Surface duplicate Plaid bank account error on USD VBBA flow#91491
Surface duplicate Plaid bank account error on USD VBBA flow#91491Abdulloh0109 wants to merge 2 commits into
Conversation
Reviewer Checklist
Screenshots/VideosScreen.Recording.2026-05-23.at.18.01.32.mov |
|
@Abdulloh0109 Couple of issues that need addressed on your side before I can complete the PR review:
Please tag me once these are completed so I can finalize PR review 🙌 |
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
|
@Abdulloh0109 Everything looks good except you're failing the ❌
Tag me once you force pushed your commit again to this PR with verified signature, then I'll review once again. Note that force push is usually not recommended due to messing with branching history - but in this case given it's only 1 commit and it's due to not having it verified, we can let it slide. There's also 1 |
04890b2 to
9977f27
Compare
|
@Abdulloh0109 Thanks for verifying your commit. There are two more things that need resolve before I can approve:
|
The page-level effect listed reimbursementAccount?.isLoading in its deps and unconditionally called hideBankAccountErrors() whenever the user was on the Plaid sub-step. When the backend duplicate-account error landed, isLoading flipped true → false, the effect re-ran on the same step, and the fresh error was wiped before AddPlaidBankAccount could render it, making Next look unresponsive on re-selecting an existing Plaid account. Gate the clear to actual step transitions via usePrevious on currentStep and achData?.subStep. Because this codebase's usePrevious initializes its ref with the current value (rather than undefined), also track the first render with isFirstRenderRef so the initial mount on the Plaid sub-step (e.g. page reload, deeplink) still clears truly stale errors from a prior session. Fixes Expensify#89064
9977f27 to
31b7ce5
Compare
|
@ikevin127 Sorry for that — removed the accidental file and merged with latest main. Ready for re-review 🙌 |
|
@Abdulloh0109 Thanks for removing the file. For some reason it's still showing merge conflicts as you simply removed the unwanted file and force-pushed again. If you can, please pull the latest The new commit should simply be a 'merge branch main' and conflicts solved type of change, no additional files should be added. Here's an example of a final merge branch main commit from another PR for reference: |



Explanation of Change
On Workspace → Workflows → Payments, when an admin adds a Plaid bank account, disconnects it, then re-runs Add bank account → Log into your bank and re-selects the same Plaid account, tapping Next previously did nothing visible — no error, no toast, no advance.
The page-level
useEffectinReimbursementAccountPage.tsxlistedreimbursementAccount?.isLoadingin its deps and unconditionally calledhideBankAccountErrors()whenever the user was on the Plaid sub-step. When the backend duplicate-account error landed,isLoadingflippedtrue → false, the effect re-ran on the same step, and the fresh error was wiped from Onyx beforeAddPlaidBankAccountcould render it via the existingFormAlertWithSubmitButton. Result: Next looked unresponsive on re-selecting an existing Plaid account.This change gates the clear to actual step transitions via
usePreviousoncurrentStepandachData?.subStep. Because this codebase'susePreviousinitializes its ref with the current value (rather thanundefined), the first render is also tracked withisFirstRenderRefso the initial mount on the Plaid sub-step (e.g. page reload, deeplink) still clears truly stale errors from a prior session — while fresh in-flight errors from a just-completed API call survive subsequentisLoadingtoggles.Per the issue's Implementation Note, only
ReimbursementAccountPage.tsxis modified — the existing error display path inAddPlaidBankAccount→FormAlertWithSubmitButtonis sufficient to render the error once the page-level wipe is gated.Fixed Issues
$ #89064
PROPOSAL: #89064 (comment)
Tests
user_good/pass_good).Offline tests
QA Steps
Same as Tests above.
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
iOS: App
20260520_201235.mp4
Windows: Chrome
20260520_174644.mp4
MacOS: Chrome / Safari
REC-20260525203836.mp4
Android: App
REC-202605252022836.mp4
Android: mWeb Chrome
1.mp4
iOS: mWeb Chrome
2.mp4