Add error handling for cases where the expense may have moved before rejection - #92860
Add error handling for cases where the expense may have moved before rejection#92860akinwale wants to merge 10 commits into
Conversation
|
Hey, I noticed you changed If you want to automatically generate translations for other locales, an Expensify employee will have to:
Alternatively, if you are an external contributor, you can run the translation script locally with your own OpenAI API key. To learn more, try running: npx ts-node ./scripts/generateTranslations.ts --helpTypically, you'd want to translate only what you changed by running |
|
@gijoe0295 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: 918e742e37
ℹ️ 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".
| errorFields: { | ||
| partial: getMicroSecondOnyxErrorWithTranslationKey('iou.rejectReport.couldNotRejectExpense'), |
There was a problem hiding this comment.
Surface the reject failure on a displayed error field
This writes the new failure message to transaction.errorFields.partial, but the transaction error consumers I found only render transaction.errorFields.route/waypoints or transaction.errors (for example ReportActionItem selects only transaction?.errorFields?.route, and MoneyRequestReceiptView merges route/waypoints/errors). In the offline move-then-reject failure path, the Onyx update is applied but no component reads partial, so the expected “expense could not be rejected” message is not shown. Please store this on a rendered report action/report error or wire partial into the transaction UI.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
@akinwale I think this is a valid concern. From my testing, we would only see the backend error message The expense has already been moved or rejected., not this client message.
|
@gijoe0295 This is ready for review. |
joekaufmanexpensify
left a comment
There was a problem hiding this comment.
Good for product 👍
|
Also, when opening the expense, it flickers from the expense view to not found page: Screen.Recording.2026-06-25.at.00.14.31.movShould we remove the transaction completely, and show the error at report level/full-screen modal since this transaction no longer exists/is not accessible? cc @joekaufmanexpensify as this is product choice and I can't access the internal link above. |
|
@joekaufmanexpensify bump on #92860 (comment) in case you missed it. |
|
I'm not totally sold on showing the error at the report level. This would potentially be an error they see much later on, right? If they were offline for a while and then go back on later, it may not be immediately clear what the error relates to. Don't we have a pattern where we show an expense on a report with lower opacity and make it dismissible with an error? I think we use it if you try and scan a receipt and the scan fails. Maybe that could work well here? |
|
friendly bump @akinwale |
This comment was marked as outdated.
This comment was marked as outdated.
|
Oops sorry @akinwale turn |
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
|
@gijoe0295 I pushed a follow-up backend fix for this. Once that is deployed, I will ping you to let you know that the App changes are ready for review. |
|
@gijoe0295 The Web-E PR was deployed to production. This is ready for review. |
|
Bug: I need to close the report RHP then reopen for the error to show up: Untitled.mov |
|
Bug: The deleted expense RHP is automatically dismissed after opening. I think we should block opening it. Screen.Recording.2026-08-17.at.16.28.11.mov |
| rejectedNextStep: 'This report was rejected. Waiting on you to fix the issues and manually resubmit.', | ||
| selectMemberError: 'Select a member to reject this report back to.', | ||
| couldNotReject: 'The report could not be rejected. Please try again.', | ||
| couldNotRejectExpense: 'The expense could not be rejected.', |
There was a problem hiding this comment.
The error message I received is The expense has already been moved or rejected. which is a backend error, not this.
| // `Transaction.errors` also carries receipt errors, which are objects rendered by their own save/delete UI, so | ||
| // keep only the plain message errors here. | ||
| const messageErrors: Errors = Object.fromEntries( | ||
| Object.entries(transaction.errors ?? {}).filter((entry): entry is [string, string | null] => typeof entry[1] === 'string' || entry[1] === null), | ||
| ); |
There was a problem hiding this comment.
I think ErrorUtils.getLatestErrorMessageField already filters out receipt errors, no?
| // A failed action (e.g. rejecting an expense that has already moved) leaves the expense in place with no pending | ||
| // action, so opacity has to be forced — `OfflineWithFeedback` only dims on its own while a write is pending. |
There was a problem hiding this comment.
| // A failed action (e.g. rejecting an expense that has already moved) leaves the expense in place with no pending | |
| // action, so opacity has to be forced — `OfflineWithFeedback` only dims on its own while a write is pending. |
This comment was marked as resolved.
This comment was marked as resolved.
|
Hey, I noticed you changed If you want to automatically generate translations for other locales, an Expensify employee will have to:
Alternatively, if you are an external contributor, you can run the translation script locally with your own OpenAI API key. To learn more, try running: npx bun ./scripts/generateTranslations.ts --helpTypically, you'd want to translate only what you changed by running |
8adab49 to
d8115a3
Compare




Explanation of Change
This handles cases where the expense may have been moved before rejection to be handled more gracefully.
Fixed Issues
$ https://github.com/Expensify/Expensify/issues/630131
PROPOSAL: https://github.com/Expensify/Expensify/issues/630131#issuecomment-4636539760
Tests
Offline tests
Same as QA steps.
QA Steps
// TODO: These must be filled out, or the issue title must include "[No QA]."
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, 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.ScrollViewcomponent to make it scrollable when more elements are added to the page.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
Screen.Recording.2026-07-30.at.18.18.12.mp4