Stop creating the transaction thread reports optimistically for RequestMoney - #63931
Conversation
…ating-optimistic-transaction-4 # Conflicts: # src/pages/home/ReportScreen.tsx # src/pages/home/report/ContextMenu/BaseReportActionContextMenu.tsx
…ating-optimistic-transaction-4 # Conflicts: # src/components/MoneyRequestReportView/MoneyRequestReportTransactionList.tsx # src/components/SelectionList/Search/ReportListItem.tsx
…istic-transaction-4
…ows if threads are not created optimistically
…tion arrows if threads are not created optimistically" This reverts commit d7e218e.
…istic-transaction-4 # Conflicts: # src/components/MoneyRequestReportView/MoneyRequestReportTransactionList.tsx # src/components/SelectionList/Search/ReportListItem.tsx # src/libs/ReportUtils.ts # src/libs/actions/IOU.ts
…ows after merging main
…istic-transaction-4 # Conflicts: # src/components/MoneyReportHeader.tsx # src/components/Search/index.tsx # src/components/SelectionList/Search/TransactionGroupListItem.tsx # src/libs/ReportUtils.ts # src/pages/Search/SearchMoneyRequestReportPage.tsx
…istic-transaction-4 # Conflicts: # src/pages/Search/SearchMoneyRequestReportPage.tsx
|
Previously reported regressions: #63490 Fixed in 38f53b8 I've retested the PR and added the recordings. I'll be OOO next week (June 30 - July 4) 🌴 |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
| params.createdReportActionIDForThread = optimisticCreatedAction.reportActionID; | ||
| } | ||
|
|
||
| API.write('HoldRequest', params, {optimisticData, successData, failureData}); |
There was a problem hiding this comment.
@VickyStash From the diff, I can see you are not the original one to blame for this, but since you are the last one that touched this, can you please make a quick PR to ensure this is referencing the constant HOLD_MONEY_REQUEST?
There was a problem hiding this comment.
Looks like the same thing happened to 'UnHoldRequest' as well
There was a problem hiding this comment.
Oh sorry for this, should've caught it.
|
🚀 Deployed to staging by https://github.com/iwiznia in version: 9.1.88-0 🚀
|
|
Hey all, an App regression occurred in the current release where LHN reports are named I'm not certain but it seems related to the optimistic report thread changes that are linked to this issue. Please could you take a look, thanks. |
|
@hungvu193 @iwiznia FYI I'm looking through all of the reported deploy blockers to define which of them can be caused by this PR. |
|
@VickyStash Thank you. Do you have solution for it? Sorry, my dev env's broken so I couldn't help much |
|
@VickyStash another one caused by this PR: #67588 |
|
Damn, this seems to never end. Should we revert this or are you fixing the issues with a new PR to CP? |
I'll try to prepare a PR, let's see how it goes. |
|
🚀 Deployed to production by https://github.com/grgia in version: 9.1.88-3 🚀
|
|
🚀 Deployed to staging by https://github.com/iwiznia in version: 9.1.89-1 🚀
|
|
🚀 Deployed to production by https://github.com/jasperhuangg in version: 9.1.89-21 🚀
|
Explanation of Change
Re-apply changes of reverted PR: #62080
Regressions reported previous time:
#63490
#63461
#63470
#63511
#63509
#63493
Fixed Issues
$ #58828
PROPOSAL: N/A
Tests
Basic testing:
2.1 In the chat tap FAB -> Create expense. Go through the steps and create a manual expense. It will be the first expense in this 1-1 chat.
2.2 Check that the RequestMoney API call didn't create a transaction thread report.
2.3 Tap on the expense -> the transaction thread report should be created and opened.
Complex testing:
Test different flows related to expenses and transaction threads overview, including:
Regressions:
Please, make sure regressions are fixed.
1. When expense has Concierge message, transaction thread with RBR appears in LHN after app reset
See steps in #63490
2. Reports - Individual expense row opens expense report instead of transaction thread
See steps in #63461
3. Reports - Expense report loads infinitely after reset app
See steps in #63470
4. After moving expense to self DM, expense thread loads infinitely with empty header when offline
See steps in #63511
5. Duplicate expenses - Expense appears twice after keeping one of the duplicates
See steps in #63509
6. Expense - Combined report is not opened when the expense report is with Manager McTest
See steps in #63493
Offline tests
Same, as in the Tests section
QA Steps
// TODO: These must be filled out, or the issue title must include "[No QA]."
Same, as in the Tests section
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.mp4
Android: mWeb Chrome
android_web.mp4
iOS: Native
ios.mp4
iOS: mWeb Safari
ios_web.mp4
MacOS: Chrome / Safari
basic_testing.mp4
1-regression.mp4
2-regression.mp4
3-regression.mp4
4-regression.mp4
5-regression.mp4
6-regression.mp4
MacOS: Desktop
desktop.mp4