Threads - There is no Leave thread option on group Chat threads

[izarutskaya]

trafficstars

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: v1.4.60-1 Reproducible in staging?: Y Reproducible in production?: Y Email or phone of affected tester (no customers): [email protected] Logs: https://stackoverflow.com/c/expensify/questions/4856 Issue reported by: Applause-Internal team

Action Performed:

1. Go to any Group chat/ Create a group chat
2. Send a message
3. Go to threads on the sent message
4. Click on the three dots on the header, Notice that there is no Leave option.
5. Send a message on the Thread.
6. Again Click on the three dots on the header, Notice that there is no Leave option.

Expected Result:

There should be a leave thread option after creating a thread.

Actual Result:

There is no Leave thread option.

Workaround:

Unknown

Platforms:

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

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

Screenshots/Videos

https://github.com/Expensify/App/assets/115492554/4728b4f0-0d75-4d6f-962e-2e88d327d434

View all open jobs on GitHub

[melvin-bot[bot]]

Triggered auto assignment to @trjExpensify (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

[izarutskaya]

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

[Nodebrute]

Proposal

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

No thread leave option in group chat threads.

What is the root cause of that problem?

It's because here !isGroupChat will be false, and that's why we don't see the 'Leave Thread' option. https://github.com/Expensify/App/blob/32b6c38cb7a08573c1f30b291595d92c8b127dca/src/pages/home/HeaderView.tsx#L147

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

We can handle this by adding a condition where when isGroupChat and is isChatThread both are true then we can show the Leave option.

something like this

const canJoinOrLeave = !isSelfDM && (!isGroupChat || isGroupChat && isChatThread) && (isChatThread || isUserCreatedPolicyRoom || canLeaveRoom);

What alternative solutions did you explore? (Optional)

When creating threads from a group chat, we should not set chatType as group. This will ensure that the threads are not considered as group chats.

[allgandalf]

Proposal

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

not able to leave/join group threads

What is the root cause of that problem?

Currently we have disabled the option for leaving/joining threads for group chats: https://github.com/Expensify/App/blob/32b6c38cb7a08573c1f30b291595d92c8b127dca/src/pages/home/HeaderView.tsx#L147

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

We should remove !isGroupChat check as it is redundant now and the expected results are to show the leave in three dot options:

const canJoinOrLeave = !isSelfDM  && (isChatThread || isUserCreatedPolicyRoom || canLeaveRoom); 

What alternative solutions did you explore? (Optional)

canJoinOrLeave is used in join and leave option both, so if we want to only enable the leave option then we have to update a || condition to allow to leave thread if the chat is a group type: https://github.com/Expensify/App/blob/32b6c38cb7a08573c1f30b291595d92c8b127dca/src/pages/home/HeaderView.tsx#L148-L149

[nkdengineer]

Proposal

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

There is no Leave thread option.

What is the root cause of that problem?

We don't allow leave or join for group chat by this check here and the chatType of thread is inherited by the parent so the thread of group chat doesn't have leave option.

https://github.com/Expensify/App/blob/f95bf85925686e64bf6c5bcfb6e66722ff6934fc/src/pages/home/HeaderView.tsx#L145

And I also see that the canLeaveRoom function return true for group chat because we don't check the chatType is group here https://github.com/Expensify/App/blob/f95bf85925686e64bf6c5bcfb6e66722ff6934fc/src/libs/ReportUtils.ts#L4884-L4889

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

1. We should remove the check isGroupChat here

https://github.com/Expensify/App/blob/f95bf85925686e64bf6c5bcfb6e66722ff6934fc/src/pages/home/HeaderView.tsx#L145

2. Add the check chatType is group here so for the group chat, the join/leave option will not appear

report?.chatType === CONST.REPORT.CHAT_TYPE.GROUP ||

https://github.com/Expensify/App/blob/f95bf85925686e64bf6c5bcfb6e66722ff6934fc/src/libs/ReportUtils.ts#L4884-L4889

and for the thread of this, join/leave option will appear by isChatThread condition

https://github.com/Expensify/App/blob/f95bf85925686e64bf6c5bcfb6e66722ff6934fc/src/pages/home/HeaderView.tsx#L145

What alternative solutions did you explore? (Optional)

If we allow the user to leave the group chat, we can simply remove !isGroupChat in this check

https://github.com/Expensify/App/blob/f95bf85925686e64bf6c5bcfb6e66722ff6934fc/src/pages/home/HeaderView.tsx#L145

And add the check !isGroupChat || isChatThread here to not show the join option for group chat since we already in the group chat.

const canJoin = canJoinOrLeave && !isWhisperAction && report.notificationPreference === CONST.REPORT.NOTIFICATION_PREFERENCE.HIDDEN && (!isGroupChat || isChatThread);

https://github.com/Expensify/App/blob/f95bf85925686e64bf6c5bcfb6e66722ff6934fc/src/pages/home/HeaderView.tsx#L146

[trjExpensify]

Moving this forward, and putting it in #vip-split as a groups bug. CC: @marcaaron @JmillsExpensify if you want to track it somewhere or it's already known as part of the Groups project rollout.

[melvin-bot[bot]]

@trjExpensify 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 - @jjcoffee (External)

[trjExpensify]

Ah whoops, needs an external label innit! @jjcoffee proposals here for you to review. Thanks! :)

[jjcoffee]

Tough decision here :smile: @Nodebrute's proposal does work, but I feel that it makes the code a bit unreadable (in an already pretty hard to parse set of conditions).

I think removing the !isGroupChat condition as in the other two proposals is the way to go here, as using that is what actually blocks any threaded messages from having the join/leave option (I imagine this might be on purpose for the self-DM). In that case, @nkdengineer's proposal is the one to go with as it also deals with keeping the join/leave option from appearing on the group chat itself.

:ribbon::eyes::ribbon: C+ reviewed

[melvin-bot[bot]]

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

[iwiznia]

We should remove the check isGroupChat here Add the check chatType is group here so for the group chat, the join/leave option will not appear

Isn't that basically the same thing except the new code will not check for || ReportUtils.isDeprecatedGroupDM(report);?

and for the thread of this, join/leave option will appear by isChatThread condition

I don't understand what you mean here, can you clarify? Is the only change the one above?

[nkdengineer]

Isn't that basically the same thing except the new code will not check for || ReportUtils.isDeprecatedGroupDM(report);?

@iwiznia Yeah we should bring all checks for isGroupChat into canLeaveRoom function instead of only adding the check chatType is group

const isGroupChat = ReportUtils.isGroupChat(report) || ReportUtils.isDeprecatedGroupDM(report);

I don't understand what you mean here, can you clarify? Is the only change the one above?

Yes, I mean after we move the check isGroupChat into canLeaveRoom function, the join/leave option will be displayed correctly for both group chat and the thread of its.

1. The join/leave option will not appear in group chat by adding the check in canLeaveRoom function.

2. And the thread of group chat will appear join/leave option with the new check.

const canJoinOrLeave = !isSelfDM  && (isChatThread || isUserCreatedPolicyRoom || canLeaveRoom); 

[iwiznia]

The join/leave option will not appear in group chat by adding the check in canLeaveRoom function.

Why would you not be able to leave a group chat? 😕

[nkdengineer]

Why would you not be able to leave a group chat? 😕

There should be a leave thread option after creating a thread.

@iwiznia I see that the expected in the description of issue is we only want to have leave thread option in the thread of group chat. Is that wrong?

[iwiznia]

Where are you seeing that? I think it is wrong, yes.

[nkdengineer]

@iwiznia I follow the expected of this issue.

[iwiznia]

Yes, that is correct, it is saying you should be able to leave a thread, it is NOT saying you should not be able to leave a group.

[iwiznia]

@jjcoffee did you also misinterpret the issue?

[nkdengineer]

4. Click on the three dots on the header, Notice that there is no Leave option.

@iwiznia Thanks for your confirmation.

I see that in the step 4 notice that there is no leave option and we don't consider this as bug in Actual result. So I think the expected for group chat is no leave/join option.

I updated my alternative solution https://github.com/Expensify/App/issues/39660#issuecomment-2039144454 to allow leave option for group chat and leave/join option for the group chat's thread.

[marcaaron]

@iwiznia I can take this one if you want.

Otherwise, the issue is about threads in Group Chats. The way to "leave" the Group Chat itself is via the report details page and a new "Leave" button (which will get merged soon). I believe that we will want to streamline all of this so that threads, rooms, Group Chats are all using a similar UI cc @shawnborton

For now - users do need the ability to leave the Group Chat thread, but leaving the Group Chat the same way as the thread (three button menu) will require more of a change and I'm not sure is what we want anyway.

[iwiznia]

Sure, go ahead

[marcaaron]

@jjcoffee @nkdengineer we are adding the ability to leave a Group Chat in this PR (previously there was no way to leave a Group DM).

For now, I think we will want to give users the ability to leave the Group Chat threads.

In this case, the problem is that isGroupChat() returns true for group chat threads so the !isGroupChat condition overrides the isChatThread logic. I believe what we ultimately want is something like this:

function isGroupChatThread(report: OnyxEntry<Report> | Partial<Report>): boolean {
    return isChatThread(report) && getChatType(report) === CONST.REPORT.CHAT_TYPE.GROUP;
}

function isGroupChat(report: OnyxEntry<Report> | Partial<Report>): boolean {
    return !isChatThread(report) && getChatType(report) === CONST.REPORT.CHAT_TYPE.GROUP;
}

However, there might be other changes to watch out for if we do this since the isGroupChat() method is called in several places.

[nkdengineer]

@marcaaron What do you think about my alternative solution here.

[jjcoffee]

@marcaaron Oooh thanks for the heads up on that PR! I like that idea in general of adding those extra helper functions, but I wonder if it's worth it if we will only (likely) use isGroupChatThread for this issue? It looks like there are cases where we currently use isGroupChat for either the main chat or the thread, so maybe we can get away with just adding a single extra isMainGroupChat instead (or maybe isParentGroupChat is a bit better)?

function isMainGroupChat(report: OnyxEntry<Report> | Partial<Report>): boolean {
    return !isChatThread(report) && getChatType(report) === CONST.REPORT.CHAT_TYPE.GROUP;
}

[jjcoffee]

What do you think about my alternative solution here.

@nkdengineer Your alternative solution would allow leaving the group chat through a different method from what has been added in the PR that @marcaaron linked, so I don't think we can go with that (for now).

[marcaaron]

I like isParentGroupChat(). I think it could still be possible that we run into some situations where we ask if something is a "Group Chat" and it's actually a "Group Chat thread" i.e. it should behave more like a "thread" than a Group Chat. But I don't know of any specific places in the code where this is happening or where we need it.

[marcaaron]

@nkdengineer your alt solution seems like it will work. But also feels like we are missing the correct place to put this logic.

You also say:

And I also see that the canLeaveRoom function return true for group chat because we don't check the chatType is group here

So, let's put your check there instead instead of tacking it onto something like:

const canLeaveOrJoin = canLeave && ...conditions related to "joining"

[marcaaron]

All of this looks due for a clean up of some kind. Feels like we could also maybe just have a canLeaveReport() method instead of a canLeaveRoom() and stop doing this confusing stuff:

https://github.com/Expensify/App/blob/a7820d2027138a1a9670b204f608d6b92ec0a454/src/pages/home/HeaderView.tsx#L146-L148

I'm unsure why we are combining the concepts of "leave" and "join" they are different things entirely and I find it quite strange that canJoinOrLeave is declared before a variable called canLeave and canJoin and we are not just setting the value of canJoinOrLeave to canLeave || canJoin.

Maybe we can just set each one individually and have something like canLeaveReport() and canJoinReport() helper methods.