[$250] Invoices - Discrepancies in bank account behavior in Invoices and Wallet page

[IuliiaHerets]

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

---

Version Number: 9.0.72-0 Reproducible in staging?: Yes Reproducible in production?: Yes If this was caught on HybridApp, is this reproducible on New Expensify Standalone?: N/A If this was caught during regression testing, add the test name, ID and link from TestRail: Exp Email or phone of affected tester (no customers): [email protected] Issue reported by: Applause Internal Team

Action Performed:

1. Go to staging.new.expensify.com
2. Go to workspace settings > Invoices.
3. Click Add bank account.
4. Add a bank account.
5. After adding bank account, click 3-dot menu > Click Make default payment method.
6. Add a second bank account.
7. After the second bank account is added, note that there is no "Default" label on the first bank account in Step 5.
8. Refresh the page, and note that there is still no "Default" label.
9. Go back to Account settings > Wallet.
10. Note that the second bank account has "Default" label, when the first bank account in Step 5 is made as default.
11. Go back to Invoices.
12. Now the first bank account only shows the "Default" label.

Expected Result:

In Step 5, there should be no "Make default payment method" option because there is only one bank account added (this option does not appear in Wallet when there is only one bank account).

In Step 8, there should be "Default" label after the second bank account is added in Invoices (The same label instantly appears when adding second bank account in Wallet page).

In Step 10 and 12, there should be consistent whether the same default bank account in Invoices should also be default in Wallet page.

Actual Result:

In Step 5, there is a "Make default payment method" option when there is only one bank account added.

In Step 8, there is no "Default" label after the second bank account is added to Invoices page. It does not appear after the page is refreshed and only appears after user opens Wallet page.

In Step 10 and 12, after adding second bank account, the first bank account remains as default in Invoices, while the second bank account is the default account in Wallet.

Workaround:

Unknown

Platforms:

• [x] Android: Standalone
• [x] Android: HybridApp
• [x] Android: mWeb Chrome
• [x] iOS: Standalone
• [x] iOS: HybridApp
• [x] iOS: mWeb Safari
• [x] MacOS: Chrome / Safari
• [x] MacOS: Desktop

Screenshots/Videos

https://github.com/user-attachments/assets/d54354ae-0214-4cce-b6f6-039296c396ca

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~021865085918655065327
• Upwork Job ID: 1865085918655065327
• Last Price Increase: 2024-12-06

Issue Owner

Current Issue Owner: @jjcoffee

[melvin-bot[bot]]

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

[Shahidullah-Muffakir]

Edited by proposal-police: This proposal was edited at 2024-12-10 10:02:03 UTC.

Proposal

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

There are discrepancies in how payment methods behave between the Wallet Page and the Invoices Page, specifically:

1. The "Make Default Payment Method" button appears incorrectly in the Invoices page.
2. The "Default" badge does not display correctly after adding a second payment method in the Invoices page. The behavior is inconsistent with the Wallet page.

What is the root cause of that problem?

We have two issues

1. Make default payment method issue The reason Make default payment method is not showing correctly the same way it does in the Wallet page is because of this condition: https://github.com/Expensify/App/blob/36b9b442a07865684ce8740fd9b1325dd1933bd3/src/pages/workspace/invoices/WorkspaceInvoiceVBASection.tsx#L58 When user add a new payment payment in the Invoices page, this value is not updating, but in the Wallet page when user add a new payment method, the new payment method is being set as the Default payment method, and then we are showing the Make default payment method for the previous payment method. and also if there is only one payment method we should not show the Make default payment method at all before checking any other condition, because that should be the default one, so the number of the payment methods is not considered in the Invoices page like it is considered in the Wallet page.
2. Default badge issue when user adds a new payment method in the Invoices page, we are only showing the Default badge for the account where: policy?.invoice?.bankAccount?.transferBankAccountID === methodID is true https://github.com/Expensify/App/blob/df7ab48f101094316aab164dfed9213914713d4f/src/pages/workspace/invoices/WorkspaceInvoiceVBASection.tsx#L116

but we art not updating the policy?.invoice?.bankAccount?.transferBankAccountID after adding a new payment method, hence it causing the issue, resulting it to show the Default badge for the old Default payment method . because now in this part of the code the isDefault will be false for the newly added payment method: https://github.com/Expensify/App/blob/f5234f74d2ae7eda37609946df0a2bfb58dd3191/src/pages/settings/Wallet/PaymentMethodList.tsx#L150-L152

causing discrepancies from the Wallet page, because in the wallet we are showing the Default badge for the recently added payment method.

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

1. To solve the first issue of Make default payment method: 1. before checking anything else we should check if there are more than one payment methods, if is one or none it should return false, same like the Wallet page.

    const hasMultiplePaymentMethods = PaymentUtils.formatPaymentMethods(bankAccountList ?? {}, fundList ?? {}, styles).length > 1;
    const shouldShowMakeDefaultButton = hasMultiplePaymentMethods && !paymentMethod.isSelectedPaymentMethodDefault;

2. we should update the policy?.invoice?.bankAccount?.transferBankAccountID to the newly added payment method, so that if there are more then one payment method, we should show the Make default payment method for the previous payment method.
3. this point number 2 have both frontend and Backend fix.
   1. the frontend fix is to call the https://github.com/Expensify/App/blob/b1c726dac0f317cd8b245b016d7ede02c56902da/src/libs/actions/PaymentMethods.ts#L559 for the newly added payment method, whenver a user add a new payment method in the Invoices page.
   2. the backend fix is that when user add a new payment method with https://github.com/Expensify/App/blob/df7ab48f101094316aab164dfed9213914713d4f/src/libs/actions/BankAccounts.ts#L199 in the invoces page update the policy?.invoice?.bankAccount?.transferBankAccountID to the account id of the new payment method using: https://github.com/Expensify/App/blob/b1c726dac0f317cd8b245b016d7ede02c56902da/src/libs/actions/PaymentMethods.ts#L559 for the newly added payment method.

In Wallet page we already update the userWallet?.walletLinkedAccountID to the account ID of the newly added payment method.

2. To solve the issue of Default badge:

1. update the policy?.invoice?.bankAccount?.transferBankAccountID to the account ID of the newly added payment method so that isDefault is true here for the newly added payment method, and directly show Default badge for the newly added payment method. https://github.com/Expensify/App/blob/f5234f74d2ae7eda37609946df0a2bfb58dd3191/src/pages/settings/Wallet/PaymentMethodList.tsx#L150-L152

2. modify this part of the code , to consider policy?.invoice?.bankAccount?.transferBankAccountID for the Invoices page and userWallet?.walletLinkedAccountID for the Wallet page when deciding to show the Default badge. if invoiceTransferBankAccountID exists in the PaymentMethodList, then it is the invoices page, because only in the inovoces page we are passing the invoiceTransferBankAccountID to PaymentMethodList in the case of Wallet page it will undefined. https://github.com/Expensify/App/blob/df7ab48f101094316aab164dfed9213914713d4f/src/pages/workspace/invoices/WorkspaceInvoiceVBASection.tsx#L188 so change the follwing code such that if it is the Invoices page, the only show the Default badge if the userWallet?.walletLinkedAccountID same as the account ID of the current payment method and in the case of Invoices page only show the Default badge if the invoiceTransferBankAccountID is same as the current payment method. https://github.com/Expensify/App/blob/f5234f74d2ae7eda37609946df0a2bfb58dd3191/src/pages/settings/Wallet/PaymentMethodList.tsx#L155-L165 this will be the modified code:

 if (invoiceTransferBankAccountID && item.methodID !== invoiceTransferBankAccountID) {
        return false;
    }

    if (item.accountType === CONST.PAYMENT_METHODS.PERSONAL_BANK_ACCOUNT) {
        if (item.accountData?.bankAccountID !== walletLinkedAccountID) {
            return false;
        }
    }
    if (item.accountType === CONST.PAYMENT_METHODS.DEBIT_CARD) {
        if (item.accountData?.fundID !== walletLinkedAccountID) {
            return false;
        }
    }

in the shouldShowDefaultBadge function.

What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?

To ensure this issue is fully covered and to avoid regressions, the following test cases should be performed for Wallet Page and Invoices Page:

Single Payment Method:

1. Add a single bank account. Verify that the 'Make Default Payment Method' button is not displayed.

Multiple Payment Methods:

1. Add two bank accounts.
2. Verify that the 'Default' label is displayed on the the latest account after the second account is added.
3. Verify that only one account is marked as the default.
4. and the first payment method has Make the payment method default option
5. Ensure the previously default account loses its 'Default' label.

Switching Default Accounts:

1. Add two bank accounts and switch the default account using the 'Make Default Payment Method' option.
2. Verify that the correct account is now marked as the default.

Consistency Across Refresh:

1. Add two bank accounts, mark one as default, and refresh the page.
2. Verify that the correct account remains marked as the default.

[melvin-bot[bot]]

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

[melvin-bot[bot]]

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

[rdipippo]

Proposal

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

There are 2 separate issues:

1. On the invoices page, the Make Default Payment Method menu item shows when there is only one account.
2. The default account on the invoices page is not the same as the default account on the wallet page (not sure if this happens all the time)

What is the root cause of that problem?

1. The function shouldShowMakeDefaultButton function defined in WalletPage.tsx is different from the function defined in WorkspaceInvoiceVBASection.tsx
2. The logic to determine the default account takes into account the invoiceTransferBankAccountID field, which is null on the wallet page but on on the invoice page. This makes the results different on each page.

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

1. Calculate the shouldShowMakeDefaultButton variable in WorkspaceInvoiceVBASection the same as the one in WalletPage.tsx.

https://github.com/Expensify/App/blob/b595f5d24141a403fcc3f44c73d00cd6951dca7d/src/pages/settings/Wallet/WalletPage/WalletPage.tsx#L338-L340

2. Do not take into account the invoiceTransferBankAccountID when determining the default account on the invoices page. Current code: https://github.com/Expensify/App/blob/b595f5d24141a403fcc3f44c73d00cd6951dca7d/src/pages/settings/Wallet/PaymentMethodList.tsx#L420-L428

The modifed code would simply pass the isDefault parameter into the shouldShowDefaultBadge function

What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?

Automated UI tests:

1. Compare the default accounts on the wallet page and invoices page. Change the default account and verify it is the same on both pages.
2. Verify Make Default Payment Method does not appear in the menu when there is one account. Do this for both pages.

What alternative solutions did you explore? (Optional)

None. Reminder: Please use plain English, be brief and avoid jargon. Feel free to use images, charts or pseudo-code if necessary. Do not post large multi-line diffs or write walls of text. Do not create PRs unless you have been hired for this job.

[melvin-bot[bot]]

📣 @rdipippo! 📣 Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork. Please follow these steps:

1. Make sure you've read and understood the contributing guidelines.
2. Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
3. Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
4. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details. Format:

Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>

[rdipippo]

Contributor details Your Expensify account email: [email protected] Upwork Profile Link: https://www.upwork.com/freelancers/~01f083226d6b4a9e09

[melvin-bot[bot]]

✅ Contributor details stored successfully. Thank you for contributing to Expensify!

[jjcoffee]

@Shahidullah-Muffakir Your proposal is missing a solution to the second issue of the labels not being consistent between invoice & wallet pages. Please also note that including unit tests is a requirement now, or at least an explanation of why you think they may not be necessary.

@rdipippo Thanks for the proposal! It looks good overall, but I'm unsure about this part:

The modifed code would simply pass the isDefault parameter into the shouldShowDefaultBadge function

Correct me if I'm wrong, but that sounds like we would ignore the invoiceTransferBankAccountID (i.e. the ID that gets set when you manually mark a payment method as default).

[rdipippo]

You are correct, but we can discuss other options if that logic can't be changed in that way. I couldn't find much in the code about what *invoiceTransferBankAccountID *is for, but it's definitely what's causing the discrepancy between the two pages, since it's null on one page (walletPage I believe) but not the other.

On Mon, Dec 9, 2024 at 9:32 AM Joel Davies @.***> wrote:

@Shahidullah-Muffakir https://github.com/Shahidullah-Muffakir Your proposal https://github.com/Expensify/App/issues/53693#issuecomment-2522797758 is missing a solution to the second issue of the labels not being consistent between invoice & wallet pages. Please also note that including unit tests is a requirement now, or at least an explanation of why you think they may not be necessary.

@rdipippo https://github.com/rdipippo Thanks for the proposal https://github.com/Expensify/App/issues/53693#issuecomment-2525317597! It looks good overall, but I'm unsure about this part:

The modifed code would simply pass the isDefault parameter into the shouldShowDefaultBadge function

Correct me if I'm wrong, but that sounds like we would ignore the invoiceTransferBankAccountID (i.e. the ID that gets set when you manually mark a payment method as default).

— Reply to this email directly, view it on GitHub https://github.com/Expensify/App/issues/53693#issuecomment-2528122966, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABO7HSPZEWUHCLG2MCV5HG32EWSWZAVCNFSM6AAAAABTEMIARSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDKMRYGEZDEOJWGY . You are receiving this because you were mentioned.Message ID: @.***>

[Shahidullah-Muffakir]

@Shahidullah-Muffakir Your proposal is missing a solution to the second issue of the labels not being consistent between invoice & wallet pages. Please also note that including unit tests is a requirement now, or at least an explanation of why you think they may not be necessary.

@rdipippo Thanks for the proposal! It looks good overall, but I'm unsure about this part:

The modifed code would simply pass the isDefault parameter into the shouldShowDefaultBadge function

Correct me if I'm wrong, but that sounds like we would ignore the invoiceTransferBankAccountID (i.e. the ID that gets set when you manually mark a payment method as default).

@jjcoffee Thank you for reviewing the proposal, I will add all the details to the proposal now.

[Shahidullah-Muffakir]

@jjcoffee , The proposal has been updated: https://github.com/Expensify/App/issues/53693#issuecomment-2522797758. Let me know if there’s anything else you’d like me to add, Thank you.

[jjcoffee]

@Shahidullah-Muffakir Thanks for the updated proposal! :pray: Could you update your RCA to include actual details of the root cause, rather than just linking to your previous PRs?

Also, in your solution section it would be more helpful to explain what changes you want to make and why, rather than just pasting entire blocks of code. This helps to keep your proposal concise, and perhaps more importantly easy to read and parse!

[Shahidullah-Muffakir]

@jjcoffee , the proposal https://github.com/Expensify/App/issues/53693#issuecomment-2522797758 has been updated. Please let me know your thoughts. Thanks!

[rdipippo]

Tagging @jjcoffee

On Mon, Dec 9, 2024 at 9:51 AM Richard DiPippo @.***> wrote:

You are correct, but we can discuss other options if that logic can't be changed in that way. I couldn't find much in the code about what *invoiceTransferBankAccountID *is for, but it's definitely what's causing the discrepancy between the two pages, since it's null on one page (walletPage I believe) but not the other.

On Mon, Dec 9, 2024 at 9:32 AM Joel Davies @.***> wrote:

@Shahidullah-Muffakir https://github.com/Shahidullah-Muffakir Your proposal https://github.com/Expensify/App/issues/53693#issuecomment-2522797758 is missing a solution to the second issue of the labels not being consistent between invoice & wallet pages. Please also note that including unit tests is a requirement now, or at least an explanation of why you think they may not be necessary.

@rdipippo https://github.com/rdipippo Thanks for the proposal https://github.com/Expensify/App/issues/53693#issuecomment-2525317597! It looks good overall, but I'm unsure about this part:

The modifed code would simply pass the isDefault parameter into the shouldShowDefaultBadge function

Correct me if I'm wrong, but that sounds like we would ignore the invoiceTransferBankAccountID (i.e. the ID that gets set when you manually mark a payment method as default).

— Reply to this email directly, view it on GitHub https://github.com/Expensify/App/issues/53693#issuecomment-2528122966, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABO7HSPZEWUHCLG2MCV5HG32EWSWZAVCNFSM6AAAAABTEMIARSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDKMRYGEZDEOJWGY . You are receiving this because you were mentioned.Message ID: @.***>

[jjcoffee]

@Shahidullah-Muffakir I still think it could be made more concise, but I think I understand it enough :smile:

In Step 10 and 12, there should be consistent whether the same default bank account in Invoices should also be default in Wallet page.

One thing I'm wondering is if the above expected result is definitely true. Is the default actually shared between the wallet and a specific policy/workspace's invoice settings? cc @garrettmknight

[jjcoffee]

@rdipippo Apologies for not responding earlier! It would be expected for you to understand what invoiceTransferBankAccountID does/is for before proposing changes that could cause regressions otherwise.

[jjcoffee]

Okay after testing this a bit more, it looks like fixing the first issue means the second issue isn't really an issue. The only remaining issue is that there's a Make default payment method option showing for the bank account that's already set as default.

I'm still unsure whether the wallet and invoice bank account should share a default. For now, let's go with @Shahidullah-Muffakir's proposal and we can adjust things on the PR.

:ribbon::eyes::ribbon: C+ reviewed

[melvin-bot[bot]]

Triggered auto assignment to @marcaaron, see https://stackoverflow.com/c/expensify/questions/7972 for more details.

[melvin-bot[bot]]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[melvin-bot[bot]]

📣 @Shahidullah-Muffakir You have been assigned to this job! Please apply to the Upwork job and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻 Once you apply to this job, your Upwork ID will be stored and you will be automatically hired for future jobs! Keep in mind: Code of Conduct | Contributing 📖

[melvin-bot[bot]]

@garrettmknight, @jjcoffee, @marcaaron, @Shahidullah-Muffakir Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[jjcoffee]

@Shahidullah-Muffakir Don't forget to let us know your ETA for raising the PR :pray:

[Shahidullah-Muffakir]

Apologies for the delay; the PR will be ready within 24 hours.

[Shahidullah-Muffakir]

Hi @NikkiWines , tagging you here because you added backend changes for the Wallet page and have relevant context: https://github.com/Expensify/App/issues/50829#issuecomment-2433389201. Could you help us clarify the expected behavior for payment methods on the Invoice page of a workspace? cc @garrettmknight @marcaaron

Current Behavior on the Invoice Page:

1. No default set initially:

   • If the user hasn't clicked the Make the payment method default button for any payment method, the Default badge is not displayed for any method. Instead, the Make the payment method default option is shown for all methods.
   • After the user clicks the button, policy?.invoice?.bankAccount?.transferBankAccountID is updated to the account ID of the selected payment method, and the Default badge is displayed for that method.

2. Adding a new payment method:

   • Unlike the Wallet page, if a user adds a new payment method in the Invoice page using the AddPersonalBankAccount API endpoint, the newly added payment method is not set as default.
   • In this case, policy?.invoice?.bankAccount?.transferBankAccountID is not updated, so the newly added payment method does not show the Default badge.

Summary:

The Invoice page behavior for displaying the Default badge and the Make the payment method default option depends entirely on the policy?.invoice?.bankAccount?.transferBankAccountID value:

• If this value is undefined, no payment method shows the Default badge, and all payment methods display the Make the payment method default option.
• If the value is defined, the method matching the account ID shows the Default badge, and other methods display the Make the payment method default option.

---

Possible Solution:

1. If no payment method is marked as default on the Invoice page, use the default payment method from the Wallet page as the fallback.
2. If a user clicks the Make the payment method default button on the Invoice page, display the Default badge next to that payment method only.
3. When a user adds a new payment method on the Invoice page using the AddPersonalBankAccount endpoint, update the policy?.invoice?.bankAccount?.transferBankAccountID value to the newly added payment method. This ensures the new method is automatically marked as default, aligning behavior with the Wallet page.(This will require BE change)
4. Instead of point 3, if we don't want to show the default badge for the latest payment method in the invoice page, we can skip point 3. We will only show the default badge and the "Make the current payment method default" based on the policy?.invoice?.bankAccount?.transferBankAccountID, which should only be updated if the user manually clicks on the "Make the current payment method default" button.

What is your thought on the proposed solution? Thanks.

[NikkiWines]

I like the idea of aligning the behavior with what we have on for the Wallet page for the sake of consistency. The proposed solution makes sense to me but let's have @marcaaron and @garrettmknight confirm too 🙇

[Shahidullah-Muffakir]

I like the idea of aligning the behavior with what we have on for the Wallet page for the sake of consistency. The proposed solution makes sense to me but let's have @marcaaron and @garrettmknight confirm too 🙇

Thank you.

[marcaaron]

I can't think of a reason why we would want paying an invoice to work inconsistently, but let's ask @madmax330 too. I see he implemented transferBankAccountID here: https://github.com/Expensify/Expensify/issues/427801

[melvin-bot[bot]]

@garrettmknight @jjcoffee @marcaaron @Shahidullah-Muffakir this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

[melvin-bot[bot]]

@garrettmknight, @jjcoffee, @marcaaron, @Shahidullah-Muffakir Whoops! This issue is 2 days overdue. Let's get this updated quick!