Clear stale bankAccountID when starting new bank account setup#83893
Clear stale bankAccountID when starting new bank account setup#83893cristipaval merged 1 commit intomainfrom
Conversation
When adding a new bank account after completing one, the backend returns existing bank account data including bankAccountID. The removeExistingBankAccountDetails function only cleared draft form values but not achData.bankAccountID, causing Manual.tsx to render routing and account number fields as disabled. Co-authored-by: Jayesh Mangwani <jayeshmangwani@users.noreply.github.com>
|
I created this PR from an upstream branch since I don't have push access to your fork. To take ownership of this branch and be able to push updates, run: Then you can close this PR and open a new one from your fork. |
|
@codex review |
|
Codex Review: Something went wrong. Try again later by commenting “@codex review”. ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
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". |
|
@MelvinBot, can you please review the usage of bankAccountID? I suspect that setting it to 0 could cause a regression. |
🔍 bankAccountID Usage Analysis
Result: Low regression riskSetting Key findingsTruthy/falsy checks (21 consumers) — no difference between
API calls: The backend treats One stylistic noteExisting reset/clear functions (e.g., An alternative approach that would be more consistent with existing patterns would be to pass ConclusionNo regression expected. The change is safe for all current consumers. |
|
@MelvinBot please mark PR as Ready for review |
|
Done — PR has been marked as Ready for review. |
|
@jayeshmangwani 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.movAndroid: mWeb Chromemweb-chorme.moviOS: HybridAppios.moviOS: mWeb Safarimweb-safari.movMacOS: Chrome / Safariweb.mov |
|
We did not find an internal engineer to review this PR, trying to assign a random engineer to #82911 as well as to this PR... Please reach out for help on Slack if no one gets assigned! |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚧 @cristipaval 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! 🧪🧪
|
|
🚀 Deployed to staging by https://github.com/cristipaval in version: 9.3.38-0 🚀
|
|
🚀 Deployed to production by https://github.com/cristipaval in version: 9.3.38-4 🚀
|
Explanation of Change
When a user completes the first bank account setup via the Expensify Card "Issue new card" flow, then tries to add a second bank account ("Add a new bank account" > "Connect manually" > "Next"), the routing number and account number fields are rendered as disabled.
The root cause is that
removeExistingBankAccountDetails()inVerifiedBankAccountFlowEntryPoint.tsxonly clears the draft form values but does not clearachData.bankAccountIDfrom theREIMBURSEMENT_ACCOUNTOnyx key. Since the backend returns the existing completed bank account data (includingbankAccountID) for the same policy,Manual.tsxsees!!achData.bankAccountIDastrueand disables the fields.This PR adds a call to
updateReimbursementAccount({bankAccountID: 0})inremoveExistingBankAccountDetails()to clear the stalebankAccountID, so the fields render as editable when starting a fresh bank account setup.Fixed Issues
$ #82911
PROPOSAL: #82911 (comment)
Tests
Offline tests
N/A - This change only affects the state cleanup when navigating between bank account setup flows. The bank account setup requires an active connection.
QA Steps
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