Fix NaN bankAccountID in VBBA flow after verify-account resume#85733
Fix NaN bankAccountID in VBBA flow after verify-account resume#85733NikkiWines merged 14 commits intoExpensify:mainfrom
Conversation
Reset reimbursement bank account draft and bankAccountID when resuming manual or Plaid setup after validation so resumed flow matches direct validated entry and prevents undefined bankAccountID values.
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
|
@MelvinBot implement the changes you suggested here in this PR. |
|
👋 I reviewed the suggested changes and here's exactly what needs to be implemented in this PR: 1. Add
|
|
@MelvinBot You are an assignee now. Make the changes. |
…ponents Add ?? CONST.DEFAULT_NUMBER_ID fallback to BankInfo, PersonalInfo, and CompleteVerification to prevent NaN when achData.bankAccountID is undefined. Also add Number.isFinite guard in connectBankAccountWithPlaid and connectBankAccountManually as a defensive API boundary check. Co-authored-by: Krishna Chaitanya <c3024@users.noreply.github.com>
|
I've pushed the changes from the proposal. Here's what was added:
These match the existing pattern used in |
|
@MelvinBot a check is failing |
Co-authored-by: Krishna Chaitanya <c3024@users.noreply.github.com>
|
Fixed the failing "Changed files ESLint check". The errors were pre-existing issues in files this PR touched:
|
|
@MelvinBot How complex are each of these to fix in this PR to avoid overriding linter? |
|
Here's a complexity breakdown of the three lint suppressions: 1.
|
|
Ok, fix this
and then fix the prettier workflow check too. |
|
@MelvinBot Ok, fix this
and then fix the prettier workflow check too. |
Replace useState with useRef for redirectedFromPlaidToManual in BankInfo.tsx since the value is only used as a guard inside useEffect and never used for rendering. Fix eslint-disable comment indentation in PersonalInfo.tsx. Co-authored-by: Krishna Chaitanya <c3024@users.noreply.github.com>
|
Fixed both issues:
|
|
@MelvinBot Jest tests are failing. |
|
The failing Jest test is unrelated to this PR's changes. Failing test: Why it's unrelated: This PR only modifies bank account/VBBA flow files ( Likely cause: The test file was recently updated on |
|
@MelvinBot merge main. |
…ountid-main-only Co-authored-by: Krishna Chaitanya <c3024@users.noreply.github.com>
|
Merged main into this branch. |
|
@ikevin127 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/VideosScreen.Recording.2026-03-19.at.18.20.38.mov |
|
We did not find an internal engineer to review this PR, trying to assign a random engineer to #85747 as well as to this PR... Please reach out for help on Slack if no one gets assigned! |
Normalize only NaN to DEFAULT_NUMBER_ID per review; avoid global isNaN coercion.
Centralize numeric bankAccountID resolution in ReimbursementAccountUtils and use it across USD reimbursement steps. Narrow useCallback deps to bankAccountID where submit read achData. Add unit tests with typed fixtures.
|
Addressed all review comments! |
…wnersStep Match other USD VBBA screens until useSubPage migration (Expensify#79039).
Place no-deprecated on line before useSubStep call; next-line only applies to following line.
|
🚧 @NikkiWines has triggered a test Expensify/App build. You can view the workflow run here. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚀 Deployed to staging by https://github.com/NikkiWines in version: 9.3.42-0 🚀
Bundle Size Analysis (Sentry): |
Explanation of Change
When an unvalidated user tries to connect a bank account to a workspace they are redirected to the validate account modal before removing the existing bank account details that sets the bankAccountID as 0. After validation, the flow resumes but
removeExistingBankAccountDetailsis not called and the bankAccountID remains unset and it becomes NaN and that is passed in the payload.Before the fix:
Screen.Recording.2026-03-19.at.1.29.30.PM.mov
This PR fixes this by calling
removeExistingBankAccountDetailsafter the account is validated.After the fix:
Screen.Recording.2026-03-19.at.1.48.04.PM.mov
Defensive
?? CONST.DEFAULT_NUMBER_IDfallback and normalizing non-finite bankAccountIDs toCONST.DEFAULT_NUMBER_IDare also added as Melvin suggested here.Fixed Issues
$ #85747
PROPOSAL: N/A
Tests
Offline tests
NA
QA Steps
Same as tests.
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
Screen.Recording.2026-03-19.at.1.48.04.PM.mov