From 5b7c186dd5909f45a1c665ace72046d805376822 Mon Sep 17 00:00:00 2001 From: MelvinBot Date: Mon, 23 Feb 2026 10:38:59 +0000 Subject: [PATCH 1/9] Redirect completed users away from onboarding routes to home 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 --- src/libs/Navigation/guards/OnboardingGuard.ts | 27 +++++++ .../Navigation/guards/OnboardingGuard.test.ts | 78 +++++++++++++++++++ 2 files changed, 105 insertions(+) diff --git a/src/libs/Navigation/guards/OnboardingGuard.ts b/src/libs/Navigation/guards/OnboardingGuard.ts index e9ffc469b20a..33f5d8a6a5c3 100644 --- a/src/libs/Navigation/guards/OnboardingGuard.ts +++ b/src/libs/Navigation/guards/OnboardingGuard.ts @@ -11,6 +11,7 @@ import {isOnboardingFlowName} from '@libs/Navigation/helpers/isNavigatorName'; import {getOnboardingInitialPath} from '@userActions/Welcome/OnboardingFlow'; import CONFIG from '@src/CONFIG'; import CONST from '@src/CONST'; +import NAVIGATORS from '@src/NAVIGATORS'; import ONYXKEYS from '@src/ONYXKEYS'; import type {Route} from '@src/ROUTES'; import ROUTES from '@src/ROUTES'; @@ -129,6 +130,25 @@ function shouldPreventReset(state: NavigationState, action: NavigationAction) { return false; } +/** + * Check if the navigation action is targeting an onboarding screen. + * This handles RESET actions (URL-based navigation / deep links). + */ +function isNavigatingToOnboardingFlow(action: NavigationAction): boolean { + if (action.type !== CONST.NAVIGATION_ACTIONS.RESET || !action.payload) { + return false; + } + + const targetFocusedRoute = findFocusedRoute(action.payload as NavigationState); + if (isOnboardingFlowName(targetFocusedRoute?.name)) { + return true; + } + + // Also check if the target state contains the onboarding modal navigator directly + const routes = (action.payload as NavigationState).routes; + return routes?.some((route) => route.name === NAVIGATORS.ONBOARDING_MODAL_NAVIGATOR) ?? false; +} + /** * OnboardingGuard handles ONLY the core NewDot onboarding flow */ @@ -148,6 +168,13 @@ const OnboardingGuard: NavigationGuard = { // eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing const isInvitedOrGroupMember = (!CONFIG.IS_HYBRID_APP && (hasNonPersonalPolicy || wasInvitedToNewDot)) ?? false; + // Redirect completed users who try to navigate to onboarding routes (e.g. via deep link) + // The OnboardingModalNavigator is not mounted when onboarding is complete, so the route would silently fail + if (isOnboardingCompleted && isNavigatingToOnboardingFlow(action)) { + Log.info('[OnboardingGuard] Redirecting completed user away from onboarding route to home'); + return {type: 'REDIRECT', route: ROUTES.HOME}; + } + const shouldSkipOnboarding = context.isLoading || isTransitioning || isOnboardingCompleted || isMigratedUser || isSingleEntry || needsExplanationModal || isInvitedOrGroupMember; if (shouldSkipOnboarding) { diff --git a/tests/unit/Navigation/guards/OnboardingGuard.test.ts b/tests/unit/Navigation/guards/OnboardingGuard.test.ts index 3f04d1ded1d2..31583750a2f3 100644 --- a/tests/unit/Navigation/guards/OnboardingGuard.test.ts +++ b/tests/unit/Navigation/guards/OnboardingGuard.test.ts @@ -3,6 +3,7 @@ import Onyx from 'react-native-onyx'; import OnboardingGuard from '@libs/Navigation/guards/OnboardingGuard'; import type {GuardContext} from '@libs/Navigation/guards/types'; import CONST from '@src/CONST'; +import NAVIGATORS from '@src/NAVIGATORS'; import ONYXKEYS from '@src/ONYXKEYS'; import SCREENS from '@src/SCREENS'; import waitForBatchedUpdates from '../../../utils/waitForBatchedUpdates'; @@ -163,6 +164,83 @@ describe('OnboardingGuard', () => { }); }); + describe('redirect completed users away from onboarding routes', () => { + it('should redirect to HOME when completed user navigates to onboarding via deep link (RESET with onboarding screen)', async () => { + await Onyx.merge(ONYXKEYS.NVP_ONBOARDING, { + hasCompletedGuidedSetupFlow: true, + }); + await waitForBatchedUpdates(); + + const resetToOnboardingAction: NavigationAction = { + type: CONST.NAVIGATION_ACTIONS.RESET, + payload: { + key: 'root', + index: 1, + routeNames: [SCREENS.HOME, NAVIGATORS.ONBOARDING_MODAL_NAVIGATOR], + routes: [ + {key: 'home', name: SCREENS.HOME}, + { + key: 'onboarding', + name: NAVIGATORS.ONBOARDING_MODAL_NAVIGATOR, + state: { + key: 'onboarding-nav', + index: 0, + routeNames: [SCREENS.ONBOARDING.PURPOSE], + routes: [{key: 'purpose', name: SCREENS.ONBOARDING.PURPOSE}], + stale: false, + type: 'stack', + }, + }, + ], + stale: false, + type: 'root', + }, + }; + + const result = OnboardingGuard.evaluate(mockState, resetToOnboardingAction, authenticatedContext) as {type: 'REDIRECT'; route: string}; + + expect(result.type).toBe('REDIRECT'); + expect(result.route).toBe('home'); + }); + + it('should ALLOW when completed user navigates to a non-onboarding route', async () => { + await Onyx.merge(ONYXKEYS.NVP_ONBOARDING, { + hasCompletedGuidedSetupFlow: true, + }); + await waitForBatchedUpdates(); + + const resetToHomeAction: NavigationAction = { + type: CONST.NAVIGATION_ACTIONS.RESET, + payload: { + key: 'root', + index: 0, + routeNames: [SCREENS.HOME], + routes: [{key: 'home', name: SCREENS.HOME}], + stale: false, + type: 'root', + }, + }; + + const result = OnboardingGuard.evaluate(mockState, resetToHomeAction, authenticatedContext); + expect(result.type).toBe('ALLOW'); + }); + + it('should ALLOW non-RESET actions for completed users (e.g. NAVIGATE)', async () => { + await Onyx.merge(ONYXKEYS.NVP_ONBOARDING, { + hasCompletedGuidedSetupFlow: true, + }); + await waitForBatchedUpdates(); + + const navigateAction: NavigationAction = { + type: 'NAVIGATE', + payload: {name: SCREENS.HOME}, + }; + + const result = OnboardingGuard.evaluate(mockState, navigateAction, authenticatedContext); + expect(result.type).toBe('ALLOW'); + }); + }); + describe('redirect to onboarding', () => { it('should redirect when authenticated user needs onboarding', async () => { await Onyx.merge(ONYXKEYS.NVP_ONBOARDING, { From 9d74f8c15c368c56c117b5239492ca0b6690a66f Mon Sep 17 00:00:00 2001 From: MelvinBot Date: Mon, 23 Feb 2026 10:43:13 +0000 Subject: [PATCH 2/9] Fix: Sort imports in de.ts to match Prettier configuration Co-authored-by: Linh Vo --- src/languages/de.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/languages/de.ts b/src/languages/de.ts index 544183b1ed77..a14798c00783 100644 --- a/src/languages/de.ts +++ b/src/languages/de.ts @@ -65,8 +65,11 @@ import type { UpdatedPolicyBudgetNotificationParams, UpdatedPolicyCategoriesParams, UpdatedPolicyCategoryMaxAmountNoReceiptParams, + UpdatedPolicyCurrencyDefaultTaxParams, + UpdatedPolicyCustomTaxNameParams, UpdatedPolicyCustomUnitSubRateParams, UpdatedPolicyDefaultTitleParams, + UpdatedPolicyForeignCurrencyDefaultTaxParams, UpdatedPolicyManualApprovalThresholdParams, UpdatedPolicyOwnershipParams, UpdatedPolicyPreventSelfApprovalParams, @@ -76,9 +79,6 @@ import type { UpdatedPolicyReportFieldDefaultValueParams, UpdatedPolicyTagFieldParams, UpdatedPolicyTagListParams, - UpdatedPolicyCurrencyDefaultTaxParams, - UpdatedPolicyCustomTaxNameParams, - UpdatedPolicyForeignCurrencyDefaultTaxParams, UpdatedPolicyTagListRequiredParams, UpdatedPolicyTagNameParams, UpdatedPolicyTagParams, From c611041ee63c27534bea8b3c62b0438cf736e3d7 Mon Sep 17 00:00:00 2001 From: MelvinBot Date: Mon, 23 Feb 2026 11:15:41 +0000 Subject: [PATCH 3/9] Add Given/When/Then documentation comments to OnboardingGuard tests 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 --- .../Navigation/guards/OnboardingGuard.test.ts | 62 ++++++++++++++++--- 1 file changed, 55 insertions(+), 7 deletions(-) diff --git a/tests/unit/Navigation/guards/OnboardingGuard.test.ts b/tests/unit/Navigation/guards/OnboardingGuard.test.ts index 31583750a2f3..37e99ab7b2e4 100644 --- a/tests/unit/Navigation/guards/OnboardingGuard.test.ts +++ b/tests/unit/Navigation/guards/OnboardingGuard.test.ts @@ -40,40 +40,50 @@ describe('OnboardingGuard', () => { describe('early return when onboarding completed', () => { it('should return ALLOW when user has completed onboarding', async () => { + // Given a user who has already completed the guided setup flow, meaning they've finished all onboarding steps await Onyx.merge(ONYXKEYS.NVP_ONBOARDING, { hasCompletedGuidedSetupFlow: true, }); await waitForBatchedUpdates(); + // When the guard evaluates a standard navigation action const result = OnboardingGuard.evaluate(mockState, mockAction, authenticatedContext); + + // Then navigation should be allowed because completed users should not be forced back into onboarding expect(result.type).toBe('ALLOW'); }); it('should return ALLOW when onboarding data is undefined (old/migrated accounts)', async () => { - // Empty/null onboarding means old account - considered completed + // Given a user with null onboarding data, which indicates an old or migrated account that predates the guided setup flow await Onyx.set(ONYXKEYS.NVP_ONBOARDING, null); await waitForBatchedUpdates(); + // When the guard evaluates a navigation action const result = OnboardingGuard.evaluate(mockState, mockAction, authenticatedContext); + + // Then navigation should be allowed because null onboarding data is treated as "completed" to avoid forcing legacy users through onboarding expect(result.type).toBe('ALLOW'); }); }); describe('early exit conditions', () => { it('should allow during app transition', () => { + // Given an authenticated user whose current URL contains a transition path, indicating the app is mid-transition between states const transitionContext: GuardContext = { isAuthenticated: true, isLoading: false, currentUrl: 'https://new.expensify.com/transition', }; + // When the guard evaluates during the transition const result = OnboardingGuard.evaluate(mockState, mockAction, transitionContext); + // Then navigation should be allowed because the guard should not interfere with app transitions to avoid breaking the transition flow expect(result.type).toBe('ALLOW'); }); it('should BLOCK RESET action when user is on onboarding and tries to reset to non-onboarding screen', async () => { - // User is currently on an onboarding screen + // Given a user who is currently on the onboarding purpose screen and has not yet completed onboarding const onboardingState: NavigationState = { key: 'root', index: 0, @@ -83,7 +93,7 @@ describe('OnboardingGuard', () => { type: 'root', }; - // RESET action trying to navigate to a non-onboarding screen (HOME) + // When a RESET action attempts to navigate them away from onboarding to the HOME screen const resetAction: NavigationAction = { type: CONST.NAVIGATION_ACTIONS.RESET, payload: { @@ -98,6 +108,7 @@ describe('OnboardingGuard', () => { const result = OnboardingGuard.evaluate(onboardingState, resetAction, authenticatedContext) as {type: 'BLOCK'; reason?: string}; + // Then the action should be blocked because users who haven't completed onboarding should not be able to skip it via a RESET action expect(result.type).toBe('BLOCK'); expect(result.reason).toBe('Cannot reset to non-onboarding screen while on onboarding'); }); @@ -105,17 +116,21 @@ describe('OnboardingGuard', () => { describe('skip onboarding conditions', () => { it('should allow when onboarding is completed', async () => { + // Given a user who has already completed the guided setup flow await Onyx.merge(ONYXKEYS.NVP_ONBOARDING, { hasCompletedGuidedSetupFlow: true, }); await waitForBatchedUpdates(); + // When the guard evaluates a navigation action const result = OnboardingGuard.evaluate(mockState, mockAction, authenticatedContext); + // Then navigation should be allowed because completed users should bypass all onboarding checks expect(result.type).toBe('ALLOW'); }); it('should allow migrated users', async () => { + // Given a user who was migrated from the classic app via the nudge migration flow, which means they already have an established account await Onyx.merge(ONYXKEYS.NVP_TRY_NEW_DOT, { classicRedirect: { dismissed: false, @@ -127,50 +142,63 @@ describe('OnboardingGuard', () => { }); await waitForBatchedUpdates(); + // When the guard evaluates a navigation action const result = OnboardingGuard.evaluate(mockState, mockAction, authenticatedContext); + // Then navigation should be allowed because migrated users should skip onboarding since they are already familiar with the product expect(result.type).toBe('ALLOW'); }); it('should allow users with single entry from HybridApp', async () => { + // Given a user who entered NewDot as a single-entry from HybridApp, meaning they are temporarily viewing NewDot from the classic app await Onyx.merge(ONYXKEYS.HYBRID_APP, { isSingleNewDotEntry: true, }); await waitForBatchedUpdates(); + // When the guard evaluates a navigation action const result = OnboardingGuard.evaluate(mockState, mockAction, authenticatedContext); + // Then navigation should be allowed because single-entry HybridApp users should not be forced into onboarding for a temporary visit expect(result.type).toBe('ALLOW'); }); it('should allow users with non-personal policies', async () => { + // Given a user who belongs to a non-personal (e.g. corporate) policy, indicating they were added to a workspace await Onyx.merge(ONYXKEYS.HAS_NON_PERSONAL_POLICY, true); await waitForBatchedUpdates(); + // When the guard evaluates a navigation action const result = OnboardingGuard.evaluate(mockState, mockAction, authenticatedContext); + // Then navigation should be allowed because users with workspace policies should skip the individual onboarding flow expect(result.type).toBe('ALLOW'); }); it('should allow invited users', async () => { + // Given a user who was invited and has already selected their intro choice (SUBMIT), indicating they came through an invitation link await Onyx.merge(ONYXKEYS.NVP_INTRO_SELECTED, { choice: CONST.INTRO_CHOICES.SUBMIT, }); await waitForBatchedUpdates(); + // When the guard evaluates a navigation action const result = OnboardingGuard.evaluate(mockState, mockAction, authenticatedContext); + // Then navigation should be allowed because invited users have a predefined purpose and should skip the onboarding purpose selection expect(result.type).toBe('ALLOW'); }); }); describe('redirect completed users away from onboarding routes', () => { it('should redirect to HOME when completed user navigates to onboarding via deep link (RESET with onboarding screen)', async () => { + // Given a user who has already completed the guided setup flow await Onyx.merge(ONYXKEYS.NVP_ONBOARDING, { hasCompletedGuidedSetupFlow: true, }); await waitForBatchedUpdates(); + // When a RESET action tries to navigate them to the onboarding modal (e.g. via a deep link like /onboarding/purpose from a Concierge message) const resetToOnboardingAction: NavigationAction = { type: CONST.NAVIGATION_ACTIONS.RESET, payload: { @@ -199,16 +227,19 @@ describe('OnboardingGuard', () => { const result = OnboardingGuard.evaluate(mockState, resetToOnboardingAction, authenticatedContext) as {type: 'REDIRECT'; route: string}; + // Then the user should be redirected to HOME because the onboarding modal is excluded from the navigation stack for completed users, and navigating there would silently fail expect(result.type).toBe('REDIRECT'); expect(result.route).toBe('home'); }); it('should ALLOW when completed user navigates to a non-onboarding route', async () => { + // Given a user who has completed the guided setup flow await Onyx.merge(ONYXKEYS.NVP_ONBOARDING, { hasCompletedGuidedSetupFlow: true, }); await waitForBatchedUpdates(); + // When a RESET action navigates to a non-onboarding screen (HOME) const resetToHomeAction: NavigationAction = { type: CONST.NAVIGATION_ACTIONS.RESET, payload: { @@ -222,27 +253,34 @@ describe('OnboardingGuard', () => { }; const result = OnboardingGuard.evaluate(mockState, resetToHomeAction, authenticatedContext); + + // Then navigation should be allowed because the redirect-to-HOME logic should only trigger when the RESET target includes an onboarding route expect(result.type).toBe('ALLOW'); }); it('should ALLOW non-RESET actions for completed users (e.g. NAVIGATE)', async () => { + // Given a user who has completed the guided setup flow await Onyx.merge(ONYXKEYS.NVP_ONBOARDING, { hasCompletedGuidedSetupFlow: true, }); await waitForBatchedUpdates(); + // When a standard NAVIGATE action (not a RESET) is evaluated const navigateAction: NavigationAction = { type: 'NAVIGATE', payload: {name: SCREENS.HOME}, }; const result = OnboardingGuard.evaluate(mockState, navigateAction, authenticatedContext); + + // Then navigation should be allowed because the deep link redirect only applies to RESET actions, which are how deep links resolve in the navigation stack expect(result.type).toBe('ALLOW'); }); }); describe('redirect to onboarding', () => { it('should redirect when authenticated user needs onboarding', async () => { + // Given a new user from a public email domain who has not completed the guided setup flow await Onyx.merge(ONYXKEYS.NVP_ONBOARDING, { hasCompletedGuidedSetupFlow: false, }); @@ -251,13 +289,16 @@ describe('OnboardingGuard', () => { }); await waitForBatchedUpdates(); + // When the guard evaluates a navigation action to a non-onboarding screen const result = OnboardingGuard.evaluate(mockState, mockAction, authenticatedContext) as {type: 'REDIRECT'; route: string}; + // Then the user should be redirected to onboarding because new users must complete the setup flow before accessing the app expect(result.type).toBe('REDIRECT'); expect(result.route).toContain('onboarding'); }); it('should redirect to correct step for users with accessible policies', async () => { + // Given a user from a private domain with accessible domain policies who has not completed onboarding await Onyx.merge(ONYXKEYS.NVP_ONBOARDING, { hasCompletedGuidedSetupFlow: false, }); @@ -267,14 +308,16 @@ describe('OnboardingGuard', () => { }); await waitForBatchedUpdates(); + // When the guard evaluates a navigation action const result = OnboardingGuard.evaluate(mockState, mockAction, authenticatedContext) as {type: 'REDIRECT'; route: string}; + // Then the user should be redirected to onboarding because their domain/policy context determines which onboarding step they should land on expect(result.type).toBe('REDIRECT'); expect(result.route).toContain('onboarding'); }); it('should redirect when user tries to access wrong onboarding step', async () => { - // User is on onboarding/purpose but should be on onboarding/work-email + // Given a new user from a public domain who is currently on the onboarding purpose screen but may need to be on a different step const onboardingState: NavigationState = { key: 'root', index: 0, @@ -292,14 +335,16 @@ describe('OnboardingGuard', () => { }); await waitForBatchedUpdates(); + // When the guard evaluates a navigation action while the user is on a potentially incorrect onboarding step const result = OnboardingGuard.evaluate(onboardingState, mockAction, authenticatedContext) as {type: 'REDIRECT'; route: string}; + // Then the user should be redirected to the correct onboarding step because the guard enforces the proper step sequence expect(result.type).toBe('REDIRECT'); expect(result.route).toContain('onboarding'); }); it('should redirect when user in onboarding tries to access non-onboarding path', async () => { - // User is on onboarding screen but tries to navigate to home + // Given a new user from a public domain who is currently on the onboarding purpose screen const onboardingState: NavigationState = { key: 'root', index: 0, @@ -309,6 +354,7 @@ describe('OnboardingGuard', () => { type: 'root', }; + // When the user attempts to navigate to the HOME screen before completing onboarding const homeAction: NavigationAction = { type: 'NAVIGATE', payload: {name: SCREENS.HOME}, @@ -324,12 +370,13 @@ describe('OnboardingGuard', () => { const result = OnboardingGuard.evaluate(onboardingState, homeAction, authenticatedContext) as {type: 'REDIRECT'; route: string}; + // Then the user should be redirected back to onboarding because they must complete the setup flow before accessing other parts of the app expect(result.type).toBe('REDIRECT'); expect(result.route).toContain('onboarding'); }); it('should always redirect to correct onboarding step when user needs onboarding', async () => { - // Even if user is on an onboarding screen, guard redirects to the correct step + // Given a new user from a public domain who is currently on the work-email onboarding step but the guard determines they belong on a different step const onboardingState: NavigationState = { key: 'root', index: 0, @@ -347,9 +394,10 @@ describe('OnboardingGuard', () => { }); await waitForBatchedUpdates(); + // When the guard evaluates a navigation action while the user is on a specific onboarding step const result = OnboardingGuard.evaluate(onboardingState, mockAction, authenticatedContext) as {type: 'REDIRECT'; route: string}; - // Guard should redirect to ensure user is on correct step + // Then the guard should redirect to the correct onboarding step because the step sequence is dynamically determined by the user's account state expect(result.type).toBe('REDIRECT'); expect(result.route).toContain('onboarding'); }); From a567667c357c3ab6ff763e7dabc73a284582f5b4 Mon Sep 17 00:00:00 2001 From: MelvinBot Date: Mon, 23 Feb 2026 11:48:08 +0000 Subject: [PATCH 4/9] Refactor isNavigatingToOnboardingFlow to match isNavigatingToTestDriveModal 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 --- src/libs/Navigation/guards/OnboardingGuard.ts | 26 +++++++++--------- .../Navigation/guards/OnboardingGuard.test.ts | 27 +++++-------------- 2 files changed, 19 insertions(+), 34 deletions(-) diff --git a/src/libs/Navigation/guards/OnboardingGuard.ts b/src/libs/Navigation/guards/OnboardingGuard.ts index 33f5d8a6a5c3..80baaf2e8525 100644 --- a/src/libs/Navigation/guards/OnboardingGuard.ts +++ b/src/libs/Navigation/guards/OnboardingGuard.ts @@ -11,7 +11,6 @@ import {isOnboardingFlowName} from '@libs/Navigation/helpers/isNavigatorName'; import {getOnboardingInitialPath} from '@userActions/Welcome/OnboardingFlow'; import CONFIG from '@src/CONFIG'; import CONST from '@src/CONST'; -import NAVIGATORS from '@src/NAVIGATORS'; import ONYXKEYS from '@src/ONYXKEYS'; import type {Route} from '@src/ROUTES'; import ROUTES from '@src/ROUTES'; @@ -131,22 +130,23 @@ function shouldPreventReset(state: NavigationState, action: NavigationAction) { } /** - * Check if the navigation action is targeting an onboarding screen. - * This handles RESET actions (URL-based navigation / deep links). + * Check if we're already on or navigating to an onboarding screen. + * This prevents redirect loops where our redirect creates new navigation actions. */ -function isNavigatingToOnboardingFlow(action: NavigationAction): boolean { - if (action.type !== CONST.NAVIGATION_ACTIONS.RESET || !action.payload) { - return false; +function isNavigatingToOnboardingFlow(state: NavigationState, action: NavigationAction): boolean { + const currentRoute = findFocusedRoute(state); + if (isOnboardingFlowName(currentRoute?.name)) { + return true; } - const targetFocusedRoute = findFocusedRoute(action.payload as NavigationState); - if (isOnboardingFlowName(targetFocusedRoute?.name)) { - return true; + if (action.type === 'RESET' && action.payload) { + const targetRoute = findFocusedRoute(action.payload as NavigationState); + if (isOnboardingFlowName(targetRoute?.name)) { + return true; + } } - // Also check if the target state contains the onboarding modal navigator directly - const routes = (action.payload as NavigationState).routes; - return routes?.some((route) => route.name === NAVIGATORS.ONBOARDING_MODAL_NAVIGATOR) ?? false; + return false; } /** @@ -170,7 +170,7 @@ const OnboardingGuard: NavigationGuard = { // Redirect completed users who try to navigate to onboarding routes (e.g. via deep link) // The OnboardingModalNavigator is not mounted when onboarding is complete, so the route would silently fail - if (isOnboardingCompleted && isNavigatingToOnboardingFlow(action)) { + if (isOnboardingCompleted && isNavigatingToOnboardingFlow(state, action)) { Log.info('[OnboardingGuard] Redirecting completed user away from onboarding route to home'); return {type: 'REDIRECT', route: ROUTES.HOME}; } diff --git a/tests/unit/Navigation/guards/OnboardingGuard.test.ts b/tests/unit/Navigation/guards/OnboardingGuard.test.ts index 37e99ab7b2e4..e2717726159d 100644 --- a/tests/unit/Navigation/guards/OnboardingGuard.test.ts +++ b/tests/unit/Navigation/guards/OnboardingGuard.test.ts @@ -3,7 +3,6 @@ import Onyx from 'react-native-onyx'; import OnboardingGuard from '@libs/Navigation/guards/OnboardingGuard'; import type {GuardContext} from '@libs/Navigation/guards/types'; import CONST from '@src/CONST'; -import NAVIGATORS from '@src/NAVIGATORS'; import ONYXKEYS from '@src/ONYXKEYS'; import SCREENS from '@src/SCREENS'; import waitForBatchedUpdates from '../../../utils/waitForBatchedUpdates'; @@ -198,30 +197,16 @@ describe('OnboardingGuard', () => { }); await waitForBatchedUpdates(); - // When a RESET action tries to navigate them to the onboarding modal (e.g. via a deep link like /onboarding/purpose from a Concierge message) + // When a RESET action tries to navigate them to the onboarding purpose screen (e.g. via a deep link like /onboarding/purpose from a Concierge message) const resetToOnboardingAction: NavigationAction = { - type: CONST.NAVIGATION_ACTIONS.RESET, + type: 'RESET', payload: { key: 'root', - index: 1, - routeNames: [SCREENS.HOME, NAVIGATORS.ONBOARDING_MODAL_NAVIGATOR], - routes: [ - {key: 'home', name: SCREENS.HOME}, - { - key: 'onboarding', - name: NAVIGATORS.ONBOARDING_MODAL_NAVIGATOR, - state: { - key: 'onboarding-nav', - index: 0, - routeNames: [SCREENS.ONBOARDING.PURPOSE], - routes: [{key: 'purpose', name: SCREENS.ONBOARDING.PURPOSE}], - stale: false, - type: 'stack', - }, - }, - ], + index: 0, + routeNames: [SCREENS.ONBOARDING.PURPOSE], + routes: [{key: 'purpose', name: SCREENS.ONBOARDING.PURPOSE}], stale: false, - type: 'root', + type: 'stack', }, }; From d7a4cc2bd00e211579830c5571fe28083d8f89dd Mon Sep 17 00:00:00 2001 From: MelvinBot Date: Mon, 23 Feb 2026 11:51:40 +0000 Subject: [PATCH 5/9] Refactor isNavigatingToOnboardingFlow to match isNavigatingToTestDriveModal 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 --- src/libs/Navigation/guards/OnboardingGuard.ts | 26 +++++++++---------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/src/libs/Navigation/guards/OnboardingGuard.ts b/src/libs/Navigation/guards/OnboardingGuard.ts index 33f5d8a6a5c3..31116484bab9 100644 --- a/src/libs/Navigation/guards/OnboardingGuard.ts +++ b/src/libs/Navigation/guards/OnboardingGuard.ts @@ -11,7 +11,6 @@ import {isOnboardingFlowName} from '@libs/Navigation/helpers/isNavigatorName'; import {getOnboardingInitialPath} from '@userActions/Welcome/OnboardingFlow'; import CONFIG from '@src/CONFIG'; import CONST from '@src/CONST'; -import NAVIGATORS from '@src/NAVIGATORS'; import ONYXKEYS from '@src/ONYXKEYS'; import type {Route} from '@src/ROUTES'; import ROUTES from '@src/ROUTES'; @@ -131,22 +130,23 @@ function shouldPreventReset(state: NavigationState, action: NavigationAction) { } /** - * Check if the navigation action is targeting an onboarding screen. - * This handles RESET actions (URL-based navigation / deep links). + * Check if the user is already on or navigating to an onboarding screen. + * This follows the same pattern as isNavigatingToTestDriveModal in TestDriveModalGuard. */ -function isNavigatingToOnboardingFlow(action: NavigationAction): boolean { - if (action.type !== CONST.NAVIGATION_ACTIONS.RESET || !action.payload) { - return false; +function isNavigatingToOnboardingFlow(state: NavigationState, action: NavigationAction): boolean { + const currentRoute = findFocusedRoute(state); + if (isOnboardingFlowName(currentRoute?.name)) { + return true; } - const targetFocusedRoute = findFocusedRoute(action.payload as NavigationState); - if (isOnboardingFlowName(targetFocusedRoute?.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; + } } - // Also check if the target state contains the onboarding modal navigator directly - const routes = (action.payload as NavigationState).routes; - return routes?.some((route) => route.name === NAVIGATORS.ONBOARDING_MODAL_NAVIGATOR) ?? false; + return false; } /** @@ -170,7 +170,7 @@ const OnboardingGuard: NavigationGuard = { // Redirect completed users who try to navigate to onboarding routes (e.g. via deep link) // The OnboardingModalNavigator is not mounted when onboarding is complete, so the route would silently fail - if (isOnboardingCompleted && isNavigatingToOnboardingFlow(action)) { + if (isOnboardingCompleted && isNavigatingToOnboardingFlow(state, action)) { Log.info('[OnboardingGuard] Redirecting completed user away from onboarding route to home'); return {type: 'REDIRECT', route: ROUTES.HOME}; } From 5f0d8e31ac6c44a1e5e7d6515c8b320393c42ef1 Mon Sep 17 00:00:00 2001 From: MelvinBot Date: Mon, 23 Feb 2026 12:05:51 +0000 Subject: [PATCH 6/9] Refactor isNavigatingToOnboardingFlow to check NAVIGATE/PUSH actions 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 --- src/libs/Navigation/guards/OnboardingGuard.ts | 23 ++++----- .../Navigation/guards/OnboardingGuard.test.ts | 50 ++++++++++++------- 2 files changed, 42 insertions(+), 31 deletions(-) diff --git a/src/libs/Navigation/guards/OnboardingGuard.ts b/src/libs/Navigation/guards/OnboardingGuard.ts index 87a809985d81..b527e13ee1b3 100644 --- a/src/libs/Navigation/guards/OnboardingGuard.ts +++ b/src/libs/Navigation/guards/OnboardingGuard.ts @@ -11,6 +11,7 @@ import {isOnboardingFlowName} from '@libs/Navigation/helpers/isNavigatorName'; import {getOnboardingInitialPath} from '@userActions/Welcome/OnboardingFlow'; import CONFIG from '@src/CONFIG'; import CONST from '@src/CONST'; +import NAVIGATORS from '@src/NAVIGATORS'; import ONYXKEYS from '@src/ONYXKEYS'; import type {Route} from '@src/ROUTES'; import ROUTES from '@src/ROUTES'; @@ -130,22 +131,18 @@ function shouldPreventReset(state: NavigationState, action: NavigationAction) { } /** - * Check if we're already on or navigating to an onboarding screen. - * This prevents redirect loops where our redirect creates new navigation actions. + * Check if the navigation action is targeting an onboarding screen. + * This handles NAVIGATE/PUSH actions that target the OnboardingModalNavigator directly. */ -function isNavigatingToOnboardingFlow(state: NavigationState, action: NavigationAction): boolean { - const currentRoute = findFocusedRoute(state); - if (isOnboardingFlowName(currentRoute?.name)) { +function isNavigatingToOnboardingFlow(action: NavigationAction): boolean { + // For NAVIGATE/PUSH actions, check if the action targets the OnboardingModalNavigator directly + if ( + (action.type === CONST.NAVIGATION.ACTION_TYPE.NAVIGATE || action.type === CONST.NAVIGATION.ACTION_TYPE.PUSH) && + (action.payload as {name?: string} | undefined)?.name === NAVIGATORS.ONBOARDING_MODAL_NAVIGATOR + ) { 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; } @@ -170,7 +167,7 @@ const OnboardingGuard: NavigationGuard = { // Redirect completed users who try to navigate to onboarding routes (e.g. via deep link) // The OnboardingModalNavigator is not mounted when onboarding is complete, so the route would silently fail - if (isOnboardingCompleted && isNavigatingToOnboardingFlow(state, action)) { + if (isOnboardingCompleted && isNavigatingToOnboardingFlow(action)) { Log.info('[OnboardingGuard] Redirecting completed user away from onboarding route to home'); return {type: 'REDIRECT', route: ROUTES.HOME}; } diff --git a/tests/unit/Navigation/guards/OnboardingGuard.test.ts b/tests/unit/Navigation/guards/OnboardingGuard.test.ts index e2717726159d..37ba7c25470e 100644 --- a/tests/unit/Navigation/guards/OnboardingGuard.test.ts +++ b/tests/unit/Navigation/guards/OnboardingGuard.test.ts @@ -3,6 +3,7 @@ import Onyx from 'react-native-onyx'; import OnboardingGuard from '@libs/Navigation/guards/OnboardingGuard'; import type {GuardContext} from '@libs/Navigation/guards/types'; import CONST from '@src/CONST'; +import NAVIGATORS from '@src/NAVIGATORS'; import ONYXKEYS from '@src/ONYXKEYS'; import SCREENS from '@src/SCREENS'; import waitForBatchedUpdates from '../../../utils/waitForBatchedUpdates'; @@ -190,29 +191,42 @@ describe('OnboardingGuard', () => { }); describe('redirect completed users away from onboarding routes', () => { - it('should redirect to HOME when completed user navigates to onboarding via deep link (RESET with onboarding screen)', async () => { + it('should redirect to HOME when completed user navigates to onboarding via NAVIGATE action', async () => { // Given a user who has already completed the guided setup flow await Onyx.merge(ONYXKEYS.NVP_ONBOARDING, { hasCompletedGuidedSetupFlow: true, }); await waitForBatchedUpdates(); - // When a RESET action tries to navigate them to the onboarding purpose screen (e.g. via a deep link like /onboarding/purpose from a Concierge message) - const resetToOnboardingAction: NavigationAction = { - type: 'RESET', - payload: { - key: 'root', - index: 0, - routeNames: [SCREENS.ONBOARDING.PURPOSE], - routes: [{key: 'purpose', name: SCREENS.ONBOARDING.PURPOSE}], - stale: false, - type: 'stack', - }, + // When a NAVIGATE action targets the OnboardingModalNavigator (e.g. via a deep link like /onboarding/purpose) + const navigateToOnboardingAction: NavigationAction = { + type: CONST.NAVIGATION.ACTION_TYPE.NAVIGATE, + payload: {name: NAVIGATORS.ONBOARDING_MODAL_NAVIGATOR}, }; - const result = OnboardingGuard.evaluate(mockState, resetToOnboardingAction, authenticatedContext) as {type: 'REDIRECT'; route: string}; + const result = OnboardingGuard.evaluate(mockState, navigateToOnboardingAction, authenticatedContext) as {type: 'REDIRECT'; route: string}; - // Then the user should be redirected to HOME because the onboarding modal is excluded from the navigation stack for completed users, and navigating there would silently fail + // Then the user should be redirected to HOME because the OnboardingModalNavigator is not mounted for completed users, and navigating there would silently fail + expect(result.type).toBe('REDIRECT'); + expect(result.route).toBe('home'); + }); + + it('should redirect to HOME when completed user navigates to onboarding via PUSH action', async () => { + // Given a user who has already completed the guided setup flow + await Onyx.merge(ONYXKEYS.NVP_ONBOARDING, { + hasCompletedGuidedSetupFlow: true, + }); + await waitForBatchedUpdates(); + + // When a PUSH action targets the OnboardingModalNavigator + const pushToOnboardingAction: NavigationAction = { + type: CONST.NAVIGATION.ACTION_TYPE.PUSH, + payload: {name: NAVIGATORS.ONBOARDING_MODAL_NAVIGATOR}, + }; + + const result = OnboardingGuard.evaluate(mockState, pushToOnboardingAction, authenticatedContext) as {type: 'REDIRECT'; route: string}; + + // Then the user should be redirected to HOME because the OnboardingModalNavigator is not mounted for completed users expect(result.type).toBe('REDIRECT'); expect(result.route).toBe('home'); }); @@ -243,22 +257,22 @@ describe('OnboardingGuard', () => { expect(result.type).toBe('ALLOW'); }); - it('should ALLOW non-RESET actions for completed users (e.g. NAVIGATE)', async () => { + it('should ALLOW NAVIGATE actions for completed users when target is not onboarding', async () => { // Given a user who has completed the guided setup flow await Onyx.merge(ONYXKEYS.NVP_ONBOARDING, { hasCompletedGuidedSetupFlow: true, }); await waitForBatchedUpdates(); - // When a standard NAVIGATE action (not a RESET) is evaluated + // When a NAVIGATE action targets a non-onboarding screen (HOME) const navigateAction: NavigationAction = { - type: 'NAVIGATE', + type: CONST.NAVIGATION.ACTION_TYPE.NAVIGATE, payload: {name: SCREENS.HOME}, }; const result = OnboardingGuard.evaluate(mockState, navigateAction, authenticatedContext); - // Then navigation should be allowed because the deep link redirect only applies to RESET actions, which are how deep links resolve in the navigation stack + // Then navigation should be allowed because the redirect only triggers when the target is the OnboardingModalNavigator expect(result.type).toBe('ALLOW'); }); }); From bf1a5d23d46c3a37b7c1cc3144fa06d7bf7eb159 Mon Sep 17 00:00:00 2001 From: MelvinBot Date: Mon, 23 Feb 2026 15:49:01 +0000 Subject: [PATCH 7/9] Fix: Format NetSuite files with Prettier to fix import ordering Co-authored-by: Linh Vo --- src/components/ConnectToNetSuiteFlow/index.tsx | 2 +- src/pages/workspace/accounting/netsuite/utils.ts | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/components/ConnectToNetSuiteFlow/index.tsx b/src/components/ConnectToNetSuiteFlow/index.tsx index 28a87da7e8ce..52824a331fa6 100644 --- a/src/components/ConnectToNetSuiteFlow/index.tsx +++ b/src/components/ConnectToNetSuiteFlow/index.tsx @@ -8,11 +8,11 @@ import {isAuthenticationError} from '@libs/actions/connections'; 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'; import type {AnchorPosition} from '@styles/index'; import CONST from '@src/CONST'; import ONYXKEYS from '@src/ONYXKEYS'; import ROUTES from '@src/ROUTES'; -import {getInitialSubPageForNetsuiteTokenInput} from '@pages/workspace/accounting/netsuite/utils'; import type {ConnectToNetSuiteFlowProps} from './types'; function ConnectToNetSuiteFlow({policyID}: ConnectToNetSuiteFlowProps) { diff --git a/src/pages/workspace/accounting/netsuite/utils.ts b/src/pages/workspace/accounting/netsuite/utils.ts index 1e47998a41f7..ada9c3b5122f 100644 --- a/src/pages/workspace/accounting/netsuite/utils.ts +++ b/src/pages/workspace/accounting/netsuite/utils.ts @@ -1,10 +1,10 @@ +import type {OnyxEntry} from 'react-native-onyx'; import type {ValueOf} from 'type-fest'; +import {isAuthenticationError} from '@libs/actions/connections'; import {canUseProvincialTaxNetSuite, canUseTaxNetSuite} from '@libs/PolicyUtils'; import CONST from '@src/CONST'; import type {NetSuiteConnectionConfig, NetSuiteSubsidiary} from '@src/types/onyx/Policy'; -import {isAuthenticationError} from '@libs/actions/connections'; import type Policy from '@src/types/onyx/Policy'; -import type {OnyxEntry} from 'react-native-onyx'; function shouldHideReimbursedReportsSection(config?: NetSuiteConnectionConfig) { return config?.reimbursableExpensesExportDestination === CONST.NETSUITE_EXPORT_DESTINATION.JOURNAL_ENTRY; From 66eaaf78a864fa441cce45055041ca6e99c78d4b Mon Sep 17 00:00:00 2001 From: MelvinBot Date: Mon, 23 Feb 2026 15:50:35 +0000 Subject: [PATCH 8/9] Add edge case tests for isNavigatingToOnboardingFlow function 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 --- .../Navigation/guards/OnboardingGuard.test.ts | 48 +++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/tests/unit/Navigation/guards/OnboardingGuard.test.ts b/tests/unit/Navigation/guards/OnboardingGuard.test.ts index 37ba7c25470e..6416d3424c65 100644 --- a/tests/unit/Navigation/guards/OnboardingGuard.test.ts +++ b/tests/unit/Navigation/guards/OnboardingGuard.test.ts @@ -275,6 +275,54 @@ describe('OnboardingGuard', () => { // Then navigation should be allowed because the redirect only triggers when the target is the OnboardingModalNavigator expect(result.type).toBe('ALLOW'); }); + + it('should NOT redirect completed user for RESET actions containing onboarding routes', async () => { + // Given a user who has completed the guided setup flow + await Onyx.merge(ONYXKEYS.NVP_ONBOARDING, { + hasCompletedGuidedSetupFlow: true, + }); + await waitForBatchedUpdates(); + + // When a RESET action contains onboarding routes in its payload (not NAVIGATE/PUSH) + const resetWithOnboardingAction: NavigationAction = { + type: CONST.NAVIGATION_ACTIONS.RESET, + payload: { + key: 'root', + index: 1, + routeNames: [SCREENS.HOME, NAVIGATORS.ONBOARDING_MODAL_NAVIGATOR], + routes: [ + {key: 'home', name: SCREENS.HOME}, + {key: 'onboarding', name: NAVIGATORS.ONBOARDING_MODAL_NAVIGATOR}, + ], + stale: false, + type: 'root', + }, + }; + + const result = OnboardingGuard.evaluate(mockState, resetWithOnboardingAction, authenticatedContext); + + // Then navigation should be allowed because isNavigatingToOnboardingFlow only checks NAVIGATE/PUSH actions, not RESET — RESET with onboarding routes does not reach the completed-user redirect + expect(result.type).toBe('ALLOW'); + }); + + it('should NOT redirect completed user for REPLACE actions targeting onboarding', async () => { + // Given a user who has completed the guided setup flow + await Onyx.merge(ONYXKEYS.NVP_ONBOARDING, { + hasCompletedGuidedSetupFlow: true, + }); + await waitForBatchedUpdates(); + + // When a REPLACE action targets the OnboardingModalNavigator + const replaceAction: NavigationAction = { + type: CONST.NAVIGATION.ACTION_TYPE.REPLACE, + payload: {name: NAVIGATORS.ONBOARDING_MODAL_NAVIGATOR}, + }; + + const result = OnboardingGuard.evaluate(mockState, replaceAction, authenticatedContext); + + // Then navigation should be allowed because isNavigatingToOnboardingFlow only handles NAVIGATE and PUSH action types, not REPLACE + expect(result.type).toBe('ALLOW'); + }); }); describe('redirect to onboarding', () => { From 04fac027e7f570b1d1edd255aadb23d62c7a5b17 Mon Sep 17 00:00:00 2001 From: MelvinBot Date: Mon, 23 Feb 2026 17:25:13 +0000 Subject: [PATCH 9/9] Remove duplicate inline comment in isNavigatingToOnboardingFlow The inline comment was redundant with the JSDoc description above the function. Co-authored-by: flaviadefaria --- src/libs/Navigation/guards/OnboardingGuard.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/libs/Navigation/guards/OnboardingGuard.ts b/src/libs/Navigation/guards/OnboardingGuard.ts index b527e13ee1b3..4f84332f40e7 100644 --- a/src/libs/Navigation/guards/OnboardingGuard.ts +++ b/src/libs/Navigation/guards/OnboardingGuard.ts @@ -135,7 +135,6 @@ function shouldPreventReset(state: NavigationState, action: NavigationAction) { * This handles NAVIGATE/PUSH actions that target the OnboardingModalNavigator directly. */ function isNavigatingToOnboardingFlow(action: NavigationAction): boolean { - // For NAVIGATE/PUSH actions, check if the action targets the OnboardingModalNavigator directly if ( (action.type === CONST.NAVIGATION.ACTION_TYPE.NAVIGATE || action.type === CONST.NAVIGATION.ACTION_TYPE.PUSH) && (action.payload as {name?: string} | undefined)?.name === NAVIGATORS.ONBOARDING_MODAL_NAVIGATOR