CRITICAL: Update Mute behavior to not show as unread in the LHN.

[quinthar]

Note: This is being actively discussed in Slack here.

Strategy:
Supporting a billion users requires supporting a wide variety of work styles. We know even from our early discussions that the behavior of the LHN is extremely personal, with very subtle variations resulting in significant visceral reactions. While we can't satisfy everyone (and if we think a particular flow is bad, we might not want to), all things being equal we should aim to be as inclusive of as many workflows as possible.

Problem:
Coming from this discussion, we've realized our current Mute behavior only partially mutes. It does disable realtime notifications on that channel, and also disables UnreadMessageSummary on that channel. But it still shows the chat as unread in the LHN. To avoid this LHN distraction requires someone to leave/join the channel frequently, which is cumbersome, and distracting to the rest of the channel (because of your leave/join messages).

Solution:
Mimic Slack and make Mute fully mute, by not showing as unread in the LHN. This has at least a few benefits:

• It doesn't return it in the LHN when "Focus mode" is enabled (unless pinned), allowing for faster OpenApp/Reconnect app
• It allows you to get notified by @here and @channel group mentions (which are presumed to be more important), without getting distracted by anything else.
• It allows you to pin a chat for quick access without being distracted by if it's read/unread.
• It allows you to read, post, and disappear without needing to explicitly leave, and without triggering any joined/left messages

So there are advantages to mimicking Slack's Mute behavior. Are there any disadvantages? Is anyone using our current Mute behavior who would be annoyed by this change?

[quinthar]

Optimistically assigning to @neil-marcellini as I know this was important to him.

[roryabraham]

Coming from slack with my understanding of the requirements here. Update OpenApp and ReconnectApp to:

• ONLY return muted reports if they are "pinned"
• DO NOT mark muted reports unread in the LHN (eg, in bold), even if they have unread messages
• DO keep showing the "unread indicator" inside the chat

I have a feeling these will be more complex than they might seem on the surface but I'm going to dig in here and try to come up with a plan.

[melvin-bot]

Current assignee @roryabraham is eligible for the Engineering assigner, not assigning anyone new.

[roryabraham]

Slight modification to the above:

when in #focus mode ONLY return muted reports if they are "pinned"

[roryabraham]

when in #focus mode ONLY return muted reports if they are "pinned"

It looks like this should be relatively easy to accomplish by updating this WHERE clause as follows:

diff --git a/auth/lib/Report.cpp b/auth/lib/Report.cpp
index c1d0e6cc5c..993ee0151a 100644
--- a/auth/lib/Report.cpp
+++ b/auth/lib/Report.cpp
@@ -5097,13 +5097,16 @@ JSON::Value Report::getChats(SQLite& db, const int64_t accountID, const list<str
             "    OR r.reportID IN parentReportsWithOutstandingChildRequest "
             //   Task assigned to accountID is not complete
             "    OR (COALESCE(JSON_EXTRACT(rnvps.rNVPs, " + SQ("$." + string(NVP_TYPE)) + "), '') = " + SQ(TYPE_TASK) + " AND r.managerID = " + SQ(accountID) + " AND status = " + SQ(STATUS_OPEN) + ") "
-            //   Reports with unread comments: lastReadTime_ is older than the lastReportAction.created
+            //   Reports with unread comments: lastReadTime_ is older than the lastReportAction.created, except those w/ notification preference "muted"
             "    OR ("
-            "         CASE "
-            "           WHEN " + lastReportActionType + " = " + SQ(ACTION_CLOSED) + " THEN lastVisibleActionCreatedBeforeArchived "
-            "           ELSE " + lastReportActionCreated + " "
-            "         END > " + lastReadTime + " "
-            "       ) "
+            "      ("
+            "        CASE "
+            "          WHEN " + lastReportActionType + " = " + SQ(ACTION_CLOSED) + " THEN lastVisibleActionCreatedBeforeArchived "
+            "          ELSE " + lastReportActionCreated + " "
+            "        END > " + lastReadTime + " "
+            "      )"
+            "      AND COALESCE(JSON_EXTRACT(rnvps.rNVPs, " + SQ("$." + string(NVP_NOTIFICATION_PREFERENCES) + "." + to_string(accountID)) + "), '') != " + SQ(NOTIFICATION_PREFERENCE_MUTE) +
+            "    ) "
             //   Retrieve the user's primary policy workspace chat if it's available so it can be accessed locally for requesting money
             + (policyWorkspaceChatID > 0 ? ownPolicyExpenseChatFilter : "") +
             //   Transaction threads are returned so they can be displayed on NewDot's chat list with RBR when they have violations

Draft PR for that piece here: https://github.com/Expensify/Auth/pull/9560

[roryabraham]

DO NOT mark muted reports unread in the LHN (eg, in bold), even if they have unread messages

For this piece, it looks like "whether a chat is unread" in the LHN is determined here, by ReportUtils.isUnread. So I think we can update it to NOT mark muted reports as unread by simply updating that function like so:

diff --git a/src/libs/ReportUtils.ts b/src/libs/ReportUtils.ts
index b41b74cd57..5a746b9e45 100644
--- a/src/libs/ReportUtils.ts
+++ b/src/libs/ReportUtils.ts
@@ -3269,6 +3269,10 @@ function isUnread(report: OnyxEntry<Report>): boolean {
         return false;
     }
 
+    if (report.notificationPreference === CONST.REPORT.NOTIFICATION_PREFERENCE.MUTE) {
+        return false;
+    }
+
     // lastVisibleActionCreated and lastReadTime are both datetime strings and can be compared directly
     const lastVisibleActionCreated = report.lastVisibleActionCreated ?? '';
     const lastReadTime = report.lastReadTime ?? '';

However, I think we'll want to control that by a flag, because there are other places where ReportUtils.isUnread is used, such as here, where we'd want to consider it unread even if its notification preference is MUTE

[roryabraham]

Have a WIP PR for the front-end bits here, but I asked some clarifying questions in slack: https://expensify.slack.com/archives/C0671JDRKQW/p1703880645818079?thread_ts=1703723716.270759&cid=C0671JDRKQW

[roryabraham]

Laying out some more clarifications about expected behavior.

---

Action taken:

1. User A has DM with user B muted
2. User A opens the report and manually marks them as unread

Expected result:

The new message indicator in the chat moves to indicate the new unread message, but the report still does not appear as bold in the LHN.

---

Action taken:

1. User A is in #focus mode
2. User A has DM with User B muted
3. User A has read all messages and performed all outstanding actions in the report, so the chat with User B is not currently visible in their LHN.
4. User B requests $5 from user A

Expected result:

User A sees their DM with User B appear in the LHN with a green dot indicating that there's some action for them to perform (in this case, pay an IOU). However, the report should not appear as bold.

---

Action taken:

1. User A is in #focus mode
2. User A has DM with User B muted
3. User A has read all messages and performed all outstanding actions in the report, so the chat with User B is not currently visible in their LHN.
4. User B assigns a task to User A

Expected result:

User A sees their DM with User B appear in the LHN with a green dot indicating that there's some action for them to perform (in this case, complete a task). However, the report should not appear as bold.

---

Action taken:

1. User A is in #focus mode
2. Users A and B are in a chat room together
3. User A has read all messages and performed all outstanding actions in the room, so the room is not currently visible in their LHN.
4. User B sends a message in the room and mentions User A

Expected result:

User A sees the room appear in their LHN with a green dot indicating that there's some action for them to perform (in this case, respond to the person that directly mentioned them). However, the report should not appear as bold.

[roryabraham]

Added very detailed test steps in #33779

[roryabraham]

Ran into some issues with my dev environment while testing this, created a thread in #vm here

[roryabraham]

The source of my woes was that due to a recent change w/ reliable updates I needed to be running bwm. Ioni sent out an email and we're discussing running bwm as a service in the VM to make the dev environment more automatic.

Anyways, during my testing I found that a number of things just weren't working as expected, and then I discovered that was due to a recent regression on staging. I created a PR to fix that regression that we'll want to CP to staging, but I was blocked on completing my testing until that's merged.

[roryabraham]

Was trying to push this over the finish line today, but was blocked by an unrelated deploy blocker.

[roryabraham]

Removing reviewing label so this doesn't appear greyed out in my K2

[roryabraham]

This is currently blocked by #34466

[quinthar]

#34466 has been merged, this is no longer blocked. What are next steps?

[roryabraham]

PR put in for review. Just a note, I think I'll create an external follow-up issue to cover my changes with automated tests in the existing UnreadIndicatorsTest file. It can all be covered by automated tests, I just didn't take the time to build them. The manual testing takes a lot of time because there are so many scenarios, which to me suggests that this will be vulnerable to regressions. Therefore the automated tests will be valuable.

[quinthar]

Can you link the PR here? I'm not sure what you are referring to with "PR put in for review".

[roryabraham]

This is the PR: #33779

[quinthar]

Woohoo deployed to staging!