search

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-07-24] [$250] [Payment card / Subscription] When clicking "Change payment", past data still shows on new form #44886

Closed izarutskaya closed 1 month ago

izarutskaya commented 2 months ago

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

Version Number: v9.0.4-0 Reproducible in staging?: Y Reproducible in production?: Unable to check Found when executing PR : https://github.com/Expensify/App/pull/44361 Email or phone of affected tester (no customers): applausetester+en@applause.expensifail.com Logs: https://stackoverflow.com/c/expensify/questions/4856 Issue reported by: Applause-Internal team

Action Performed:

  1. Access staging.new.expensify.com
  2. Sign into a valid account (Beta enabled)
  3. Access https://staging.new.expensify.com/settings/subscription/add-payment-card
  4. Input valid card data
  5. Click on "Change Payment" after the card data has been set

Expected Result:

User expects that if the fields are cleared, then all fields should be cleared.

Actual Result:

The expiration date still shows from the previous card (And is in the correct format, missing a digit) and the Zip code is still showing as well.

Workaround:

Unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

Screenshots/Videos

https://github.com/Expensify/App/assets/115492554/4c7b43bb-3b0f-4e17-ae62-521fae36a05b

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01f36eea0f55c219c3
  • Upwork Job ID: 1809282474389988839
  • Last Price Increase: 2024-07-05
Issue OwnerCurrent Issue Owner: @mallenexpensify
melvin-bot[bot] commented 2 months ago

Triggered auto assignment to @mallenexpensify (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 2 months ago

Triggered auto assignment to @madmax330 (DeployBlockerCash), see https://stackoverflowteams.com/c/expensify/questions/9980/ for more details.

github-actions[bot] commented 2 months ago

:wave: Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open `StagingDeployCash` deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

  1. Identify the pull request that introduced this issue and revert it.
  2. Find someone who can quickly fix the issue.
  3. Fix the issue yourself.
izarutskaya commented 2 months ago

@mallenexpensify I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors.

izarutskaya commented 2 months ago

We think this issue might be related to the #collect project.

mountiny commented 2 months ago

This is minor and subscriptions in ND is still hidden for new users so we can demote

melvin-bot[bot] commented 2 months ago

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

melvin-bot[bot] commented 2 months ago

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

Pujan92 commented 2 months ago

Proposal

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

Past data shows in the form when we try to change payment card or opening the add payment card page directly

What is the root cause of that problem?

In the PaymentCard component, we are setting the payment card form with the previously added card details. Due to that some of the fields are populated with the existing card data.

https://github.com/Expensify/App/blob/c5eafd33463b2ba4c0ce2eb0c48261f66b085dc7/src/pages/settings/Subscription/PaymentCard/index.tsx#L48-L55

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

I don't think we need this(setPaymentCardForm) method or don't need to set payment card form details. We can get rid of that useEffect block. Also we need to remove the prop shouldShowPaymentCardForm bcoz it should be always true for web. Reason - We don't have any edit card functionality for which we need to populate the existing details and for currency updation we use fundList to derive default card currency.

https://github.com/Expensify/App/blob/c5eafd33463b2ba4c0ce2eb0c48261f66b085dc7/src/pages/settings/Subscription/PaymentCard/ChangeBillingCurrency/index.tsx#L36

trjExpensify commented 1 month ago

@fedirjh what do you think?

melvin-bot[bot] commented 1 month ago

@blimpich, @mallenexpensify, @fedirjh Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

fedirjh commented 1 month ago

@Pujan92 Can you elaborate on the root cause? Why are some fields cleared and others not?

Also, following the testing steps in PR #43572, it seems that there is an edit card functionality. @blimpich, @narefyev91, can you please confirm this? Should we always clear the card form, or should we populate it with the saved card data?

Edit payment card:

  1. You need to have saved card in subscription and click on 3 dots to change a card
  2. Verify that you see payment card fields. Not all fields should be filled - based on the API type structure we should see filed fields - card number, zip, expiration date, currency.
  3. Change some fields, fill rest of them
  4. Click save
  5. Verify that API call AddPaymentCard executed
blimpich commented 1 month ago

The entire form should be emptied when the user clicks on "change payment card." We originally decided to have edit functionality but changed our minds and decided that when the user clicks "change payment card" the entire form should be emptied.

No real root cause, its a new feature and we are still figuring out how exactly we want it. Less of a bug and more a new feature.

trjExpensify commented 1 month ago

@fedirjh @Pujan92 how we looking here? Are we aligned, can we move forward?

fedirjh commented 1 month ago

@blimpich @trjExpensify Since this is a newer feature, should we consider validating the edited card number on the front end? For instance, if someone tries to add the same card number again, should we show an error message?

@Pujan92 Proposal looks good and we can move proceed with it.

trjExpensify commented 1 month ago

should we consider validating the edited card number on the front end? For instance, if someone tries to add the same card number again, should we show an error message?

I don't really see the value in erroring out here. If someone did that, it doesn't cause any harm, they just have the same card details on file as they did before.

blimpich commented 1 month ago

Yeah we don't care about them putting in the same card number. Lets keep it simple 👍

blimpich commented 1 month ago

Looks like automation didn't work for automatically assigning the contract. @Pujan92 please feel free to raise a PR. @mallenexpensify will make sure you get paid.

Pujan92 commented 1 month ago

@blimpich I will raise PR soon, the contract isn't assigned bcoz now I am about to get payments via Newdot.

mallenexpensify commented 1 month ago

Noting that assign date is after eligible date, so @Pujan92 will be paid via NewDot

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.7-8 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-07-24. :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:

fedirjh commented 1 month ago

BugZero Checklist:

Regression Test Proposal

1. Open App
2. Go to /settings/subscription/add-payment-card
3. Enter valid card details and add it
4. Verify the card data has been successfully added to your account
5. Click on "Change Payment" option 
6. Verify previously added card details doesn't show in the form
mallenexpensify commented 1 month ago

@Pujan92 does not require payment (Contractor)

@Pujan92 this should be fixed now to denote you're paid via NewDot. Comment here if you see any assigned issues from today forward where it's not.

@fedirjh , thanks for test case steps

melvin-bot[bot] commented 1 month ago

Payment Summary

Upwork Job

BugZero Checklist (@mallenexpensify)

JmillsExpensify commented 1 month ago

@mallenexpensify can you confirm the checklist above so I can approve payments?

melvin-bot[bot] commented 1 month ago

@Pujan92, @blimpich, @mallenexpensify, @fedirjh Whoops! This issue is 2 days overdue. Let's get this updated quick!

mallenexpensify commented 1 month ago

Contributor: @Pujan92 due $250 via NewDot
Contributor+: @fedirjh due $250 via NewDot

Thanks!

JmillsExpensify commented 1 month ago

$250 approved for @fedirjh