From a6e8d512d002c5aab0ce104174d3d0bef42ecd64 Mon Sep 17 00:00:00 2001 From: Florian Duros Date: Mon, 24 Mar 2025 17:00:47 +0100 Subject: [PATCH] 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 | 17 ++-- src/stores/notifications/NotificationState.ts | 26 ++++++ .../notifications/RoomNotificationState.ts | 54 +++++++++++- .../RoomNotificationState-test.ts | 88 +++++++++++++++++++ .../list-ordering/ImportanceAlgorithm-test.ts | 9 +- .../list-ordering/NaturalAlgorithm-test.ts | 1 + 6 files changed, 183 insertions(+), 12 deletions(-) diff --git a/src/RoomNotifs.ts b/src/RoomNotifs.ts index e6064d2691..d5254d523d 100644 --- a/src/RoomNotifs.ts +++ b/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, }; } diff --git a/src/stores/notifications/NotificationState.ts b/src/stores/notifications/NotificationState.ts index a453d7ca42..b3f30229a2 100644 --- a/src/stores/notifications/NotificationState.ts +++ b/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, diff --git a/src/stores/notifications/RoomNotificationState.ts b/src/stores/notifications/RoomNotificationState.ts index c4f677130d..3447257e96 100644 --- a/src/stores/notifications/RoomNotificationState.ts +++ b/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); diff --git a/test/unit-tests/stores/notifications/RoomNotificationState-test.ts b/test/unit-tests/stores/notifications/RoomNotificationState-test.ts index 88847b42e8..f9ed754c17 100644 --- a/test/unit-tests/stores/notifications/RoomNotificationState-test.ts +++ b/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); + }); + }); }); diff --git a/test/unit-tests/stores/room-list/algorithms/list-ordering/ImportanceAlgorithm-test.ts b/test/unit-tests/stores/room-list/algorithms/list-ordering/ImportanceAlgorithm-test.ts index 5573157b14..aed501eef9 100644 --- a/test/unit-tests/stores/room-list/algorithms/list-ordering/ImportanceAlgorithm-test.ts +++ b/test/unit-tests/stores/room-list/algorithms/list-ordering/ImportanceAlgorithm-test.ts @@ -67,9 +67,9 @@ describe("ImportanceAlgorithm", () => { }; const unreadStates: Record> = { - red: { symbol: null, count: 1, level: NotificationLevel.Highlight }, - grey: { symbol: null, count: 1, level: NotificationLevel.Notification }, - none: { symbol: null, count: 0, level: NotificationLevel.None }, + red: { symbol: null, count: 1, level: NotificationLevel.Highlight, invited: false }, + grey: { symbol: null, count: 1, level: NotificationLevel.Notification, invited: false }, + 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); diff --git a/test/unit-tests/stores/room-list/algorithms/list-ordering/NaturalAlgorithm-test.ts b/test/unit-tests/stores/room-list/algorithms/list-ordering/NaturalAlgorithm-test.ts index cf0fd59cd4..6fdf71b02d 100644 --- a/test/unit-tests/stores/room-list/algorithms/list-ordering/NaturalAlgorithm-test.ts +++ b/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, }); });