Guard against empty receipt source in ReceiptFileValidator#89313
Conversation
Add itemReceiptPath truthiness check so that receipts with empty source (after blob URL expiry clears the source) are not added to receiptFiles, preventing the confirmation page from showing an enabled button with no receipt preview. Co-authored-by: Fitsum Abebe <FitseTLT@users.noreply.github.com>
|
@MelvinBot request my review |
|
Review requested from |
|
@MelvinBot the test steps should be Navigate to workspace chat or Click global + |
|
Updated the Tests and QA Steps sections with the provided test steps ✅ |
|
@MelvinBot complete your checklist and make it ready for review |
|
|
||
| if (!isLocalFile) { | ||
| if (item.receipt) { | ||
| if (item.receipt && itemReceiptPath) { |
There was a problem hiding this comment.
The check should be added for the above line @MelvinBot
There was a problem hiding this comment.
Added the itemReceiptPath guard to the isLocalFileFileUtils call as well — ReceiptFileValidator.tsx:62. Pushed in a91fbec.
There was a problem hiding this comment.
Ok @MelvinBot add &&itemReceiptPath check on line 64 that's all that is needed remove other changes.
There was a problem hiding this comment.
Moved the itemReceiptPath guard to line 64 (if (!isLocalFile && itemReceiptPath)) and reverted line 65 back to if (item.receipt). Pushed in 0f67f2c.
|
Completed the PR Author Checklist and marked the PR as ready for review. Note for co-author: The following items are left unchecked as they require manual verification:
Please complete these if applicable before merge. |
Co-authored-by: FitseTLT <48335213+FitseTLT@users.noreply.github.com> Co-authored-by: Fitsum Abebe <FitseTLT@users.noreply.github.com>
|
@Julesssss 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] |
Per review feedback, remove the isLocalFile guard change and keep only the && itemReceiptPath check in the if (item.receipt) condition. Co-authored-by: FitseTLT <73498291+FitseTLT@users.noreply.github.com> Co-authored-by: Fitsum Abebe <FitseTLT@users.noreply.github.com>
|
Done — reverted the |
Co-authored-by: Fitsum Abebe <FitseTLT@users.noreply.github.com>
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb Chrome2026-05-05.23-40-53.mp4iOS: HybridAppiOS: mWeb Safari2026-05-05.23-46-13.mp4MacOS: Chrome / Safari2026-05-05.23-42-25.mp4 |
Co-authored-by: Fitsum Abebe <FitseTLT@users.noreply.github.com>
|
Fixed the failing check: the test mock for |
|
@MelvinBot fix jest failure |
…ssing Navigation mock - Add early return in ReceiptFileValidator when no receipt path/filename exists (e.g. distance requests), preventing false onFailure and draft removal - Add getActiveRouteWithoutParams to Navigation mock in test Co-authored-by: Fitsum Abebe <FitseTLT@users.noreply.github.com>
|
Fixed the jest failure. Two issues:
|
|
|
||
| // No receipt at all (e.g. distance requests) — nothing to validate | ||
| if (!itemReceiptPath && !itemReceiptFilename) { | ||
| return Promise.resolve(); | ||
| } | ||
|
|
There was a problem hiding this comment.
Removed the early return guard block. Pushed in 42bb7b0.
Co-authored-by: Fitsum Abebe <FitseTLT@users.noreply.github.com>
|
Asking question https://github.com/Expensify/App/pull/86865/changes#r3197679578 |
|
@MelvinBot revert the current change
|
Move setMoneyRequestReceipt call from inside the onFailure callback to after Promise.all resolves, preventing the transaction change from retriggering the effect before the promise chain completes. Co-authored-by: Fitsum Abebe <FitseTLT@users.noreply.github.com>
|
@MelvinBot U forgot to revert all the previous changes before making the latest changes 😢 |
The previous commit kept the original `&& itemReceiptPath` guard on the non-local file check. This reverts that guard so only the deferred receipt reset logic remains as the fix. Co-authored-by: Fitsum Abebe <FitseTLT@users.noreply.github.com>
|
Reverted the original |
|
@MelvinBot remove changes to tests/ui/components/IOURequestStepConfirmationPageTest.tsx |
Co-authored-by: Fitsum Abebe <FitseTLT@users.noreply.github.com>
|
Reverted changes to |
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
|
U can proceed @Julesssss |
Julesssss
left a comment
There was a problem hiding this comment.
Looking good. Comment is brief and clear
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚧 @Julesssss has triggered a test Expensify/App build. You can view the workflow run here. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
🚀 Deployed to staging by https://github.com/Julesssss in version: 9.3.69-0 🚀
Bundle Size Analysis (Sentry): |
|
No help site changes are required for this PR. This is a purely internal bug fix that defers a |
|
Deploy Blocker ##90025 was identified to be related to this PR. |
|
🚀 Deployed to production by https://github.com/Beamanator in version: 9.3.69-18 🚀
|
Explanation of Change
When a browser page refreshes,
blob://URLs in the transaction draft become invalid.ReceiptFileValidatordetects this and callsonFailure, which clears the receipt source to''. On re-render, the non-local file branch (isLocalFile('')returnsfalse) adds the receipt with empty source toreceiptFilesbecause it only checksitem.receipt(truthy object) — not whether the source is actually present. This hides the loading state and enables the "Create Expense" button with no receipt preview.This PR adds an
itemReceiptPathtruthiness check so that receipts with an empty source are not treated as valid non-local files.Fixed Issues
$ #88975
PROPOSAL: #88975 (comment)
Tests
Offline tests
This change only affects how receipt sources are validated after page refresh. Offline behavior is unchanged — the guard only prevents empty-source receipts from being treated as valid non-local files. No new offline-specific flows are introduced.
QA Steps
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
N/A — no UI changes, logic-only fix
Android: mWeb Chrome
N/A — no UI changes, logic-only fix
iOS: Native
N/A — no UI changes, logic-only fix
iOS: mWeb Safari
N/A — no UI changes, logic-only fix
MacOS: Chrome / Safari
N/A — no UI changes, logic-only fix