Room notification state: add clearer methods, documentation and deprecation (#29564)

* feat(notification state): add clearer methods, documentation and deprecation

* test(room notification state): add tests for new attributes

* doc: more explicit documentation for `hasUnreadCount`

* doc: add link to `RoomNotificationState.isMention` in `hasMentions` doc

* refactor: change `isSilent` to `hasAnyNotificationOrActivity`

* refactor: add `invited` to `determineUnreadState` and use it in `NotificationState` & `RoomNotificationState`

* test: update `RoomNotificationState` test to use `invited`

* test: update other tests to add `invited`

* refactor: remove count check in `isNotification`

src/RoomNotifs.ts

@@ -238,25 +238,25 @@ export function determineUnreadState(
     room?: Room,
     threadId?: string,
     includeThreads?: boolean,
-): { level: NotificationLevel; symbol: string | null; count: number } {
+): { level: NotificationLevel; symbol: string | null; count: number; invited: boolean } {
     if (!room) {
-        return { symbol: null, count: 0, level: NotificationLevel.None };
+        return { symbol: null, count: 0, level: NotificationLevel.None, invited: false };
     }
 
     if (getUnsentMessages(room, threadId).length > 0) {
-        return { symbol: "!", count: 1, level: NotificationLevel.Unsent };
+        return { symbol: "!", count: 1, level: NotificationLevel.Unsent, invited: false };
     }
 
     if (getEffectiveMembership(room.getMyMembership()) === EffectiveMembership.Invite) {
-        return { symbol: "!", count: 1, level: NotificationLevel.Highlight };
+        return { symbol: "!", count: 1, level: NotificationLevel.Highlight, invited: true };
     }
 
     if (SettingsStore.getValue("feature_ask_to_join") && isKnockDenied(room)) {
-        return { symbol: "!", count: 1, level: NotificationLevel.Highlight };
+        return { symbol: "!", count: 1, level: NotificationLevel.Highlight, invited: false };
     }
 
     if (getRoomNotifsState(room.client, room.roomId) === RoomNotifState.Mute) {
-        return { symbol: null, count: 0, level: NotificationLevel.None };
+        return { symbol: null, count: 0, level: NotificationLevel.None, invited: false };
     }
 
     const redNotifs = getUnreadNotificationCount(
@@ -269,12 +269,12 @@ export function determineUnreadState(
 
     const trueCount = greyNotifs || redNotifs;
     if (redNotifs > 0) {
-        return { symbol: null, count: trueCount, level: NotificationLevel.Highlight };
+        return { symbol: null, count: trueCount, level: NotificationLevel.Highlight, invited: false };
     }
 
     const markedUnreadState = getMarkedUnreadState(room);
     if (greyNotifs > 0 || markedUnreadState) {
-        return { symbol: null, count: trueCount, level: NotificationLevel.Notification };
+        return { symbol: null, count: trueCount, level: NotificationLevel.Notification, invited: false };
     }
 
     // We don't have any notified messages, but we might have unread messages. Let's find out.
@@ -293,5 +293,6 @@ export function determineUnreadState(
         symbol: null,
         count: trueCount,
         level: hasUnread ? NotificationLevel.Activity : NotificationLevel.None,
+        invited: false,
     };
 }

src/stores/notifications/NotificationState.ts

@@ -18,6 +18,7 @@ export interface INotificationStateSnapshotParams {
     level: NotificationLevel;
     muted: boolean;
     knocked: boolean;
+    invited: boolean;
 }
 
 export enum NotificationStateEvents {
@@ -38,6 +39,7 @@ export abstract class NotificationState
     protected _level: NotificationLevel = NotificationLevel.None;
     protected _muted = false;
     protected _knocked = false;
+    protected _invited = false;
 
     private watcherReferences: string[] = [];
 
@@ -70,10 +72,22 @@ export abstract class NotificationState
         return this._knocked;
     }
 
+    /**
+     * True if the notification is an invitation notification.
+     * Invite notifications are a special case of highlight notifications
+     */
+    public get invited(): boolean {
+        return this._invited;
+    }
+
     public get isIdle(): boolean {
         return this.level <= NotificationLevel.None;
     }
 
+    /**
+     * True if the notification is higher than an activity notification or if the feature_hidebold is disabled with an activity notification.
+     * The "unread" term used here is different from the "Unread" in the UI. Unread in the UI doesn't include activity notifications even with feature_hidebold disabled.
+     */
     public get isUnread(): boolean {
         if (this.level > NotificationLevel.Activity) {
             return true;
@@ -83,10 +97,19 @@ export abstract class NotificationState
         }
     }
 
+    /**
+     * True if the notification has a count or a symbol and is equal or greater than an NotificationLevel.Notification.
+     */
     public get hasUnreadCount(): boolean {
         return this.level >= NotificationLevel.Notification && (!!this.count || !!this.symbol);
     }
 
+    /**
+     * True if the notification is a mention, an invitation, a knock or a unset message.
+     *
+     * @deprecated because the name is confusing. A mention is not an invitation, a knock or an unsent message.
+     * In case of a {@link RoomNotificationState}, use {@link RoomNotificationState.isMention} instead.
+     */
     public get hasMentions(): boolean {
         return this.level >= NotificationLevel.Highlight;
     }
@@ -116,6 +139,7 @@ export class NotificationStateSnapshot {
     private readonly level: NotificationLevel;
     private readonly muted: boolean;
     private readonly knocked: boolean;
+    private readonly isInvitation: boolean;
 
     public constructor(state: INotificationStateSnapshotParams) {
         this.symbol = state.symbol;
@@ -123,6 +147,7 @@ export class NotificationStateSnapshot {
         this.level = state.level;
         this.muted = state.muted;
         this.knocked = state.knocked;
+        this.isInvitation = state.invited;
     }
 
     public isDifferentFrom(other: INotificationStateSnapshotParams): boolean {
@@ -132,6 +157,7 @@ export class NotificationStateSnapshot {
             level: this.level,
             muted: this.muted,
             knocked: this.knocked,
+            is: this.isInvitation,
         };
         const after = {
             count: other.count,

src/stores/notifications/RoomNotificationState.ts

@@ -17,6 +17,7 @@ import * as RoomNotifs from "../../RoomNotifs";
 import { NotificationState } from "./NotificationState";
 import SettingsStore from "../../settings/SettingsStore";
 import { MARKED_UNREAD_TYPE_STABLE, MARKED_UNREAD_TYPE_UNSTABLE } from "../../utils/notifications";
+import { NotificationLevel } from "./NotificationLevel";
 
 export class RoomNotificationState extends NotificationState implements IDestroyable {
     public constructor(
@@ -51,6 +52,52 @@ export class RoomNotificationState extends NotificationState implements IDestroy
         cli.removeListener(ClientEvent.AccountData, this.handleAccountDataUpdate);
     }
 
+    /**
+     * True if the notification is a mention.
+     */
+    public get isMention(): boolean {
+        if (this.invited || this.knocked) return false;
+
+        return this.level === NotificationLevel.Highlight;
+    }
+
+    /**
+     * True if the notification is an unset message.
+     */
+    public get isUnsetMessage(): boolean {
+        return this.level === NotificationLevel.Unsent;
+    }
+
+    /**
+     * Activity notifications are the lowest level of notification (except none and muted)
+     */
+    public get isActivityNotification(): boolean {
+        return this.level === NotificationLevel.Activity;
+    }
+
+    /**
+     * This is the case for notifications with a level:
+     * - is a knock
+     * - greater Activity
+     * - equal Activity and feature_hidebold is disabled.
+     */
+    public get hasAnyNotificationOrActivity(): boolean {
+        if (this.knocked) return true;
+
+        // If the feature_hidebold is enabled, we don't want to show activity notifications
+        const hideBold = SettingsStore.getValue("feature_hidebold");
+        if (!hideBold && this.level === NotificationLevel.Activity) return true;
+
+        return this.level >= NotificationLevel.Notification;
+    }
+
+    /**
+     * True if the notification is a NotificationLevel.Notification.
+     */
+    public get isNotification(): boolean {
+        return this.level === NotificationLevel.Notification;
+    }
+
     private handleLocalEchoUpdated = (): void => {
         this.updateNotificationState();
     };
@@ -95,7 +142,11 @@ export class RoomNotificationState extends NotificationState implements IDestroy
     private updateNotificationState(): void {
         const snapshot = this.snapshot();
 
-        const { level, symbol, count } = RoomNotifs.determineUnreadState(this.room, undefined, this.includeThreads);
+        const { level, symbol, count, invited } = RoomNotifs.determineUnreadState(
+            this.room,
+            undefined,
+            this.includeThreads,
+        );
         const muted =
             RoomNotifs.getRoomNotifsState(this.room.client, this.room.roomId) === RoomNotifs.RoomNotifState.Mute;
         const knocked =
@@ -105,6 +156,7 @@ export class RoomNotificationState extends NotificationState implements IDestroy
         this._count = count;
         this._muted = muted;
         this._knocked = knocked;
+        this._invited = invited;
 
         // finally, publish an update if needed
         this.emitIfUpdated(snapshot);

test/unit-tests/stores/notifications/RoomNotificationState-test.ts

@@ -24,6 +24,9 @@ import { RoomNotificationState } from "../../../../src/stores/notifications/Room
 import { NotificationStateEvents } from "../../../../src/stores/notifications/NotificationState";
 import { NotificationLevel } from "../../../../src/stores/notifications/NotificationLevel";
 import { createMessageEventContent } from "../../../test-utils/events";
+import SettingsStore from "../../../../src/settings/SettingsStore";
+import * as RoomStatusBarModule from "../../../../src/components/structures/RoomStatusBar";
+import * as UnreadModule from "../../../../src/Unread";
 
 describe("RoomNotificationState", () => {
     let room: Room;
@@ -36,6 +39,10 @@ describe("RoomNotificationState", () => {
         });
     });
 
+    afterEach(() => {
+        jest.resetAllMocks();
+    });
+
     function addThread(room: Room): void {
         const threadId = "thread_id";
         jest.spyOn(room, "eventShouldLiveIn").mockReturnValue({
@@ -200,4 +207,85 @@ describe("RoomNotificationState", () => {
         expect(roomNotifState.level).toBe(NotificationLevel.Activity);
         expect(roomNotifState.symbol).toBe(null);
     });
+
+    describe("computed attributes", () => {
+        beforeEach(() => {
+            jest.spyOn(RoomStatusBarModule, "getUnsentMessages").mockReturnValue([]);
+            jest.spyOn(UnreadModule, "doesRoomHaveUnreadMessages").mockReturnValue(false);
+        });
+
+        it("should has invited at true", () => {
+            room.updateMyMembership(KnownMembership.Invite);
+            const roomNotifState = new RoomNotificationState(room, false);
+            expect(roomNotifState.invited).toBe(true);
+        });
+
+        it("should has isUnsetMessage at true", () => {
+            jest.spyOn(RoomStatusBarModule, "getUnsentMessages").mockReturnValue([{} as MatrixEvent]);
+            const roomNotifState = new RoomNotificationState(room, false);
+            expect(roomNotifState.isUnsetMessage).toBe(true);
+        });
+
+        it("should has isMention at false if the notification is invitation, an unset message or a knock", () => {
+            setUnreads(room, 0, 2);
+
+            const roomNotifState = new RoomNotificationState(room, false);
+            expect(roomNotifState.isMention).toBe(true);
+
+            room.updateMyMembership(KnownMembership.Invite);
+            expect(roomNotifState.isMention).toBe(false);
+
+            jest.spyOn(SettingsStore, "getValue").mockReturnValue(true);
+            room.updateMyMembership(KnownMembership.Knock);
+            expect(roomNotifState.isMention).toBe(false);
+
+            jest.spyOn(RoomStatusBarModule, "getUnsentMessages").mockReturnValue([{} as MatrixEvent]);
+            room.updateMyMembership(KnownMembership.Join);
+            expect(roomNotifState.isMention).toBe(false);
+        });
+
+        it("should has isNotification at true", () => {
+            setUnreads(room, 1, 0);
+
+            const roomNotifState = new RoomNotificationState(room, false);
+            expect(roomNotifState.isNotification).toBe(true);
+        });
+
+        it("should has isActivityNotification at true", () => {
+            jest.spyOn(UnreadModule, "doesRoomHaveUnreadMessages").mockReturnValue(true);
+
+            const roomNotifState = new RoomNotificationState(room, false);
+            expect(roomNotifState.isActivityNotification).toBe(true);
+        });
+
+        it("should has hasAnyNotificationOrActivity at true", () => {
+            // Hidebold is disabled
+            jest.spyOn(SettingsStore, "getValue").mockReturnValue(false);
+            // Unread message, generate activity notification
+            jest.spyOn(UnreadModule, "doesRoomHaveUnreadMessages").mockReturnValue(true);
+            // Highlight notification
+            setUnreads(room, 0, 1);
+
+            // There is one highlight notification
+            const roomNotifState = new RoomNotificationState(room, false);
+            expect(roomNotifState.hasAnyNotificationOrActivity).toBe(true);
+
+            // Activity notification
+            setUnreads(room, 0, 0);
+            // Trigger update
+            room.updateMyMembership(KnownMembership.Join);
+            // hidebold is disabled and we have an activity notification
+            expect(roomNotifState.hasAnyNotificationOrActivity).toBe(true);
+
+            // hidebold is enabled and we have an activity notification
+            jest.spyOn(SettingsStore, "getValue").mockReturnValue(true);
+            room.updateMyMembership(KnownMembership.Join);
+            expect(roomNotifState.hasAnyNotificationOrActivity).toBe(false);
+
+            // No unread
+            jest.spyOn(UnreadModule, "doesRoomHaveUnreadMessages").mockReturnValue(false);
+            room.updateMyMembership(KnownMembership.Join);
+            expect(roomNotifState.hasAnyNotificationOrActivity).toBe(false);
+        });
+    });
 });

test/unit-tests/stores/room-list/algorithms/list-ordering/ImportanceAlgorithm-test.ts

@@ -67,9 +67,9 @@ describe("ImportanceAlgorithm", () => {
     };
 
     const unreadStates: Record<string, ReturnType<(typeof RoomNotifs)["determineUnreadState"]>> = {
-        red: { symbol: null, count: 1, level: NotificationLevel.Highlight },
+        red: { symbol: null, count: 1, level: NotificationLevel.Highlight, invited: false },
-        grey: { symbol: null, count: 1, level: NotificationLevel.Notification },
+        grey: { symbol: null, count: 1, level: NotificationLevel.Notification, invited: false },
-        none: { symbol: null, count: 0, level: NotificationLevel.None },
+        none: { symbol: null, count: 0, level: NotificationLevel.None, invited: false },
     };
 
     beforeEach(() => {
@@ -77,6 +77,7 @@ describe("ImportanceAlgorithm", () => {
             symbol: null,
             count: 0,
             level: NotificationLevel.None,
+            invited: false,
         });
     });
 
@@ -183,6 +184,7 @@ describe("ImportanceAlgorithm", () => {
                 symbol: null,
                 count: 0,
                 level: NotificationLevel.None,
+                invited: false,
             });
             const algorithm = setupAlgorithm(sortAlgorithm);
 
@@ -353,6 +355,7 @@ describe("ImportanceAlgorithm", () => {
                 symbol: null,
                 count: 0,
                 level: NotificationLevel.None,
+                invited: false,
             });
             const algorithm = setupAlgorithm(sortAlgorithm);
 

test/unit-tests/stores/room-list/algorithms/list-ordering/NaturalAlgorithm-test.ts

@@ -190,6 +190,7 @@ describe("NaturalAlgorithm", () => {
                 symbol: null,
                 count: 0,
                 level: NotificationLevel.None,
+                invited: false,
             });
         });