Reuse existing Plaid feed on reconnect instead of creating a duplicate - #96349
Conversation
…licate Co-authored-by: Maxence Coulibaly <madmax330@users.noreply.github.com>
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
|
I looked into the failing checks — there's nothing to fix in code. Every automated check passes (typecheck, ESLint, all 8 The one red check is
That's a human-only check — it verifies the author has ticked the boxes in the PR Author Checklist and filled out the
I intentionally don't fill these out on an author's behalf. To turn this check green, |
|
@MelvinBot can you merge main? |
…ExistingFeed Co-authored-by: Maxence Coulibaly <madmax330@users.noreply.github.com>
|
🤖 Merged the latest |
|
There's no code fix to push here — all real CI checks are green. The only failing check is PR Author Checklist / checklist, and that isn't a code failure. It fails by design because the author checklist boxes are unticked and the Check statusPassing: Bun tests, Failing: Still running (non-blocking): |
|
@fedirjh it's the same as this for plaid: #95214 (comment) |
![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: 476b143e1a
ℹ️ 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".
| importPlaidAccounts( | ||
| publicToken, | ||
| plaidConnectedFeed, | ||
| feed, |
There was a problem hiding this comment.
When RefreshCardFeedConnectionPage refreshes an expired Plaid feed, this corrected import is immediately followed by setAssignCardStepAndData, which mounts BankConnection; both its web and native effects then call useImportPlaidAccounts(). That hook reads the bare plaidConnectedFeed stored as institutionId, so it queues another ImportPlaidAccounts request without the plaid. prefix or domainAccountID. Because this command has no conflict resolver, the second request can still take the create-new-feed branch and undo the intended duplicate-feed fix; preserve the existing feed in the state consumed by that hook or skip the second import for reconnects.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in a6893be — the concern was real, but only on native. Pushed a fix.
What was happening (native only): After the repair importPlaidAccounts call, PlaidConnectionStep runs setAssignCardStepAndData({cardToAssign: {institutionId: plaidConnectedFeed /* bare */}}, BANK_CONNECTION) and RefreshCardFeedConnectionPage mounts BankConnection with the feed prop. In the native effect the guard was if (feed && !isFeedExpired) — so while the repair is still in flight and the feed still reads as expired, that block is skipped and execution falls through to if (isPlaid) { onImportPlaidAccounts(); }, which reads the bare institutionId (no plaid. prefix, no domainAccountID) and queues a second ImportPlaidAccounts. With no conflict resolver, that second request can take the server's create-new-feed branch and undo the fix.
Why web was already safe: the web effect uses if (feed) { if (!isFeedExpired) {…} if (isPlaid) { return; } }, so a still-expired Plaid feed returns early and never fires a second import.
Fix: aligned the native effect with the web one so a Plaid reconnect returns instead of re-importing:
src/pages/workspace/companyCards/BankConnection/index.native.tsx:122-139
The add-new-feed path (feed undefined) is unchanged and still imports via onImportPlaidAccounts(); OAuth reconnects (feed present, not Plaid) are also unchanged.
Checks run locally
- ESLint (
scripts/lint.shon the changed file) — pass - React Compiler compliance (
check, both Babel + OXC) — compiled tests/unit/RefreshCardFeedConnectionPageTest.tsx— 10/10 passtypecheck-tsgo— no new errors in the changed file (the reported errors are pre-existing, in unrelated files); fulltscOOM'd at the 8 GB ceiling in this environment, not a type error.
|
There's nothing for me to fix here — all 36 real CI checks pass (build, tests, typecheck, ESLint, Oxfmt, spellcheck, perf, etc.). The only failing check is PR Reviewer Checklist / To clear it: the reviewer needs to complete the PR Reviewer Checklist (check all boxes in their review). The |
|
The only failing check is PR Reviewer Checklist / checklist, which failed with: This isn't a code or CI failure — it's not something I can fix by pushing a commit. That check scans the PR comments for a completed reviewer checklist and stays red until a human reviewer posts one. Every other check on this PR is green (tests, ESLint, typecheck, Oxfmt, build, Storybook, perf, etc.). To clear it: a reviewer needs to complete the PR Reviewer Checklist on this PR (it's normally posted by the assigned C+/reviewer). Once that checklist comment exists, this check will pass on the next run. No code changes are warranted here. |
Co-authored-by: {"message":"Not Found","documentation_url":"https://docs.github.com/rest/issues/comments#get-an-issue-comment","status":"404"} <{"message":"Not Found","documentation_url":"https://docs.github.com/rest/issues/comments#get-an-issue-comment","status":"404"}@users.noreply.github.com>
|
@ChavdaSachin 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] |
|
Nothing has changed since my previous comment. The only failing check is still PR Reviewer Checklist / checklist, failing with This is not a code/CI failure and re-running or pushing a commit won't clear it. It fails because no completed reviewer checklist has been posted on this PR yet. A reviewer (the assigned C+) needs to copy in and complete the PR Reviewer Checklist — once that comment exists, the check will pass on its next run. All other checks are green. |
|
@fedirjh can you review when you get a chance? |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeiOS: HybridAppiOS: mWeb SafariMacOS: Chrome / SafariCleanShot.2026-07-28.at.16.30.45.mp4
|
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚧 madmax330 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/madmax330 in version: 9.4.46-0 🚀
|
|
@madmax330 @mountiny Do we need any QA for this? |
|
🤖 I reviewed the changes in this PR against the help site files under No help site changes are required. Why: This PR is an internal bug fix. It changes how the NewDot "fix broken connection" flow builds the request it sends to the server (passing the existing prefixed The relevant article — Fix a Broken Company Card Feed Connection — already documents the intended behavior this fix restores:
The pre-fix duplicate-feed behavior was a bug, never documented, so the article is already accurate and needs no update. Accordingly, I did not create a draft docs PR. @madmax330, if you believe there's a user-facing aspect here worth documenting (e.g. guidance specific to Plaid feed reconnects), let me know and I'll draft the article changes. |
6 participants
Explanation of Change
The NewDot "fix broken connection" flow for Plaid company-card feeds created a duplicate feed instead of repairing the existing one. On reconnect,
PlaidConnectionStepderived a fresh feed from the Plaid Link metadata — a bare, unprefixed institution ID (e.g.ins_123456) — and passed that toimportPlaidAccounts, discarding the existingfeedprop it already held.Server-side,
ImportPlaidAccountsdecides repair-vs-create purely on whether the incomingfeedstarts with theplaid.prefix. A bare institution ID misses the prefix, so the server took its create-new-feed branch and minted a duplicate.This change makes the reconnect branch pass the existing
feed(which keeps itsplaid.prefix and originating#domainID) toimportPlaidAccountsinstead of the freshly-derived bare institution ID.importPlaidAccountsrunsgetCompanyCardFeed(feed), which strips only the#domainIDsuffix and preserves theplaid.prefix — so the server now sees a prefixed feed and repairs the existing feed in place rather than creating a second one. The new public token still refreshes the credentials.It also forwards the feed's originating
domainID(parsed from theCompanyCardFeedWithDomainID) to the server as a newdomainAccountIDparam when repairing an existing feed. This is the Plaid analog of the Chase reconnect fix (#95214): for a Classic domain-level feed surfaced into a workspace via "preferred workspace", it lets the server refresh credentials on the domain the broken cards actually belong to instead of the synthetic workspace-policy domain. The paired server change lives in Web-Expensify PR https://github.com/Expensify/Web-Expensify/pull/54605.When no feed is being repaired (adding a new feed), behavior is unchanged.
Fixed Issues
$ https://github.com/Expensify/Expensify/issues/653959
PROPOSAL: https://github.com/Expensify/Expensify/issues/653959#issuecomment-4935721846
Tests
// TODO: The human co-author must fill out the tests you ran before marking this PR as "ready for review". At minimum, verify that reconnecting a broken Plaid company-card feed repairs the existing feed instead of creating a duplicate.
Offline tests
QA Steps
// TODO: The human co-author must fill out the QA tests you ran before marking this PR as "ready for review", or the issue title must include "[No QA]."
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: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari