Skip to content

migrate DYNAMIC_SETTINGS_WRITE_CAPABILITY #85109

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
mjasikowski merged 8 commits into from
Mar 20, 2026
Merged

migrate DYNAMIC_SETTINGS_WRITE_CAPABILITY #85109

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
986cf6e
migrate report setting write capability
huult Mar 12, 2026
531a384
remove useCallback
huult Mar 12, 2026
f68cc22
Merge remote-tracking branch 'upstream/main' into 83368-migrate-REPOR…
huult Mar 16, 2026
cc38225
update route path
huult Mar 16, 2026
4c903dc
update import path
huult Mar 16, 2026
baf65c5
update import
huult Mar 16, 2026
2fe203f
update order
huult Mar 16, 2026
55535fb
Merge remote-tracking branch 'upstream/main' into 83368-migrate-REPOR…
huult Mar 20, 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
10 changes: 4 additions & 6 deletions src/ROUTES.ts
View file
Open in desktop
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_WRITE_CAPABILITY: {
path: 'who-can-post',
entryScreens: [SCREENS.REPORT_SETTINGS.ROOT],
},
REPORT_SETTINGS_VISIBILITY: {
path: 'visibility',
entryScreens: [SCREENS.REPORT_SETTINGS.ROOT],
Expand Down Expand Up @@ -806,12 +810,6 @@ const ROUTES = {
// eslint-disable-next-line no-restricted-syntax -- Legacy route generation
getRoute: (reportID: string, backTo?: string) => getUrlWithBackToParam(`r/${reportID}/settings/notification-preferences` as const, backTo),
},
REPORT_SETTINGS_WRITE_CAPABILITY: {
route: 'r/:reportID/settings/who-can-post',

// eslint-disable-next-line no-restricted-syntax -- Legacy route generation
getRoute: (reportID: string, backTo?: string) => getUrlWithBackToParam(`r/${reportID}/settings/who-can-post` as const, backTo),
},
REPORT_CHANGE_APPROVER: {
route: 'r/:reportID/change-approver',

Expand Down
3 changes: 2 additions & 1 deletion src/SCREENS.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -445,7 +445,8 @@ const SCREENS = {
ROOT: 'Report_Settings_Root',
NAME: 'Report_Settings_Name',
NOTIFICATION_PREFERENCES: 'Report_Settings_Notification_Preferences',
WRITE_CAPABILITY: 'Report_Settings_Write_Capability',
DYNAMIC_SETTINGS_WRITE_CAPABILITY: 'Dynamic_Report_Settings_Write_Capability',
VISIBILITY: 'Report_Settings_Visibility',
DYNAMIC_SETTINGS_VISIBILITY: 'Dynamic_Report_Settings_Visibility',
REPORT_LAYOUT: 'Report_Settings_Report_Layout',
COLUMNS: 'Report_Settings_Columns',
Expand Down
2 changes: 1 addition & 1 deletion src/libs/Navigation/AppNavigator/ModalStackNavigators/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,7 @@ const ReportSettingsModalStackNavigator = createModalStackNavigator<ReportSettin
[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.NOTIFICATION_PREFERENCES]: () => require<ReactComponentModule>('../../../../pages/settings/Report/NotificationPreferencePage').default,
[SCREENS.REPORT_SETTINGS.WRITE_CAPABILITY]: () => require<ReactComponentModule>('../../../../pages/settings/Report/WriteCapabilityPage').default,
[SCREENS.REPORT_SETTINGS.DYNAMIC_SETTINGS_WRITE_CAPABILITY]: () => require<ReactComponentModule>('../../../../pages/settings/Report/DynamicWriteCapabilityPage').default,
[SCREENS.REPORT_SETTINGS.DYNAMIC_SETTINGS_VISIBILITY]: () => require<ReactComponentModule>('../../../../pages/settings/Report/DynamicVisibilityPage').default,
[SCREENS.REPORT_SETTINGS.REPORT_LAYOUT]: () => require<ReactComponentModule>('../../../../pages/settings/Report/ReportLayoutPage').default,
[SCREENS.REPORT_SETTINGS.COLUMNS]: () => require<ReactComponentModule>('../../../../pages/settings/Report/ReportDetailsColumnsPage').default,
Expand Down
4 changes: 1 addition & 3 deletions src/libs/Navigation/linkingConfig/config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1402,9 +1402,7 @@ const config: LinkingOptions<RootNavigatorParamList>['config'] = {
[SCREENS.REPORT_SETTINGS.NOTIFICATION_PREFERENCES]: {
path: ROUTES.REPORT_SETTINGS_NOTIFICATION_PREFERENCES.route,
},
[SCREENS.REPORT_SETTINGS.WRITE_CAPABILITY]: {
path: ROUTES.REPORT_SETTINGS_WRITE_CAPABILITY.route,
},
[SCREENS.REPORT_SETTINGS.DYNAMIC_SETTINGS_WRITE_CAPABILITY]: DYNAMIC_ROUTES.REPORT_SETTINGS_WRITE_CAPABILITY.path,
[SCREENS.REPORT_SETTINGS.DYNAMIC_SETTINGS_VISIBILITY]: DYNAMIC_ROUTES.REPORT_SETTINGS_VISIBILITY.path,
[SCREENS.REPORT_SETTINGS.REPORT_LAYOUT]: {
path: ROUTES.REPORT_SETTINGS_REPORT_LAYOUT.route,
Expand Down
4 changes: 1 addition & 3 deletions src/libs/Navigation/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1685,10 +1685,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.WRITE_CAPABILITY]: {
[SCREENS.REPORT_SETTINGS.DYNAMIC_SETTINGS_WRITE_CAPABILITY]: {
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.DYNAMIC_SETTINGS_VISIBILITY]: {
reportID: string;
Expand Down
1 change: 1 addition & 0 deletions src/pages/inbox/report/withReportOrNotFound.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,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_WRITE_CAPABILITY>
| PlatformStackScreenProps<ReportSettingsNavigatorParamList, typeof SCREENS.REPORT_SETTINGS.DYNAMIC_SETTINGS_VISIBILITY>
| PlatformStackScreenProps<RoomMembersNavigatorParamList, typeof SCREENS.ROOM_MEMBERS.DETAILS>
| PlatformStackScreenProps<ReportChangeWorkspaceNavigatorParamList, typeof SCREENS.REPORT_CHANGE_WORKSPACE.ROOT>
Expand Down
21 changes: 9 additions & 12 deletions ...s/settings/Report/WriteCapabilityPage.tsx → ...ngs/Report/DynamicWriteCapabilityPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,28 +1,25 @@
import {useRoute} from '@react-navigation/native';
import React, {useCallback} from 'react';
import type {ValueOf} from 'type-fest';
import FullPageNotFoundView from '@components/BlockingViews/FullPageNotFoundView';
import HeaderWithBackButton from '@components/HeaderWithBackButton';
import ScreenWrapper from '@components/ScreenWrapper';
import SelectionList from '@components/SelectionList';
import RadioListItem from '@components/SelectionList/ListItem/RadioListItem';
import useDynamicBackPath from '@hooks/useDynamicBackPath';
import useLocalize from '@hooks/useLocalize';
import useReportIsArchived from '@hooks/useReportIsArchived';
import {updateWriteCapability as updateWriteCapabilityUtil} from '@libs/actions/Report';
import Navigation from '@libs/Navigation/Navigation';
import type {PlatformStackRouteProp, PlatformStackScreenProps} from '@libs/Navigation/PlatformStackNavigation/types';
import {canEditWriteCapability} from '@libs/ReportUtils';
import type {ReportSettingsNavigatorParamList} from '@navigation/types';
import withReportOrNotFound from '@pages/inbox/report/withReportOrNotFound';
import type {WithReportOrNotFoundProps} from '@pages/inbox/report/withReportOrNotFound';
import CONST from '@src/CONST';
import ROUTES from '@src/ROUTES';
import type SCREENS from '@src/SCREENS';
import {DYNAMIC_ROUTES} from '@src/ROUTES';

type WriteCapabilityPageProps = WithReportOrNotFoundProps & PlatformStackScreenProps<ReportSettingsNavigatorParamList, typeof SCREENS.REPORT_SETTINGS.WRITE_CAPABILITY>;
type DynamicWriteCapabilityPageProps = WithReportOrNotFoundProps;

function WriteCapabilityPage({report, policy}: WriteCapabilityPageProps) {
const route = useRoute<PlatformStackRouteProp<ReportSettingsNavigatorParamList, typeof SCREENS.REPORT_SETTINGS.WRITE_CAPABILITY>>();
function DynamicWriteCapabilityPage({report, policy}: DynamicWriteCapabilityPageProps) {
const backPath = useDynamicBackPath(DYNAMIC_ROUTES.REPORT_SETTINGS_WRITE_CAPABILITY.path);
const {translate} = useLocalize();
const writeCapabilityOptions = Object.values(CONST.REPORT.WRITE_CAPABILITIES).map((value) => ({
value,
Expand All @@ -34,9 +31,9 @@ function WriteCapabilityPage({report, policy}: WriteCapabilityPageProps) {
const isReportArchived = useReportIsArchived(report.reportID);
const isAbleToEdit = canEditWriteCapability(report, policy, isReportArchived);

const goBack = useCallback(() => {
Navigation.goBack(ROUTES.REPORT_SETTINGS.getRoute(report.reportID, route.params.backTo));
}, [report.reportID, route.params.backTo]);
const goBack = () => {
Navigation.goBack(backPath);
Copy link
Contributor

@github-actions github-actions bot Mar 12, 2026

Choose a reason for hiding this comment

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

❌ CLEAN-REACT-PATTERNS-0 (docs)

React Compiler is enabled in this codebase (babel-plugin-react-compiler in babel.config.js). It automatically memoizes closures based on their captured variables, making manual useCallback wrappers redundant. The goBack and updateWriteCapability functions (lines 35 and 39) are both wrapped in useCallback, but the compiler will handle this memoization automatically.

Remove the useCallback wrappers and use plain functions instead:

const goBack = () => {
    Navigation.goBack(backPath);
};

const updateWriteCapability = (newValue: ValueOf<typeof CONST.REPORT.WRITE_CAPABILITIES>) => {
    updateWriteCapabilityUtil(report.reportID, newValue);
    Navigation.goBack(backPath);
};

Also remove useCallback from the import React, {useCallback} from 'react'; statement on line 1.

Please rate this suggestion with 👍 or 👎 to help us improve! Reactions are used to monitor reviewer efficiency.

};

const updateWriteCapability = useCallback(
(newValue: ValueOf<typeof CONST.REPORT.WRITE_CAPABILITIES>) => {
Expand Down Expand Up @@ -69,4 +66,4 @@ function WriteCapabilityPage({report, policy}: WriteCapabilityPageProps) {
);
}

export default withReportOrNotFound()(WriteCapabilityPage);
export default withReportOrNotFound()(DynamicWriteCapabilityPage);
2 changes: 1 addition & 1 deletion src/pages/settings/Report/ReportSettingsPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ function ReportSettingsPage({report, policy, route}: ReportSettingsPageProps) {
shouldShowRightIcon
title={writeCapabilityText}
description={translate('writeCapabilityPage.label')}
onPress={() => Navigation.navigate(ROUTES.REPORT_SETTINGS_WRITE_CAPABILITY.getRoute(reportID, backTo))}
onPress={() => Navigation.navigate(createDynamicRoute(DYNAMIC_ROUTES.REPORT_SETTINGS_WRITE_CAPABILITY.path))}
Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot Mar 12, 2026

Choose a reason for hiding this comment

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

P1 Badge Pass reportID when opening dynamic write-capability route

This navigation call builds the dynamic URL with only DYNAMIC_ROUTES.REPORT_SETTINGS_WRITE_CAPABILITY.path, but getStateForDynamicRoute() only carries query params into route.params; it does not recover reportID from the base path. The target page is wrapped with withReportOrNotFound(), which reads route.params.reportID, so opening this screen from Report Settings can leave reportID undefined and send users to the NotFound/loading fallback instead of the write-capability selector.

Useful? React with 👍 / 👎.

Copy link
Contributor

@mjasikowski mjasikowski Mar 19, 2026

Choose a reason for hiding this comment

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

@huult was this addressed?

Copy link
Collaborator

@mananjadhav mananjadhav Mar 19, 2026

Choose a reason for hiding this comment

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

I explicitly tested this and I didn't come across this issue.

Copy link
Contributor Author

@huult huult Mar 20, 2026

Choose a reason for hiding this comment

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

@mjasikowski This issue is resolved

/>
) : (
<View style={[styles.ph5, styles.pv3]}>
Expand Down
Loading