Thread - LHN displays "removed 1 user" instead of "left the chat" after leaving thread

[izarutskaya]

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com 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:

1. Go to staging.new.expensify.com/
2. Go to any chat.
3. Send a message.
4. Right click on the message > Reply in thread.
5. Send a reply in thread.
6. Click 3-dot menu > Leave.
7. 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?

• [x] Android: Native
• [x] Android: mWeb Chrome
• [x] iOS: Native
• [x] iOS: mWeb Safari
• [x] MacOS: Chrome / Safari
• [x] MacOS: Desktop

Screenshots/Videos

https://github.com/Expensify/App/assets/115492554/ea7de16f-68de-4e17-bf82-4b9951179d90

View all open jobs on GitHub

Issue Owner

Current Issue Owner: @eh2077

[melvin-bot[bot]]

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]

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

[bernhardoj]

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. https://github.com/Expensify/App/blob/4622e19c1da5137bc7b227f5b65bf3d83f96a909/src/libs/SidebarUtils.ts#L351-L375

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]

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[bot]]

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

[melvin-bot[bot]]

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

[melvin-bot[bot]]

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

[eh2077]

@dominictb I think you overlooked method ReportActionsUtils.isRoomChangeLogAction https://github.com/Expensify/App/blob/cd72f1b13d4a5d32227ab03ebd7df07a594e482d/src/libs/ReportActionsUtils.ts#L266-L268 because it does exactly same as you suggested.

---

~@bernhardoj 's proposal looks good to me. We should exclude thread action when preparing LHN message for room log action.~

As pointed out by @dominictb, we should be able fix the issue if we check

[
    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

~🎀👀🎀 C+ reviewed~

[melvin-bot[bot]]

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

[dominictb]

@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.

https://github.com/Expensify/App/blob/55546bb6066fcc14a9005d512554e45c1d159166/src/libs/SidebarUtils.ts#L374

[eh2077]

@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[bot]]

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

[arosiclair]

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]

@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]

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

[melvin-bot[bot]]

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

[dominictb]

PR will be ready by today