[CP staging] Fix Plaid redirect issue on iOS#87996
Conversation
melvin-bot
bot
requested review from
ShridharGoel and
joekaufmanexpensify
and removed request for
a team
|
@ShridharGoel 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] |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| // The Plaid OAuth redirect URI for iOS (partners/plaid/oauth_ios) is not a registered route. | ||
| // When this URL is processed — either as a deep link or opened in Safari — redirect to home | ||
| // to avoid showing NotFound. The native Plaid SDK handles the actual OAuth callback separately. | ||
| if (normalizedPath.startsWith('/partners/plaid/oauth_ios')) { |
There was a problem hiding this comment.
❌ CONSISTENCY-2 (docs)
The string 'partners/plaid/oauth_ios' is a magic value used here and also in src/libs/actions/Link.ts. The adjacent code already follows the pattern of using CONST.SIGNIN_ROUTE for the /signin redirect path. This string should be defined as a named constant in CONST (e.g., CONST.PLAID_OAUTH_IOS_ROUTE) and referenced in both locations, as well as in src/libs/getPlaidLinkTokenParameters/index.ios.ts which also uses this string.
// In src/CONST/index.ts
PLAID_OAUTH_IOS_ROUTE: '/partners/plaid/oauth_ios',
// Then use it here:
if (normalizedPath.startsWith(CONST.PLAID_OAUTH_IOS_ROUTE)) {Reviewed at: 100003e | Please rate this suggestion with 👍 or 👎 to help us improve! Reactions are used to monitor reviewer efficiency.
| } | ||
|
|
||
| // The Plaid OAuth redirect URI is handled by the native Plaid SDK on iOS — skip navigation to avoid showing NotFound | ||
| if (route?.includes('partners/plaid/oauth_ios')) { |
There was a problem hiding this comment.
❌ CONSISTENCY-2 (docs)
Same magic string 'partners/plaid/oauth_ios' duplicated here. Extract to a shared constant in CONST alongside the existing CONST.SIGNIN_ROUTE pattern, and reuse it in both this file and getAdaptedStateFromPath.ts.
if (route?.includes(CONST.PLAID_OAUTH_IOS_ROUTE)) {
return;
}Reviewed at: 100003e | Please rate this suggestion with 👍 or 👎 to help us improve! Reactions are used to monitor reviewer efficiency.
Codecov Report❌ Looks like you've decreased code coverage for some files. Please write tests to increase, or at least maintain, the existing level of code coverage. See our documentation here for how to interpret this table.
|
![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: 100003e5c2
ℹ️ 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".
| if (normalizedPath.startsWith('/partners/plaid/oauth_ios')) { | ||
| normalizedPath = '/'; |
There was a problem hiding this comment.
Mapping '/partners/plaid/oauth_ios' to '/' still makes React Navigation treat the OAuth callback as a normal deep link and navigate away from the current screen. Because linkingConfig.subscribe forwards every incoming URL to the navigation listener, this path rewrite sends users to Home when they return from Plaid, which can unmount the in-progress Plaid connection flow before it completes (the expected behavior in this flow is to stay on the card-import step). This callback URL should be excluded from navigation handling rather than redirected to root.
Useful? React with 👍 / 👎.
|
🚧 @amyevans has triggered a test Expensify/App build. You can view the workflow run here. |
This comment has been minimized.
This comment has been minimized.
|
I will test this on iOS in the AM! |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeiOS: HybridAppiOS: mWeb SafariMacOS: Chrome / Safari |
amyevans
changed the title
amyevans
left a comment
amyevans
left a comment
There was a problem hiding this comment.
Per this thread we can only test this on staging, not ad hoc, so I'm approving and getting this CP'ed
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
…irect-issue [CP staging] Fix Plaid redirect issue on iOS (cherry picked from commit fca974f) (cherry-picked to staging by AndrewGable)
|
🚧 @amyevans 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! 🧪🧪
|
|
🚀 Cherry-picked to staging by https://github.com/AndrewGable in version: 9.3.60-6 🚀
Bundle Size Analysis (Sentry): |
|
No help site changes are required for this PR. This is a bug fix for iOS Plaid OAuth redirect handling — it prevents users from seeing a NotFound page when returning from Plaid bank authentication. The existing help articles (Manage Personal Cards, Set up a Direct Company Card Feed Connection) describe user-facing flows at the right abstraction level and don't need to document iOS-specific redirect behavior. The user experience is unchanged. |
5 participants
Explanation of Change
Fixed Issues
$ #87757
PROPOSAL:
Tests
Offline tests
QA Steps
// TODO: These must be filled out, or the issue title must include "[No QA]."
Same as Tests
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
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari