Expensify / App

Welcome to New Expensify: a complete re-imagination of financial collaboration, centered around chat. Help us build the next generation of Expensify by sharing feedback and contributing to the code.
https://new.expensify.com
MIT License
3.29k stars 2.72k forks source link

[HOLD for payment 2024-08-01] [$125] [Payment card / Subscription] Make payment card only display last four digits #45309

Closed blimpich closed 1 month ago

blimpich commented 1 month ago

Problem

Right now when you add a payment card to the new subscription page, it shows the 16 characters instead of just four:

Image

It should show just the last four:

Image

Solution

Lets fix this by only showing the last four digits in the subscription page.

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01a9042029c460e7a0
  • Upwork Job ID: 1811455490190961117
  • Last Price Increase: 2024-07-11
Issue OwnerCurrent Issue Owner: @CortneyOfstad / @isabelastisser
melvin-bot[bot] commented 1 month ago

Triggered auto assignment to @isabelastisser (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

melvin-bot[bot] commented 1 month ago

Job added to Upwork: https://www.upwork.com/jobs/~01a9042029c460e7a0

melvin-bot[bot] commented 1 month ago

Triggered auto assignment to Contributor-plus team member for initial proposal review - @sobitneupane (External)

melvin-bot[bot] commented 1 month ago

Upwork job price has been updated to $125

blimpich commented 1 month ago

Lowering price because this is pretty straightforward string manipulation.

blimpich commented 1 month ago

Anyone who tests this should be able to simulate adding a test card by populating the fundList onyx key based off the types we define in the frontend, or you can add a real credit card, though know that we currently don't have a way of deleting credit cards added to the app.

Krishna2323 commented 1 month ago

Proposal

Please re-state the problem that we are trying to solve in this issue.

[Payment card / Subscription] Make payment card only display last four digits

What is the root cause of that problem?

Value is not sliced. https://github.com/Expensify/App/blob/c53bd5cd52a5236cbd5c83ed4f68dd43cdf4ff6c/src/pages/settings/Subscription/CardSection/CardSection.tsx#L116

What changes do you think we should make in order to solve the problem?

Slice the value to obtain last four digits. defaultCard?.accountData?.cardNumber.slice(-4)

fundlist can be populated for testing by using object below.

{
Card: {accountData: {addressName: 'Max', cardYear: 2029, cardMonth: 9, currency: 'USD', cardNumber: '1234567887654321', additionalData: {isBillingCard: true}}},
}

What alternative solutions did you explore? (Optional)

Result

Monosnap (83) New Expensify 2024-07-11 23-33-29
kabeer95 commented 1 month ago

Feature Request: Mask Payment Card Numbers on Expensify

Summary:

To enhance security and compliance, we propose masking payment card numbers on Expensify, displaying only the last four digits. This feature will provide an additional layer of protection for sensitive payment information.

Current State:

Currently, Expensify displays the full payment card number, which may pose a security risk if unauthorized access is gained.

Proposed Solution:

Implement a feature to mask payment card numbers, displaying only the last four digits. For example:

Original card number: 424242XXXXXX4242 Masked card number: ****4242 Benefits:

Enhanced Security: Masking payment card numbers reduces the risk of unauthorized access to sensitive information. Compliance: This feature aligns with industry standards for payment card security, such as PCI-DSS. User Trust: By protecting payment information, Expensify demonstrates a commitment to user security and trust. Implementation:

To implement this feature, we recommend the following:

Update the payment card number display to show only the last four digits. Store the full payment card number securely, using industry-standard encryption and access controls. Ensure that the masked card number is displayed consistently throughout the Expensify platform. Estimated Development Time:

We estimate 20 hours to implement this feature and mask payment card numbers on Expensify.

Priority:

We consider this feature a high priority, as it directly impacts the security and trust of Expensify users.

Please let us know if you would like to discuss this feature further or proceed with implementation

tienifr commented 1 month ago

Proposal

Please re-state the problem that we are trying to solve in this issue.

Screenshot 2024-07-13 at 12 26 51

In card title:

https://github.com/Expensify/App/blob/7255ab50bb25e74a4cc17a539af089dc7abce5a3/src/pages/settings/Subscription/CardSection/CardSection.tsx#L116

In card billing status:

https://github.com/Expensify/App/blob/7255ab50bb25e74a4cc17a539af089dc7abce5a3/src/pages/settings/Subscription/CardSection/CardSection.tsx#L84-L192 with the subtitle is from: https://github.com/Expensify/App/blob/7255ab50bb25e74a4cc17a539af089dc7abce5a3/src/pages/settings/Subscription/CardSection/utils.ts#L74 https://github.com/Expensify/App/blob/7255ab50bb25e74a4cc17a539af089dc7abce5a3/src/pages/settings/Subscription/CardSection/utils.ts#L82 with cardEnding is from: https://github.com/Expensify/App/blob/7255ab50bb25e74a4cc17a539af089dc7abce5a3/src/pages/settings/Subscription/CardSection/utils.ts#L27

What changes do you think we should make in order to solve the problem?

Result:

image

sobitneupane commented 1 month ago

Thanks for the proposal everyone.

Proposal from @tienifr looks good to me.

🎀 👀 🎀 C+ reviewed

melvin-bot[bot] commented 1 month ago

Current assignee @blimpich is eligible for the choreEngineerContributorManagement assigner, not assigning anyone new.

tienifr commented 1 month ago

@blimpich Based on comment, to test this issue, I just need to:

simulate adding a test card by populating the fundList onyx key based off the types we define in the frontend

without:

add a real credit card

right?

blimpich commented 1 month ago

Yes that should be fine 👍

tienifr commented 1 month ago

PR https://github.com/Expensify/App/pull/45598 is ready

isabelastisser commented 1 month ago

Hi @sobitneupane, PR is ready for review. Thanks!

melvin-bot[bot] commented 1 month ago

Triggered auto assignment to @CortneyOfstad (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

isabelastisser commented 1 month ago

I will be OOO tomorrow and next week, so I am reassigning this until I return on July 29. Thanks, @CortneyOfstad!

Status: Waiting for PR review.

melvin-bot[bot] commented 1 month ago

Reviewing label has been removed, please complete the "BugZero Checklist".

melvin-bot[bot] commented 1 month ago

The solution for this issue has been :rocket: deployed to production :rocket: in version 9.0.11-5 and is now subject to a 7-day regression period :calendar:. Here is the list of pull requests that resolve this issue:

If no regressions arise, payment will be issued on 2024-08-01. :confetti_ball:

For reference, here are some details about the assignees on this issue:

melvin-bot[bot] commented 1 month ago

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

isabelastisser commented 1 month ago

I'm back and taking this back. Thanks, @CortneyOfstad !

isabelastisser commented 1 month ago

Not overdue.

isabelastisser commented 1 month ago

Hey @sobitneupane, please complete the BZ list above. Thanks!

melvin-bot[bot] commented 1 month ago

Payment Summary

Upwork Job

BugZero Checklist (@isabelastisser)

sobitneupane commented 1 month ago

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

  • [ ] [@sobitneupane] The PR that introduced the bug has been identified. Link to the PR:

https://github.com/Expensify/App/pull/44072

  • [ ] [@sobitneupane] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:

https://github.com/Expensify/App/pull/44072#issuecomment-2268631759

  • [ ] [@sobitneupane] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:

Not required.

  • [ ] [@sobitneupane] Determine if we should create a regression test for this bug.

Yes.

  • [ ] [@sobitneupane] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.

https://github.com/Expensify/App/issues/45309#issuecomment-2268639094

sobitneupane commented 1 month ago

Regression Test Proposal:

Do we agree 👍 or 👎

isabelastisser commented 1 month ago

@blimpich, can you please review the regression test above? Thanks!

isabelastisser commented 1 month ago

Payment summary:

C+ review @sobitneupane owed $250 via NewDot Contributor @tienifr owed $250 via NewDot

isabelastisser commented 1 month ago

All set.

garrettmknight commented 3 days ago

$250 approved for @sobitneupane based on this summary.

garrettmknight commented 1 day ago

$250 approved for @tienifr