Honor the selected bank account when paying an expense report from the report preview#93110
Honor the selected bank account when paying an expense report from the report preview#93110KioCoan wants to merge 11 commits into
Conversation
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
|
@marufsharifi Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeiOS: HybridAppiOS: mWeb SafariMacOS: Chrome / Safari |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
ReviewThe change is correct and faithfully mirrors the existing header pay flow. The Verified against the header (
All CI checks (typecheck, tests, lint) are green, and the two added test files cover the wiring. One minor non-blockerThe intermediate component still types onHoldMenuOpen: (requestType: string, paymentType?: PaymentMethodType, canPay?: boolean) => void;It forwards this prop straight into Why the runtime path is safe despite the type gap
LGTM once the optional type tidy-up above is considered. Nice job matching the existing header behavior and adding targeted tests. |
|
@KioCoan, is this a No QA pr? thanks. |
@danieldoglas should we add a No QA to this? Or maybe handle it internally. |
trjExpensify
left a comment
There was a problem hiding this comment.
Thanks for catching this! 👍
|
I think our QA team should be able to test this. @KioCoan, I think this PR does not use the accountNumber in the returned in reportAction message, which will show the correct bank account. |
|
I included the account number changes and added the test evidences to the screenshot section. |
Explanation of Change
When a payer chooses a specific business bank account (VBBA) to reimburse an expense report, that selection has to reach the backend as a top-level
bankAccountIDso the payment is drawn from the chosen account rather than the workspace's default reimburser.The report header pay flows already send the selected account. The report preview pay flow (paying from the expense preview inside the chat) was dropping the selected
methodID, so the backend always fell back to the workspace default. This PR threads the selected account through both report-preview pay paths so they match the header behavior.Changes:
PayActionButton.tsxonHoldMenuOpenprop now carries a trailingmethodID?: number.methodIDtoonHoldMenuOpen.payMoneyRequestcall now sendsmethodID: type === VBBA ? methodID : undefined, matching the header components.MoneyRequestReportPreviewContent.tsxmethodIDin state, captures it inhandleHoldMenuOpen, and passes it to<ProcessMoneyReportHoldMenu>(which already forwardsmethodID/bankAccountIDtopayMoneyRequest).tests/ui/components/PayActionButtonTest.tsxcovering the wiring.Fixed Issues
$ https://github.com/Expensify/Expensify/issues/644593
Tests
Prerequisite test data (payer = User A, recipient = User B):
Pay from the report preview inside the chat (the expense preview card, not the report header):
Offline tests
N/A
QA Steps
Prerequisite test data (payer = User A, recipient = User B):
Pay from the report preview inside the chat (the expense preview card, not the report header):
PayMoneyRequestrequest is sent with a top-levelbankAccountIDequal to the selected account's id.PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari
Report preview in chat with the Pay dropdown open. Both of the payer's verified business bank accounts are listed (Ending in 4191 and Ending in 0000, the workspace default reimburser), so the payer can choose which one to pay from.

Report paid from the preview after selecting the non-default account (4191). The PayMoneyRequest request body included bankAccountID: 2050811 and the backend reimbursed from that account: the REIMBURSED action returned accountNumber: "XXXXXX4191" and the report message shows "paid with bank account 4191" instead of the workspace default (0000).

Control test: a second report paid selecting the workspace default account (0000) in the same dropdown. The message correctly shows "paid with bank account 0000", confirming the displayed account follows the selection and not a hardcoded value.

Same paid report viewed by the submitter: the message renders "paid with bank account" with no digits. This is because the IOU pay action's originalMessage doesn't include accountNumber (only the REIMBURSED action does, and that action is hidden in NewDot). The FE now prefers accountNumber wherever present, so adding it to the IOU action message in PayMoneyRequest would complete this for all viewers.
