Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
b5d5fe4
Preserve deep-linked report route during onboarding guard redirect
MelvinBot Mar 26, 2026
ed1c51f
Merge remote-tracking branch 'origin/main' into claude-preserveDeepLi…
MelvinBot Mar 27, 2026
2e3e098
Merge remote-tracking branch 'origin/main' into claude-preserveDeepLi…
MelvinBot Apr 2, 2026
0ff0fa0
Merge remote-tracking branch 'origin/main' into claude-preserveDeepLi…
MelvinBot Apr 4, 2026
71716c7
Refactor redirect logic to only preserve routes for modal guard targets
MelvinBot Apr 7, 2026
4b4ac60
Update unit test to match modal-only redirect preservation logic
MelvinBot Apr 7, 2026
a9d22dc
Fix: skip redirect when target is already the focused route
MelvinBot Apr 7, 2026
edfa6c7
Merge remote-tracking branch 'origin/claude-preserveDeepLinkDuringOnb…
MelvinBot Apr 7, 2026
6e5d049
Revert "Fix: skip redirect when target is already the focused route"
MelvinBot Apr 8, 2026
c58257c
Remove HOME navigation fallback from navigateAfterOnboarding
MelvinBot Apr 8, 2026
36a1718
Fix: use last fullscreen route instead of first for modal redirects
MelvinBot Apr 10, 2026
cb55ea1
Merge remote-tracking branch 'origin/main' into claude-preserveDeepLi…
MelvinBot Apr 22, 2026
634b23f
Merge remote-tracking branch 'origin/main' into claude-preserveDeepLi…
MelvinBot Apr 29, 2026
e235263
Remove navigation to Home after sign-in in SignInModal
MelvinBot May 5, 2026
a206209
Merge remote-tracking branch 'origin/main' into claude-preserveDeepLi…
MelvinBot May 5, 2026
c238561
Remove unused eslint-disable directives for naming-convention
MelvinBot May 5, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import {CommonActions, StackRouter} from '@react-navigation/native';
import type {RouterConfigOptions, StackActionType, StackNavigationState} from '@react-navigation/native';
import type {NavigationState, PartialState, RouterConfigOptions, StackActionType, StackNavigationState} from '@react-navigation/native';
import type {ParamListBase} from '@react-navigation/routers';
import {createGuardContext, evaluateGuards} from '@libs/Navigation/guards';
import getAdaptedStateFromPath from '@libs/Navigation/helpers/getAdaptedStateFromPath';
Expand Down Expand Up @@ -71,6 +71,12 @@ function isPreloadAction(action: RootStackNavigatorAction): action is PreloadAct
return action.type === CONST.NAVIGATION.ACTION_TYPE.PRELOAD;
}

const MODAL_GUARD_REDIRECT_TARGETS = new Set<string>([NAVIGATORS.ONBOARDING_MODAL_NAVIGATOR, NAVIGATORS.MIGRATED_USER_MODAL_NAVIGATOR]);

function isModalGuardRedirectTarget(name: string) {
return MODAL_GUARD_REDIRECT_TARGETS.has(name);
}

/**
* Evaluates navigation guards and handles BLOCK/REDIRECT results
*
Expand Down Expand Up @@ -101,10 +107,23 @@ function handleNavigationGuards(
return null;
}

const isModalRedirect = redirectState.routes.some((route) => isModalGuardRedirectTarget(route.name));

let resetRoutes: typeof redirectState.routes = redirectState.routes;
if (isModalRedirect) {
const redirectRoute = redirectState.routes.at(-1);
const existingFullScreenRoute = state.routes.findLast((route) => isFullScreenName(route.name));
// When the current stack already has a fullscreen route (e.g., a deep-linked report),
// append only the redirect target on top of the existing routes so the user returns
// to them after the redirect screen is dismissed. Otherwise (fresh app with no stack),
// use the full redirect state which includes the base route (e.g., Home).
resetRoutes = existingFullScreenRoute && redirectRoute ? ([existingFullScreenRoute, redirectRoute] as typeof redirectState.routes) : redirectState.routes;
}

const resetAction = CommonActions.reset({
index: redirectState.index ?? redirectState.routes.length - 1,
routes: redirectState.routes,
});
index: resetRoutes.length - 1,
routes: resetRoutes,
} as PartialState<NavigationState>);

return stackRouter.getStateForAction(state, resetAction, configOptions);
}
Expand Down
3 changes: 0 additions & 3 deletions src/libs/navigateAfterOnboarding.ts
Original file line number Diff line number Diff line change
Expand Up @@ -93,9 +93,6 @@ function navigateAfterOnboarding(
);
if (reportID) {
Navigation.navigate(ROUTES.REPORT_WITH_ID.getRoute(reportID));
} else {
// Navigate to home to trigger guard evaluation
Navigation.navigate(ROUTES.HOME);
Comment on lines -96 to -98

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is to fix what I wrote here: #86390 (comment)

(I asked if it safe to remove it here)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But the comment above the code says:

// Navigate to home to trigger guard evaluation

So our guard code change will handle it. NO?

@bernhardoj bernhardoj Apr 17, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's for test drive modal guard, which doesn't exist anymore.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand how the problem you explained here relates with the scope of this pr. Is there any specific circumstance that this code makes the expected result of this pr fail?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, the 6th step

Verify that after onboarding, you land on the deep-linked report room instead of the home page

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But without this change I was able to land on the report. Are u saying for specific onboarding flow? For instance, the manage teams flow landed on the admin's room but I thought that was expected.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you test choosing Something else and complete the onboarding?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I couldn't test because of a new bug on main here

}
}

Expand Down
2 changes: 0 additions & 2 deletions src/pages/signin/SignInModal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import Navigation from '@libs/Navigation/Navigation';
import {waitForIdle} from '@libs/Network/SequentialQueue';
import CONST from '@src/CONST';
import ONYXKEYS from '@src/ONYXKEYS';
import ROUTES from '@src/ROUTES';
import SCREENS from '@src/SCREENS';
import SignInPageWrapped, {SignInPage} from './SignInPage';
import type {SignInPageRef} from './SignInPage';
Expand Down Expand Up @@ -58,7 +57,6 @@ function SignInModal() {
}

Navigation.dismissModal();
Navigation.navigate(ROUTES.HOME);
}, [isLoadingApp]);

return (
Expand Down
184 changes: 184 additions & 0 deletions tests/unit/Navigation/guards/handleNavigationGuardRedirect.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,184 @@
import type {StackNavigationState} from '@react-navigation/native';
import type {ParamListBase} from '@react-navigation/routers';
import RootStackRouter from '@libs/Navigation/AppNavigator/createRootStackNavigator/RootStackRouter';
import {evaluateGuards} from '@libs/Navigation/guards';
import getAdaptedStateFromPath from '@libs/Navigation/helpers/getAdaptedStateFromPath';
import NAVIGATORS from '@src/NAVIGATORS';
import SCREENS from '@src/SCREENS';

jest.mock('@libs/Navigation/guards', () => ({
__esModule: true,
createGuardContext: jest.fn(() => ({
isAuthenticated: true,
isLoading: false,
currentUrl: '',
})),
evaluateGuards: jest.fn(() => ({type: 'ALLOW'})),
registerGuard: jest.fn(),
clearGuards: jest.fn(),
getRegisteredGuards: jest.fn(() => []),
}));

jest.mock('@libs/Navigation/helpers/getAdaptedStateFromPath', () => ({
__esModule: true,
default: jest.fn(),
}));

const mockedEvaluateGuards = evaluateGuards as jest.Mock;
const mockedGetAdaptedStateFromPath = getAdaptedStateFromPath as jest.Mock;

const routeNames = [SCREENS.HOME, NAVIGATORS.REPORTS_SPLIT_NAVIGATOR, NAVIGATORS.ONBOARDING_MODAL_NAVIGATOR, NAVIGATORS.SETTINGS_SPLIT_NAVIGATOR];

describe('handleNavigationGuards - REDIRECT stack preservation', () => {
const router = RootStackRouter({});

const configOptions = {
routeNames,
routeParamList: {} as ParamListBase,
routeGetIdList: {} as Record<string, ((params: Record<string, unknown>) => string) | undefined>,
};

const mockAction = {
type: 'NAVIGATE' as const,
payload: {name: SCREENS.HOME},
};

beforeEach(() => {
jest.clearAllMocks();
mockedEvaluateGuards.mockReturnValue({type: 'ALLOW'});
});

it('should preserve existing fullscreen routes and append redirect target on top', () => {
// Given the current stack has a deep-linked report (a fullscreen route)
const state: StackNavigationState<ParamListBase> = {
key: 'root',
index: 0,
routeNames,
routes: [{key: 'report-1', name: NAVIGATORS.REPORTS_SPLIT_NAVIGATOR, params: undefined}],
stale: false,
type: 'stack',
preloadedRoutes: [],
};

// When the guard returns REDIRECT to onboarding and getAdaptedStateFromPath returns a state with Home + OnboardingModalNavigator
mockedEvaluateGuards.mockReturnValue({type: 'REDIRECT', route: 'onboarding/purpose'});
mockedGetAdaptedStateFromPath.mockReturnValue({
routes: [
{name: SCREENS.HOME, key: 'home-1'},
{name: NAVIGATORS.ONBOARDING_MODAL_NAVIGATOR, key: 'onboarding-1'},
],
});

const result = router.getStateForAction(state, mockAction, configOptions);

// Then the deep-linked report should be preserved and onboarding should be appended on top
expect(result).not.toBeNull();
expect(result?.routes).toHaveLength(2);
expect(result?.routes[0].name).toBe(NAVIGATORS.REPORTS_SPLIT_NAVIGATOR);
expect(result?.routes[1].name).toBe(NAVIGATORS.ONBOARDING_MODAL_NAVIGATOR);
expect(result?.index).toBe(1);
});

it('should use the full redirect state when no existing fullscreen route is present', () => {
// Given a fresh app state with no fullscreen routes (e.g., only a non-fullscreen route)
const state: StackNavigationState<ParamListBase> = {
key: 'root',
index: 0,
routeNames: [...routeNames, 'SomeNonFullScreenRoute'],
routes: [{key: 'other-1', name: 'SomeNonFullScreenRoute', params: undefined}],
stale: false,
type: 'stack',
preloadedRoutes: [],
};

// When the guard returns REDIRECT to onboarding
mockedEvaluateGuards.mockReturnValue({type: 'REDIRECT', route: 'onboarding/purpose'});
mockedGetAdaptedStateFromPath.mockReturnValue({
routes: [
{name: SCREENS.HOME, key: 'home-1'},
{name: NAVIGATORS.ONBOARDING_MODAL_NAVIGATOR, key: 'onboarding-1'},
],
});

const result = router.getStateForAction(state, mockAction, configOptions);

// Then the full redirect state (Home + Onboarding) should be used since there's no fullscreen route to preserve
expect(result).not.toBeNull();
expect(result?.routes).toHaveLength(2);
expect(result?.routes[0].name).toBe(SCREENS.HOME);
expect(result?.routes[1].name).toBe(NAVIGATORS.ONBOARDING_MODAL_NAVIGATOR);
expect(result?.index).toBe(1);
});

it('should use the full redirect state for non-modal redirects even when fullscreen routes exist', () => {
// Given the current stack has a deep-linked report (a fullscreen route)
const state: StackNavigationState<ParamListBase> = {
key: 'root',
index: 0,
routeNames,
routes: [{key: 'report-1', name: NAVIGATORS.REPORTS_SPLIT_NAVIGATOR, params: undefined}],
stale: false,
type: 'stack',
preloadedRoutes: [],
};

// When the guard returns a non-modal REDIRECT (e.g., to SettingsSplitNavigator)
mockedEvaluateGuards.mockReturnValue({type: 'REDIRECT', route: 'settings'});
mockedGetAdaptedStateFromPath.mockReturnValue({
routes: [{name: NAVIGATORS.SETTINGS_SPLIT_NAVIGATOR, key: 'settings-1'}],
});

const result = router.getStateForAction(state, mockAction, configOptions);

// Then the full redirect state should be used (no route preservation for non-modal redirects)
expect(result).not.toBeNull();
expect(result?.routes).toHaveLength(1);
expect(result?.routes[0].name).toBe(NAVIGATORS.SETTINGS_SPLIT_NAVIGATOR);
expect(result?.index).toBe(0);
});

it('should not process redirect when guard allows navigation', () => {
// Given the guard allows navigation
const state: StackNavigationState<ParamListBase> = {
key: 'root',
index: 0,
routeNames,
routes: [{key: 'home-1', name: SCREENS.HOME, params: undefined}],
stale: false,
type: 'stack',
preloadedRoutes: [],
};

mockedEvaluateGuards.mockReturnValue({type: 'ALLOW'});

// Normal routing may error with minimal config — we only care that redirect logic was not triggered
try {
router.getStateForAction(state, mockAction, configOptions);
} catch {
// Expected: StackRouter needs full config for normal routing
}

// Then getAdaptedStateFromPath should NOT have been called (no redirect processing)
expect(mockedGetAdaptedStateFromPath).not.toHaveBeenCalled();
});

it('should return unchanged state when guard blocks navigation', () => {
// Given the guard blocks navigation
const state: StackNavigationState<ParamListBase> = {
key: 'root',
index: 0,
routeNames,
routes: [{key: 'home-1', name: SCREENS.HOME, params: undefined}],
stale: false,
type: 'stack',
preloadedRoutes: [],
};

mockedEvaluateGuards.mockReturnValue({type: 'BLOCK', reason: 'Test block'});

const result = router.getStateForAction(state, mockAction, configOptions);

// Then the state should be returned unchanged
expect(result).toEqual(state);
});
});
Loading