[HOLD for payment 2024-07-25] [$250] [Payment card / Subscription] When adding Payment card, there is a slight delay before it shows

[lanitochka17]

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: 9.0.4-6
Reproducible in staging?: Y
Reproducible in production?: N
If this was caught during regression testing, add the test name, ID and link from TestRail: N/A
Issue reported by: Applause - Internal Team

Issue found when executing PR #44361

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 & Save

Expected Result:

User expects that after saving the card data immediately loads and displays

Actual Result:

Several seconds go by before the card actually appears on the Sub page, leading the user to think the details did not save

Workaround:

Unknown

Platforms:

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

• Android: Native
• Android: mWeb Chrome
• iOS: Native
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

Bug6534057_1720183810076.Add_payment_takes_a_while_to_load.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01b996dd4f4317410f
• Upwork Job ID: 1809282225502876268
• Last Price Increase: 2024-07-05
• Automatic offers:
• dominictb | Contributor | 103066195

Issue Owner

Current Issue Owner: @getusha

[melvin-bot]

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

[github-actions]

👋 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.

[lanitochka17]

@mountiny FYI 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

[mountiny]

Not a blocker since this is behind beta

@blimpich I hope you dont mind I will assign you.

Are we not handling this optimistically? Feels like we should, but if not there should be some loader

[blimpich]

Yeah this is a bug, we should be handling this optimistically.

[melvin-bot]

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

[melvin-bot]

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

[melvin-bot]

Triggered auto assignment to @sonialiap (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.

[cretadn22]

Proposal

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

When adding Payment card, there is a slight delay before it shows

What is the root cause of that problem?

In the addSubscriptionPaymentCard function, we do not include card information in the optimistic data. Additionally, we do not update the successData when successful or the error in the failure data.

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

In the addSubscriptionPaymentCard function, we must incorporate optimisticData to mirror the backend's response. Furthermore, we should update the successData upon successful execution and the failureData in case of errors.

What alternative solutions did you explore? (Optional)

Additionally, we could display a skeleton while awaiting the response from the backend (By using addPaymentCardForm.loading from Onyx)

[dominictb]

Proposal

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

• Several seconds go by before the card actually appears on the Sub page, leading the user to think the details did not save

What is the root cause of that problem?

• In function:
  

  App/src/libs/actions/PaymentMethods.ts

  Line 210 in b0947e0

  function addSubscriptionPaymentCard(cardData: {

  
  we call Navigation.goBack() right after we call AddPaymentCard API, that will close the payment form regardless the API is called successfully or not.

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

• We should remove:
  

  App/src/libs/actions/PaymentMethods.ts

  Line 261 in b0947e0

  Navigation.goBack();

  
  so the form is still displayed after clicking on the CTA button.
• And we should add the logic to close the payment form if the API is called successfully by adding the below logic to
  this line

    const [formData] = useOnyx(ONYXKEYS.FORMS.ADD_PAYMENT_CARD_FORM);
    const prevFormDataSetupComplete = usePrevious(!!formData?.setupComplete);

    useEffect(() => {
        if (prevFormDataSetupComplete || !formData?.setupComplete) {
            return;
        }

        PaymentMethods.continueSetup();
    }, [prevFormDataSetupComplete, formData?.setupComplete]);

if the API is called successfully, the setupComplete is set to true, based on that, we can auto close the form.

• There is the same issue with the same solution should be fixed here.

What alternative solutions did you explore? (Optional)

[sonialiap]

@getusha what do you think of the above proposals?

[trjExpensify]

@dominictb do you think we can have the draft PR in review today?

[dominictb]

• PR feat only close the modal once the API is done #45281 is ready to review, it will fix the bug #44904

• Related to issue #45124, we should hold it until we fix the BE issue, and then I will raise the PR the fix its original issue using the solution.

cc @trjExpensify @blimpich

[trjExpensify]

Nice one! Can you complete the video/screenshots in the OP of the PR linked above? Thanks!

[dominictb]

@blimpich I found another BE bug when calling API AddPaymentCard. When we already added the payment card 1, its findList['101'].accountData.additionalData.isBillingCard is true. Then I add the new payment card 2, BE only sets findList['102'].accountData.additionalData.isBillingCard to true but does not setfindList['101'].accountData.additionalData.isBillingCard to false unless we refresh the page.

[blimpich]

@dominictb thank you for reporting the bug! I believe that has the same root cause as the first BE bug. Lets come back to that once the first BE bug is completed 👍

[melvin-bot]

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

[melvin-bot]

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

• feat only close the modal once the API is done #45281

If no regressions arise, payment will be issued on 2024-07-25. 🎊

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

• @getusha requires payment through NewDot Manual Requests
• @dominictb requires payment automatic offer (Contributor)

[melvin-bot]

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:

• [@getusha] The PR that introduced the bug has been identified. Link to the PR:
• [@getusha] 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:
• [@getusha] 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:
• [@getusha] Determine if we should create a regression test for this bug.
• [@getusha] 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.
• [@sonialiap] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[sonialiap]

Payment summary:
@getusha $250 - please request in ND - please complete the checklist
@dominictb $250 - paid ✔️

[melvin-bot]

@sonialiap, @blimpich, @getusha, @dominictb Eep! 4 days overdue now. Issues have feelings too...

[getusha]

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:

• The PR that introduced the bug has been identified. Link to the PR: remove GBP as a currency for the subscription page #44858
• 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/44858/files#r1697402990
• 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: N/a
• Determine if we should create a regression test for this bug. No, seems like a slight improvement / polish
• 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. N/a

[JmillsExpensify]

$250 approved for @getusha