-
Notifications
You must be signed in to change notification settings - Fork 4k
Reuse existing Plaid feed on reconnect instead of creating a duplicate #96349
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
+30
−10
Merged
Changes from all commits
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When
RefreshCardFeedConnectionPagerefreshes an expired Plaid feed, this corrected import is immediately followed bysetAssignCardStepAndData, which mountsBankConnection; both its web and native effects then calluseImportPlaidAccounts(). That hook reads the bareplaidConnectedFeedstored asinstitutionId, so it queues anotherImportPlaidAccountsrequest without theplaid.prefix ordomainAccountID. 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@MelvinBot can you address this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Addressed in a6893be — the concern was real, but only on native. Pushed a fix.
What was happening (native only): After the repair
importPlaidAccountscall,PlaidConnectionSteprunssetAssignCardStepAndData({cardToAssign: {institutionId: plaidConnectedFeed /* bare */}}, BANK_CONNECTION)andRefreshCardFeedConnectionPagemountsBankConnectionwith thefeedprop. In the native effect the guard wasif (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 toif (isPlaid) { onImportPlaidAccounts(); }, which reads the bareinstitutionId(noplaid.prefix, nodomainAccountID) and queues a secondImportPlaidAccounts. 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-139The add-new-feed path (
feedundefined) is unchanged and still imports viaonImportPlaidAccounts(); OAuth reconnects (feed present, not Plaid) are also unchanged.Checks run locally
scripts/lint.shon the changed file) — passcheck, both Babel + OXC) — compiledtests/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.