Invisible report is created after submit scan expense from self DM to workspace chat#72179
Conversation
…-after-submit-scan-expense
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppScreen.Recording.2025-10-09.at.18.39.57.movAndroid: mWeb ChromeScreen.Recording.2025-10-09.at.18.25.11.moviOS: HybridAppScreen.Recording.2025-10-09.at.18.30.14.moviOS: mWeb SafariScreen.Recording.2025-10-09.at.18.41.45.movMacOS: Chrome / SafariScreen.Recording.2025-10-09.at.18.22.18.movMacOS: DesktopScreen.Recording.2025-10-09.at.18.15.59.mov |
| * - we have one, but we can't add more transactions to it due to: report is approved or settled | ||
| */ | ||
| function shouldCreateNewMoneyRequestReport(existingIOUReport: OnyxInputOrEntry<Report> | undefined, chatReport: OnyxInputOrEntry<Report>, isScanRequest: boolean): boolean { | ||
| function shouldCreateNewMoneyRequestReport( |
There was a problem hiding this comment.
Could you add a bullet to the comment above explaining why we don't create a new money request report for submit actions specifically?
There was a problem hiding this comment.
@chuckdries I think it would be best to check with the internal team about the logic for when a new money request report should be created. Once we have clarity, we can document all the relevant cases directly in a comment on the shouldCreateNewMoneyRequestReport function—rather than only explaining why we don’t create a new report for submit actions specifically.
There was a problem hiding this comment.
I'm a little confused at what you mean. There already is a comment on shouldCreateNewMoneyRequestReport that explains when a new money request report should be created. I'm just saying it'd be helpful to add to it a note about why we're adding an exception for submit actions.
There was a problem hiding this comment.
@chuckdries I updated the comment to more clearly. Please help to check it.
There was a problem hiding this comment.
Hi @thelullabyy, sorry to be so pedantic about this - I should've been more clear from the get-go. Can you please put in the comment why we're excluding submit actions specifically. Just briefly describe what the bug that we're solving is.
There was a problem hiding this comment.
@chuckdries In the comment I added to the function:
- It is a SmartScan request, the user is in the ASAP Submit beta, and the action is not "submit"
I was simply highlighting that we create a new report only if the action is not "submit". However, as you pointed out, I wasn’t able to provide a clear explanation for why we don’t create a report when the action is "submit".
The reason I included the action !== CONST.IOU.ACTION.SUBMIT condition was purely to align with the current backend behavior.
If we need a definitive explanation for why a report isn't created in the "submit" case, we should probably reach out to the backend or internal team for clarification.
There was a problem hiding this comment.
Similar to the previous comment we added here:
Line 10140 in 35e6f75
—it simply notes that when the it is waiting on the payee to add a bank account, we create a new report. However, it doesn't explain why that behavior is necessary in this case.
|
Quick question: does this break the submitting an expense from the self DM to the workspace chat when there is no report yet in the workspace chat? |
If there is no report, we still return true because the Line 10149 in 35e6f75 |
chuckdries
left a comment
There was a problem hiding this comment.
Tested it, works like a charm. Thanks for putting up with my comments!
|
✋ 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/chuckdries in version: 9.2.30-0 🚀
|
|
🚀 Deployed to production by https://github.com/mountiny in version: 9.2.30-6 🚀
|
Explanation of Change
Fixed Issues
$#70744
PROPOSAL:#70744 (comment)
Tests
Precondition:
Offline tests
QA Steps
Precondition:
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
ndroid.mov
Android: mWeb Chrome
android_ch.mov
iOS: Native
ios.mov
iOS: mWeb Safari
ios_safari.mov
MacOS: Chrome / Safari
chorme.mov
MacOS: Desktop
desktop.mov