Keep the KYC continuation route when the bank account flow is left mid-setup - #97305
Keep the KYC continuation route when the bank account flow is left mid-setup#97305mukhrr wants to merge 2 commits into
Conversation
…d-setup Cancelling the personal bank account flow unmounted AddPersonalBankAccountPage, whose cleanup cleared the whole PERSONAL_BANK_ACCOUNT key. That dropped the onSuccessFallbackRoute seeded by the Pay/KYC entry point, so completing the flow on a second attempt fell back to goBack() instead of continuing to KYC. Preserve the routing data across an unmount that isn't a deliberate exit. Fixes Expensify#94659
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| }; | ||
|
|
||
| useEffect(() => clearPersonalBankAccount, []); | ||
| useEffect(() => { |
There was a problem hiding this comment.
❌ PERF-6 (docs)
This useEffect has no dependency array, so it runs on every render purely to write a value derived from personalBankAccount into flowRoutingDataRef. It performs no external side effect — it just recomputes derived state — which is exactly the "you might not need an Effect" case: a derived value should be computed directly in the component body rather than synced into a ref via an effect that fires every render.
Compute the routing data inline and keep the ref updated during the same render path that guards hasExitedFlowRef, e.g.:
const {onSuccessFallbackRoute, exitReportID} = personalBankAccount ?? {};
const flowRoutingData = !onSuccessFallbackRoute && !exitReportID ? undefined : {onSuccessFallbackRoute, exitReportID};
if (!hasExitedFlowRef.current) {
flowRoutingDataRef.current = flowRoutingData;
}This removes the extra effect while still capturing the latest routing data for the unmount cleanup.
Reviewed at: fe77cd7 | Please rate this suggestion with 👍 or 👎 to help us improve! Reactions are used to monitor reviewer efficiency.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fe77cd7c12
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| return; | ||
| } | ||
| const {onSuccessFallbackRoute, exitReportID} = personalBankAccount ?? {}; | ||
| flowRoutingDataRef.current = !onSuccessFallbackRoute && !exitReportID ? undefined : {onSuccessFallbackRoute, exitReportID}; |
There was a problem hiding this comment.
When an add-bank-account flow that has exitReportID reaches the success screen, the success/back branches call dismissModalWithReport() without setting hasExitedFlowRef, so this new preservation path writes {exitReportID} back to PERSONAL_BANK_ACCOUNT during unmount. That leaves a stale report redirect behind; a later EnablePayments add-bank-account attempt can start with openPersonalBankAccountSetupWithPlaid() (which only merges the setup type when no ID is passed) and AddBankAccount.exitFlow() prioritizes any existing exitReportID, dismissing to the old report instead of Wallet/KYC. Treat the exitReportID success dismissal as an exited flow, or only preserve it while the user is still mid-setup.
Useful? React with 👍 / 👎.
Preserving onSuccessFallbackRoute and exitReportID on unmount also re-seeded them after a flow that had already finished, so a later add-bank-account attempt could dismiss to a stale report instead of the wallet. Reaching exitFlow always means leaving on purpose, so mark the flow exited before any branch runs rather than only in the fallback one. Also scope the routing-data effect to personalBankAccount so it stops running on every render.
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
|
@dmkt9 kindly bump. thanks |
|
oh wait.. #95946 refactored the personal bank account flow into Retested on staging: after cancelling the bank account flow, REC-20260802181601.mp4 |
1 participant
Explanation of Change
When you pay someone from a 1:1 chat and pick "Pay with personal account", the KYC wall seeds
PERSONAL_BANK_ACCOUNTwith anonSuccessFallbackRoute(andexitReportIDwhere relevant) so the app knows where to continue once a bank account exists.AddPersonalBankAccountPagecleared that whole Onyx key on unmount:Backing out of the Plaid step calls
Navigation.goBack, which unmounts the page and wipes the key along with the continuation route. Re-entering throughAccountFlowEntryPointjust navigates, it never re-seeds it, so on the second, successful attemptexitFlow(true)can no longer satisfyshouldContinue && onSuccessFallbackRouteand falls through togoBack(). That drops you back on the Add bank account entry point instead of starting KYC.The fix keeps the routing data across an unmount that isn't a deliberate exit. A ref tracks the latest
onSuccessFallbackRoute/exitReportIDand hands it toclearPersonalBankAccount(preservedData)in the cleanup, and a second ref makes sure a deliberate exit still clears everything, so a stale route can't leak into the next flow regardless of render ordering.clearPersonalBankAccountalready acceptspreservedData, so nothing in the action layer changes.AccountFlowEntryPointdoes the same preservation on entry, so this is consistent with the existing pattern.Fixed Issues
$ #94659
PROPOSAL: #94659 (comment)
Tests
Check that a deliberate exit still clears the flow:
Offline tests
Adding a bank account needs the API (Plaid and
AddPersonalBankAccount), so there is no offline path through this flow. This change only touches in-memory routing state kept across an unmount, so offline behavior is unchanged.QA Steps
Same as tests.
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectionAvatar, 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.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_app.mp4
Android: mWeb Chrome
android_web.mp4
iOS: Native
ios_app.mp4
iOS: mWeb Safari
ios_web.mp4
MacOS: Chrome / Safari
web.mp4