Fix distance waypoint "Not here" after Scan receipt then Track distan…#94772
Conversation
…ce shortcut Launcher quick-action deeplinks (the home-screen "Scan receipt" / "Track distance" shortcuts) enter NewDot through IOURequestRedirectToStartPage, which navigates straight to the per-request route without going through startMoneyRequest. A previous shortcut's draft therefore survived under OPTIMISTIC_TRANSACTION_ID; a leftover "scan" draft has no comment.waypoints, so the distance start page showed placeholder Start/Stop waypoints (rendered locally) while the waypoint editor read the empty waypoints from Onyx and rendered the "Not here" page. Clear the stale draft in the redirect (mirroring startMoneyRequest) so the destination start page rebuilds a fresh draft with the correct shape, including the distance start/stop waypoints. The OPTIMISTIC_TRANSACTION_ID fallback covers the cold-start case where the draft collection has not hydrated yet. Add a regression test that fails without the fix. Expensify#88183
|
@ChavdaSachin Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7f62f38b10
ℹ️ 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".
…e-init - codex P1: `validTransactionDraftIDsSelector` returns `[]` (not `undefined`) before the draft collection hydrates, so the clear must always include OPTIMISTIC_TRANSACTION_ID. A `?? [...]` fallback alone passed an empty list on a cold start / hard-refresh and removed nothing, letting the stale scan draft survive. - PERF-16: guard the empty-dependency effect with a per-mount `useRef` so the non-idempotent `clearMoneyRequest` + redirect run exactly once under Strict Mode's dev double-invoke. A module-level flag would be wrong here — this page remounts on every quick-action deeplink, so a flag that never resets would block every shortcut after the first. - Add a regression test for the empty-selector (cold-start) case.
2c4decc to
a1018ef
Compare
Resolve import-block conflict in IOURequestRedirectToStartPage.tsx: keep the stale-draft clear (clearMoneyRequest + useOnyx selector + per-mount useRef guard) on top of main's updated request-type routing and new import grouping.
|
Hey @ChavdaSachin, this one had drifted a bit behind It was only in Should be ready for another look whenever you get a chance 🙏 |
The test file predated main's codebase-wide oxfmt pass; reformat its import block to match the current oxfmt 0.55.0 grouping so the fmt CI check passes.
Main refactored IOURequestRedirectToStartPage to route through startDistanceRequest/startMoneyRequest and added its own didRedirectRef guard. Those helpers only clear the draft IDs passed to them and the redirect invokes them with undefined (and the DISTANCE_ODOMETER branch navigates directly), so the stale OPTIMISTIC_TRANSACTION_ID draft still survives. Re-applied the stale-draft clear on top of the refactor (dropping our now-redundant ref guard in favor of main's didRedirectRef).
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppScreen.Recording.2026-07-13.at.5.54.28.AM.movAndroid: mWeb ChromeiOS: HybridAppScreen.Recording.2026-07-13.at.5.54.28.AM.moviOS: mWeb SafariMacOS: Chrome / Safari |
marcochavezf
left a comment
There was a problem hiding this comment.
Thanks for the changes! Just a few requested changes in the code comments
| // This page is the entry point for launcher quick-action deeplinks (e.g. the home-screen "Scan receipt" / | ||
| // "Track distance" shortcuts). A previous shortcut's draft survives under OPTIMISTIC_TRANSACTION_ID: a | ||
| // leftover "scan" draft has no `comment.waypoints`, and the distance start page only shows placeholder | ||
| // waypoints locally without writing them back, so tapping a waypoint opens the "Not here" page. The | ||
| // start*Request helpers below only clear the draft IDs they are handed and we invoke them with `undefined`, | ||
| // and the DISTANCE_ODOMETER branch navigates directly without them, so clear the stale draft here to cover | ||
| // every branch. OPTIMISTIC_TRANSACTION_ID is always included so the leftover draft is removed even before | ||
| // the draft collection hydrates — the selector returns `[]` (not `undefined`) while loading, so a `?? [...]` | ||
| // fallback alone would pass an empty list and clear nothing (#88183). |
There was a problem hiding this comment.
Could we make this comment a bit more concise and remove the reference to #88183? It feels a little too verbose and reads a bit AI-generated
There was a problem hiding this comment.
Done - trimmed it down and dropped the issue reference.
| }); | ||
| }); | ||
|
|
||
| // Regression test for https://github.com/Expensify/App/issues/88183 |
There was a problem hiding this comment.
Also here, instead of referencing the direct regression, can we just state what this test is covering and why?
There was a problem hiding this comment.
Done - reworded it to say what the test covers and why instead of linking the issue.
| expect(Navigation.navigate).toHaveBeenCalledTimes(1); | ||
| }); | ||
|
|
||
| // PR review (codex P1): on a cold start / hard-refresh the draft collection may not be hydrated when this |
There was a problem hiding this comment.
Also here, just stating what we're covering in the codebase and why
There was a problem hiding this comment.
Done - same here, described what it's covering and why.
Per reviewer feedback, make the clear-draft comment and the two test descriptions concise and state what/why they cover instead of linking the issue or the codex review.
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
|
Hi, @marcochavezf Can you check last changes? |
|
🚧 marcochavezf has triggered a test Expensify/App build. You can view the workflow run here. |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
🚀 Deployed to staging by https://github.com/marcochavezf in version: 9.4.36-0 🚀
|
Help site review — no changes requiredI reviewed the changes in this PR against the help site articles under Why: This is an internal bug fix, not a behavior or feature change. The PR clears a stale draft transaction ( The fix simply restores the already-expected behavior for Track distance / distance waypoints — it introduces no new UI, settings, labels, tabs, or user-facing workflows. The existing articles that touch distance expenses and waypoints (e.g. Distance-Expenses.md, Create-an-Expense.md) already describe the correct behavior and remain accurate after this change. Since no help-site content is affected, I did not open a draft docs PR.
|
Explanation of Change
Launcher quick-action deeplinks (the home-screen "Scan receipt" / "Track distance" shortcuts) enter NewDot through
IOURequestRedirectToStartPage, which navigates straight to the per-request route without going throughstartMoneyRequest. A previous shortcut's draft therefore survived underOPTIMISTIC_TRANSACTION_ID; a leftover "scan" draft has nocomment.waypoints, so the distance start page showed placeholder Start/Stop waypoints (rendered locally) while the waypoint editor read the empty waypoints from Onyx and rendered the "Not here" page.This clears the stale draft in the redirect (mirroring
startMoneyRequest) so the destination start page rebuilds a fresh draft with the correct shape, including the distance start/stop waypoints. TheOPTIMISTIC_TRANSACTION_IDfallback covers the cold-start case where the draft collection has not hydrated yet. A regression test that fails without the fix is added.Fixed Issues
$ #88183
PROPOSAL: #88183 (comment)
Tests
mWeb/Web (no home-screen shortcut): open
/start/create/scan, hard-refresh the page, then open/start/create/distance-map, and tap the Start (or Stop) waypoint. Verify the editor opens.Offline tests
Same steps as in Tests.
clearMoneyRequestis a synchronous OnyxmultiSetwith no network involvement, so the fix behaves identically while offline (the waypoint editor opens instead of the "Not here" page).QA Steps
Same as Tests above.
mWeb/Web: open
/start/create/scan, hard-refresh the page, then open/start/create/distance-map, and tap the Start (or Stop) waypoint. Verify the editor opens.PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)Avatar, 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
distance-scan-fix.mov
Android: mWeb Chrome
android.-.mweb.mp4
iOS: Native
ios-distance-.mp4
iOS: mWeb Safari
ios.-.mweb.MOV
MacOS: Chrome / Safari
web.-.distance.mp4