fix: Redirect completed users from onboarding deep link routes to home#83193
fix: Redirect completed users from onboarding deep link routes to home#83193Beamanator merged 13 commits intomainfrom
Conversation
When a user who has completed onboarding clicks a deep link to /onboarding/purpose (e.g. from a Concierge message), the navigation silently fails because the OnboardingModalNavigator is not mounted. Add a check in OnboardingGuard to detect when a completed user is navigating to an onboarding route via deep link (RESET action) and redirect them to the home screen instead. Co-authored-by: Linh Vo <linhvovan29546@users.noreply.github.com>
Co-authored-by: Linh Vo <linhvovan29546@users.noreply.github.com>
|
Fixed the failing Prettier check. The imports in |
|
@MelvinBot Please add a comment to the test based on the instructions in this README: https://github.com/Expensify/App/blob/main/tests/README.md |
Following the test documentation guidelines in tests/README.md, each test now includes Given/When/Then comment sections that explain why the test exists and what behavior it verifies. Co-authored-by: Linh Vo <linhvovan29546@users.noreply.github.com>
|
Added Given/When/Then documentation comments to all 17 tests in
All 17 tests still pass after the changes. |
…eModal pattern Restructured the function to follow the same approach as isNavigatingToTestDriveModal in TestDriveModalGuard: - Accept state parameter and check current focused route first - Use string literal 'RESET' and findFocusedRoute on the action payload - Remove the manual routes array check for ONBOARDING_MODAL_NAVIGATOR - Remove unused NAVIGATORS import from both guard and test files - Simplified the RESET test payload to match the cleaner pattern Co-authored-by: Linh Vo <linhvovan29546@users.noreply.github.com>
…eModal pattern Restructures the function to follow the same approach as isNavigatingToTestDriveModal in TestDriveModalGuard: checks the current focused route first, then checks the RESET action payload. Removes the extra routes.some() check for ONBOARDING_MODAL_NAVIGATOR since findFocusedRoute traverses into nested states. Co-authored-by: Linh Vo <linhvovan29546@users.noreply.github.com>
Preferred CONST.NAVIGATION_ACTIONS.RESET over literal 'RESET' string for consistency with the rest of the file. Co-authored-by: Linh Vo <linhvovan29546@users.noreply.github.com>
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
| function isNavigatingToOnboardingFlow(state: NavigationState, action: NavigationAction): boolean { | ||
| const currentRoute = findFocusedRoute(state); | ||
| if (isOnboardingFlowName(currentRoute?.name)) { | ||
| return true; | ||
| } | ||
|
|
||
| if (action.type === CONST.NAVIGATION_ACTIONS.RESET && action.payload) { | ||
| const targetRoute = findFocusedRoute(action.payload as NavigationState); | ||
| if (isOnboardingFlowName(targetRoute?.name)) { | ||
| return true; | ||
| } | ||
| } | ||
|
|
||
| return false; | ||
| } | ||
|
|
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
Applied this suggestion. The isNavigatingToOnboardingFlow function now checks for NAVIGATE/PUSH actions targeting NAVIGATORS.ONBOARDING_MODAL_NAVIGATOR instead of checking RESET action payloads with findFocusedRoute. Updated tests accordingly — all 18 tests pass.
|
The only failing check is the PR Reviewer Checklist ( All code-related checks are passing: tests (all 8 jobs), typecheck, ESLint, Prettier, all builds, Storybook tests, codecov, verifySignedCommits, and CLA. No code fix is needed. |
|
The only failing check is the PR Reviewer Checklist (
This is not a code issue — it fails because no reviewer has posted the reviewer checklist comment on the PR yet. All code-related checks (ESLint, typecheck, Prettier, tests, builds, spellcheck, signed commits, etc.) are passing. This check will resolve automatically once a reviewer posts their checklist. |
|
@MelvinBot Apply this suggestion #83193 (comment) |
Apply review suggestion: instead of checking RESET action payloads with findFocusedRoute, check for NAVIGATE/PUSH actions that directly target NAVIGATORS.ONBOARDING_MODAL_NAVIGATOR. Updated tests to match. Co-authored-by: Linh Vo <linhvovan29546@users.noreply.github.com>
|
The only failing check is |
|
@MelvinBot Please update the PR title to follow the PR_AUTHOR_REVIEWER_BEST_PRACTICES.md guidelines https://github.com/Expensify/App/blob/main/contributingGuides/PR_AUTHOR_REVIEWER_BEST_PRACTICES.md |
MelvinBot
changed the title
|
Updated the PR title to follow the PR_AUTHOR_REVIEWER_BEST_PRACTICES.md convention: Before: Added the |
|
@MelvinBot Make the PR |
|
Marked this PR as Ready for review. |
Add two additional tests that verify the boundary conditions of the isNavigatingToOnboardingFlow function: - RESET actions containing onboarding routes are NOT intercepted - REPLACE actions targeting onboarding navigator are NOT intercepted These confirm the function only handles NAVIGATE and PUSH action types. Co-authored-by: Situ Chandra Shil <situchan@users.noreply.github.com>
…ttps://github.com/Expensify/App into claude-redirectCompletedUsersFromOnboardingRoutes Co-authored-by: Situ Chandra Shil <situchan@users.noreply.github.com>
There was a problem hiding this comment.
@MelvinBot update Tests step 4 a bit.
/onboarding/work path not exist. It leads to not found page. The correct path is /onboarding/work-email
|
@codex review |
![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: 95a3a542a6
ℹ️ 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".
|
Tests pass Screen.Recording.2026-02-23.at.10.16.32.PM.mov |
| import {getAdminPoliciesConnectedToNetSuite} from '@libs/actions/Policy/Policy'; | ||
| import Navigation from '@libs/Navigation/Navigation'; | ||
| import {useAccountingContext} from '@pages/workspace/accounting/AccountingContext'; | ||
| import {getInitialSubPageForNetsuiteTokenInput} from '@pages/workspace/accounting/netsuite/utils'; |
There was a problem hiding this comment.
FYI: This prettier issue is being fixed in #83221
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 95a3a542a6
ℹ️ 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".
…tedUsersFromOnboardingRoutes Co-authored-by: Situ Chandra Shil <situchan@users.noreply.github.com>
|
Merged |
|
@Beamanator all yours |
The inline comment was redundant with the JSDoc description above the function. Co-authored-by: flaviadefaria <flaviadefaria@users.noreply.github.com>
Beamanator
left a comment
Beamanator
left a comment
There was a problem hiding this comment.
Loving the minimal changes with all the clarified & added tests! 🙏
Beamanator
deleted the
claude-redirectCompletedUsersFromOnboardingRoutes
branch
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚧 @Beamanator 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! 🧪🧪
|
|
@linhvovan29546 @situchan This PR is failing because of a regression issue #83273 The issue is reproducible in: Web 
|
|
@mitarachim Apologies, that’s not a bug, just a typo generated by the AI. |
|
@linhvovan29546 Noted, Thanks for letting me know |
|
🚀 Deployed to staging by https://github.com/Beamanator in version: 9.3.25-0 🚀
|
Melvin skipped my message 😄 |
|
Updated the PR body — changed |
|
🚀 Deployed to production by https://github.com/puneetlath in version: 9.3.25-13 🚀
|
6 participants
Explanation of Change
When a user who has completed onboarding clicks a "Continue Onboarding" deep link (
/onboarding/purpose) from a Concierge message, nothing happens — the navigation silently fails. This is because theOnboardingModalNavigatoris conditionally excluded from the navigation stack whenisOnboardingCompleted !== false(inAuthScreens.tsx), so the route can't resolve. TheOnboardingGuardblindly returnsALLOWwhen onboarding is completed, without checking whether the user is trying to navigate to an onboarding route.This PR adds a check in
OnboardingGuard.evaluate()that detects when a completed user is navigating to an onboarding route via deep link (RESET action) and redirects them to the home screen instead of silently failing. This follows the same pattern established byTestDriveModalGuard, which redirects toROUTES.HOMEwhen a user navigates to a dismissed test drive modal.Fixed Issues
$ #79588
Tests
https://staging.new.expensify.com/onboarding/purposedirectly (or any/onboarding/*route)/onboarding/personal-detailsand/onboarding/work-emailnpx jest tests/unit/Navigation/guards/OnboardingGuard.test.ts— all tests passOffline tests
This change only affects deep link navigation routing logic, which is handled synchronously in the navigation guard. No network requests are involved. Offline behavior is unaffected.
QA Steps
https://staging.new.expensify.com/onboarding/purpose/onboarding/personal-detailsand/onboarding/work-emailPR 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
N/A - Navigation guard logic only, no UI changes
Android: mWeb Chrome
N/A - Navigation guard logic only, no UI changes
iOS: Native
N/A - Navigation guard logic only, no UI changes
iOS: mWeb Safari
N/A - Navigation guard logic only, no UI changes
MacOS: Chrome / Safari
N/A - Navigation guard logic only, no UI changes