-
Notifications
You must be signed in to change notification settings - Fork 3.5k
Improve UX for scanned receipts on Reports #78301
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
|
@heyjennahay or @JmillsExpensify Do we intend to only show This PR's behaviour is currently that we always display desktop-chrome-multi-2025-12-23_14.24.15.mp4 |
Reviewer Checklist
Screenshots/VideosMacOS: Chrome / Safaridesktop-chrome-2025-12-23_14.21.28.mp4 |
|
Commented in the bug report GH #78007 (comment) Let me know if I need to clarify further |
|
@jjcoffee i updated the logic, please check again |
|
@daledah It now shows desktop-chrome-new-2025-12-30_14.56.54.mp4Can you also add additional test steps to cover the case where we add another expense? To verify that any existing amount shows. |
|
I'll give an update soon |
@jjcoffee If we scan with a valid receipt, this issue will not be reproduced. This doesn't seem to be a front-end issue, it appears the back-end returned the scan data to snapshot data before returning the scan data to the onyx data report. |
@daledah Could you explain this more? The receipt in my example does scan and has an amount eventually (it just gets flagged as AI generated).
I'm not sure I understand this, why would we be showing $0.00 unless the snapshot is updated with incorrect data first? To be clear, what's happening is:
|
|
@jjcoffee I tried today and couldn't reproduce it. It seems this problem is because Pusher updated the snapshot data first (used to display on the Reports page) and then updated the expense data. |
|
@daledah Hmm I can still reproduce it (
But is it updating the snapshot data incorrectly? Is it still showing as pending scanning? I wonder if you're relying on some data that's only set optimistically and then the BE updates it with some other data that you'd also need to handle. |
|
Also, just a heads up that I'll be OOO Mon-Thurs (12-15th), and 50% tomorrow. |
|
@jjcoffee i fixed the bug |
src/components/SelectionListWithSections/Search/ExpenseReportListItemRow.tsx
Outdated
Show resolved
Hide resolved
src/components/SelectionListWithSections/Search/ExpenseReportListItemRow.tsx
Outdated
Show resolved
Hide resolved
|
@daledah Thanks, that fixed it for me! One other thing I've noticed is that the
|
|
Friendly bump @daledah 🙏 |
@jjcoffee The Total spend value comes directly from the snapshot data returned by the backend. Fixing this would require further investigation and changes, so I think it’s likely out of scope for this issue. |
jjcoffee
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
jasperhuangg
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
code looks good, handing off to @heyjennahay for the final product review!
|
@heyjennahay bump on this product review 🙇 |
heyjennahay
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Product change LGTM
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚀 Deployed to staging by https://github.com/jasperhuangg in version: 9.3.11-16 🚀
|





Explanation of Change
Fixed Issues
$ #78007
PROPOSAL: #78007 (comment)
Tests
Test case 1:
Scanningis displayedTest case 2:
Offline tests
Same as tests
QA Steps
Same as tests
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: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari
web.mov