Skip to content

BT: Migrate CHANGE_APPROVER #88361

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 3 commits into from
Apr 21, 2026
Merged

BT: Migrate CHANGE_APPROVER #88361

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d91fede
feat: migrate change approver to dynamic route
mananjadhav Apr 20, 2026
99b7ea3
fix: remove useCallback
mananjadhav Apr 21, 2026
b760d8a
fix: typo
mananjadhav Apr 21, 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 @@ -328,6 +328,10 @@ const DYNAMIC_ROUTES = {
path: 'exit-survey/confirm',
entryScreens: [SCREENS.SETTINGS.DYNAMIC_EXIT_SURVEY_REASON, SCREENS.SETTINGS.TROUBLESHOOT],
},
REPORT_CHANGE_APPROVER: {
path: 'change-approver',
entryScreens: [SCREENS.REPORT, SCREENS.RIGHT_MODAL.SEARCH_REPORT, SCREENS.RIGHT_MODAL.EXPENSE_REPORT, SCREENS.RIGHT_MODAL.SEARCH_MONEY_REQUEST_REPORT],
},
FLAG_COMMENT: {
path: 'flag/:reportID/:reportActionID',
entryScreens: [SCREENS.REPORT, SCREENS.RIGHT_MODAL.SEARCH_REPORT, SCREENS.RIGHT_MODAL.EXPENSE_REPORT, SCREENS.RIGHT_MODAL.SEARCH_MONEY_REQUEST_REPORT],
Expand Down Expand Up @@ -1045,12 +1049,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_CHANGE_APPROVER: {
route: 'r/:reportID/change-approver',

// eslint-disable-next-line no-restricted-syntax -- Legacy route generation
getRoute: (reportID: string, backTo?: string) => getUrlWithBackToParam(`r/${reportID}/change-approver` as const, backTo),
},
REPORT_CHANGE_APPROVER_ADD_APPROVER: {
route: 'r/:reportID/change-approver/add',
getRoute: (reportID: string) => `r/${reportID}/change-approver/add` as const,
Expand Down
2 changes: 1 addition & 1 deletion src/SCREENS.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -973,7 +973,7 @@ const SCREENS = {
CONFIRMATION: 'ScheduleCall_Confirmation',
},
REPORT_CHANGE_APPROVER: {
ROOT: 'Report_Change_Approver_Root',
DYNAMIC_ROOT: 'Dynamic_Report_Change_Approver_Root',
ADD_APPROVER: 'Report_Change_Approver_Add_Approver',
},
TEST_TOOLS_MODAL: {
Expand Down
5 changes: 3 additions & 2 deletions src/hooks/useExpenseActions.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import initSplitExpense from '@libs/actions/SplitExpenses';
import getNonEmptyStringOnyxID from '@libs/getNonEmptyStringOnyxID';
import {getExistingTransactionID} from '@libs/IOUUtils';
import Log from '@libs/Log';
import createDynamicRoute from '@libs/Navigation/helpers/dynamicRoutesUtils/createDynamicRoute';
import Navigation from '@libs/Navigation/Navigation';
import {isPolicyAccessible} from '@libs/PolicyUtils';
import {getIOUActionForTransactionID, getOriginalMessage, isMoneyRequestAction} from '@libs/ReportActionsUtils';
Expand Down Expand Up @@ -43,7 +44,7 @@ import {getNavigationUrlOnMoneyRequestDelete} from '@userActions/IOU/DeleteMoney
import {setDeleteTransactionNavigateBackUrl} from '@userActions/Report';
import CONST from '@src/CONST';
import ONYXKEYS from '@src/ONYXKEYS';
import ROUTES from '@src/ROUTES';
import ROUTES, {DYNAMIC_ROUTES} from '@src/ROUTES';
import type {Route} from '@src/ROUTES';
import type * as OnyxTypes from '@src/types/onyx';
import useConfirmModal from './useConfirmModal';
Expand Down Expand Up @@ -451,7 +452,7 @@ function useExpenseActions({reportID, isReportInSearch = false, backTo, onDuplic
Log.warn('Change approver secondary action triggered without moneyRequestReport data.');
return;
}
Navigation.navigate(ROUTES.REPORT_CHANGE_APPROVER.getRoute(moneyRequestReport.reportID, Navigation.getActiveRoute()));
Navigation.navigate(createDynamicRoute(DYNAMIC_ROUTES.REPORT_CHANGE_APPROVER.path));
},
},
[CONST.REPORT.SECONDARY_ACTIONS.DELETE]: {
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 @@ -267,7 +267,7 @@ const ReportChangeWorkspaceModalStackNavigator = createModalStackNavigator<Repor
});

const ReportChangeApproverModalStackNavigator = createModalStackNavigator<ReportChangeApproverParamList>({
[SCREENS.REPORT_CHANGE_APPROVER.ROOT]: () => require<ReactComponentModule>('../../../../pages/ReportChangeApproverPage').default,
[SCREENS.REPORT_CHANGE_APPROVER.DYNAMIC_ROOT]: () => require<ReactComponentModule>('../../../../pages/DynamicReportChangeApproverPage').default,
[SCREENS.REPORT_CHANGE_APPROVER.ADD_APPROVER]: () => require<ReactComponentModule>('../../../../pages/ReportAddApproverPage').default,
});

Expand Down
2 changes: 1 addition & 1 deletion src/libs/Navigation/linkingConfig/config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2119,7 +2119,7 @@ const config: LinkingOptions<RootNavigatorParamList>['config'] = {
},
[SCREENS.RIGHT_MODAL.REPORT_CHANGE_APPROVER]: {
screens: {
[SCREENS.REPORT_CHANGE_APPROVER.ROOT]: ROUTES.REPORT_CHANGE_APPROVER.route,
[SCREENS.REPORT_CHANGE_APPROVER.DYNAMIC_ROOT]: DYNAMIC_ROUTES.REPORT_CHANGE_APPROVER.path,
[SCREENS.REPORT_CHANGE_APPROVER.ADD_APPROVER]: ROUTES.REPORT_CHANGE_APPROVER_ADD_APPROVER.route,
},
},
Expand Down
2 changes: 1 addition & 1 deletion src/libs/Navigation/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3209,7 +3209,7 @@ type ScheduleCallParamList = {
};

type ReportChangeApproverParamList = {
[SCREENS.REPORT_CHANGE_APPROVER.ROOT]: {
[SCREENS.REPORT_CHANGE_APPROVER.DYNAMIC_ROOT]: {
reportID: string;
};
[SCREENS.REPORT_CHANGE_APPROVER.ADD_APPROVER]: {
Expand Down
18 changes: 12 additions & 6 deletions src/pages/ReportChangeApproverPage.tsx → ...pages/DynamicReportChangeApproverPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import SelectionList from '@components/SelectionList';
import RadioListItem from '@components/SelectionList/ListItem/RadioListItem';
import type {ListItem} from '@components/SelectionList/types';
import useCurrentUserPersonalDetails from '@hooks/useCurrentUserPersonalDetails';
import useDynamicBackPath from '@hooks/useDynamicBackPath';
import useEnvironment from '@hooks/useEnvironment';
import useLocalize from '@hooks/useLocalize';
import useOnyx from '@hooks/useOnyx';
Expand All @@ -22,7 +23,7 @@ import {isControlPolicy, isPolicyAdmin} from '@libs/PolicyUtils';
import {hasViolations as hasViolationsReportUtils, isAllowedToApproveExpenseReport, isMoneyRequestReport, isMoneyRequestReportPendingDeletion} from '@libs/ReportUtils';
import CONST from '@src/CONST';
import ONYXKEYS from '@src/ONYXKEYS';
import ROUTES from '@src/ROUTES';
import ROUTES, {DYNAMIC_ROUTES} from '@src/ROUTES';
import type SCREENS from '@src/SCREENS';
import {isEmptyObject} from '@src/types/utils/EmptyObject';
import NotFoundPage from './ErrorPage/NotFoundPage';
Expand All @@ -36,9 +37,9 @@ const APPROVER_TYPE = {

type ApproverType = ValueOf<typeof APPROVER_TYPE>;

type ReportChangeApproverPageProps = WithReportOrNotFoundProps & PlatformStackScreenProps<ReportChangeApproverParamList, typeof SCREENS.REPORT_CHANGE_APPROVER.ROOT>;
type DynamicReportChangeApproverPageProps = WithReportOrNotFoundProps & PlatformStackScreenProps<ReportChangeApproverParamList, typeof SCREENS.REPORT_CHANGE_APPROVER.DYNAMIC_ROOT>;

function ReportChangeApproverPage({report, policy, isLoadingReportData}: ReportChangeApproverPageProps) {
function DynamicReportChangeApproverPage({report, policy, isLoadingReportData}: DynamicReportChangeApproverPageProps) {
const reportID = report?.reportID;
const {translate} = useLocalize();
const styles = useThemeStyles();
Expand All @@ -53,6 +54,11 @@ function ReportChangeApproverPage({report, policy, isLoadingReportData}: ReportC
const [reportNextStep] = useOnyx(`${ONYXKEYS.COLLECTION.NEXT_STEP}${reportID}`);
const hasAutoAppliedRef = useRef(false);
const hasNavigatedToAddApproverRef = useRef(false);
const backPath = useDynamicBackPath(DYNAMIC_ROUTES.REPORT_CHANGE_APPROVER.path);
Copy link
Copy Markdown
Contributor

@github-actions github-actions bot Apr 20, 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)

This file compiles with React Compiler (verified via check-compiler.sh). The new useCallback wrapping goBack is redundant because React Compiler automatically memoizes closures based on their captured variables.

Remove the useCallback wrapper and use a plain function instead:

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

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

Copy link
Copy Markdown
Contributor

@huult huult Apr 21, 2026

Choose a reason for hiding this comment

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

@mananjadhav Could you please update this?

Copy link
Copy Markdown
Contributor

@huult huult Apr 21, 2026

Choose a reason for hiding this comment

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

@mananjadhav We only need to update this. After that, I’ll approve the PR and we can merge.

Copy link
Copy Markdown
Collaborator Author

@mananjadhav mananjadhav Apr 21, 2026

Choose a reason for hiding this comment

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

Done.


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

const changeApprover = useCallback(() => {
if (!selectedApproverType) {
Expand Down Expand Up @@ -140,13 +146,13 @@ function ReportChangeApproverPage({report, policy, isLoadingReportData}: ReportC

return (
<ScreenWrapper
testID="ReportChangeApproverPage"
testID="DynamicReportChangeApproverPage"
includeSafeAreaPaddingBottom
shouldEnableMaxHeight
>
<HeaderWithBackButton
title={translate('iou.changeApprover.title')}
onBackButtonPress={Navigation.goBack}
onBackButtonPress={goBack}
/>
<SelectionList
data={approverTypes}
Expand Down Expand Up @@ -176,6 +182,6 @@ function ReportChangeApproverPage({report, policy, isLoadingReportData}: ReportC
);
}

export default withReportOrNotFound()(ReportChangeApproverPage);
export default withReportOrNotFound()(DynamicReportChangeApproverPage);
export {APPROVER_TYPE};
export type {ApproverType};
5 changes: 3 additions & 2 deletions src/pages/ReportAddApproverPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import useOnyx from '@hooks/useOnyx';
import usePermissions from '@hooks/usePermissions';
import useThemeStyles from '@hooks/useThemeStyles';
import {addReportApprover} from '@libs/actions/IOU/ReportWorkflow';
import createDynamicRoute from '@libs/Navigation/helpers/dynamicRoutesUtils/createDynamicRoute';
import Navigation from '@libs/Navigation/Navigation';
import type {PlatformStackScreenProps} from '@libs/Navigation/PlatformStackNavigation/types';
import type {ReportChangeApproverParamList} from '@libs/Navigation/types';
Expand All @@ -24,7 +25,7 @@ import {
} from '@libs/ReportUtils';
import CONST from '@src/CONST';
import ONYXKEYS from '@src/ONYXKEYS';
import ROUTES from '@src/ROUTES';
import ROUTES, {DYNAMIC_ROUTES} from '@src/ROUTES';
import type SCREENS from '@src/SCREENS';
import type {WithReportOrNotFoundProps} from './inbox/report/withReportOrNotFound';
import withReportOrNotFound from './inbox/report/withReportOrNotFound';
Expand Down Expand Up @@ -128,7 +129,7 @@ function ReportAddApproverPage({report, isLoadingReportData, policy}: ReportAddA
testID="ReportAddApproverPage"
headerTitle={translate('iou.changeApprover.actions.addApprover')}
onBackButtonPress={() => {
Navigation.goBack(ROUTES.REPORT_CHANGE_APPROVER.getRoute(report.reportID), {compareParams: false});
Navigation.goBack(createDynamicRoute(DYNAMIC_ROUTES.REPORT_CHANGE_APPROVER.path, ROUTES.REPORT_WITH_ID.getRoute(report.reportID)), {compareParams: false});
}}
subtitle={<Text style={[styles.ph5, styles.pb3]}>{translate('iou.changeApprover.addApprover.subtitle')}</Text>}
isLoadingReportData={isLoadingReportData}
Expand Down
4 changes: 2 additions & 2 deletions src/pages/Search/SearchChangeApproverPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@ import {openBulkChangeApproverPage} from '@libs/actions/Search';
import Navigation from '@libs/Navigation/Navigation';
import {isControlPolicy, isPolicyAdmin} from '@libs/PolicyUtils';
import {hasViolations as hasViolationsReportUtils, isAllowedToApproveExpenseReport} from '@libs/ReportUtils';
import {APPROVER_TYPE} from '@pages/ReportChangeApproverPage';
import type {ApproverType} from '@pages/ReportChangeApproverPage';
import {APPROVER_TYPE} from '@pages/DynamicReportChangeApproverPage';
import type {ApproverType} from '@pages/DynamicReportChangeApproverPage';
import CONST from '@src/CONST';
import ONYXKEYS from '@src/ONYXKEYS';
import ROUTES from '@src/ROUTES';
Expand Down
2 changes: 1 addition & 1 deletion src/pages/inbox/report/withReportOrNotFound.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ type ScreenProps =
| PlatformStackScreenProps<ReportSettingsNavigatorParamList, typeof SCREENS.REPORT_SETTINGS.DYNAMIC_SETTINGS_VISIBILITY>
| PlatformStackScreenProps<RoomMembersNavigatorParamList, typeof SCREENS.ROOM_MEMBERS.DETAILS>
| PlatformStackScreenProps<ReportChangeWorkspaceNavigatorParamList, typeof SCREENS.REPORT_CHANGE_WORKSPACE.ROOT>
| PlatformStackScreenProps<ReportChangeApproverParamList, typeof SCREENS.REPORT_CHANGE_APPROVER.ROOT>;
| PlatformStackScreenProps<ReportChangeApproverParamList, typeof SCREENS.REPORT_CHANGE_APPROVER.DYNAMIC_ROOT>;

type WithReportOrNotFoundProps = WithReportOrNotFoundOnyxProps & {
route: ScreenProps['route'];
Expand Down
4 changes: 2 additions & 2 deletions tests/unit/pages/Search/SearchChangeApproverPageTest.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import {APPROVER_TYPE} from '@pages/ReportChangeApproverPage';
import type {ApproverType} from '@pages/ReportChangeApproverPage';
import {APPROVER_TYPE} from '@pages/DynamicReportChangeApproverPage';
import type {ApproverType} from '@pages/DynamicReportChangeApproverPage';
import {shouldAutoApplyApprover} from '@pages/Search/SearchChangeApproverPage';
import type {Report} from '@src/types/onyx';

Expand Down
Loading