Migrate money request time step to dynamic routes (#83850 part 2 - #97762
Migrate money request time step to dynamic routes (#83850 part 2#97762suneox wants to merge 3 commits into
Conversation
…rt 2, re-land) Re-lands PR Expensify#96963 (reverted by Expensify#97582 as deploy blocker Expensify#97558) together with the fix for the regression that caused the revert. Migration: MONEY_REQUEST_STEP_TIME / _TIME_EDIT become the dynamic suffixes `per-diem-time` / `per-diem-time-edit`, the page is renamed to DynamicIOURequestStepTime, `backTo` is dropped from both screen params and the old static paths redirect through OldRoutes. Regression fix (Expensify#97558): the per diem workspace selector retargets the report when it opens the destination step, but the start route beneath it kept the report the flow was started from. Because the dynamic suffixes carry no `:reportID`, the URL - and therefore any back path derived from it - described the stale report, so `goUp` could not match the destination route in the stack and replaced the time step instead of popping to it, duplicating the suffix and landing on a not found page. The selector now retargets its own route params so the URL stays truthful, and the wizard back target is rebuilt from the time route's own params instead of being derived from the URL. Adds tests/navigation/PerDiemTimeGoBackTests.tsx, which locks in the live (non-refresh) back path for both the matching and the stale-reportID case.
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
`retarget` is not in the cspell dictionary, so the comments and the test helper name use plain wording instead, and each explanation is trimmed to three lines.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2e7a9a8ea4
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| // makes the URL describe a report that has no per diem rates: refreshing or sharing the destination/time | ||
| // URL then resolves an empty policy and renders Not Found, and the start page treats its own draft as | ||
| // stale (`isStaleTransactionDraft`) because the draft's reportID no longer matches the route's. | ||
| navigation.setParams({reportID: targetReport.reportID}); |
There was a problem hiding this comment.
When the workspace selector retargets a global-create per diem flow to the self-DM/unreported case, getInitialPerDiemTargetReport() can change targetIouType to TRACK, but this only updates the base route's reportID. Because the new destination/time steps are dynamic suffixes and their browser URL is rebuilt from the underlying start route, refreshing or sharing the time URL in that case rehydrates with the original iouType instead of TRACK, even though the transaction was retargeted. Please set the start route's iouType to targetIouType at the same time as the reportID.
Useful? React with 👍 / 👎.
| // the wizard variant to the destination-based suffix. Query is not preserved (trailing wildcard would swallow it) - | ||
| // acceptable because callers no longer append `?backTo=`. | ||
| '/*/*/time/*/*/edit': '/$1/$2/confirmation/$3/$4/per-diem-time-edit', | ||
| '/*/*/time/*/*': '/$1/$2/destination/$3/$4/per-diem-time', |
There was a problem hiding this comment.
The removed static time route accepted an optional backToReport path segment, so existing links like /create/submit/time/<transactionID>/<reportID>/<backToReport> no longer match this redirect because the pattern ends after the reportID. Those legacy URLs now fall through after MONEY_REQUEST_STEP_TIME was removed from the linking config instead of opening the migrated dynamic time step; add a more specific redirect that carries the extra segment into the new start/destination/time path.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
/*/*/time/*/* does not end at the reportID. In getMatchingNewRoute a trailing * compiles to (.*) (multi-segment) while non-trailing wildcards compile to ([^/]+) (exactly one segment). So the last wildcard swallows <reportID>/<backToReport> and $4 carries both segments into the new path.
…-request-part-2-v2-suneox
2 participants
Explanation of Change
Fixed Issues
$ #83850
$ #97558
PROPOSAL:
Tests
DYNAMIC_ROUTES.MONEY_REQUEST_STEP_TIME— wizard time step (non-edit)Case 1.1 — Destination → Time forward (multi-policy, through the workspace selector)
Target:
src/pages/iou/request/step/DynamicIOURequestStepDestination.tsx:183·src/pages/iou/request/step/IOURequestStepPerDiemWorkspace.tsx:101Precondition: at least 2 workspaces with per diem enabled and rates, and the currently selected workspace without per diem, so the per diem tab shows the workspace selector.
Steps:
Expected: the Time step opens at
create/create/start/1/<workspace expense chat reportID>/per-diem-destination/per-diem-time. The reportID in the URL is the picked workspace's expense chat, not the report the flow was started from.Case 1.2 ⭐ — Time Back → Destination (the deploy blocker case, #97558)
Target:
src/pages/iou/request/step/DynamicIOURequestStepTime.tsx:90Precondition: Case 1.1 completed — the Time step is open in the multi-policy flow.
Steps:
Expected: the Destination step opens with a single
per-diem-destinationsuffix in the URL — no duplicatedper-diem-destination/per-diem-destinationand no "Oops, this page cannot be found".Case 1.3 — Time Save → Subrate (forward; the subrate step is still a static route)
Precondition: the Time step is open.
start = end = 12:00 AMis invalid.Steps:
Expected: the Subrate step opens.
Case 1.4 — Subrate Back → Time (cross-file goBack)
Target:
src/pages/iou/request/step/IOURequestStepSubrate.tsx:131Precondition: Case 1.3 completed — the Subrate step is open.
Steps:
Expected: the Time step opens again at
…/per-diem-destination/per-diem-time, with no duplicated suffix and no "Oops, this page cannot be found".Case 1.5 — Refresh on the Time URL (multi-policy) → Back
Precondition: the Time step is open in the multi-policy flow (Case 1.1).
Steps:
Expected: the Time step is restored with its policy resolved (the URL carries the workspace expense chat reportID), and Back returns to the Destination step.
Case 1.6 — Single per diem policy: Time Back → start page
Target:
src/pages/iou/request/step/DynamicIOURequestStepTime.tsx:90Precondition: exactly 1 workspace with per diem rates, so the destination step renders inline on the start page.
Steps:
…/start/1/<reportID>/per-diem-time, without aper-diem-destinationsuffix.Expected: the start page's Per diem tab opens on the first tap.
Case 1.7 — Report-bound flow: Time Back → start page Per diem tab
Target:
src/pages/iou/request/step/DynamicIOURequestStepTime.tsx:90Precondition: an expense chat of a workspace with per diem enabled and rates.
Steps:
Expected: the start page's Per diem tab opens (
…/start/1/<reportID>/per-diem). Not the Destination RHP and not "Oops, this page cannot be found".Case 1.8 — Old URL redirect (non-edit)
Target:
src/libs/Navigation/linkingConfig/OldRoutes.ts:14Precondition: none — the redirect is evaluated while building the state from the URL.
Steps:
create/submit/time/<transactionID>/<reportID>in the address bar.Expected: it redirects to⚠️ A
create/submit/destination/<transactionID>/<reportID>/per-diem-timeand the Time step opens.?backTo=query on the old URL is not preserved (the trailing wildcard swallows it) — acceptable because no call site produces the old path with a query anymore.DYNAMIC_ROUTES.MONEY_REQUEST_STEP_TIME_EDIT— editing time from the confirmation stepCase 2.1 — Confirmation → edit Time (forward)
Target:
src/components/MoneyRequestConfirmationList/sections/PerDiemFields.tsx:129Precondition: a per diem expense driven all the way to the Confirmation step (destination → time → subrate with a quantity → Save).
Steps:
Expected: the Time step opens in edit mode (header "Time") at
…/confirmation/…/per-diem-time-edit.Case 2.2 — Edit Back → Confirmation (value unchanged)
Target:
src/pages/iou/request/step/DynamicIOURequestStepTime.tsx:86Precondition: Case 2.1 completed — the edit page is open.
Steps:
Expected: the Confirmation step opens again with the time unchanged and no "Oops, this page cannot be found".
Case 2.3 — Edit, change the time and Save → Confirmation (updated)
Precondition: Case 2.1 completed — the edit page is open.
Steps:
Expected: the Confirmation step opens again and the Time field shows the new value.
Case 2.4 — Refresh on the
per-diem-time-editURL → BackPrecondition: Case 2.1 completed — the edit page is open.
Steps:
Expected: the edit page is restored on top of the Confirmation base, and Back returns to Confirmation.
Case 2.5 — Old URL redirect (edit)
Target:
src/libs/Navigation/linkingConfig/OldRoutes.ts:13Precondition: none.
Steps:
create/submit/time/<transactionID>/<reportID>/editin the address bar.Expected: it redirects to⚠️ Known gap across the whole migration (
create/submit/confirmation/<transactionID>/<reportID>/per-diem-time-editand the edit page opens.MoneyRequestInitializerre-initialises the flow when the transaction draft no longer exists) — if it lands onstart/1/<reportID>/per-dieminstead, that is the known gap, not a regression of this PR.Offline tests
QA Steps
Test case 1: Per diem time step opens and goes back correctly when a workspace is picked first
/per-diem-destination/per-diem-time./per-diem-destination, and no "Oops, this page cannot be found" is shown.Test case 2: Time can be edited from the confirmation step
/per-diem-time-edit.PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectionAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari
CleanShot.2026-08-04.at.14.04.23.1.mp4
Case 1.1 — Destination → Time forward (multi-policy, through the workspace selector)
Case 1.2 — Time Back → Destination (the deploy blocker case, #97558)
Case 1.3 — Time Save → Subrate
Case 1.4 — Subrate Back → Time
Case 1.5 — Refresh on the Time URL → Back
Case 1.6 — Single per diem policy: Time Back → start page
Case 1.7 — Report-bound flow: Time Back → start page Per diem tab
Case 1.8 — Old URL redirect (non-edit)
Case 2.1 — Confirmation → edit Time
Case 2.2 — Edit Back → Confirmation
Case 2.3 — Edit, change the time and Save → Confirmation
Case 2.4 — Refresh on the per-diem-time-edit URL → Back
Case 2.5 — Old URL redirect (edit)