-
Notifications
You must be signed in to change notification settings - Fork 3.2k
[$250] LHN - Deleted in 1:1 chat expense remains in LHN with the receive's name changed to the email #57694
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 @lschurr ( |
Job added to Upwork: https://www.upwork.com/jobs/~021897023196486831591 |
Triggered auto assignment to Contributor-plus team member for initial proposal review - @brunovjk ( |
This comment has been minimized.
This comment has been minimized.
I can reproduce, waiting for proposals. Screen.Recording.2025-03-06.at.16.14.24.mov |
ProposalPlease re-state the problem that we are trying to solve in this issue.Deleted in 1:1 chat expense remains in LHN with the receive's name changed to the email What is the root cause of that problem?When the user B deletes a message, for user A the reportAction is deleted but the lastMessageText and lastVisibleActionCreated of the report is not updated. Due to this, when the expense is deleted and the updated last visible action is a ADD_COMMENT, the LHN shows lastMessageText and the report is shown as unread which is incorrect as it shows stale data What changes do you think we should make in order to solve the problem?For LHN text:The LHN text based on the last report is handled by getLastMessageTextForReport which will fallback to report.lastMessageText for report actions like ADD_COMMENT as they don't have any special handling When we already do this for closed report action so we just need to add a or condition for our case. App/src/libs/OptionsListUtils.ts Lines 732 to 734 in f49f6c4
so the updated code will be if (reportID && !isArchivedReport(reportNameValuePairs) && (isDeletedAction(lastOriginalReportAction) || report.lastActionType === CONST.REPORT.ACTIONS.TYPE.CLOSED)) {
return lastMessageTextFromReport || (getReportLastMessage(reportID).lastMessageText ?? '');
} For read/unread highlightThe unread highlight is handled by isUnread() which gets lastVisibleActionCreated from getReportLastVisibleActionCreated. We can update getReportLastVisibleActionCreated to use report actions whenever possible function getReportLastVisibleActionCreated(report: OnyxEntry<Report>, oneTransactionThreadReport: OnyxEntry<Report>) {
const lastVisibleReportActionCreated = getLastVisibleActionReportActionsUtils(report?.reportID)?.created ?? report?.lastVisibleActionCreated ?? ''
const lastVisibleActionCreated =
(oneTransactionThreadReport?.lastVisibleActionCreated ?? '') > lastVisibleReportActionCreated
? oneTransactionThreadReport?.lastVisibleActionCreated
: lastVisibleReportActionCreated;
return lastVisibleActionCreated;
} What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?Test getReportLastVisibleActionCreated and getLastMessageTextForReport return correct values when there is a deleted report action What alternative solutions did you explore? (Optional)Reminder: Please use plain English, be brief and avoid jargon. Feel free to use images, charts or pseudo-code if necessary. Do not post large multi-line diffs or write walls of text. Do not create PRs unless you have been hired for this job. |
@jaydamani's proposal makes sense to me, I tested it and it works well. 🎀👀🎀 C+ reviewed |
Triggered auto assignment to @thienlnam, see https://stackoverflow.com/c/expensify/questions/7972 for more details. |
📣 @brunovjk 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app! |
📣 @jaydamani 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app! Offer link |
Working on this, PR should be up soon |
created draft PR |
Not overdue, Contributor is working on PR. |
Update: We are still working on the PR. |
@brunovjk Whoops! This issue is 2 days overdue. Let's get this updated quick! |
PR is in progress, waiting on some back end changes |
Not overdue 🚀 |
Update: We are still working on the PR |
This issue has not been updated in over 15 days. @thienlnam, @lschurr, @jaydamani, @brunovjk eroding to Monthly issue. P.S. Is everyone reading this sure this is really a near-term priority? Be brave: if you disagree, go ahead and close it out. If someone disagrees, they'll reopen it, and if they don't: one less thing to do! |
Update: Same as above |
1 similar comment
Update: Same as above |
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: 9.1.8-0
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught on HybridApp, is this reproducible on New Expensify Standalone?: N/A
If this was caught during regression testing, add the test name, ID and link from TestRail: N/A
Issue reported by: Applause - Internal Team
Action Performed:
User A logged in on the main testing device, User B - on secondary device
Expected Result:
Deleted in 1:1 chat expense should be removed from LHN, the last remaining message in the chat should be shown
Actual Result:
Deleted in 1:1 chat expense remains in LHN with the receiver's name changed to the email
Workaround:
Unknown
Platforms:
Which of our officially supported platforms is this issue occurring on?
Screenshots/Videos
Add any screenshot/video evidence
Bug6760189_1741028585300.Deleted_expense_LHN.mp4
View all open jobs on GitHub
Upwork Automation - Do Not Edit
Issue Owner
Current Issue Owner: @brunovjkThe text was updated successfully, but these errors were encountered: