[$250] Invoices - Add bank account button in invoice report remains after bank account is added

[lanitochka17]

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.46-3 Reproducible in staging?: Y Reproducible in production?: Y If this was caught during regression testing, add the test name, ID and link from TestRail: https://expensify.testrail.io/index.php?/tests/view/5054125 Email or phone of affected tester (no customers): [email protected] Issue reported by: Applause - Internal Team. Refined by @neil-marcellini. Slack conversation (hyperlinked to channel name): Expensify-billpay

Action Performed:

1. Go to staging.new.expensify.com
2. Create a workspace if needed
3. Go to More features, scroll down and enable Invoices
4. Click green plus, Send invoice
5. [User A] Send an invoice to User B
6. [User B] Go to invoice chat and pay the invoice as business
7. [User A] Go to invoice report
8. [User A] Click Add bank account
9. [User A] Note that the button leads to Invoices on workspace settings
10. [User A] Click Add bank account and add a bank account
11. [User A] Go back to workspace chat

Expected Result:

After adding a bank account for the invoice report the user should be navigated back to it, and the add bank account button should be gone. We could potentially have a system message saying that they added a bank account to receive the invoice payment. Also, maybe instead of directing the the workspace settings after clicking add bank account, we should instead have an informational page at the end of the flow in the RHP that explains the account has been added as the invoice transfer bank account.

Actual Result:

The add bank account button on the invoice report navigates to settings, then after adding it you land back on the settings page and have to manually navigate back to the invoice report. Additionally, the add bank account button will remain present until you go to invoicing settings, click on the overflow menu of the bank account, and make it the default payment method.

Workaround:

Unknown

Platforms:

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

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

Screenshots/Videos

Add any screenshot/video evidence

Current report

https://github.com/user-attachments/assets/ec372a3a-121c-4bdb-9916-7b1627f7e600

Original report

https://github.com/user-attachments/assets/fffd223f-363b-4ac8-88fe-2cfe0272673a

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~021846353790748148311
• Upwork Job ID: 1846353790748148311
• Last Price Increase: 2024-12-06
• Automatic offers:
• abzokhattab | Contributor | 104666639

Issue Owner

Current Issue Owner: @neil-marcellini

[melvin-bot[bot]]

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

[lanitochka17]

@twisterdotcom 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

[lanitochka17]

We think that this bug might be related to #vip-bills

[abzokhattab]

Edited by proposal-police: This proposal was edited at 2024-10-27 14:58:25 UTC.

Proposal

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

The "Add Bank Account" button in the invoice report remains visible even after a bank account is added.

What is the root cause of that problem?

The issue arises because the visibility of the "Add Bank Account" button is controlled by a function that checks whether the policy?.invoice?.bankAccount?.transferBankAccountID exists. If it doesn't exist, the function returns false, causing the button to remain visible. The root cause is that the backend does not set the transferBankAccountID when adding a bank account, which is why the button stays visible even after an account has been added.

https://github.com/Expensify/App/blob/3f9bf24aab01110a69869b763bdf146189df17b3/src/libs/ReportUtils.ts#L8444

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

To resolve this, we need the backend to set the transferBankAccountID. This can be achieved by adding a new parameter to the body of the AddPersonalBankAccount request sent to the backend. To understand how this process works when the user clicks on the "Add Bank Account" button in the invoice page, follow the flow:

1. When the "Add Bank Account" button is clicked, the paymentMethodPressed function is called, which sets setShouldShowAddPaymentMenu(true) .
2. The shouldShowAddPaymentMenu state controls whether the AddPaymentMethodMenu is visible .
3. Once isVisible is true, the useEffect inside the AddPaymentMethodMenu is triggered, which calls the onItemSelected function.
4. The onItemSelected function calls addPaymentMethodTypePressed .
5. The addPaymentMethodTypePressed function then calls openPersonalBankAccountSetupView, which triggers the Plaid integration function here .
6. After the Plaid process finishes, the addPersonalBankAccount request is sent to the backend, passing the account details as parameters.

To fix the issue, i propose the following changes:

1. Add an isInvoiceBankAccount = false property to the openPersonalBankAccountSetupView function. Then, inside the Plaid clear promise, merge this isInvoiceBankAccount value into the ONYXKEYS.PERSONAL_BANK_ACCOUNT onyx key similar to the exitReportID arg

2. In the BankAccounts.openPersonalBankAccountSetupView , set the isInvoiceBankAccount parameter to true.

3. Finally, in the addPersonalBankAccount function, pass the isInvoiceBankAccount parameter to the backend request.

this way the backend can differentiate between invoices bank accounts and others

What alternative solutions did you explore? (Optional)

Potential issue after fixing the `transferBankAccountID` issue

The condition that determines whether the button should be shown uses `ReportUtils.hasMissingInvoiceBankAccount`, which only calculates once because the report ID does not change when adding a bank account. [https://github.com/Expensify/App/blob/6439b7bbc2cc31c73618bb9bd8da04e850d7b17b/src/pages/home/report/ReportActionItemMessage.tsx#L139](https://github.com/Expensify/App/blob/6439b7bbc2cc31c73618bb9bd8da04e850d7b17b/src/pages/home/report/ReportActionItemMessage.tsx#L139)

Solution: The condition that determines whether the button should be shown uses ReportUtils.hasMissingInvoiceBankAccount, which only calculates once because the report ID does not change when adding a bank account. https://github.com/Expensify/App/blob/6439b7bbc2cc31c73618bb9bd8da04e850d7b17b/src/pages/home/report/ReportActionItemMessage.tsx#L139

• We should modify the hasMissingInvoiceBankAccount function so that it takes the policy as an input.
• Then, in ReportActionItemMessage, we should create a small component that would display the button and subscribe to the policy using Onyx and pass it to the function so we will move this button to the new component: sudo code:

ReportActionItemMessage:

...
...action.actionName === CONST.REPORT.ACTIONS.TYPE.IOU && ReportUtils.hasMissingInvoiceBankAccount(reportID) && ( <AddBankAccountActionButton/> )

AddBankAccountActionButton

const [invoiceReport] = useOnyx(REPORTS_KEY, reportID)
const [policy] = useOnyx(POLICY_KEY, `invoiceReport?.policyID`);

useEffect( () => {
ReportUtils.hasMissingInvoiceBankAccount(reportID)
},[reportID,policy])


return <Button 
....

optional: we can modify the function to take the report itself, not the id.

Alternatively:

An alternative is to use useMemo on ReportUtils.hasMissingInvoiceBankAccount with the report policy (policy?.invoice?.bankAccount?.transferBankAccountID) as a dependency. This approach ensures the function recalculates if the account ID within the policy changes.

[melvin-bot[bot]]

@twisterdotcom Eep! 4 days overdue now. Issues have feelings too...

[twisterdotcom]

Not sure about that solution because the bank account isn't policy linked is it? Anyway, I agree this is an issue, but minor.

[melvin-bot[bot]]

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

[melvin-bot[bot]]

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

[melvin-bot[bot]]

Upwork job price has been updated to $125

[parasharrajat]

@abzokhattab your proposal looks good to me but it is going to add a subscription to each msg item which is heavy. How can we optimize this?

[abzokhattab]

1. As an optimization technique, we can use useSelector on the policy?.invoice?.bankAccount?.transferBankAccountID object when fetching the policy.
2. In addition to the proposal details, we can optionally move the ReportUtils.hasMissingInvoiceBankAccount call from the ReportActionItemMessage component to the ReportActionsListItemRenderer . We can then pass this value as a prop to the ReportActionItem component and further down to the ReportActionItemMessage. This way, the policy subscription is called only once.

let me know what you think about that

[melvin-bot[bot]]

@twisterdotcom @parasharrajat 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]]

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

[twisterdotcom]

bump on @abzokhattab's Q @parasharrajat

[parasharrajat]

@abzokhattab What about we move the button and report subscription to a small component inside the ReportActionItemMessage so that we only subscribe where the button is visible. This component will not render until action.actionName === CONST.REPORT.ACTIONS.TYPE.IOU && ReportUtils.hasMissingInvoiceBankAccount(reportID) is true.

[abzokhattab]

Good idea, looks good to me

Updated my proposal to implement this approach

[parasharrajat]

OK. Thanks @abzokhattab's proposal looks good to me.

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

[melvin-bot[bot]]

Triggered auto assignment to @neil-marcellini, 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? 💸

[neil-marcellini]

OK. Thanks @abzokhattab's proposal looks good to me.

🎀 👀 🎀 C+ reviewed

I agree. Hiring.

[melvin-bot[bot]]

📣 @abzokhattab 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link Upwork job Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻 Keep in mind: Code of Conduct | Contributing 📖

[melvin-bot[bot]]

@twisterdotcom @parasharrajat @neil-marcellini @abzokhattab this issue is now 4 weeks old, please consider:

• Finding a contributor to fix the bug
• Closing the issue if BZ has been unable to add the issue to a VIP or Wave project
• If you have any questions, don't hesitate to start a discussion in #expensify-open-source

Thanks!

[parasharrajat]

@abzokhattab Any update?

[neil-marcellini]

@abzokhattab please raise a PR in the next couple days, or we might have to re-assign the issue.

[abzokhattab]

Sorry i missed the notifications, working on it today!

[abzokhattab]

It seems that modifying the button alone won’t be sufficient to fix the issue. There's another problem where the button still appears even after a page refresh.

https://github.com/user-attachments/assets/20483aca-f833-460f-91df-ac98ecc57b95

In order to debug this, I added a log in the hasMissingInvoiceBankAccount function to output the getPolicy(invoiceReport?.policyID)?.invoice value. Below is the result after adding the bank account

In the screenshot, the bankAccount object contains a stripeConnectAccountID. However, in hasMissingInvoiceBankAccount, we are checking for transferBankAccountID.

If this parameter has been updated in the backend, then we may need to replace all instances of transferBankAccountID with stripeConnectAccountID. Could you confirm if transferBankAccountID is still in use on the backend?

Another approach could be to implement the same logic used in hasBankAccount, which retrieves the accounts list via the ONYXKEYS.BANK_ACCOUNT_LIST subscription and checks if any accounts are available. You can see this implementation here .

Let me know your thoughts on how we should proceed based on these observations. cc @parasharrajat, @neil-marcellini

[parasharrajat]

Another approach could be to implement the same logic used in hasBankAccount, which retrieves the accounts list via the ONYXKEYS.BANK_ACCOUNT_LIST subscription and checks if any accounts are available. You can see this implementation here .

This can't be done as we add many accounts. Not all are used for invoicing.

[parasharrajat]

@neil-marcellini Can you please help us determine what's going on? Are there new changes that need to be updated for invoicing?

[abzokhattab]

This can't be done as we add many accounts. Not all are used for invoicing.

I see but why are we using the ONYXKEYS.BANK_ACCOUNT_LIST in the WorkspaceInvoiceVBASection component then? i mean this component is related to invoices as well

@neil-marcellini, could you please share your perspective on this?

[parasharrajat]

@neil-marcellini Bump.