[$125] 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

Action Performed:

1. Go to staging.new.expensify.com
2. [User A] Send an invoice to User B
3. [User B] Go to invoice chat and pay the invoice as business
4. [User A] Go to invoice report
5. [User A] Click Add bank account
6. [User A] Note that the button leads to Invoices on workspace settings
7. [User A] Click Add bank account and add a bank account
8. [User A] Go back to workspace chat and click Add bank account button after adding a bank account
9. [User A] Note that the button leads to workspace list when bank account is added

Expected Result:

In Step 8, the button should disappear after bank account is added

Actual Result:

In Step 8, the button remains after bank account is added
In Step 9, the button leads to workspace list when bank account is added

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

Bug6628504_1728403401110.20241008_235741.mp4

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-10-30
• Automatic offers:
• abzokhattab | Contributor | 104666639

Issue Owner

Current Issue Owner: @parasharrajat

[melvin-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 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.

App/src/pages/home/report/ReportActionItemMessage.tsx

Line 139 in 6439b7b

{action.actionName === CONST.REPORT.ACTIONS.TYPE.IOU && ReportUtils.hasMissingInvoiceBankAccount(reportID) && (

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

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

What alternative solutions did you explore? (Optional)

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]

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

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

[melvin-bot]

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

[melvin-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]

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

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

🎀 👀 🎀 C+ reviewed

[melvin-bot]

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

[melvin-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]

📣 @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]

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

Screen.Recording.2024-11-07.at.23.27.23.mov

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?