[HOLD for payment 2024-07-24] Thread - LHN displays "removed 1 user" instead of "left the chat" after leaving thread #44610

izarutskaya · 2024-06-28T12:15:55Z

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.0-3.1
Reproducible in staging?: Y
Reproducible in production?: Y
Logs: https://stackoverflow.com/c/expensify/questions/4856
Issue reported by: Applause-Internal team

Action Performed:

Go to staging.new.expensify.com/
Go to any chat.
Send a message.
Right click on the message > Reply in thread.
Send a reply in thread.
Click 3-dot menu > Leave.
Go to the thread again.

Expected Result:

Thread in LHN will display "left the chat".

Actual Result:

Thread in LHN displays "removed 1 user" after leaving thread.

Workaround:

Unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

Screenshots/Videos

Bug6527092_1719558928457.20240628_151205.mp4

View all open jobs on GitHub

Issue Owner

Current Issue Owner: @eh2077

melvin-bot · 2024-06-28T12:16:06Z

Triggered auto assignment to @sonialiap (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

izarutskaya · 2024-06-28T12:16:19Z

We think this issue might be related to the #vip-vsb

bernhardoj · 2024-06-29T12:17:19Z

Proposal

Please re-state the problem that we are trying to solve in this issue.

The LHN shows removed 1 user instead of left the chat when leaving a thread.

What is the root cause of that problem?

Leaving a thread will add a LEAVEROOM action from the BE response.

And that's considered as a room change log action.

App/src/libs/SidebarUtils.ts

Lines 351 to 375 in 4622e19

    
                   } else if (ReportActionsUtils.isRoomChangeLogAction(lastAction)) { 
        
                       const lastActionOriginalMessage = lastAction?.actionName ? ReportActionsUtils.getOriginalMessage(lastAction) : null; 
        
                       const targetAccountIDs = lastActionOriginalMessage?.targetAccountIDs ?? []; 
        
                       const targetAccountIDsLength = targetAccountIDs.length !== 0 ? targetAccountIDs.length : report.lastMessageHtml?.match(/<mention-user[^>]*><\/mention-user>/g)?.length ?? 0; 
        
                       const verb = 
        
                           lastActionName === CONST.REPORT.ACTIONS.TYPE.ROOM_CHANGE_LOG.INVITE_TO_ROOM || lastActionName === CONST.REPORT.ACTIONS.TYPE.POLICY_CHANGE_LOG.INVITE_TO_ROOM 
        
                               ? Localize.translate(preferredLocale, 'workspace.invite.invited') 
        
                               : Localize.translate(preferredLocale, 'workspace.invite.removed'); 
        
                       const users = Localize.translate(preferredLocale, targetAccountIDsLength > 1 ? 'workspace.invite.users' : 'workspace.invite.user'); 
        
                       result.alternateText = `${lastActorDisplayName} ${verb} ${targetAccountIDsLength} ${users}`.trim(); 
        
                       const roomName = lastActionOriginalMessage?.roomName ?? ''; 
        
                       if (roomName) { 
        
                           const preposition = 
        
                               lastAction.actionName === CONST.REPORT.ACTIONS.TYPE.ROOM_CHANGE_LOG.INVITE_TO_ROOM || lastAction.actionName === CONST.REPORT.ACTIONS.TYPE.POLICY_CHANGE_LOG.INVITE_TO_ROOM 
        
                                   ? ` ${Localize.translate(preferredLocale, 'workspace.invite.to')}` 
        
                                   : ` ${Localize.translate(preferredLocale, 'workspace.invite.from')}`; 
        
                           result.alternateText += `${preposition} ${roomName}`; 
        
                       } 
        
                       if (lastActionName === CONST.REPORT.ACTIONS.TYPE.ROOM_CHANGE_LOG.UPDATE_ROOM_DESCRIPTION) { 
        
                           result.alternateText = `${lastActorDisplayName} ${Localize.translate(preferredLocale, 'roomChangeLog.updateRoomDescription')} ${ 
        
                               lastActionOriginalMessage?.description 
        
                           }`.trim(); 
        
                       } 
        
                   } else if (lastAction?.actionName === CONST.REPORT.ACTIONS.TYPE.POLICY_CHANGE_LOG.LEAVE_POLICY) {

Because the action is not an invited action, the removed message is being used.

What changes do you think we should make in order to solve the problem?

We can make sure the room change log condition is only true if it's not a thread.
ReportActionsUtils.isRoomChangeLogAction(lastAction) && !result.isThread.

This way, the last message of the thread report will be used, that is, left the chat.

What alternative solutions did you explore? (Optional)

We can improve the room change log message by adding the case for the leave room that returns left the chat (and it's translation). Currently, it only handles invite, remove, and update description.

dominictb · 2024-07-01T14:00:41Z

Proposal

Please re-state the problem that we are trying to solve in this issue.

Thread in LHN displays "removed 1 user" after leaving thread.

What is the root cause of that problem?

In the past, we moved to this case if the report action is invite or removed action. See the diff here for more details.

Now, we use isRoomChangeLogAction function that checks whether the action is a room change log action or not and it's not correct with the previous check. And then when we leave the room, the last action is LEAVEROOM action, and removed 1 user appears.

What changes do you think we should make in order to solve the problem?

We should create a new function like isInviteOrRemovedAction that will return true if the report action is one of four action types that we used in the past here.

function isInviteOrRemovedAction(reportAction: OnyxInputOrEntry<ReportAction>): reportAction is ReportAction<ValueOf<typeof CONST.REPORT.ACTIONS.TYPE.POLICY_CHANGE_LOG | typeof CONST.REPORT.ACTIONS.TYPE.ROOM_CHANGE_LOG>> {
    return isActionOfType(reportAction, CONST.REPORT.ACTIONS.TYPE.ROOM_CHANGE_LOG.INVITE_TO_ROOM, CONST.REPORT.ACTIONS.TYPE.ROOM_CHANGE_LOG.REMOVE_FROM_ROOM, CONST.REPORT.ACTIONS.TYPE.POLICY_CHANGE_LOG.INVITE_TO_ROOM, CONST.REPORT.ACTIONS.TYPE.POLICY_CHANGE_LOG.REMOVE_FROM_ROOM);
}

And then use this function here to update the condition correctly

What alternative solutions did you explore? (Optional)

melvin-bot · 2024-07-01T18:46:23Z

@sonialiap Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

melvin-bot · 2024-07-02T13:18:43Z

Unable to auto-create job on Upwork. The BZ team member should create it manually for this issue.

melvin-bot · 2024-07-02T13:18:48Z

Triggered auto assignment to Contributor-plus team member for initial proposal review - @eh2077 (External)

melvin-bot · 2024-07-03T14:55:44Z

Triggered auto assignment to @arosiclair, see https://stackoverflow.com/c/expensify/questions/7972 for more details.

dominictb · 2024-07-03T15:58:50Z

@eh2077 Please see the diff, in the past we check if the action is invited or removed action for room or policy, we will move to this case and display in LHN as invited x users or removed x users.

But isRoomChangeLogAction function check if the action is one of CONST.REPORT.ACTIONS.TYPE.ROOM_CHANGE_LOG that is not correct with the check we did in the past, this doesn't contain the case for CONST.REPORT.ACTIONS.TYPE.POLICY_CHANGE_LOG.INVITE_TO_ROOM and CONST.REPORT.ACTIONS.TYPE.POLICY_CHANGE_LOG.REMOVE_FROM_ROOM and has the incorrect case for LEAVEROOM

For the UPDATEROOMDESCRIPTION case that is just added here , we can move this as a separate case to make the code more cleaner.

App/src/libs/SidebarUtils.ts

Line 374 in 55546bb

    
           if (lastActionName === CONST.REPORT.ACTIONS.TYPE.ROOM_CHANGE_LOG.UPDATE_ROOM_DESCRIPTION) {

eh2077 · 2024-07-03T16:24:47Z

@dominictb Thanks for pointing out that! Yes, I agreed with you - we should fix the issue by checking

[
    CONST.REPORT.ACTIONS.TYPE.ROOM_CHANGE_LOG.INVITE_TO_ROOM,
    CONST.REPORT.ACTIONS.TYPE.ROOM_CHANGE_LOG.REMOVE_FROM_ROOM,
    CONST.REPORT.ACTIONS.TYPE.POLICY_CHANGE_LOG.INVITE_TO_ROOM, 
    CONST.REPORT.ACTIONS.TYPE.POLICY_CHANGE_LOG.REMOVE_FROM_ROOM
]

in method ReportActionsUtils.isRoomChangeLogAction just like we did before the refactoring.

That's said, I think we should go with @dominictb 's proposal.

@arosiclair All yours.

🎀👀🎀 C+ reviewed

Sorry for the back and forth!

melvin-bot · 2024-07-03T16:24:56Z

Current assignee @arosiclair is eligible for the choreEngineerContributorManagement assigner, not assigning anyone new.

arosiclair · 2024-07-04T13:54:58Z

And then use this function here to update the condition correctly

@dominictb how should we update this exactly? I don't see where LEAVEROOM would be handled if we only check for isInviteOrRemovedAction.

dominictb · 2024-07-04T16:21:33Z

@arosiclair In the past we also didn't have the special case for LEAVEROOM and we use the lastMessageText as default for this case.

arosiclair · 2024-07-04T20:09:12Z

Gotcha sounds like the default text will work. Please move forward with the PR

melvin-bot · 2024-07-08T18:28:23Z

@arosiclair, @sonialiap, @eh2077, @dominictb Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

dominictb · 2024-07-09T08:30:55Z

PR will be ready by today

melvin-bot · 2024-07-17T00:07:19Z

Reviewing label has been removed, please complete the "BugZero Checklist".

melvin-bot · 2024-07-17T00:07:24Z

The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.7-8 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

fix: check for leaveroom last action type #44945

If no regressions arise, payment will be issued on 2024-07-24. 🎊

For reference, here are some details about the assignees on this issue:

@eh2077 requires payment (Needs manual offer from BZ)
@dominictb requires payment (Needs manual offer from BZ)

melvin-bot · 2024-07-17T00:07:25Z

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

[@eh2077] The PR that introduced the bug has been identified. Link to the PR:
[@eh2077] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
[@eh2077] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
[@eh2077] Determine if we should create a regression test for this bug.
[@eh2077] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
[@sonialiap] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

sonialiap · 2024-07-24T10:16:11Z

Payment summary:

@eh2077 $250 - offer sent in upwork - please complete the checklist - paid ✔️
@dominictb $250 - offer sent in upwork - paid ✔️

https://www.upwork.com/ab/applicants/1816054414293401460/job-details

melvin-bot · 2024-07-24T18:05:01Z

Payment Summary

Upwork Job

ROLE: @eh2077 paid $(AMOUNT) via Upwork (LINK)
ROLE: @dominictb paid $(AMOUNT) via Upwork (LINK)

BugZero Checklist (@sonialiap)

I have verified the correct assignees and roles are listed above and updated the neccesary manual offers
I have verified that there are no duplicate or incorrect contracts on Upwork for this job (https://www.upwork.com/ab/applicants//hired)
I have paid out the Upwork contracts or cancelled the ones that are incorrect
I have verified the payment summary above is correct

melvin-bot · 2024-07-29T18:12:58Z

@arosiclair, @sonialiap, @eh2077, @dominictb Huh... This is 4 days overdue. Who can take care of this?

eh2077 · 2024-07-30T01:37:51Z

Checklist

[@eh2077] The PR that introduced the bug has been identified. Link to the PR: feature: Stop using reportAction.originalMessage or reportAction.messge #40168
[@eh2077] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment: https://github.com/Expensify/App/pull/40168/files#r1696157600
[@eh2077] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion: N/A
[@eh2077] Determine if we should create a regression test for this bug. NO

izarutskaya added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Jun 28, 2024

melvin-bot bot assigned sonialiap Jun 28, 2024

melvin-bot bot added the Overdue label Jul 1, 2024

sonialiap added the External Added to denote the issue can be worked on by a contributor label Jul 2, 2024

melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Jul 2, 2024

melvin-bot bot assigned eh2077 Jul 2, 2024

melvin-bot bot removed the Overdue label Jul 2, 2024

sonialiap added this to [#whatsnext] #vip-vsb Jul 2, 2024

This comment was marked as outdated.

Sign in to view

melvin-bot bot assigned arosiclair Jul 3, 2024

arosiclair assigned dominictb Jul 4, 2024

melvin-bot bot removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Jul 4, 2024

dominictb mentioned this issue Jul 8, 2024

fix: check for leaveroom last action type #44945

Merged

50 tasks

melvin-bot bot added the Overdue label Jul 8, 2024

melvin-bot bot added Reviewing Has a PR in review Weekly KSv2 and removed Overdue Daily KSv2 labels Jul 9, 2024

melvin-bot bot added Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Weekly KSv2 labels Jul 17, 2024

melvin-bot bot changed the title ~~Thread - LHN displays "removed 1 user" instead of "left the chat" after leaving thread~~ [HOLD for payment 2024-07-24] Thread - LHN displays "removed 1 user" instead of "left the chat" after leaving thread Jul 17, 2024

melvin-bot bot removed the Reviewing Has a PR in review label Jul 17, 2024

melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels Jul 23, 2024

melvin-bot bot added the Overdue label Jul 26, 2024

eh2077 mentioned this issue Jul 30, 2024

feature: Stop using reportAction.originalMessage or reportAction.messge #40168

Merged

50 tasks

melvin-bot bot removed the Overdue label Jul 30, 2024

sonialiap closed this as completed Jul 30, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[HOLD for payment 2024-07-24] Thread - LHN displays "removed 1 user" instead of "left the chat" after leaving thread #44610

[HOLD for payment 2024-07-24] Thread - LHN displays "removed 1 user" instead of "left the chat" after leaving thread #44610

izarutskaya commented Jun 28, 2024 •

edited by sonialiap

Loading

melvin-bot bot commented Jun 28, 2024

izarutskaya commented Jun 28, 2024

bernhardoj commented Jun 29, 2024

dominictb commented Jul 1, 2024

melvin-bot bot commented Jul 1, 2024

melvin-bot bot commented Jul 2, 2024

melvin-bot bot commented Jul 2, 2024

This comment was marked as outdated.

melvin-bot bot commented Jul 3, 2024

dominictb commented Jul 3, 2024 •

edited

Loading

eh2077 commented Jul 3, 2024

melvin-bot bot commented Jul 3, 2024

arosiclair commented Jul 4, 2024

dominictb commented Jul 4, 2024

arosiclair commented Jul 4, 2024

melvin-bot bot commented Jul 8, 2024

dominictb commented Jul 9, 2024

melvin-bot bot commented Jul 17, 2024

melvin-bot bot commented Jul 17, 2024

melvin-bot bot commented Jul 17, 2024

sonialiap commented Jul 24, 2024 •

edited

Loading

melvin-bot bot commented Jul 24, 2024

melvin-bot bot commented Jul 29, 2024

eh2077 commented Jul 30, 2024

[HOLD for payment 2024-07-24] Thread - LHN displays "removed 1 user" instead of "left the chat" after leaving thread #44610

[HOLD for payment 2024-07-24] Thread - LHN displays "removed 1 user" instead of "left the chat" after leaving thread #44610

Comments

izarutskaya commented Jun 28, 2024 • edited by sonialiap Loading

Action Performed:

Expected Result:

Actual Result:

Workaround:

Platforms:

Screenshots/Videos

melvin-bot bot commented Jun 28, 2024

izarutskaya commented Jun 28, 2024

bernhardoj commented Jun 29, 2024

Proposal

Please re-state the problem that we are trying to solve in this issue.

What is the root cause of that problem?

What changes do you think we should make in order to solve the problem?

What alternative solutions did you explore? (Optional)

dominictb commented Jul 1, 2024

Proposal

Please re-state the problem that we are trying to solve in this issue.

What is the root cause of that problem?

What changes do you think we should make in order to solve the problem?

What alternative solutions did you explore? (Optional)

melvin-bot bot commented Jul 1, 2024

melvin-bot bot commented Jul 2, 2024

melvin-bot bot commented Jul 2, 2024

This comment was marked as outdated.

melvin-bot bot commented Jul 3, 2024

dominictb commented Jul 3, 2024 • edited Loading

eh2077 commented Jul 3, 2024

melvin-bot bot commented Jul 3, 2024

arosiclair commented Jul 4, 2024

dominictb commented Jul 4, 2024

arosiclair commented Jul 4, 2024

melvin-bot bot commented Jul 8, 2024

dominictb commented Jul 9, 2024

melvin-bot bot commented Jul 17, 2024

melvin-bot bot commented Jul 17, 2024

melvin-bot bot commented Jul 17, 2024

sonialiap commented Jul 24, 2024 • edited Loading

melvin-bot bot commented Jul 24, 2024

Payment Summary

BugZero Checklist (@sonialiap)

melvin-bot bot commented Jul 29, 2024

eh2077 commented Jul 30, 2024

Checklist

izarutskaya commented Jun 28, 2024 •

edited by sonialiap

Loading

dominictb commented Jul 3, 2024 •

edited

Loading

sonialiap commented Jul 24, 2024 •

edited

Loading