Fix submitted message is shown on an empty one-transaction report last message#62952
Conversation
|
@mananjadhav 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] |
|
On native, opening a report immediately crashes the app. |
|
Reviewed the code, will test it today. |
|
Still testing it. @bernhardoj Can you please fix the conflicts? |
|
Fixed |
Reviewer Checklist
Screenshots/Videos |
mananjadhav
left a comment
There was a problem hiding this comment.
I tried to test the previous $ 0.0 amount to ensure we're not adding any regression but the receipt scan always ends up in a message from Concierge.
Before we merge @bernhardoj can you confirm if you've tested? Would be best if you can also upload a video for the same atleast on the Web platform.
|
Here it is: a.mp4It shows No activity yet, and when the scan completes, the pusher sets the last message to "The receipt couldn't be scanned...", but when we reopen the app, the last message is cleared. We should fix this on the pusher if we don't want that message to show. |
cc @trjExpensify @JmillsExpensify, what do you think? This PR added the action |
|
I'm a bit confused by what we're looking at here. Did the receipt scan actually fail? If so, where is the system message for it as part of actionable system messages thing? (CC: @deetergp too).
If it didn't actually fail, then I agree, we shouldn't be showing this message at all. 👍 |
The scan was successful as shown on the video, so I agree we shouldn't show the failed message at all. |
I'm a bit confused too… The receipt scan failed system message is part of improved system messages. In your screenshot @tom it looks like it did fail since there is neither a merchant nor an amount. |
|
Indeed, what is weird here is that the system message on the transaction thread didn't appear. It only appeared in the LHN, and disappears once it is opened again ( |
My screenshot is from the doc to convey the expectation if it did fail. 😅 In this video that Bernhard shared, the scan didn't seem to fail to me:
|
|
@bernhardoj is the issue we're talking about reproducible only on your PR or not? From my understanding, it's an Onyx update issue that is not tied to your changes. |
|
@lakchote it's only observable from this PR. On
|
Thanks, @bernhardoj. From a backend perspective, putting an error as the last message text when the scan was successful looks wrong. @deetergp, could you please take a look since you've authored this new report action? cc @trjExpensify for your opinion on this too |
|
So I did a bit more digging into this and I think there real issue is that we are masking the fact that the receipt did fail. Looking it up in the DB, the receipt is in state What is the actual date on the receipt? In @bernhardoj's screenshot, the date shown on the expense is 2025-06-07 which did not come from the back end since we don't have that data. |
|
If you go into the receipt tool in supportal, you can grab the receipt image. It's clearly a fakey, so posting it here with no qualms:
You're right, it has no date value on the receipt, and was marked as illegible by AZ according to the tool. That said, our guidelines for SmartScan (internal peeps doc only, sorry) state:
I'm a bit confused by that with some other competing lines in that ref, so I've followed-up in thread here to ask about it. |
|
Alright, so I confirmed in thread that this was right to be marked as illegible.
|
|
@trjExpensify should we proceed with the PR given the issue we're talking about is preexisting to this one? |
|
Yeah, I think we handle that outside of this PR as part of https://github.com/Expensify/Expensify/issues/484573 |
|
🚀 Deployed to staging by https://github.com/lakchote in version: 9.1.66-0 🚀
|
|
🚀 Deployed to production by https://github.com/lakchote in version: 9.1.66-5 🚀
|
|
🚀 Deployed to production by https://github.com/lakchote in version: 9.1.66-5 🚀
|
|
🚀 Deployed to production by https://github.com/lakchote in version: 9.1.66-5 🚀
|










Explanation of Change
Fixed Issues
$ #62323
PROPOSAL: #62323 (comment)
Tests
Same as QA Steps
Offline tests
Same as QA Steps
QA Steps
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))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
MacOS: Desktop