[HOLD for payment 2024-06-28] [HOLD for payment 2024-06-24] [$250] Group chat thread - Inconsistency when leaving thread via 3-dot menu and group details page

[lanitochka17]

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: 1.4-79-3
Reproducible in staging?: Y
Reproducible in production?: Y
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:

1. Go to staging.new.expensify.com
2. Go to group chat.
3. Send a message and send a reply in thread.
4. Go to the thread.
5. Leave the thread via 3-dot menu > Leave.
6. Go to the same thread again.
7. Note that when leaving thread via 3-dot menu, there is no "Heads up" warning and thread does not load when reopening the thread after leaving.
8. Send a message and send a reply in thread.
9. Go to the thread.
10. Leave the thread via chat header > Leave.
11. Go to the same thread again.
12. Note that when leaving thread via group menu, there is "Heads up" warning and thread loads when reopening the thread after leaving.

Expected Result:

There should be consistency when leaving thread and reopening thread after leaving via 3-dot menu (Step 7) and group details page (Step 12)

Actual Result:

In Step 7. when leaving thread via 3-dot menu and reopening thread, there is no "Heads up" warning and thread does not load when reopening the thread after leaving
In Step 12, when leaving thread via group menu, there is "Heads up" warning and thread loads when reopening the thread after leaving

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

Add any screenshot/video evidence

Bug6502481_1717577093194.bandicam_2024-06-05_16-39-37-515.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~0166b95168c5d28cbd
• Upwork Job ID: 1798387156425131546
• Last Price Increase: 2024-06-05
• Automatic offers:
• akinwale | Reviewer | 102676476
• bernhardoj | Contributor | 102676480

Issue Owner

Current Issue Owner: @puneetlath

[melvin-bot]

Triggered auto assignment to @puneetlath (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.

[lanitochka17]

@puneetlath FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors

[lanitochka17]

We think that this bug might be related to #vip-vsp

[dragnoir]

Proposal

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

Leave from 3 dots on the header is not the same as leave from the report details page

What is the root cause of that problem?

On HeaderView, for the 3 dots leave action we call for Report.leaveRoom

App/src/pages/home/HeaderView.tsx

Line 174 in 0efe60a

onSelected: Session.checkIfActionIsAllowed(() => Report.leaveRoom(reportID, isWorkspaceMemberLeavingWorkspaceRoom)),

But on ReportDetailsPage we call for Report.leaveGroupChat

App/src/pages/ReportDetailsPage.tsx

Line 131 in 0efe60a

Report.leaveGroupChat(report.reportID);

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

We should do the same on HeaderView, for the 3 dots.

We shoudl check if isChatRoom if so we call Report.leaveRoom, else we call for Report.leaveGroupChat

https://github.com/Expensify/App/blob/0efe60a7e681b02606b4e3d58bd000e3c88014cd/src/pages/ReportDetailsPage.tsx#L126-L131C9

onSelected: Session.checkIfActionIsAllowed(() => {
  if (isChatRoom) {
    Report.leaveRoom(report.reportID, isWorkspaceMemberLeavingWorkspaceRoom);
    return;
  }
   Report.leaveGroupChat(report.reportID);
  }),

What alternative solutions did you explore?

[ShridharGoel]

This is not a bug, confirmation modal is only needed when last member leaves the group chat and not a thread.

[bernhardoj]

Proposal

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

Pressing leave from the report detail page shows a modal confirmation but not from the 3-dot menu.

What is the root cause of that problem?

It's expected that if we press Leave from a 3-dot, it won't show a confirmation modal. It's only for a group chat in detail page as you can see here.

App/src/pages/ReportDetailsPage.tsx

Lines 216 to 217 in 8dbec8d

	if (ReportUtils.getParticipantAccountIDs(report.reportID, true).length === 1 && isGroupChat) {
	setIsLastMemberLeavingGroupModalVisible(true);

The real problem here is that, the leave button on the detail page shouldn't show for a thread. The condition to show the leave button is if it's either a group chat or a chat room and canLeaveChat is true.

App/src/pages/ReportDetailsPage.tsx

Line 209 in 8dbec8d

if (isGroupChat || (isChatRoom && ReportUtils.canLeaveChat(report, policy ?? null))) {

When we create a thread inside a group chat, it's still detected as a group chat because thread chatType is inherited from its parent.

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

Don't show the leave button option if it's a thread.

if (!isThread && (isGroupChat || (isChatRoom && ReportUtils.canLeaveChat(report, policy ?? null)))) {

We did this before in the original PR, but treated as out of scope.

[ShridharGoel]

Proposal

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

Group chat thread - Inconsistency when leaving thread via 3-dot menu and group details page.

What is the root cause of that problem?

This is not a bug, confirmation modal is only needed when last member leaves the group chat and not a thread.

I think join and leave options are needed in threads to set notification preferences. But if we want to avoid showing them, then that can be done.

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

In canJoinChat, instead of isRootGroupChat we can use isGroupChat at this place:

// Anyone viewing these chat types is already a participant and therefore cannot join
if (isGroupChat(report) || isSelfDM(report) || isInvoiceRoom(report) || isSystemChat(report)) {
    return false;
}

In canLeaveChat, again instead of isRootGroupChat we can use isGroupChat at this place:

// Anyone viewing these chat types is already a participant and therefore cannot leave
if (isSelfDM(report) || isGroupChat(report)) {
    return false;
}

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~0166b95168c5d28cbd

[melvin-bot]

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

[akinwale]

We can move forward with @bernhardoj's proposal here since it's the best approach to solving the issue.

🎀👀🎀 C+ reviewed.

[melvin-bot]

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

[ShridharGoel]

@akinwale Can you give some feedback on my proposal? I think we should be updating the checks in the join and leave methods.

…

On Sat, Jun 8, 2024, 6:56 AM melvin-bot[bot] ***@***.***> wrote: Current assignee @puneetlath <https://github.com/puneetlath> is eligible for the choreEngineerContributorManagement assigner, not assigning anyone new. — Reply to this email directly, view it on GitHub <#43126 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AIPLJHHMDARPXBEZRYCHLPDZGJMS7AVCNFSM6AAAAABI2ZT62OVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNJVG42DGNBRG4> . You are receiving this because you commented.Message ID: ***@***.***>

[melvin-bot]

📣 @akinwale 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job

[melvin-bot]

📣 @bernhardoj 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job
Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

[bernhardoj]

PR is ready

cc: @akinwale

[melvin-bot]

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

[melvin-bot]

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

• Don't show leave button in report details page if it's a thread #43454

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

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

• @akinwale requires payment automatic offer (Reviewer)
• @bernhardoj requires payment automatic offer (Contributor)

[melvin-bot]

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:

• [@akinwale] The PR that introduced the bug has been identified. Link to the PR:
• [@akinwale] 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:
• [@akinwale] 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:
• [@akinwale] Determine if we should create a regression test for this bug.
• [@akinwale] 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.
• [@puneetlath] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[melvin-bot]

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

• Don't show leave button in report details page if it's a thread #43454

If no regressions arise, payment will be issued on 2024-06-28. 🎊

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

• @akinwale requires payment automatic offer (Reviewer)
• @bernhardoj requires payment automatic offer (Contributor)

[melvin-bot]

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:

• [@akinwale] The PR that introduced the bug has been identified. Link to the PR:
• [@akinwale] 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:
• [@akinwale] 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:
• [@akinwale] Determine if we should create a regression test for this bug.
• [@akinwale] 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.
• [@puneetlath] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[puneetlath]

@akinwale bump on the checklist so we can pay.

[akinwale]

• [@akinwale] The PR that introduced the bug has been identified. Link to the PR:
• [@akinwale] 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:
• [@akinwale] 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:'

Not a regression.

• [@akinwale] Determine if we should create a regression test for this bug.
• [@akinwale] 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 Steps

1. Launch the Expensify app.
2. Create a group chat or open an existing group chat.
3. Send a message.
4. Reply in the thread of the new message.
5. Click or tap on the thread header.
6. Verify that there is no leave button on the details page.

Do we agree 👍 or 👎?

[akinwale]

@puneetlath Done.

[puneetlath]

Regression issue here:

All paid. Thanks y'all!