-
Notifications
You must be signed in to change notification settings - Fork 3.2k
AU - Chat - Deleted message with a thread disappears from chat #62191
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
Comments
Triggered auto assignment to @youssef-lr ( |
Triggered auto assignment to @RachCHopkins ( |
💬 A slack conversation has been started in #expensify-open-source |
👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open `StagingDeployCash` deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:
|
Offending PR: #62002 and #62151 This is quite a weird one. I think the diff shown in #62002 is not correct and eventually cause the revert PR not to restore to the correct state on My guess is because of these force pushes so Github displays the wrong diff. So in short, the correct revert should be: App/src/pages/home/ReportScreen.tsx Lines 269 to 276 in 41108ae
const {reportActions, linkedAction, sortedAllReportActions, hasNewerActions, hasOlderActions} = usePaginatedReportActions(reportID, reportActionIDFromRoute); You can verify that by tracing back cc @JakubKorytko @mountiny @aldo-expensify because you worked on the revert. |
My guess is that the problem with the revert was probably not caused by the force push, but rather by the fact that two PRs were merged very close together: #61777 and #62002. They both changed the same line just below |
I think I'm missing something, I see that the lines in the current staging and App/src/pages/home/ReportScreen.tsx Lines 269 to 276 in dc23284
are the same the PR #61777 changed to: ![]() Can you help me see what is the line that is wrong after the revert? 🙇 |
Actually, I think my approach to filtering these actions was incorrect. Feel free to remove this line if it fixes the issue and I think there's another file with the same logic for filtering actions - so that should also be considered. However, please don't revert the entire #61777 because more changes were made than just this one. I will reconsider this on Monday and prepare a follow-up for these two PRs (#61777 & reverted #62002) because removing this line will most likely reopen two linked issues in the PR. The important thing is deciding which issue is more important to fix, because either way, I will start working on a follow-up on Monday to correct these two bad boys. |
If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!
Version Number: v9.1.46-0
Reproducible in staging?: Yes
Reproducible in production?: No
If this was caught during regression testing, add the test name, ID and link from TestRail: https://expensify.testrail.io/index.php?/tests/view/6122620
Email or phone of affected tester (no customers): [email protected]
Issue reported by: Applause Internal Team
Device used: Windows 10, iPhone 13/iOS 18.4.1
App Component: Chat Report View
Action Performed:
Expected Result:
The parent message is now displayed as [Deleted message]
Actual Result:
The message disappears from the chat
Workaround:
Unknown
Platforms:
Screenshots/Videos
1.mp4
View all open jobs on GitHub
The text was updated successfully, but these errors were encountered: