Create messages in single transaction thread / Fix resolving whispers#68415
Create messages in single transaction thread / Fix resolving whispers#68415
Conversation
| * @param reportID - The report ID where the comment should be added and the report action should be updated | ||
| * @param reportActionID - The specific report action ID to update | ||
| * @param selectedCategory - The category selected by the user | ||
| */ | ||
| function resolveConciergeCategoryOptions(reportID: string | undefined, actionReportID: string | undefined, reportActionID: string | undefined, selectedCategory: string) { | ||
| if (!reportID || !actionReportID || !reportActionID) { | ||
| function resolveConciergeCategoryOptions(reportID: string | undefined, reportActionID: string | undefined, selectedCategory: string) { | ||
| if (!reportID || !reportActionID) { |
There was a problem hiding this comment.
|
Added @jayeshmangwani as C+ reviewer since he has some context and reviewed the PR #67707 that was later reverted |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid.movAndroid: mWeb Chromemweb-chrome.moviOS: HybridAppios-1.movios-2.moviOS: mWeb Safarimweb-safari.movMacOS: Chrome / Safariweb-1.movweb-2.movMacOS: Desktopdesktop.mov |
|
@jayeshmangwani I didn't mention it, but you need to disable the beta BETAS.NO_OPTIMISTIC_TRANSACTION_THREADS to test this properly |
|
Pulled |
Okay, Testing this by manually disabling this beta |
jayeshmangwani
left a comment
There was a problem hiding this comment.
Changes work well, PR LGTM 🚀
|
🎯 @jayeshmangwani, thanks for reviewing and testing this PR! 🎉 An E/App issue has been created to issue payment here: #68544. |
|
✋ 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/blimpich in version: 9.1.95-0 🚀
|
|
Hello @aldo-expensify @jayeshmangwani, QA team is experiencing an issue where category suggestions are not displayed when following the steps below using the Expensifail account:
Screen.Recording.2025-08-15.at.12.21.18.movIs this the expected behavior? Please note that the issue is not reproducible when using a Gmail account Screen.Recording.2025-08-15.at.12.18.16.mov |
|
@mitarachim hmm it is not expected, but it sounds unrelated. I'll give it a try. |
|
🚀 Deployed to production by https://github.com/AndrewGable in version: 9.1.95-1 🚀
|
Explanation of Change
Re-implementing #67707 while making sure I don't break actionable whispers #68239
The previous PR broke whispers because the code resolving whispers was using the
reportIDwhich pointed to the expense report when the whisper was in an expense report with a single transaction, even if the whisper reportAction itself was really linked to the transaction thread.This solves the issue by first trying to use
originalReportID.originalReportIDholds the correctreportIDcorresponding to theactionbeing handled byPureReportActionItem.Fixed Issues
$ https://github.com/Expensify/Expensify/issues/526433
PROPOSAL:
Tests
You need to disable the beta
BETAS.NO_OPTIMISTIC_TRANSACTION_THREADSto test this properlyMessages are stored in the correct report:
Set category to "Travel"Mention whisper:
Suggested category actionable whisper:
dsgdgrewerfwefaegand don't select any categoryset the category to "Other"Report mention whisper:
#1232534543634522 hello#asdasda214234 asdsasfOffline 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))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