diff --git a/src/libs/Navigation/guards/OnboardingGuard.ts b/src/libs/Navigation/guards/OnboardingGuard.ts index bdee4a17e895..e46109ebd0b3 100644 --- a/src/libs/Navigation/guards/OnboardingGuard.ts +++ b/src/libs/Navigation/guards/OnboardingGuard.ts @@ -198,6 +198,14 @@ const OnboardingGuard: NavigationGuard = { return {type: 'ALLOW'}; } + // If the OnboardingModalNavigator is the currently focused route, the user is already + // on the onboarding flow. Redirecting again would produce a redundant state reset that + // triggers further actions, creating an infinite navigation loop (APP-7FR). + const isOnboardingFocused = state.routes[state.index]?.name === NAVIGATORS.ONBOARDING_MODAL_NAVIGATOR; + if (isOnboardingFocused) { + return {type: 'ALLOW'}; + } + // User needs onboarding - calculate the correct step and redirect const onboardingRoute = getOnboardingRoute(); diff --git a/tests/unit/Navigation/guards/OnboardingGuard.test.ts b/tests/unit/Navigation/guards/OnboardingGuard.test.ts index 6416d3424c65..d27fcfcb5ca6 100644 --- a/tests/unit/Navigation/guards/OnboardingGuard.test.ts +++ b/tests/unit/Navigation/guards/OnboardingGuard.test.ts @@ -326,7 +326,7 @@ describe('OnboardingGuard', () => { }); describe('redirect to onboarding', () => { - it('should redirect when authenticated user needs onboarding', async () => { + it('should redirect when authenticated user needs onboarding and is not on 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, @@ -336,7 +336,7 @@ describe('OnboardingGuard', () => { }); await waitForBatchedUpdates(); - // When the guard evaluates a navigation action to a non-onboarding screen + // When the guard evaluates a navigation action while the user is on 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 @@ -355,25 +355,43 @@ describe('OnboardingGuard', () => { }); await waitForBatchedUpdates(); - // When the guard evaluates a navigation action + // When the guard evaluates a navigation action while the user is on 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 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 () => { - // 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, - routeNames: [SCREENS.ONBOARDING.PURPOSE], - routes: [{key: 'purpose', name: SCREENS.ONBOARDING.PURPOSE}], - stale: false, - type: 'root', - }; + describe('infinite loop prevention (APP-7FR)', () => { + // A realistic navigation state that matches what the guard's REDIRECT reset produces: + // HOME at the bottom, OnboardingModalNavigator on top (focused). + const stateWithOnboardingNavigator: NavigationState = { + key: 'root', + index: 1, + routeNames: [SCREENS.HOME, NAVIGATORS.ONBOARDING_MODAL_NAVIGATOR], + routes: [ + {key: 'home', name: SCREENS.HOME}, + { + key: 'onboarding-modal', + name: NAVIGATORS.ONBOARDING_MODAL_NAVIGATOR, + state: { + key: 'onboarding-stack', + index: 0, + routeNames: [SCREENS.ONBOARDING.WORK_EMAIL], + routes: [{key: 'work-email', name: SCREENS.ONBOARDING.WORK_EMAIL}], + stale: false, + type: 'stack', + }, + }, + ], + stale: false, + type: 'stack', + }; + it('should ALLOW when user is already on onboarding to prevent redirect loop', async () => { + // Given a HybridApp user who needs onboarding (all shouldSkipOnboarding conditions are false) await Onyx.merge(ONYXKEYS.NVP_ONBOARDING, { hasCompletedGuidedSetupFlow: false, }); @@ -382,31 +400,39 @@ 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}; + // When the guard evaluates any action while the user is already on the OnboardingModalNavigator + const result = OnboardingGuard.evaluate(stateWithOnboardingNavigator, mockAction, authenticatedContext); - // 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'); + // Then navigation should be ALLOWED because the user is already on onboarding; + // redirecting again would produce a redundant state reset that causes an infinite loop + expect(result.type).toBe('ALLOW'); }); - it('should redirect when user in onboarding tries to access non-onboarding path', async () => { - // Given a new user from a public domain who is currently on the onboarding purpose screen - const onboardingState: NavigationState = { - key: 'root', - index: 0, - routeNames: [SCREENS.ONBOARDING.PURPOSE], - routes: [{key: 'purpose', name: SCREENS.ONBOARDING.PURPOSE}], - stale: false, - type: 'root', - }; + it('should prove the guard reaches a stable state (no infinite loop)', async () => { + // Given a user who needs onboarding + await Onyx.merge(ONYXKEYS.NVP_ONBOARDING, { + hasCompletedGuidedSetupFlow: false, + }); + await Onyx.merge(ONYXKEYS.ACCOUNT, { + isFromPublicDomain: true, + }); + await waitForBatchedUpdates(); - // When the user attempts to navigate to the HOME screen before completing onboarding - const homeAction: NavigationAction = { - type: 'NAVIGATE', - payload: {name: SCREENS.HOME}, - }; + // When the guard first evaluates on a non-onboarding state, it redirects + const firstResult = OnboardingGuard.evaluate(mockState, mockAction, authenticatedContext); + expect(firstResult.type).toBe('REDIRECT'); + + // And then subsequent evaluations on the post-redirect state (OnboardingModalNavigator mounted) + // reach a stable ALLOW state, breaking any potential loop + const secondResult = OnboardingGuard.evaluate(stateWithOnboardingNavigator, mockAction, authenticatedContext); + expect(secondResult.type).toBe('ALLOW'); + + const thirdResult = OnboardingGuard.evaluate(stateWithOnboardingNavigator, mockAction, authenticatedContext); + expect(thirdResult.type).toBe('ALLOW'); + }); + it('should still redirect when user is NOT on onboarding and needs it', async () => { + // Given a user who needs onboarding and is on the HOME screen (not on onboarding) await Onyx.merge(ONYXKEYS.NVP_ONBOARDING, { hasCompletedGuidedSetupFlow: false, }); @@ -415,22 +441,27 @@ describe('OnboardingGuard', () => { }); await waitForBatchedUpdates(); - const result = OnboardingGuard.evaluate(onboardingState, homeAction, authenticatedContext) as {type: 'REDIRECT'; route: string}; + // When the guard evaluates on a state without OnboardingModalNavigator + const result = OnboardingGuard.evaluate(mockState, mockAction, 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 + // Then the guard should redirect because the user needs onboarding and isn't on it yet expect(result.type).toBe('REDIRECT'); expect(result.route).toContain('onboarding'); }); - it('should always redirect to correct onboarding step when user needs onboarding', async () => { - // 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 = { + it('should still redirect when onboarding is in routes but not focused', async () => { + // Given a user who needs onboarding, and a state where OnboardingModalNavigator + // exists in routes but HOME is focused (index: 0) + const stateWithOnboardingUnfocused: NavigationState = { key: 'root', index: 0, - routeNames: [SCREENS.ONBOARDING.WORK_EMAIL], - routes: [{key: 'work-email', name: SCREENS.ONBOARDING.WORK_EMAIL}], + routeNames: [SCREENS.HOME, NAVIGATORS.ONBOARDING_MODAL_NAVIGATOR], + routes: [ + {key: 'home', name: SCREENS.HOME}, + {key: 'onboarding-modal', name: NAVIGATORS.ONBOARDING_MODAL_NAVIGATOR}, + ], stale: false, - type: 'root', + type: 'stack', }; await Onyx.merge(ONYXKEYS.NVP_ONBOARDING, { @@ -441,12 +472,50 @@ 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}; + // When the guard evaluates while onboarding is NOT focused + const result = OnboardingGuard.evaluate(stateWithOnboardingUnfocused, mockAction, authenticatedContext) as {type: 'REDIRECT'; route: string}; - // Then the guard should redirect to the correct onboarding step because the step sequence is dynamically determined by the user's account state + // Then the guard should still redirect because the user isn't actively on onboarding expect(result.type).toBe('REDIRECT'); expect(result.route).toContain('onboarding'); }); + + it('should still BLOCK RESET to non-onboarding even when on onboarding', async () => { + // Given a user on onboarding who has not completed it + await Onyx.merge(ONYXKEYS.NVP_ONBOARDING, { + hasCompletedGuidedSetupFlow: false, + }); + await waitForBatchedUpdates(); + + // Note: shouldPreventReset uses findFocusedRoute which checks the deepest focused route name. + // In a state with onboarding screens at the root level (as used by shouldPreventReset tests), + // the focused route IS an onboarding screen name. + const onboardingRootState: NavigationState = { + key: 'root', + index: 0, + routeNames: [SCREENS.ONBOARDING.PURPOSE], + routes: [{key: 'purpose', name: SCREENS.ONBOARDING.PURPOSE}], + stale: false, + type: 'root', + }; + + const resetToHome: 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(onboardingRootState, resetToHome, authenticatedContext) as {type: 'BLOCK'; reason?: string}; + + // Then the RESET should still be blocked by shouldPreventReset (runs before the new check) + expect(result.type).toBe('BLOCK'); + expect(result.reason).toBe('Cannot reset to non-onboarding screen while on onboarding'); + }); }); });