[HOLD for payment 2024-08-20] [$250] Threads - Join thread menu item is not shown for system generated threads

[izarutskaya]

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.12-0
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 staging.new.expensify.com
2. Click on Green plus icon
3. Click on submit expense
4. Insert an amount and click next
5. Insert an email address and select the user
6. Click on submit expense
7. Click on the submitted expense
8. Click on Amount
9. Change the amount
10. Hover over the system generated message and click on reply in thread
11. Send something inside the thread
12. Click on Header
13. Click on Leave
14. Right click on the system message which has a reply

Expected Result:

Join thread menu item is inside the pop up menu.

Actual Result:

Join thread menu item is not inside the pop up menu which is a different behavior to other threads (Other threads have join thread menu item).

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

Bug6551773_1721843983545.Screen_Recording_20240724_204909_Chrome.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~018a37718a2aa2c10d
• Upwork Job ID: 1818200916076747170
• Last Price Increase: 2024-07-30
• Automatic offers:
• mkhutornyi | Reviewer | 103347859

Issue Owner

Current Issue Owner: @kadiealexander

[melvin-bot]

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

[nkdengineer]

Proposal

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

Join thread menu item is not inside the pop up menu which is a different behavior to other threads (Other threads have join thread menu item).

What is the root cause of that problem?

We only show Join thread option for comment action so the system message doesn't have this option though it can reply in thread

App/src/pages/home/report/ContextMenu/ContextMenuActions.tsx

Line 306 in b0f810d

return !subscribed && !isWhisperAction && isCommentAction && (!isDeletedAction || shouldDisplayThreadReplies);

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

We should replace isCommentAction check with shouldDisableThread function that we used to show the reply in thread option here

!subscribed && !isWhisperAction && !ReportUtils.shouldDisableThread(reportAction, reportID) && (!isDeletedAction || shouldDisplayThreadReplies); 

App/src/pages/home/report/ContextMenu/ContextMenuActions.tsx

Line 306 in b0f810d

return !subscribed && !isWhisperAction && isCommentAction && (!isDeletedAction || shouldDisplayThreadReplies);

OPTIONAL: we can also remove !isWhisperAction check since shouldDisableThread function already covered this case

What alternative solutions did you explore? (Optional)

[bernhardoj]

Proposal

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

There is no join thread option on a system message.

What is the root cause of that problem?

Currently, we only show join thread if the action is an ADDCOMMENT.

App/src/pages/home/report/ContextMenu/ContextMenuActions.tsx

Lines 304 to 306 in e9ec3e9

	const isCommentAction = reportAction?.actionName === CONST.REPORT.ACTIONS.TYPE.ADD_COMMENT && !ReportUtils.isThreadFirstChat(reportAction, reportID);
	const isWhisperAction = ReportActionsUtils.isWhisperAction(reportAction);
	return !subscribed && !isWhisperAction && isCommentAction && (!isDeletedAction || shouldDisplayThreadReplies);

So, system message that has different action type won't have join thread option even though the user can open the thread, leave, rejoin from the report.

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

We already have a logic for canJoinChat. This covers thread, group chat, etc.

App/src/libs/ReportUtils.ts

Lines 6986 to 7010 in e9ec3e9

	function canJoinChat(report: OnyxInputOrEntry<Report>, parentReportAction: OnyxInputOrEntry<ReportAction>, policy: OnyxInputOrEntry<Policy>): boolean {
	// We disabled thread functions for whisper action
	// So we should not show join option for existing thread on whisper message that has already been left, or manually leave it
	if (ReportActionsUtils.isWhisperAction(parentReportAction)) {
	return false;
	}
	// If the notification preference of the chat is not hidden that means we have already joined the chat
	if (report?.notificationPreference !== CONST.REPORT.NOTIFICATION_PREFERENCE.HIDDEN) {
	return false;
	}
	const isExpenseChat = isMoneyRequestReport(report) || isMoneyRequest(report) || isInvoiceReport(report) || isTrackExpenseReport(report);
	// Anyone viewing these chat types is already a participant and therefore cannot join
	if (isRootGroupChat(report) || isSelfDM(report) || isInvoiceRoom(report) || isSystemChat(report) || isExpenseChat) {
	return false;
	}
	// The user who is a member of the workspace has already joined the public announce room.
	if (isPublicAnnounceRoom(report) && !isEmptyObject(policy)) {
	return false;
	}
	return isChatThread(report) || isUserCreatedPolicyRoom(report) || isNonAdminOrOwnerOfPolicyExpenseChat(report, policy);
	}

However, it requires a child report and it's not always guaranteed that the child report is available after relogin. If we are fine with that, then we can use that replacing the whole current condition.

So, we need a solution that heavily rely on the report action itself. We need to know what kind of action that allows the Join thread option. If we look at the canJoinChat, it has some common condition with the condition to show join thread option. Don't allow whisper (isWhisperAction) and don't allow if notification preference is not hidden (subscribed). The thing that we don't have yet is the expense chat check (there is no join/leave in expense chat).

1. A report action that leads to an expense chat is IOU and REPORTPREVIEW, so we need to add that.

const isExpenseReportAction = ReportActionsUtils.isMoneyRequestAction(reportAction) || ReportActionsUtils.isReportPreviewAction(reportAction);

2. Then, we can throw this condition which only allows ADDCOMMENT, but also keep the !isThreadFirstChat condition,
   

   App/src/pages/home/report/ContextMenu/ContextMenuActions.tsx

   Line 304 in e9ec3e9

   const isCommentAction = reportAction?.actionName === CONST.REPORT.ACTIONS.TYPE.ADD_COMMENT && !ReportUtils.isThreadFirstChat(reportAction, reportID);

const isThreadFirstChat = ReportUtils.isThreadFirstChat(reportAction, reportID);

so we can join thread on any kind of action except the expense action and except the thread first chat. (We can add more case if needed)

3. We need to not show the join thread for archived room too, but only if the message doesn't have child, just like the deleted condition.

(!isDeletedAction && !isArchivedRoom) || shouldDisplayThreadReplies

4. Last, we need to update isWhisperAction to include actionable track expense action because we are not allowed to create thread for it.

const isWhisperAction = ReportActionsUtils.isWhisperAction(reportAction) || ReportActionsUtils.isActionableTrackExpense(reportAction);

Final condition

return !subscribed && !isWhisperAction && !isExpenseReportAction && !isThreadFirstChat && (shouldDisplayThreadReplies || (!isDeletedAction && !isArchivedRoom));

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~018a37718a2aa2c10d

[melvin-bot]

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

[mkhutornyi]

Thanks for the proposals.
@nkdengineer your proposal causes regressions like ReportPreview having Join thread.
@bernhardoj's proposal looks good. We need to test all posssble cases after refactoring Join thread display logic.
🎀👀🎀 C+ reviewed

[melvin-bot]

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

[melvin-bot]

📣 @mkhutornyi 🎉 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

[bernhardoj]

PR is ready

cc: @mkhutornyi

[melvin-bot]

⚠️ Looks like this issue was linked to a Deploy Blocker here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

[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 9.0.19-11 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• [CP Staging] Fix reply in thread missing after rejoined system message thread #47234

If no regressions arise, payment will be issued on 2024-08-20. 🎊

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

• @bernhardoj requires payment through NewDot Manual Requests
• @mkhutornyi requires payment automatic offer (Reviewer)

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

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

[kadiealexander]

@mkhutornyi don't forget the checklist!

[mkhutornyi]

This is not a regression. The bug existed for long time. We didn't consider Join thread option for system messages while implementing this feature.

Regression Test Steps

1. Submit a money request to any chat
2. Open the transaction thread
3. Edit any transaction field
4. Right-click or long-press the edit system message
5. Verify there is no "Join thread" option yet
6. Press "Reply in thread"
7. Press the header and choose "Leave"
8. Right-click or long-press the edit system message again
9. Verify there is a "Join thread" option

[bernhardoj]

Requested in ND.

[JmillsExpensify]

Can I get a payment summary for this issue?

[roryabraham]

Looks like:

• $250 to @bernhardoj (contributor role)
• $250 to @mkhutornyi (C+ role)

[kadiealexander]

Apologies for missing this one! Just a note - $250 to @mkhutornyi (C+ role) was paid via Upwork.

[JmillsExpensify]

$250 approved for @bernhardoj