fix: Dismissed photo appears on confirmation page when uploading receipt after taking photo#77816
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes an issue where a dismissed photo would appear on the confirmation page when uploading a new receipt after taking a photo. The fix ensures that draft transactions are properly cleaned up when multi-scan mode is not enabled.
Key changes:
- Added
removeDraftTransactions(true)call when multi-scan is disabled to clear old draft transactions before processing new files - Updated transaction reuse logic to check for
initialTransactionexistence and modified conditions to properly handle multi-scan vs single-scan scenarios
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
src/pages/iou/request/step/IOURequestStepScan/index.tsx |
Added draft transaction cleanup for non-multi-scan mode and refined transaction reuse logic to prevent dismissed photos from appearing |
src/pages/iou/request/step/IOURequestStepScan/index.native.tsx |
Applied the same fixes as the web version to ensure consistent behavior across platforms |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| initialTransaction && | ||
| (!shouldAcceptMultipleFiles || | ||
| (index === 0 && (!isMultiScanEnabled || (transactions.length === 1 && (!initialTransaction.receipt?.source || initialTransaction.receipt?.isTestReceipt))))) |
There was a problem hiding this comment.
This boolean condition has become quite complex with multiple nested levels of logic. Consider extracting this into a helper function with a descriptive name (e.g., shouldReuseInitialTransaction) to improve readability and maintainability. The function could take parameters like initialTransaction, shouldAcceptMultipleFiles, index, isMultiScanEnabled, and transactions and return a boolean indicating whether to reuse the initial transaction.
There was a problem hiding this comment.
Yes @abzokhattab, this is indeed a great suggestion and I’m happy you’re thinking the same way. Let me implement this.
Codecov Report❌ Looks like you've decreased code coverage for some files. Please write tests to increase, or at least maintain, the existing level of code coverage. See our documentation here for how to interpret this table.
|
|
@abzokhattab All yours. |
joekaufmanexpensify
left a comment
There was a problem hiding this comment.
Makes sense from a product perspective 👍
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppScreen.Recording.2025-12-18.at.12.34.53.movAndroid: mWeb ChromeScreen.Recording.2025-12-18.at.12.31.31.moviOS: HybridAppScreen.Recording.2025-12-18.at.12.28.18.moviOS: mWeb SafariScreen.Recording.2025-12-18.at.12.30.02.movMacOS: Chrome / SafariScreen.Recording.2025-12-18.at.12.27.31.mov |
|
LGTM |
|
🚀 Deployed to staging by https://github.com/iwiznia in version: 9.2.82-0 🚀
|
|
🚀 Deployed to staging by https://github.com/iwiznia in version: 9.2.82-0 🚀
|
|
🚀 Deployed to staging by https://github.com/iwiznia in version: 9.2.82-0 🚀
|
|
🚀 Deployed to staging by https://github.com/iwiznia in version: 9.2.84-0 🚀
|
|
🚀 Deployed to production by https://github.com/AndrewGable in version: 9.2.84-8 🚀
|
Explanation of Change
Fixed Issues
$ #76110
PROPOSAL: #76110 (comment)
Tests
Offline tests
Same as test
QA Steps
Same as test
// TODO: These must be filled out, or the issue title must include "[No QA]."
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectioncanBeMissingparam foruseOnyxtoggleReportand 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.Native.mp4
Android: mWeb Chrome
Android.mWeb.Chrome.mov
iOS: Native
IOS.Native.mov
iOS: mWeb Safari
IOS.mWeb.Safari.mov
MacOS: Chrome / Safari
MAC.OS.Chrome.mov