Fix scan-failed expense sheet showing not found after reconnect by re… - #98863
Conversation
…conciling the moved thread and route with the backend report
|
This PR adds a new |
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
![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: ab945f7bdb
ℹ️ 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 params instead of substring before redirecting
| }; | ||
| } | ||
|
|
||
| export {getReportFromHoldRequestsOnyxData, putOnHold, putTransactionsOnHold, unholdRequest}; |
There was a problem hiding this comment.
NAB: Do we need to add a JSDoc comment here? Thanks.
| }, | ||
| }); | ||
| } | ||
|
|
There was a problem hiding this comment.
NAB: Same here, I think a comment is missing.
|
@yusufdeveloper2903 I think I found a bug:
Screen.Recording.2026-08-18.at.10.40.44.mov |
…ument the moved scan-failed reconciliation helpers
Thanks for the catch Fixed now every route referencing the optimistic report gets repointed, not just the focused one (JSDoc comments added too). Here's a video following your repro steps , could you take another look Thanks REC-20260818201640.mp4 |
garrettmknight
left a comment
garrettmknight
left a comment
There was a problem hiding this comment.
Flow makes sense from a product perspective.
| // The backend's report actions for the moved expense may arrive after the transaction update, and this runs from the | ||
| // action layer where no view exists to subscribe with useOnyx, so connectWithoutView is the only way to wait for them. |
There was a problem hiding this comment.
This is all triggered from payMoneyRequest() which is called from a UI component, so can't the data be gathered there with useOnyx() and passed through parameters?
|
Bump @yusufdeveloper2903 please prioritize this since it's fixing a deploy blocker |
|
Heads up: we are going to revert PR 97084 to clear the deploy blocker rather than fix forward here. Revert PR: #98914 The reasoning is deploy timing, not the quality of this work. Issue 98821 is the last unchecked blocker on the 2026-08-18 deploy checklist, the regression is not in production yet, and the revert restores exactly what production runs today. That puts issue 96257 back on the board at Weekly priority, so the optimistic split can be re-landed without deadline pressure. @yusufdeveloper2903 thank you for the quick turnaround here, and sorry to send it back. One thing worth considering for the next attempt: the App already reconciles "the backend used a different report than the optimistic one" through |
6 participants
Explanation of Change
When a scan-failed expense is optimistically moved to a new report on Pay (#97084), the backend creates its own report instead of reusing
optimisticHoldReportID, so the optimistic report is dropped insuccessDataon reconnect. If the user opened the moved expense while offline, its sheet stayed pointed at the dropped optimistic report and its client-only copied action, so on reconnect the sheet showed the "Hmm... it's not here" page instead of the expense.This PR adds
watchMovedScanFailedTransactions: after Pay is sent, it watches each moved transaction until the backend assigns its real report ID, then re-points the expense thread (parentReportID/parentReportActionID/chatReportID) to the backend report and its real IOU action, and redirects any route still showing the optimistic report to the real one. The reconciliation also handles the failure path (transaction returning to its original report).Known limitation: the watcher lives in memory, so if the app is reloaded between the offline Pay and reconnect, the reconciliation does not run. Fully closing that edge requires the backend to reuse
optimisticHoldReportID.Fixed Issues
$ #96257
$ #98821
PROPOSAL: #96257 (comment)
Tests
Precondition: workspace with payments enabled, a report containing one valid manual expense and one scan-failed expense (amount 0, missing merchant), approved and ready to pay.
Offline tests
QA Steps
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
After fix:REC-20260818163046.mp4