Skip to content
10 changes: 4 additions & 6 deletions src/ROUTES.ts
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,10 @@ const DYNAMIC_ROUTES = {
path: 'owner-selector',
entryScreens: [],
},
REPORT_SETTINGS_NAME: {
path: 'settings/name',
entryScreens: [SCREENS.REPORT_DETAILS.ROOT, SCREENS.RIGHT_MODAL.REPORT_SETTINGS, SCREENS.REPORT, SCREENS.RIGHT_MODAL.SEARCH_REPORT, SCREENS.SEARCH.ROOT],
},
ADDRESS_COUNTRY: {
path: 'country',
entryScreens: [
Expand Down Expand Up @@ -792,12 +796,6 @@ const ROUTES = {
// eslint-disable-next-line no-restricted-syntax -- Legacy route generation
getRoute: (reportID: string, backTo?: string) => getUrlWithBackToParam(`r/${reportID}/settings` as const, backTo),
},
REPORT_SETTINGS_NAME: {
route: 'r/:reportID/settings/name',

// eslint-disable-next-line no-restricted-syntax -- Legacy route generation
getRoute: (reportID: string, backTo?: string) => getUrlWithBackToParam(`r/${reportID}/settings/name` as const, backTo),
},
REPORT_SETTINGS_NOTIFICATION_PREFERENCES: {
route: 'r/:reportID/settings/notification-preferences',

Expand Down
2 changes: 1 addition & 1 deletion src/SCREENS.ts
Original file line number Diff line number Diff line change
Expand Up @@ -443,7 +443,7 @@ const SCREENS = {

REPORT_SETTINGS: {
ROOT: 'Report_Settings_Root',
NAME: 'Report_Settings_Name',
DYNAMIC_SETTINGS_NAME: 'Dynamic_Report_Settings_Name',
NOTIFICATION_PREFERENCES: 'Report_Settings_Notification_Preferences',
WRITE_CAPABILITY: 'Report_Settings_Write_Capability',
VISIBILITY: 'Report_Settings_Visibility',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,7 @@ const ReportChangeApproverModalStackNavigator = createModalStackNavigator<Report

const ReportSettingsModalStackNavigator = createModalStackNavigator<ReportSettingsNavigatorParamList>({
[SCREENS.REPORT_SETTINGS.ROOT]: () => require<ReactComponentModule>('../../../../pages/settings/Report/ReportSettingsPage').default,
[SCREENS.REPORT_SETTINGS.NAME]: () => require<ReactComponentModule>('../../../../pages/settings/Report/NamePage').default,
[SCREENS.REPORT_SETTINGS.DYNAMIC_SETTINGS_NAME]: () => require<ReactComponentModule>('../../../../pages/settings/Report/DynamicNamePage').default,
[SCREENS.REPORT_SETTINGS.NOTIFICATION_PREFERENCES]: () => require<ReactComponentModule>('../../../../pages/settings/Report/NotificationPreferencePage').default,
[SCREENS.REPORT_SETTINGS.WRITE_CAPABILITY]: () => require<ReactComponentModule>('../../../../pages/settings/Report/WriteCapabilityPage').default,
[SCREENS.REPORT_SETTINGS.VISIBILITY]: () => require<ReactComponentModule>('../../../../pages/settings/Report/VisibilityPage').default,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
type LeafRoute = {
name: string;
path: string;
params?: Record<string, string>;
params?: Record<string, unknown>;
};

type NestedRoute = {
Expand Down Expand Up @@ -54,10 +54,10 @@
return null;
}

function getStateForDynamicRoute(path: string, dynamicRouteName: keyof typeof DYNAMIC_ROUTES) {
function getStateForDynamicRoute(path: string, dynamicRouteName: keyof typeof DYNAMIC_ROUTES, parentRouteParams?: Record<string, unknown>) {
const routeConfig = getRouteNamesForDynamicRoute(DYNAMIC_ROUTES[dynamicRouteName].path);
const [, query] = splitPathAndQuery(path);
const params = getParamsFromQuery(query);

Check failure on line 60 in src/libs/Navigation/helpers/dynamicRoutesUtils/getStateForDynamicRoute.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check

'params' is assigned a value but never used

Check failure on line 60 in src/libs/Navigation/helpers/dynamicRoutesUtils/getStateForDynamicRoute.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check

'params' is assigned a value but never used

Check failure on line 60 in src/libs/Navigation/helpers/dynamicRoutesUtils/getStateForDynamicRoute.ts

View workflow job for this annotation

GitHub Actions / ESLint check

'params' is assigned a value but never used

Check failure on line 60 in src/libs/Navigation/helpers/dynamicRoutesUtils/getStateForDynamicRoute.ts

View workflow job for this annotation

GitHub Actions / ESLint check

'params' is assigned a value but never used
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@collectioneur How about const params = getParamsFromQuery(query);? Do we still need this?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we do need to keep this. Dynamic routes can also have query parameters, and this seems to be the most reliable way to extract them

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated. thanks


if (!routeConfig) {
throw new Error(`No route configuration found for dynamic route '${dynamicRouteName}'`);
Expand All @@ -67,12 +67,12 @@
const buildNestedState = (routes: string[], currentIndex: number): RouteNode => {
const currentRoute = routes.at(currentIndex);

// If this is the last route, create leaf node with path
// If this is the last route, create leaf node with path and inherited params
if (currentIndex === routes.length - 1) {
return {
name: currentRoute ?? '',
path,
params,
...(parentRouteParams ? {params: parentRouteParams} : {}),
};
}

Expand Down
2 changes: 1 addition & 1 deletion src/libs/Navigation/helpers/getStateFromPath.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ function getStateFromPath(path: Route): PartialState<NavigationState> {
if (focusedRoute?.name) {
if (entryScreens.includes(focusedRoute.name as Screen)) {
// Generate navigation state for the dynamic route
const dynamicRouteState = getStateForDynamicRoute(normalizedPath, dynamicRoute as DynamicRouteKey);
const dynamicRouteState = getStateForDynamicRoute(normalizedPath, dynamicRoute as DynamicRouteKey, focusedRoute?.params as Record<string, unknown> | undefined);
return dynamicRouteState;
}

Expand Down
4 changes: 1 addition & 3 deletions src/libs/Navigation/linkingConfig/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1396,9 +1396,7 @@ const config: LinkingOptions<RootNavigatorParamList>['config'] = {
[SCREENS.REPORT_SETTINGS.ROOT]: {
path: ROUTES.REPORT_SETTINGS.route,
},
[SCREENS.REPORT_SETTINGS.NAME]: {
path: ROUTES.REPORT_SETTINGS_NAME.route,
},
[SCREENS.REPORT_SETTINGS.DYNAMIC_SETTINGS_NAME]: DYNAMIC_ROUTES.REPORT_SETTINGS_NAME.path,
[SCREENS.REPORT_SETTINGS.NOTIFICATION_PREFERENCES]: {
path: ROUTES.REPORT_SETTINGS_NOTIFICATION_PREFERENCES.route,
},
Expand Down
4 changes: 1 addition & 3 deletions src/libs/Navigation/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1683,10 +1683,8 @@ type ReportSettingsNavigatorParamList = {
// eslint-disable-next-line no-restricted-syntax -- `backTo` usages in this file are legacy. Do not add new `backTo` params to screens. See contributingGuides/NAVIGATION.md
backTo?: Routes;
};
[SCREENS.REPORT_SETTINGS.NAME]: {
[SCREENS.REPORT_SETTINGS.DYNAMIC_SETTINGS_NAME]: {
reportID: string;
// eslint-disable-next-line no-restricted-syntax -- `backTo` usages in this file are legacy. Do not add new `backTo` params to screens. See contributingGuides/NAVIGATION.md
backTo?: Routes;
};
[SCREENS.REPORT_SETTINGS.NOTIFICATION_PREFERENCES]: {
reportID: string;
Expand Down
7 changes: 5 additions & 2 deletions src/pages/ReportDetailsPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
import useThemeStyles from '@hooks/useThemeStyles';
import getBase62ReportID from '@libs/getBase62ReportID';
import getNonEmptyStringOnyxID from '@libs/getNonEmptyStringOnyxID';
import createDynamicRoute from '@libs/Navigation/helpers/createDynamicRoute';

Check failure on line 46 in src/pages/ReportDetailsPage.tsx

View workflow job for this annotation

GitHub Actions / typecheck

Cannot find module '@libs/Navigation/helpers/createDynamicRoute' or its corresponding type declarations.
import Navigation, {navigationRef} from '@libs/Navigation/Navigation';
import type {PlatformStackScreenProps} from '@libs/Navigation/PlatformStackNavigation/types';
import type {ReportDetailsNavigatorParamList, RightModalNavigatorParamList} from '@libs/Navigation/types';
Expand Down Expand Up @@ -125,7 +126,7 @@
import type {TranslationPaths} from '@src/languages/types';
import ONYXKEYS from '@src/ONYXKEYS';
import type {Route} from '@src/ROUTES';
import ROUTES from '@src/ROUTES';
import ROUTES, {DYNAMIC_ROUTES} from '@src/ROUTES';
import SCREENS from '@src/SCREENS';
import type * as OnyxTypes from '@src/types/onyx';
import type DeepValueOf from '@src/types/utils/DeepValueOf';
Expand Down Expand Up @@ -818,7 +819,9 @@
furtherDetails={chatRoomSubtitle && !isGroupChat ? additionalRoomDetails : ''}
furtherDetailsNumberOfLines={isWorkspaceChat ? 0 : undefined}
furtherDetailsStyle={isWorkspaceChat ? [styles.textAlignCenter, styles.breakWord] : undefined}
onPress={() => Navigation.navigate(ROUTES.REPORT_SETTINGS_NAME.getRoute(report.reportID, backTo))}
onPress={() => {
Navigation.navigate(createDynamicRoute(DYNAMIC_ROUTES.REPORT_SETTINGS_NAME.path));

Check failure on line 823 in src/pages/ReportDetailsPage.tsx

View workflow job for this annotation

GitHub Actions / Changed files ESLint check

Unsafe call of a(n) `error` type typed value

Check failure on line 823 in src/pages/ReportDetailsPage.tsx

View workflow job for this annotation

GitHub Actions / Changed files ESLint check

Unsafe argument of type error typed assigned to a parameter of type `Route`

Check failure on line 823 in src/pages/ReportDetailsPage.tsx

View workflow job for this annotation

GitHub Actions / Changed files ESLint check

Unsafe call of a(n) `error` type typed value

Check failure on line 823 in src/pages/ReportDetailsPage.tsx

View workflow job for this annotation

GitHub Actions / Changed files ESLint check

Unsafe argument of type error typed assigned to a parameter of type `Route`

Check failure on line 823 in src/pages/ReportDetailsPage.tsx

View workflow job for this annotation

GitHub Actions / ESLint check

Unsafe call of a(n) `error` type typed value

Check failure on line 823 in src/pages/ReportDetailsPage.tsx

View workflow job for this annotation

GitHub Actions / ESLint check

Unsafe argument of type error typed assigned to a parameter of type `Route`

Check failure on line 823 in src/pages/ReportDetailsPage.tsx

View workflow job for this annotation

GitHub Actions / ESLint check

Unsafe call of a(n) `error` type typed value

Check failure on line 823 in src/pages/ReportDetailsPage.tsx

View workflow job for this annotation

GitHub Actions / ESLint check

Unsafe argument of type error typed assigned to a parameter of type `Route`
}}
numberOfLinesTitle={isThread ? 2 : 0}
shouldBreakWord
/>
Expand Down
1 change: 1 addition & 0 deletions src/pages/inbox/report/withReportOrNotFound.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ type ScreenProps =
| PlatformStackScreenProps<ReportDetailsNavigatorParamList, typeof SCREENS.REPORT_DETAILS.ROOT>
| PlatformStackScreenProps<ReportDetailsNavigatorParamList, typeof SCREENS.REPORT_DETAILS.SHARE_CODE>
| PlatformStackScreenProps<ReportSettingsNavigatorParamList, typeof SCREENS.REPORT_SETTINGS.ROOT>
| PlatformStackScreenProps<ReportSettingsNavigatorParamList, typeof SCREENS.REPORT_SETTINGS.DYNAMIC_SETTINGS_NAME>
| PlatformStackScreenProps<RoomMembersNavigatorParamList, typeof SCREENS.ROOM_MEMBERS.DETAILS>
| PlatformStackScreenProps<ReportChangeWorkspaceNavigatorParamList, typeof SCREENS.REPORT_CHANGE_WORKSPACE.ROOT>
| PlatformStackScreenProps<ReportChangeApproverParamList, typeof SCREENS.REPORT_CHANGE_APPROVER.ROOT>;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,17 +1,21 @@
import React from 'react';
import useDynamicBackPath from '@hooks/useDynamicBackPath';
import type {PlatformStackScreenProps} from '@libs/Navigation/PlatformStackNavigation/types';
import {isGroupChat, isTripRoom} from '@libs/ReportUtils';
import type {ReportSettingsNavigatorParamList} from '@navigation/types';
import GroupChatNameEditPage from '@pages/GroupChatNameEditPage';
import withReportOrNotFound from '@pages/inbox/report/withReportOrNotFound';
import type {WithReportOrNotFoundProps} from '@pages/inbox/report/withReportOrNotFound';
import TripChatNameEditPage from '@pages/TripChatNameEditPage';
import {DYNAMIC_ROUTES} from '@src/ROUTES';
import type SCREENS from '@src/SCREENS';
import RoomNamePage from './RoomNamePage';

type NamePageProps = WithReportOrNotFoundProps & PlatformStackScreenProps<ReportSettingsNavigatorParamList, typeof SCREENS.REPORT_SETTINGS.NAME>;
type DynamicNamePageProps = WithReportOrNotFoundProps & PlatformStackScreenProps<ReportSettingsNavigatorParamList, typeof SCREENS.REPORT_SETTINGS.DYNAMIC_SETTINGS_NAME>;

function DynamicNamePage({report}: DynamicNamePageProps) {
const backPath = useDynamicBackPath(DYNAMIC_ROUTES.REPORT_SETTINGS_NAME.path);

function NamePage({report}: NamePageProps) {
if (isTripRoom(report)) {
return <TripChatNameEditPage report={report} />;
}
Expand All @@ -20,7 +24,12 @@ function NamePage({report}: NamePageProps) {
return <GroupChatNameEditPage report={report} />;
}

return <RoomNamePage report={report} />;
return (
<RoomNamePage
report={report}
navigateBackTo={backPath}
/>
);
}

export default withReportOrNotFound()(NamePage);
export default withReportOrNotFound()(DynamicNamePage);
16 changes: 6 additions & 10 deletions src/pages/settings/Report/RoomNamePage.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import {useIsFocused, useRoute} from '@react-navigation/native';
import {useIsFocused} from '@react-navigation/native';
import React, {useCallback, useRef} from 'react';
import {View} from 'react-native';
import FullPageNotFoundView from '@components/BlockingViews/FullPageNotFoundView';
Expand All @@ -15,35 +15,31 @@ import useReportIsArchived from '@hooks/useReportIsArchived';
import useThemeStyles from '@hooks/useThemeStyles';
import {addErrorMessage} from '@libs/ErrorUtils';
import Navigation from '@libs/Navigation/Navigation';
import type {PlatformStackRouteProp} from '@libs/Navigation/PlatformStackNavigation/types';
import type {ReportSettingsNavigatorParamList} from '@libs/Navigation/types';
import {shouldDisableRename} from '@libs/ReportUtils';
import {isExistingRoomName, isReservedRoomName, isValidRoomNameWithoutLimits} from '@libs/ValidationUtils';
import {updatePolicyRoomName as updatePolicyRoomNameReportAction} from '@userActions/Report';
import CONST from '@src/CONST';
import ONYXKEYS from '@src/ONYXKEYS';
import ROUTES from '@src/ROUTES';
import type SCREENS from '@src/SCREENS';
import type {Route} from '@src/ROUTES';
import INPUT_IDS from '@src/types/form/RoomNameForm';
import type {Report} from '@src/types/onyx';

type RoomNamePageProps = {
report: Report;
navigateBackTo?: Route;
};

function RoomNamePage({report}: RoomNamePageProps) {
const route = useRoute<PlatformStackRouteProp<ReportSettingsNavigatorParamList, typeof SCREENS.REPORT_SETTINGS.NAME>>();
function RoomNamePage({report, navigateBackTo}: RoomNamePageProps) {
const styles = useThemeStyles();
const roomNameInputRef = useRef<AnimatedTextInputRef>(null);
const isFocused = useIsFocused();
const {translate} = useLocalize();
const reportID = report?.reportID;
const [reports] = useOnyx(ONYXKEYS.COLLECTION.REPORT);
const isReportArchived = useReportIsArchived(report?.reportID);

const goBack = useCallback(() => {
Navigation.setNavigationActionToMicrotaskQueue(() => Navigation.goBack(ROUTES.REPORT_WITH_ID_DETAILS.getRoute(reportID, route.params.backTo)));
}, [reportID, route.params.backTo]);
Navigation.setNavigationActionToMicrotaskQueue(() => Navigation.goBack(navigateBackTo));
}, [navigateBackTo]);

const validate = useCallback(
(values: FormOnyxValues<typeof ONYXKEYS.FORMS.ROOM_NAME_FORM>) => {
Expand Down
20 changes: 20 additions & 0 deletions tests/navigation/getStateForDynamicRouteTests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ jest.mock('@src/ROUTES', () => ({
type LeafRoute = {
name: string;
path: string;
params?: Record<string, unknown>;
};

type NestedRoute = {
Expand Down Expand Up @@ -111,4 +112,23 @@ describe('getStateForDynamicRoute', () => {
expect(state?.index).toBe(0);
expect(Array.isArray(state?.routes)).toBe(true);
});

it('should inherit parent route params on the leaf node', () => {
const path = '/r/12345/settings/name';
const parentParams = {reportID: '12345'};
const result = getStateForDynamicRoute(path, KEY_TEST as unknown as keyof typeof DYNAMIC_ROUTES, parentParams);

const rootRoute = result.routes.at(0) as NestedRoute | undefined;
const leafRoute = rootRoute?.state.routes.at(0) as LeafRoute | undefined;
expect(leafRoute?.params).toEqual(parentParams);
});

it('should not include params on the leaf node when parentRouteParams is undefined', () => {
const path = '/some/path/test-path';
const result = getStateForDynamicRoute(path, KEY_TEST as unknown as keyof typeof DYNAMIC_ROUTES);

const rootRoute = result.routes.at(0) as NestedRoute | undefined;
const leafRoute = rootRoute?.state.routes.at(0) as LeafRoute | undefined;
expect(leafRoute?.params).toBeUndefined();
});
});
5 changes: 3 additions & 2 deletions tests/navigation/getStateFromPathTests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,17 +57,18 @@ describe('getStateFromPath', () => {
it('should generate dynamic state when authorized screen is focused', () => {
const fullPath = '/settings/wallet/verify-account';
const baseRouteState = {routes: [{name: 'Wallet'}]};
const focusedRouteParams = {walletID: '456'};

mockRNGetStateFromPath.mockReturnValue(baseRouteState);
mockFindFocusedRoute.mockReturnValue({name: 'Wallet'});
mockFindFocusedRoute.mockReturnValue({name: 'Wallet', params: focusedRouteParams});

const expectedDynamicState = {routes: [{name: 'DynamicRoot'}]};
mockGetStateForDynamicRoute.mockReturnValue(expectedDynamicState);

const result = getStateFromPath(fullPath as unknown as Route);

expect(result).toBe(expectedDynamicState);
expect(mockGetStateForDynamicRoute).toHaveBeenCalledWith(fullPath, 'VERIFY_ACCOUNT');
expect(mockGetStateForDynamicRoute).toHaveBeenCalledWith(fullPath, 'VERIFY_ACCOUNT', focusedRouteParams);
});

it('should fallback to standard RN parsing if focused screen is NOT authorized for dynamic route', () => {
Expand Down
Loading