Unify unread notification state determination (#9941)

* Add tests for unread notification facilities Add some tests to guarantee some consistency in `useUnreadNotifications` and `RoomNotificationState`. Signed-off-by: Clark Fischer <clark.fischer@gmail.com> * Add RoomNotifs#determineUnreadState Intended as a singular replacement for the divergent implementations before. Signed-off-by: Clark Fischer <clark.fischer@gmail.com> * Unify room unread state determination Have both the class-based facility and the hook use the new unified logic in `RoomNotifs#determineUnreadState`. Addresses element-hq/element-web#24229 Signed-off-by: Clark Fischer <clark.fischer@gmail.com> --------- Signed-off-by: Clark Fischer <clark.fischer@gmail.com> Co-authored-by: Michael Telatynski <7t3chguy@gmail.com>
matrix-org · Jan 31, 2023 · 431afaa · 431afaa
1 parent 53a9b64
commit 431afaa
Show file tree

Hide file tree

Showing 9 changed files with 500 additions and 232 deletions.
diff --git a/src/RoomNotifs.ts b/src/RoomNotifs.ts
@@ -1,6 +1,5 @@
 /*
-Copyright 2016 OpenMarket Ltd
-Copyright 2019 The Matrix.org Foundation C.I.C.
+Copyright 2016, 2019, 2023 The Matrix.org Foundation C.I.C.
 
 Licensed under the Apache License, Version 2.0 (the "License");
 you may not use this file except in compliance with the License.
@@ -16,18 +15,18 @@ limitations under the License.
 */
 
 import { PushProcessor } from "matrix-js-sdk/src/pushprocessor";
-import { NotificationCountType, Room } from "matrix-js-sdk/src/models/room";
-import {
-    ConditionKind,
-    IPushRule,
-    PushRuleActionName,
-    PushRuleKind,
-    TweakName,
-} from "matrix-js-sdk/src/@types/PushRules";
+import { NotificationCountType } from "matrix-js-sdk/src/models/room";
+import { ConditionKind, PushRuleActionName, PushRuleKind, TweakName } from "matrix-js-sdk/src/@types/PushRules";
 import { EventType } from "matrix-js-sdk/src/@types/event";
-import { MatrixClient } from "matrix-js-sdk/src/matrix";
 
+import type { IPushRule } from "matrix-js-sdk/src/@types/PushRules";
+import type { Room } from "matrix-js-sdk/src/models/room";
+import type { MatrixClient } from "matrix-js-sdk/src/matrix";
 import { MatrixClientPeg } from "./MatrixClientPeg";
+import { NotificationColor } from "./stores/notifications/NotificationColor";
+import { getUnsentMessages } from "./components/structures/RoomStatusBar";
+import { doesRoomHaveUnreadMessages, doesRoomOrThreadHaveUnreadMessages } from "./Unread";
+import { getEffectiveMembership, EffectiveMembership } from "./utils/membership";
 
 export enum RoomNotifState {
     AllMessagesLoud = "all_messages_loud",
@@ -36,7 +35,7 @@ export enum RoomNotifState {
     Mute = "mute",
 }
 
-export function getRoomNotifsState(client: MatrixClient, roomId: string): RoomNotifState {
+export function getRoomNotifsState(client: MatrixClient, roomId: string): RoomNotifState | null {
     if (client.isGuest()) return RoomNotifState.AllMessages;
 
     // look through the override rules for a rule affecting this room:
@@ -177,7 +176,7 @@ function setRoomNotifsStateUnmuted(roomId: string, newState: RoomNotifState): Pr
     return Promise.all(promises);
 }
 
-function findOverrideMuteRule(roomId: string): IPushRule {
+function findOverrideMuteRule(roomId: string): IPushRule | null {
     const cli = MatrixClientPeg.get();
     if (!cli?.pushRules?.global?.override) {
         return null;
@@ -201,3 +200,44 @@ function isRuleForRoom(roomId: string, rule: IPushRule): boolean {
 function isMuteRule(rule: IPushRule): boolean {
     return rule.actions.length === 1 && rule.actions[0] === PushRuleActionName.DontNotify;
 }
+
+export function determineUnreadState(
+    room: Room,
+    threadId?: string,
+): { color: NotificationColor; symbol: string | null; count: number } {
+    if (getUnsentMessages(room, threadId).length > 0) {
+        return { symbol: "!", count: 1, color: NotificationColor.Unsent };
+    }
+
+    if (getEffectiveMembership(room.getMyMembership()) === EffectiveMembership.Invite) {
+        return { symbol: "!", count: 1, color: NotificationColor.Red };
+    }
+
+    if (getRoomNotifsState(room.client, room.roomId) === RoomNotifState.Mute) {
+        return { symbol: null, count: 0, color: NotificationColor.None };
+    }
+
+    const redNotifs = getUnreadNotificationCount(room, NotificationCountType.Highlight, threadId);
+    const greyNotifs = getUnreadNotificationCount(room, NotificationCountType.Total, threadId);
+
+    const trueCount = greyNotifs || redNotifs;
+    if (redNotifs > 0) {
+        return { symbol: null, count: trueCount, color: NotificationColor.Red };
+    }
+
+    if (greyNotifs > 0) {
+        return { symbol: null, count: trueCount, color: NotificationColor.Grey };
+    }
+
+    // We don't have any notified messages, but we might have unread messages. Let's
+    // find out.
+    let hasUnread = false;
+    if (threadId) hasUnread = doesRoomOrThreadHaveUnreadMessages(room.getThread(threadId)!);
+    else hasUnread = doesRoomHaveUnreadMessages(room);
+
+    return {
+        symbol: null,
+        count: trueCount,
+        color: hasUnread ? NotificationColor.Bold : NotificationColor.None,
+    };
+}
diff --git a/src/hooks/useUnreadNotifications.ts b/src/hooks/useUnreadNotifications.ts
@@ -1,5 +1,5 @@
 /*
-Copyright 2022 The Matrix.org Foundation C.I.C.
+Copyright 2022 - 2023 The Matrix.org Foundation C.I.C.
 
 Licensed under the Apache License, Version 2.0 (the "License");
 you may not use this file except in compliance with the License.
@@ -14,15 +14,12 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-import { NotificationCount, NotificationCountType, Room, RoomEvent } from "matrix-js-sdk/src/models/room";
-import { Thread } from "matrix-js-sdk/src/models/thread";
+import { RoomEvent } from "matrix-js-sdk/src/models/room";
 import { useCallback, useEffect, useState } from "react";
 
-import { getUnsentMessages } from "../components/structures/RoomStatusBar";
-import { getRoomNotifsState, getUnreadNotificationCount, RoomNotifState } from "../RoomNotifs";
-import { NotificationColor } from "../stores/notifications/NotificationColor";
-import { doesRoomOrThreadHaveUnreadMessages } from "../Unread";
-import { EffectiveMembership, getEffectiveMembership } from "../utils/membership";
+import type { NotificationCount, Room } from "matrix-js-sdk/src/models/room";
+import { determineUnreadState } from "../RoomNotifs";
+import type { NotificationColor } from "../stores/notifications/NotificationColor";
 import { useEventEmitter } from "./useEventEmitter";
 
 export const useUnreadNotifications = (
@@ -53,40 +50,10 @@ export const useUnreadNotifications = (
     useEventEmitter(room, RoomEvent.MyMembership, () => updateNotificationState());
 
     const updateNotificationState = useCallback(() => {
-        if (getUnsentMessages(room, threadId).length > 0) {
-            setSymbol("!");
-            setCount(1);
-            setColor(NotificationColor.Unsent);
-        } else if (getEffectiveMembership(room.getMyMembership()) === EffectiveMembership.Invite) {
-            setSymbol("!");
-            setCount(1);
-            setColor(NotificationColor.Red);
-        } else if (getRoomNotifsState(room.client, room.roomId) === RoomNotifState.Mute) {
-            setSymbol(null);
-            setCount(0);
-            setColor(NotificationColor.None);
-        } else {
-            const redNotifs = getUnreadNotificationCount(room, NotificationCountType.Highlight, threadId);
-            const greyNotifs = getUnreadNotificationCount(room, NotificationCountType.Total, threadId);
-
-            const trueCount = greyNotifs || redNotifs;
-            setCount(trueCount);
-            setSymbol(null);
-            if (redNotifs > 0) {
-                setColor(NotificationColor.Red);
-            } else if (greyNotifs > 0) {
-                setColor(NotificationColor.Grey);
-            } else {
-                // We don't have any notified messages, but we might have unread messages. Let's
-                // find out.
-                let roomOrThread: Room | Thread = room;
-                if (threadId) {
-                    roomOrThread = room.getThread(threadId)!;
-                }
-                const hasUnread = doesRoomOrThreadHaveUnreadMessages(roomOrThread);
-                setColor(hasUnread ? NotificationColor.Bold : NotificationColor.None);
-            }
-        }
+        const { symbol, count, color } = determineUnreadState(room, threadId);
+        setSymbol(symbol);
+        setCount(count);
+        setColor(color);
     }, [room, threadId]);
 
     useEffect(() => {

diff --git a/src/stores/local-echo/RoomEchoChamber.ts b/src/stores/local-echo/RoomEchoChamber.ts
@@ -58,10 +58,9 @@ export class RoomEchoChamber extends GenericEchoChamber<RoomEchoContext, CachedR
     };
 
     private updateNotificationVolume(): void {
-        this.properties.set(
-            CachedRoomKey.NotificationVolume,
-            getRoomNotifsState(this.matrixClient, this.context.room.roomId),
-        );
+        const state = getRoomNotifsState(this.matrixClient, this.context.room.roomId);
+        if (state) this.properties.set(CachedRoomKey.NotificationVolume, state);
+        else this.properties.delete(CachedRoomKey.NotificationVolume);
         this.markEchoReceived(CachedRoomKey.NotificationVolume);
         this.emit(PROPERTY_UPDATED, CachedRoomKey.NotificationVolume);
     }

diff --git a/src/stores/notifications/RoomNotificationState.ts b/src/stores/notifications/RoomNotificationState.ts
@@ -1,5 +1,5 @@
 /*
-Copyright 2020 The Matrix.org Foundation C.I.C.
+Copyright 2020, 2023 The Matrix.org Foundation C.I.C.
 
 Licensed under the Apache License, Version 2.0 (the "License");
 you may not use this file except in compliance with the License.
@@ -14,21 +14,19 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-import { MatrixEvent, MatrixEventEvent } from "matrix-js-sdk/src/models/event";
-import { NotificationCountType, Room, RoomEvent } from "matrix-js-sdk/src/models/room";
+import { MatrixEventEvent } from "matrix-js-sdk/src/models/event";
+import { RoomEvent } from "matrix-js-sdk/src/models/room";
 import { ClientEvent } from "matrix-js-sdk/src/client";
 import { Feature, ServerSupport } from "matrix-js-sdk/src/feature";
 
-import { NotificationColor } from "./NotificationColor";
-import { IDestroyable } from "../../utils/IDestroyable";
+import type { Room } from "matrix-js-sdk/src/models/room";
+import type { MatrixEvent } from "matrix-js-sdk/src/models/event";
+import type { IDestroyable } from "../../utils/IDestroyable";
 import { MatrixClientPeg } from "../../MatrixClientPeg";
-import { EffectiveMembership, getEffectiveMembership } from "../../utils/membership";
 import { readReceiptChangeIsFor } from "../../utils/read-receipts";
 import * as RoomNotifs from "../../RoomNotifs";
-import * as Unread from "../../Unread";
 import { NotificationState, NotificationStateEvents } from "./NotificationState";
-import { getUnsentMessages } from "../../components/structures/RoomStatusBar";
-import { ThreadsRoomNotificationState } from "./ThreadsRoomNotificationState";
+import type { ThreadsRoomNotificationState } from "./ThreadsRoomNotificationState";
 
 export class RoomNotificationState extends NotificationState implements IDestroyable {
     public constructor(public readonly room: Room, private readonly threadsState?: ThreadsRoomNotificationState) {
@@ -49,10 +47,6 @@ export class RoomNotificationState extends NotificationState implements IDestroy
         this.updateNotificationState();
     }
 
-    private get roomIsInvite(): boolean {
-        return getEffectiveMembership(this.room.getMyMembership()) === EffectiveMembership.Invite;
-    }
-
     public destroy(): void {
         super.destroy();
         const cli = this.room.client;
@@ -112,58 +106,10 @@ export class RoomNotificationState extends NotificationState implements IDestroy
     private updateNotificationState(): void {
         const snapshot = this.snapshot();
 
-        if (getUnsentMessages(this.room).length > 0) {
-            // When there are unsent messages we show a red `!`
-            this._color = NotificationColor.Unsent;
-            this._symbol = "!";
-            this._count = 1; // not used, technically
-        } else if (
-            RoomNotifs.getRoomNotifsState(this.room.client, this.room.roomId) === RoomNotifs.RoomNotifState.Mute
-        ) {
-            // When muted we suppress all notification states, even if we have context on them.
-            this._color = NotificationColor.None;
-            this._symbol = null;
-            this._count = 0;
-        } else if (this.roomIsInvite) {
-            this._color = NotificationColor.Red;
-            this._symbol = "!";
-            this._count = 1; // not used, technically
-        } else {
-            const redNotifs = RoomNotifs.getUnreadNotificationCount(this.room, NotificationCountType.Highlight);
-            const greyNotifs = RoomNotifs.getUnreadNotificationCount(this.room, NotificationCountType.Total);
-
-            // For a 'true count' we pick the grey notifications first because they include the
-            // red notifications. If we don't have a grey count for some reason we use the red
-            // count. If that count is broken for some reason, assume zero. This avoids us showing
-            // a badge for 'NaN' (which formats as 'NaNB' for NaN Billion).
-            const trueCount = greyNotifs ? greyNotifs : redNotifs ? redNotifs : 0;
-
-            // Note: we only set the symbol if we have an actual count. We don't want to show
-            // zero on badges.
-
-            if (redNotifs > 0) {
-                this._color = NotificationColor.Red;
-                this._count = trueCount;
-                this._symbol = null; // symbol calculated by component
-            } else if (greyNotifs > 0) {
-                this._color = NotificationColor.Grey;
-                this._count = trueCount;
-                this._symbol = null; // symbol calculated by component
-            } else {
-                // We don't have any notified messages, but we might have unread messages. Let's
-                // find out.
-                const hasUnread = Unread.doesRoomHaveUnreadMessages(this.room);
-                if (hasUnread) {
-                    this._color = NotificationColor.Bold;
-                } else {
-                    this._color = NotificationColor.None;
-                }
-
-                // no symbol or count for this state
-                this._count = 0;
-                this._symbol = null;
-            }
-        }
+        const { color, symbol, count } = RoomNotifs.determineUnreadState(this.room);
+        this._color = color;
+        this._symbol = symbol;
+        this._count = count;
 
         // finally, publish an update if needed
         this.emitIfUpdated(snapshot);