Support dynamic room predecessors in RoomNotificationStateStore (#10297)

* Tests for RoomNotificationStateStore emitting events

* Support dynamic room predecessors in RoomNotificationStateStore

* Remove unused arguments from emit call.

UPDATE_STATUS_INDICATOR is used in:
* SpacePanel
* MatrixChat
* RoomHeaderButtons

but these arguments are not used in any of those places. Remove them so
when I refactor I don't have to make up values for them.

* Fix broken test (wrong expected args to emit)

UPDATE_STATUS_INDICATOR is used in:
* SpacePanel
* MatrixChat
* RoomHeaderButtons

but these arguments are not used in any of those places. Remove them so
when I refactor I don't have to make up values for them.

* Update the RoomNotificationStore whenever the predecessor labs flag changes

* Fix type errors

* Fix other tests that trigger our new watcher

src/stores/notifications/RoomNotificationStateStore.ts

@@ -15,18 +15,19 @@ limitations under the License.
 */
 
 import { Room } from "matrix-js-sdk/src/models/room";
-import { ISyncStateData, SyncState } from "matrix-js-sdk/src/sync";
+import { SyncState } from "matrix-js-sdk/src/sync";
 import { ClientEvent } from "matrix-js-sdk/src/client";
 
 import { ActionPayload } from "../../dispatcher/payloads";
 import { AsyncStoreWithClient } from "../AsyncStoreWithClient";
-import defaultDispatcher from "../../dispatcher/dispatcher";
+import defaultDispatcher, { MatrixDispatcher } from "../../dispatcher/dispatcher";
 import { DefaultTagID, TagID } from "../room-list/models";
 import { FetchRoomFn, ListNotificationState } from "./ListNotificationState";
 import { RoomNotificationState } from "./RoomNotificationState";
 import { SummarizedNotificationState } from "./SummarizedNotificationState";
 import { VisibilityProvider } from "../room-list/filters/VisibilityProvider";
 import { PosthogAnalytics } from "../../PosthogAnalytics";
+import SettingsStore from "../../settings/SettingsStore";
 
 interface IState {}
 
@@ -43,8 +44,22 @@ export class RoomNotificationStateStore extends AsyncStoreWithClient<IState> {
     private listMap = new Map<TagID, ListNotificationState>();
     private _globalState = new SummarizedNotificationState();
 
-    private constructor() {
-        super(defaultDispatcher, {});
+    private constructor(dispatcher = defaultDispatcher) {
+        super(dispatcher, {});
+        SettingsStore.watchSetting("feature_dynamic_room_predecessors", null, () => {
+            // We pass SyncState.Syncing here to "simulate" a sync happening.
+            // The code that receives these events actually doesn't care
+            // what state we pass, except that it behaves differently if we
+            // pass SyncState.Error.
+            this.emitUpdateIfStateChanged(SyncState.Syncing, false);
+        });
+    }
+
+    /**
+     * @internal Public for test only
+     */
+    public static testInstance(dispatcher: MatrixDispatcher): RoomNotificationStateStore {
+        return new RoomNotificationStateStore();
     }
 
     /**
@@ -93,11 +108,22 @@ export class RoomNotificationStateStore extends AsyncStoreWithClient<IState> {
         return RoomNotificationStateStore.internalInstance;
     }
 
-    private onSync = (state: SyncState, prevState: SyncState | null, data?: ISyncStateData): void => {
+    private onSync = (state: SyncState, prevState: SyncState | null): void => {
+        this.emitUpdateIfStateChanged(state, state !== prevState);
+    };
+
+    /**
+     * If the SummarizedNotificationState of this room has changed, or forceEmit
+     * is true, emit an UPDATE_STATUS_INDICATOR event.
+     *
+     * @internal public for test
+     */
+    public emitUpdateIfStateChanged = (state: SyncState, forceEmit: boolean): void => {
         // Only count visible rooms to not torment the user with notification counts in rooms they can't see.
         // This will include highlights from the previous version of the room internally
+        const msc3946ProcessDynamicPredecessor = SettingsStore.getValue("feature_dynamic_room_predecessors");
         const globalState = new SummarizedNotificationState();
-        const visibleRooms = this.matrixClient.getVisibleRooms();
+        const visibleRooms = this.matrixClient.getVisibleRooms(msc3946ProcessDynamicPredecessor);
 
         let numFavourites = 0;
         for (const room of visibleRooms) {
@@ -115,10 +141,10 @@ export class RoomNotificationStateStore extends AsyncStoreWithClient<IState> {
             this.globalState.count !== globalState.count ||
             this.globalState.color !== globalState.color ||
             this.globalState.numUnreadStates !== globalState.numUnreadStates ||
-            state !== prevState
+            forceEmit
         ) {
             this._globalState = globalState;
-            this.emit(UPDATE_STATUS_INDICATOR, globalState, state, prevState, data);
+            this.emit(UPDATE_STATUS_INDICATOR, globalState, state);
         }
     };
 

test/components/structures/RoomView-test.tsx

@@ -203,11 +203,19 @@ describe("RoomView", () => {
             expect(instance.getHiddenHighlightCount()).toBe(23);
         });
 
-        it("and feature_dynamic_room_predecessors is enabled it should pass the setting to findPredecessor", async () => {
-            SettingsStore.setValue("feature_dynamic_room_predecessors", null, SettingLevel.DEVICE, true);
-            expect(instance.getHiddenHighlightCount()).toBe(0);
-            expect(room.findPredecessor).toHaveBeenCalledWith(true);
-            SettingsStore.setValue("feature_dynamic_room_predecessors", null, SettingLevel.DEVICE, null);
+        describe("and feature_dynamic_room_predecessors is enabled", () => {
+            beforeEach(() => {
+                instance.setState({ msc3946ProcessDynamicPredecessor: true });
+            });
+
+            afterEach(() => {
+                instance.setState({ msc3946ProcessDynamicPredecessor: false });
+            });
+
+            it("should pass the setting to findPredecessor", async () => {
+                expect(instance.getHiddenHighlightCount()).toBe(0);
+                expect(room.findPredecessor).toHaveBeenCalledWith(true);
+            });
         });
     });
 

test/components/views/dialogs/SpotlightDialog-test.tsx

@@ -198,11 +198,17 @@ describe("Spotlight Dialog", () => {
 
     describe("when MSC3946 dynamic room predecessors is enabled", () => {
         beforeEach(() => {
-            SettingsStore.setValue("feature_dynamic_room_predecessors", null, SettingLevel.DEVICE, true);
+            jest.spyOn(SettingsStore, "getValue").mockImplementation((settingName, roomId, excludeDefault) => {
+                if (settingName === "feature_dynamic_room_predecessors") {
+                    return true;
+                } else {
+                    return []; // SpotlightSearch.recentSearches
+                }
+            });
         });
 
         afterEach(() => {
-            SettingsStore.setValue("feature_dynamic_room_predecessors", null, SettingLevel.DEVICE, null);
+            jest.restoreAllMocks();
         });
 
         it("should call getVisibleRooms with MSC3946 dynamic room predecessors", async () => {

test/stores/RoomNotificationStateStore-test.ts

@@ -0,0 +1,131 @@
+/*
+Copyright 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.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+import { mocked } from "jest-mock";
+import { ClientEvent, MatrixClient, Room } from "matrix-js-sdk/src/matrix";
+import { SyncState } from "matrix-js-sdk/src/sync";
+
+import { createTestClient, setupAsyncStoreWithClient } from "../test-utils";
+import {
+    RoomNotificationStateStore,
+    UPDATE_STATUS_INDICATOR,
+} from "../../src/stores/notifications/RoomNotificationStateStore";
+import SettingsStore from "../../src/settings/SettingsStore";
+import { MatrixDispatcher } from "../../src/dispatcher/dispatcher";
+
+describe("RoomNotificationStateStore", function () {
+    let store: RoomNotificationStateStore;
+    let client: MatrixClient;
+    let dis: MatrixDispatcher;
+
+    beforeEach(() => {
+        client = createTestClient();
+        dis = new MatrixDispatcher();
+        jest.resetAllMocks();
+        store = RoomNotificationStateStore.testInstance(dis);
+        store.emit = jest.fn();
+        setupAsyncStoreWithClient(store, client);
+    });
+
+    it("Emits no event when a room has no unreads", async () => {
+        // Given a room with 0 unread messages
+        const room = fakeRoom(0);
+
+        // When we sync and the room is visible
+        mocked(client.getVisibleRooms).mockReturnValue([room]);
+        client.emit(ClientEvent.Sync, SyncState.Syncing, SyncState.Syncing);
+
+        // Then we emit an event from the store
+        expect(store.emit).not.toHaveBeenCalled();
+    });
+
+    it("Emits an event when a room has unreads", async () => {
+        // Given a room with 2 unread messages
+        const room = fakeRoom(2);
+
+        // When we sync and the room is visible
+        mocked(client.getVisibleRooms).mockReturnValue([room]);
+        client.emit(ClientEvent.Sync, SyncState.Syncing, SyncState.Syncing);
+
+        // Then we emit an event from the store
+        expect(store.emit).toHaveBeenCalledWith(UPDATE_STATUS_INDICATOR, expect.anything(), "SYNCING");
+    });
+
+    it("Emits an event when a feature flag changes notification state", async () => {
+        // Given we have synced already
+        let room = fakeRoom(0);
+        mocked(store.emit).mockReset();
+        mocked(client.getVisibleRooms).mockReturnValue([room]);
+        client.emit(ClientEvent.Sync, SyncState.Syncing, SyncState.Syncing);
+        expect(store.emit).not.toHaveBeenCalled();
+
+        // When we update the feature flag and it makes us have a notification
+        room = fakeRoom(2);
+        mocked(client.getVisibleRooms).mockReturnValue([room]);
+        jest.spyOn(SettingsStore, "getValue").mockReturnValue(true);
+        store.emitUpdateIfStateChanged(SyncState.Syncing, false);
+
+        // Then we get notified
+        expect(store.emit).toHaveBeenCalledWith(UPDATE_STATUS_INDICATOR, expect.anything(), "SYNCING");
+    });
+
+    describe("If the feature_dynamic_room_predecessors is not enabled", () => {
+        beforeEach(() => {
+            // Turn off feature_dynamic_room_predecessors setting
+            jest.spyOn(SettingsStore, "getValue").mockReturnValue(false);
+        });
+
+        it("Passes the dynamic predecessor flag to getVisibleRooms", async () => {
+            // When we sync
+            mocked(client.getVisibleRooms).mockReturnValue([]);
+            client.emit(ClientEvent.Sync, SyncState.Syncing, SyncState.Syncing);
+
+            // Then we check visible rooms, using the dynamic predecessor flag
+            expect(client.getVisibleRooms).toHaveBeenCalledWith(false);
+            expect(client.getVisibleRooms).not.toHaveBeenCalledWith(true);
+        });
+    });
+
+    describe("If the feature_dynamic_room_predecessors is enabled", () => {
+        beforeEach(() => {
+            // Turn on feature_dynamic_room_predecessors setting
+            jest.spyOn(SettingsStore, "getValue").mockImplementation(
+                (settingName) => settingName === "feature_dynamic_room_predecessors",
+            );
+        });
+
+        it("Passes the dynamic predecessor flag to getVisibleRooms", async () => {
+            // When we sync
+            mocked(client.getVisibleRooms).mockReturnValue([]);
+            client.emit(ClientEvent.Sync, SyncState.Syncing, SyncState.Syncing);
+
+            // Then we check visible rooms, using the dynamic predecessor flag
+            expect(client.getVisibleRooms).toHaveBeenCalledWith(true);
+            expect(client.getVisibleRooms).not.toHaveBeenCalledWith(false);
+        });
+    });
+
+    let roomIdx = 0;
+
+    function fakeRoom(numUnreads: number): Room {
+        roomIdx++;
+        const ret = new Room(`room${roomIdx}`, client, "@user:example.com");
+        ret.getPendingEvents = jest.fn().mockReturnValue([]);
+        ret.isSpaceRoom = jest.fn().mockReturnValue(false);
+        ret.getUnreadNotificationCount = jest.fn().mockReturnValue(numUnreads);
+        return ret;
+    }
+});