[$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. There should be a system message saying that they added a bank account as their invoice transfer 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?

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

Screenshots/Videos

Add any screenshot/video evidence

Current report

InvoiceConfusing2024-11-22_14-31-39.mp4

Original report

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-12-06
• Automatic offers:
• abzokhattab | Contributor | 104666639

Issue Owner

Current Issue Owner: @neil-marcellini

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

App/src/libs/ReportUtils.ts

Line 8444 in 3f9bf24

return invoiceReport?.ownerAccountID === currentUserAccountID && !getPolicy(invoiceReport?.policyID)?.invoice?.bankAccount?.transferBankAccountID && isSettled(iouReportID);

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.

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

Line 139 in 6439b7b

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

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

@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

[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?

[parasharrajat]

@neil-marcellini Bump.

[neil-marcellini]

Ok wow, this is a weird one. I tested it out myself and here's what I see for the response.

AddPersonalBankAccount response

{
    "jsonCode": 200,
    "requestID": "FcZtuw",
    "onyxData": [
        {
            "key": "userWallet",
            "onyxMethod": "merge",
            "value": {
                "walletLinkedAccountID": 97,
                "walletLinkedAccountType": "bankAccount"
            }
        },
        {
            "key": "bankAccountList",
            "onyxMethod": "merge",
            "value": {
                "97": {
                    "accountData": {
                        "accountNumber": "XXXXXXXXXXXX0000",
                        "additionalData": {
                            "acceptTerms": true,
                            "bankName": "",
                            "country": "US",
                            "currency": "USD",
                            "fieldsType": "local",
                            "plaidAccessToken": "",
                            "plaidAccountID": "dxg9K4nD6vck6rwbD3kZfMKjX9NkQxcJnaBv1"
                        },
                        "addressName": "Plaid Checking",
                        "allowDebit": false,
                        "bankAccountID": 97,
                        "created": "2024-11-22 18:54:57",
                        "defaultCredit": true,
                        "expensifyCardDomain": "",
                        "isExpensifyCardSettlementAccount": false,
                        "isSavings": false,
                        "processor": "bancorp",
                        "riskChecked": false,
                        "routingNumber": "011401533",
                        "sharees": [],
                        "state": "OPEN",
                        "type": "PERSONAL"
                    },
                    "accountType": "bankAccount",
                    "description": "Account ending in 0000",
                    "isDefault": true,
                    "key": "bankAccount-97",
                    "methodID": 97,
                    "title": "Plaid Checking"
                }
            }
        }
    ],
    "previousUpdateID": 7563,
    "lastUpdateID": 7564
}

So adding that bank account has nothing to do with setting the invoicing bank account, because we should see an Onyx update to send the policy.invoice.transferBankAccountID. However, I noticed that if you click on the bank account that has just been added under invoicing, then there is an option to make it a default payment method. When I click that it triggers SetInvoicingTransferBankAccount and that updates the transferBankAccountID. The add bank account button still doesn't go away until I refresh, but I think @abzokhattab's original proposal fixes that.

So overall I think we just need to add this extra step of making the bank account the default payment method. I will start a Slack discussion about the UX because it's super confusing and ideally if you choose to add a bank account for an invoice report it should automatically be the default invoicing bank account.

SetInvoicingTransferBankAccount response

{
    "accountID": 99,
    "accounts": {
        "99": {
            "lastUpdateID": 7565,
            "previousUpdateID": 7564,
            "updateIDs": [
                7565
            ]
        }
    },
    "authToken": "6D352EC54F5C8648B3EB17ED9F136809428D3786C0C11FBD244B71E94F2127E0838A3CB9B57AF1859E07AB6919134E80C1DC3EFDA736CC2BA499CC07197FB7D1A98EB430FDB50542EC956218A324629FA2C083C8FEA65BA4984A4C13234B3F1194B41368049D18C87E78746B280135907615FB4247371E664D2731C0B705A1683B64104396BC8559F1AE5883996BACE098EE7D2387482024500D3B35211F1FAF3E5E9083F2184837FC6C47284860F9541CBAE3949539CBD3351989C98A3E2048B588E722FF7C1C2DD914426179CE53390772BEEB5FE23BF265C040CD9AFB66723CA5CB333CBB411F5502C9B854DD83300BC111E66340807A2CEF15379660186926A8236746FE3C27041DE240E18761381D12FCD33D1053C3E782F1B3255AE652EE2827433E442534C101CBE19CAA7693",
    "email": "[email protected]",
    "fetchableOnyxUpdates": [
        "7565"
    ],
    "onyxUpdates": {
        "7565": [
            {
                "key": "policy_7E85A0D7A12E3885",
                "onyxMethod": "merge",
                "value": {
                    "invoice": {
                        "bankAccount": {
                            "transferBankAccountID": 97
                        }
                    }
                }
            }
        ]
    },
    "httpCode": 200,
    "jsonCode": 200,
    "authResponseMessage": "200 OK",
    "requestID": "Iu6amW",
    "onyxData": [
        {
            "key": "policy_7E85A0D7A12E3885",
            "onyxMethod": "merge",
            "value": {
                "invoice": {
                    "bankAccount": {
                        "transferBankAccountID": 97
                    }
                }
            }
        }
    ],
    "previousUpdateID": 7564,
    "lastUpdateID": 7565
}

[abzokhattab]

However, I noticed that if you click on the bank account that has just been added under invoicing, then there is an option to make it a default payment method. When I click that it triggers SetInvoicingTransferBankAccount and that updates the transferBankAccountID

so should we make this added bank account as the default acc first then implement my approach? or how should we go with this ?

[abzokhattab]

Kind reminder on this

[parasharrajat]

@neil-marcellini Bump.

[neil-marcellini]

Hi thanks. For some reason I accidentally created a separate issue which I have since closed. I think I forgot there was already an issue for this, sorry. I have updated the description with the new expected result. Please update your proposals accordingly.

[neil-marcellini]

However, I noticed that if you click on the bank account that has just been added under invoicing, then there is an option to make it a default payment method. When I click that it triggers SetInvoicingTransferBankAccount and that updates the transferBankAccountID

so should we make this added bank account as the default acc first then implement my approach? or how should we go with this ?

Good question. Yes I think I should update AddPersonalBankAccount to set policy.invoice.transferBankAccountID, when we trigger this flow from the button on an invoice report. Would you please update your proposal to describe how the backend can know that we're adding a personal bank account for invoicing? Maybe we pass a special parameter? Include all the changes you'll make on the frontend to achieve the expected result, and detail what you need the backend to do as well as the responses you expect to receive. Thanks!

[neil-marcellini]

@twisterdotcom why did we reduce the price to $125? Can we please bump it back up to $250 now that it's more involved?

[melvin-bot]

Upwork job price has been updated to $250

[abzokhattab]

Great i updated the proposal to add a new param to the AddPersonalBankAccount request sent to the backend... this way we can differentiate between invoices bank accounts and others

[neil-marcellini]

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.

Thanks @abzokhattab. Your updated proposal as of now looks good, and you can start on the frontend portion. I think we still need to make it clear to the user that the personal bank account was set as the invoice transfer bank accountID. I will plan to add a system message for this on the backend, and I don't think it will need any frontend changes.

I'll update the expected result to remove the idea of adding an informational page at the end of the bank account flow, because the system message should solve the problem.

[parasharrajat]

@abzokhattab Any update?

[abzokhattab]

I added the new param here #54052 and added a web video showing the value being set to true when adding a new invoice bank account...

please review and let me know about the next steps