Repair the originating domain feed on NewDot company card reconnect - #95214
Conversation
Co-authored-by: Maxence Coulibaly <madmax330@users.noreply.github.com>
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
|
No code fix is needed — neither red check is a real problem with this PR's code. Every code/CI check passes: 1. Why it's unrelated
2. tl;dr: re-run the iOS build, complete the author checklist — no code change required. |
|
🤖 I analyzed all the checks. No code fix is needed — neither failure is caused by this PR's changes. Two checks are red:
Everything that could be affected by the code change is green: Recommended next step: Re-run the failed |
There was a problem hiding this comment.
Pull request overview
This PR updates the company card bank reconnect URL generation so that, when repairing an existing company-card feed, the reconnect flow can target the feed’s originating domain (via a domainAccountID query param) instead of always targeting the synthetic workspace-policy domain.
Changes:
- Extend
getCompanyCardBankConnection()to accept an optionalfeedand, when present, derive and appenddomainAccountIDto the OAuth callback URL. - Forward the
feedprop into the URL builder from both web and nativeBankConnectionentry points.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/pages/workspace/companyCards/BankConnection/index.tsx | Passes feed into the company-card bank connection URL builder (web). |
| src/pages/workspace/companyCards/BankConnection/index.native.tsx | Passes feed into the company-card bank connection URL builder (native). |
| src/libs/actions/getCompanyCardBankConnection/index.tsx | Adds optional feed handling and appends domainAccountID to the callback URL when available. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
🤖 I looked into the failing checks. No code fix is needed — the only real failure is unrelated to this PR. Failing checks:
Every code-quality check passed: typecheck, ESLint, Prettier, Bun tests, all 8 Jest shards, Storybook, knip, and the Why the iOS build failure is unrelatedThis PR changes only 3 TypeScript files under The failed build ended with a generic To clear it, re-run the failed |
|
@MelvinBot can you merge main? |
1 similar comment
|
@MelvinBot can you merge main? |
|
🤖 I looked into the failing checks — no code fix is needed. Here's the breakdown: 1. 2. This is an infrastructure flake, not a real failure:
Recommended action: Just re-run the failed iOS build job — it should pass on retry. I don't have Actions permission to re-run it myself ( |
|
@thelullabyy 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] |
|
🤖 No fix needed — the failing Investigation details
Current status: |
|
🤖 No failing checks to fix — all checks on this PR are currently passing. The only check that ever reported a failure was No push was needed. |
|
@fedirjh @madmax330 Please let me know if needing my review on this PR. Thanks |
|
@thelullabyy no need, thanks. @fedirjh can you review please? |
|
@madmax330 I think we need to fix it for plaid as well, This still creates duplicate feed
|
|
@fedirjh we'll do Plaid as a follow up based on Melvin's analysis here: https://github.com/Expensify/Expensify/issues/653959#issuecomment-4935721846 Since we need a separate Web-Expensify PR for it |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeiOS: HybridAppiOS: mWeb SafariMacOS: Chrome / Safari |
|
🚧 mountiny 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/mountiny in version: 9.4.34-0 🚀
|
|
🤖 No help site changes required. I reviewed the changes in this PR against the help articles under Why: This is an internal plumbing fix. It changes how the reconnect flow builds the OAuth callback URL — passing the feed's originating
The fix restores the intended behavior that the existing article already documents — that selecting the broken-connection error and following the prompts reconnects the feed and resumes importing transactions:
Both articles describe the correct behavior after this fix, so no edits are warranted.
@madmax330, no linked help site PR was created since no documentation changes are required. If you believe a specific article should be updated to reflect this change, let me know which one and I'll draft the update. |
|
Any QA steps here? |
|
@IuliiaHerets Not sure if you can test it, can you follow the testing steps from OP https://github.com/Expensify/Expensify/issues/653959 ? |
|
@fedirjh I don't think soo |
|
🚀 Deployed to production by https://github.com/roryabraham in version: 9.4.34-14 🚀
|
|
🚀 Deployed to production by https://github.com/roryabraham in version: 9.4.34-14 🚀
Bundle Size Analysis (Sentry): |

Explanation of Change
When a Chase (OAuth) company-card feed is set up at the domain level in Classic and surfaced into a NewDot workspace via "preferred workspace", clicking "Log in to your bank to fix it" does not repair the original domain feed. NewDot's reconnect flow always targets the synthetic workspace-policy domain (
expensify-policy<policyID>.exfy), so the fresh credentials land on a separate workspace-scoped feed while the original domain feed and its cards stay broken (import fails with438"Could not obtain fresh credentials").Root cause:
getCompanyCardBankConnectionalways sendsdomainName: getDomainNameForPolicy(policyID), which unconditionally resolves to the synthetic policy domain — there is no path that identifies the real feed being repaired.This PR passes the feed's originating domain account ID to the reconnect callback so the server can target the correct feed:
getCompanyCardBankConnectionnow accepts thefeed(aCompanyCardFeedWithDomainID, i.e.feedName#domainID). When a feed is present (repairing an existing feed, not adding a new one), it parses thedomainIDout of the feed key viasplitCardFeedWithDomainIDand adds it to the callback URL as a newdomainAccountIDparam.BankConnectionentry points (web and native) forward the existingfeedprop into the URL builder.When no feed is provided (the add-new-card flow), no
domainAccountIDis sent and the behavior is unchanged.This is one half of a cross-repo fix and does not repair feeds on its own. The server must consume the new
domainAccountIDparam to resolve the real domain and write credentials there. Companion PR: https://github.com/Expensify/Web-Expensify/pull/54188Fixed Issues
$ https://github.com/Expensify/Expensify/issues/653959
PROPOSAL: https://github.com/Expensify/Expensify/issues/653959#issuecomment-4847372405
Tests
// TODO: The human co-author must fill out the tests you ran before marking this PR as "ready for review"
// Please describe what tests you performed that validates your change worked.
Offline tests
// TODO: The human co-author must fill out the offline tests you ran before marking this PR as "ready for review".
QA Steps
// TODO: The human co-author must fill out the QA tests you ran before marking this PR as "ready for review".
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectioncanBeMissingparam foruseOnyxtoggleReportand 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
MacOS: Chrome / Safari