[Awaiting Payment 29th May] Threads - There is no Leave thread option on group Chat threads

[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: v1.4.60-1
Reproducible in staging?: Y
Reproducible in production?: Y
Email or phone of affected tester (no customers): dave0123seife+hl5@gmail.com
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?

• Android: Native
• Android: mWeb Chrome
• iOS: Native
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Bug6437737_1712221240801.Issue_NO_leave_Option.mp4

View all open jobs on GitHub

[melvin-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.

App/src/pages/home/HeaderView.tsx

Line 147 in 32b6c38

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

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:

App/src/pages/home/HeaderView.tsx

Line 147 in 32b6c38

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

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:

App/src/pages/home/HeaderView.tsx

Lines 148 to 149 in 32b6c38

	const canJoin = canJoinOrLeave && !isWhisperAction && report.notificationPreference === CONST.REPORT.NOTIFICATION_PREFERENCE.HIDDEN;
	const canLeave = canJoinOrLeave && ((isChatThread && !!report.notificationPreference?.length) || isUserCreatedPolicyRoom || canLeaveRoom);

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

App/src/pages/home/HeaderView.tsx

Line 145 in f95bf85

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

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

App/src/libs/ReportUtils.ts

Lines 4884 to 4889 in f95bf85

	report?.chatType === CONST.REPORT.CHAT_TYPE.POLICY_ADMINS ||
	report?.chatType === CONST.REPORT.CHAT_TYPE.POLICY_ANNOUNCE ||
	report?.chatType === CONST.REPORT.CHAT_TYPE.POLICY_EXPENSE_CHAT ||
	report?.chatType === CONST.REPORT.CHAT_TYPE.DOMAIN_ALL ||
	report?.chatType === CONST.REPORT.CHAT_TYPE.SELF_DM ||
	!report?.chatType

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

1. We should remove the check isGroupChat here

App/src/pages/home/HeaderView.tsx

Line 145 in f95bf85

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

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

App/src/libs/ReportUtils.ts

Lines 4884 to 4889 in f95bf85

	report?.chatType === CONST.REPORT.CHAT_TYPE.POLICY_ADMINS ||
	report?.chatType === CONST.REPORT.CHAT_TYPE.POLICY_ANNOUNCE ||
	report?.chatType === CONST.REPORT.CHAT_TYPE.POLICY_EXPENSE_CHAT ||
	report?.chatType === CONST.REPORT.CHAT_TYPE.DOMAIN_ALL ||
	report?.chatType === CONST.REPORT.CHAT_TYPE.SELF_DM ||
	!report?.chatType

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

App/src/pages/home/HeaderView.tsx

Line 145 in f95bf85

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

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

App/src/pages/home/HeaderView.tsx

Line 145 in f95bf85

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

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);

App/src/pages/home/HeaderView.tsx

Line 146 in f95bf85

const canJoin = canJoinOrLeave && !isWhisperAction && report.notificationPreference === CONST.REPORT.NOTIFICATION_PREFERENCE.HIDDEN;

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

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

[melvin-bot]

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

[melvin-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 😄 @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.

🎀👀🎀 C+ reviewed

[melvin-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.

[nkdengineer]

@jjcoffee The PR is here.

[melvin-bot]

This issue has not been updated in over 15 days. @trjExpensify, @jjcoffee, @marcaaron, @nkdengineer 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!

[trjExpensify]

PR was merged earlier today, Melv.

[saracouto]

@trjExpensify what is left to be done here?

[trjExpensify]

The PR hit production 2 days ago, so we'll now wait 5 more days for any regressions. Then we'll pay it out and close. :)

[jjcoffee]

• The PR that introduced the bug has been identified. Link to the PR: N/A - this has always been the behaviour as far as I can tell
• 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: N/A
• 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
• Determine if we should create a regression test for this bug. Yes
• 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.

Regression Test Proposal

1. Create a group chat
2. Go to the group chat and send a message
3. Click on three-dot menu in the header
4. Verify that leave option doesn't appear
5. Reply in thread to the group chat message in (2)
6. Repeat step 3 and verify that the leave option appears

Do we agree 👍 or 👎

[jjcoffee]

Just a heads up that this issue was created before the default payment changed to $250 (on April 5) so the bounty should be $500.

[marcaaron]

LGTM @jjcoffee

[trjExpensify]

Thanks for the steps! Why can't you leave a group in step #3 though? I thought anyone could leave those now. 🤔

If that's accurate, my suggested edit would be:

1. Create a group chat
2. Send a message in the group chat
3. Click on the three-dot overflow menu in the header of the group chat
4. Verify the leave option appears
5. Reply in thread to the group chat message sent in (2)
6. Click on the three-dot overflow menu in the header of the thread
7. Verify that the leave option appears

[jjcoffee]

@trjExpensify Ah yeah you are right that you can leave groups but it's not via the three-dot menu. You have to tap the header and then the modal has the leave option:

We could add an extra step to tap the header and verify that the leave option is there? It'd be good to keep the step verifying that the leave option isn't in the three-dot menu though.

[trjExpensify]

Hm, so actually.. we're in the process of updating all of these in a project called "Details revamp" in implementation. So there won't be a overflow menu in any of the headers, the buttons will reside on the Details page instead.

So I'm going to hold off on this regression test, and the project when wrapped will add/update them all centrally.

[trjExpensify]

Payment summary as follows:

• $500 to @nkdengineer for the fix
• $500 to @jjcoffee for the C+ review

Offers sent!

[jjcoffee]

@trjExpensify Thanks, offer accepted!

[trjExpensify]

Joel, paid!

[nkdengineer]

@trjExpensify Thank you, I accepted the offer

[trjExpensify]

Perfect, paid. Closing!