[HOLD for payment 2024-01-25] [$500] Room - No Join button in thread header for thread with no reply

[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.22-0
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: Applause - Internal Team
Slack conversation:

Issue found when executing PR #33791

Action Performed:

1. Go to workspace chat
2. Send a message
3. Right click on the message > Unsubscribe from thread
4. Right click on the message > Reply in thread

Expected Result:

There is a Join button on the right of the header

Actual Result:

There is no Join button on the header

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

Bug6333414_1704479548749.20240106_023028.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01e0087435438fe2da
• Upwork Job ID: 1743344144612663296
• Last Price Increase: 2024-01-05
• Automatic offers:
• Ollyws | Reviewer | 28093246
• paultsimura | Contributor | 28093247

[melvin-bot]

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

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~01e0087435438fe2da

[melvin-bot]

Bug0 Triage Checklist (Main S/O)

• This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
• This bug is not a duplicate report (check E/App issues and #expensify-bugs)
  • If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
• This bug is reproducible using the reproduction steps in the OP. S/O
  • If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
  • If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
• This issue is filled out as thoroughly and clearly as possible
  • Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
• I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

[melvin-bot]

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

[paultsimura]

Proposal

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

No "Join" button is displayed on the empty thread header

What is the root cause of that problem?

When showing the "Join" button, we check the thread to be not empty:

App/src/pages/home/HeaderView.js

Lines 156 to 158 in fb0b1d1

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

There is another issue that the "Join" button is visible for an optimistic thread (can be tested via sending a new message and opening it as a thread all in offline mode). This is because the new threads are created with Hidden notification preference, but the API response returns Always:

App/src/libs/actions/Report.ts

Line 742 in 596e56b

CONST.REPORT.NOTIFICATION_PREFERENCE.HIDDEN,

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

Remove the !isEmptyChat check:

const canJoinOrLeave = isChatThread || isUserCreatedPolicyRoom || canLeaveRoom;

To fix the optimistic thread issue, we should set the optimistic notification preference to "Always", if the user is the author of the original message, when clicking "Reply in thread" (i.e. creating a new thread) for the first time. It is replaced with "Always" on the API response if requested by the message author, and we're doing the same check do it when subscribing to the thread:

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

Lines 176 to 177 in e18b1d0

	const isActionCreator = ReportUtils.isActionCreator(reportAction);
	childReportNotificationPreference = isActionCreator ? CONST.REPORT.NOTIFICATION_PREFERENCE.ALWAYS : CONST.REPORT.NOTIFICATION_PREFERENCE.HIDDEN;

Therefore, this change will make the optimistic match the actual BE response.

We should change the following line (742):

- CONST.REPORT.NOTIFICATION_PREFERENCE.HIDDEN
+ ReportUtils.isActionCreator(parentReportAction) ? CONST.REPORT.NOTIFICATION_PREFERENCE.ALWAYS : CONST.REPORT.NOTIFICATION_PREFERENCE.HIDDEN

App/src/libs/actions/Report.ts

Lines 732 to 745 in 596e56b

	const newChat = ReportUtils.buildOptimisticChatReport(
	participantAccountIDs,
	parentReportAction?.message?.[0]?.text,
	parentReport?.chatType,
	parentReport?.policyID ?? CONST.POLICY.OWNER_EMAIL_FAKE,
	CONST.POLICY.OWNER_ACCOUNT_ID_FAKE,
	false,
	'',
	undefined,
	undefined,
	CONST.REPORT.NOTIFICATION_PREFERENCE.HIDDEN,
	parentReportAction.reportActionID,
	parentReportID,
	);

What alternative solutions did you explore? (Optional)

[paultsimura]

@srikarparsi could you please share why you decided to hide the "Join" button on the empty threads here?

[samilabud]

Proposal

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

Room - No Join button in thread header for thread with no reply

What is the root cause of that problem?

When we reply in the same thread and we are unsubscribed we do not take into consideration if the parent's response is empty to show the join button.

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

We should take into consideration if the parent message is empty or not to show the join button:

const isParentEmpty = !props.parentReport.lastMessageText && !props.report.lastMessageTranslationKey;
.
.
.
 const canJoinOrLeave = (isChatThread && (!isEmptyChat || !isParentEmpty)) || isUserCreatedPolicyRoom || canLeaveRoom;

Result:

Join.button.in.thread.test.mp4

[aim-salam]

.

[srikarparsi]

@srikarparsi could you please share why you decided to hide the "Join" button on the empty threads here?

I believe it was because of the words "join" and "leave" since joining and leaving a nonexistent thread didn't make much sense at the time. Subscribe and unsubscribing made more sense in that context. But I believe we're changing the language across the menus to be "join" and "leave." So it would make sense to be able to "join" a nonexistent thread now.

[paultsimura]

So it would make sense to be able to "join" a nonexistent thread now.

Cool, thank you for an expanded explanation 👍

[paultsimura]

Proposal

Updated proposal #34054 (comment) to cover the optimistic button flickering.

@srikarparsi was there any specific reason to create the thread with notification: hidden, considering the API response replaces it with Always right after?

[srikarparsi]

Hey @Ollyws, can you please take a look at these proposals whenever you get a chance? This proposal looks mostly good to me since it uses the same logic as the context menu actions.

I'm not sure if we need to check for other cases for when the notification preference is set to always, such as when the workspace admin comments as brought up here, because then the thread will no longer have no replies right?

If the proposal looks good and there are only a few minor changes, I think we should be able to get this done by Wednesday or Thursday?

[paultsimura]

The notification preference conditions needs to be handled for buildOptimisticIOUReport, buildOptimisticExpenseReport and buildTransactionThread.

No need for this, as the user cannot Join any of those reports. The list of reports a user can join is strictly defined (omitted extra code for clarity):

const canJoinOrLeave = isChatThread || isUserCreatedPolicyRoom;

[Ollyws]

This proposal looks mostly good to me since it uses the same logic as the context menu actions.

@srikarparsi I'm guessing you were talking about @paultsimura's proposal here.
LGTM too.
🎀👀🎀 C+ reviewed

[melvin-bot]

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

[melvin-bot]

📣 @Ollyws 🎉 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]

📣 @paultsimura 🎉 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 📖

[paultsimura]

Thanks. The PR is ready for review: #34376

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

• fix: Show Join & Leave buttons on empty threads #34376

If no regressions arise, payment will be issued on 2024-01-25. 🎊

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

• @Ollyws requires payment automatic offer (Reviewer)
• @paultsimura 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:

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

[sophiepintoraetz]

@paultsimura - you're all paid - @Ollyws , I'll release payment once the BZ checklist is complete!

[Ollyws]

BugZero Checklist:

• The PR that introduced the bug has been identified. Link to the PR:

This behaviour was introduced in #27748 but there was reasoning behind it, so I'm not sure if this can really be defined as a bug it's more of a new feature.

• 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. Go to any chat
2. Send a message
3. Right-click on the message > Unsubscribe from thread
4. Right-click on the message > Reply in the thread
5. Verify the "Join" button is present in the Header

Do we agree 👍 or 👎

[sophiepintoraetz]

Awesome, thanks @Ollyws! I need you to accept the offer though before I pay
https://www.upwork.com/nx/wm/offer/28093246

[Ollyws]

Accepted, thanks.