From b8d502be2e0790c5587f59362765ae11510da7ee Mon Sep 17 00:00:00 2001 From: Andy Balaam Date: Wed, 8 Mar 2023 14:18:03 +0000 Subject: [PATCH] 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 --- .../RoomNotificationStateStore.ts | 42 ++++-- test/components/structures/RoomView-test.tsx | 18 ++- .../views/dialogs/SpotlightDialog-test.tsx | 10 +- .../stores/RoomNotificationStateStore-test.ts | 131 ++++++++++++++++++ 4 files changed, 186 insertions(+), 15 deletions(-) create mode 100644 test/stores/RoomNotificationStateStore-test.ts diff --git a/src/stores/notifications/RoomNotificationStateStore.ts b/src/stores/notifications/RoomNotificationStateStore.ts index b7e7a3863e..b0cafc0064 100644 --- a/src/stores/notifications/RoomNotificationStateStore.ts +++ b/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 { private listMap = new Map(); 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 { 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 { 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); } }; diff --git a/test/components/structures/RoomView-test.tsx b/test/components/structures/RoomView-test.tsx index f7425072ef..bad6f27c66 100644 --- a/test/components/structures/RoomView-test.tsx +++ b/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); + }); }); }); diff --git a/test/components/views/dialogs/SpotlightDialog-test.tsx b/test/components/views/dialogs/SpotlightDialog-test.tsx index f51b0647d9..f6fa68bde0 100644 --- a/test/components/views/dialogs/SpotlightDialog-test.tsx +++ b/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 () => { diff --git a/test/stores/RoomNotificationStateStore-test.ts b/test/stores/RoomNotificationStateStore-test.ts new file mode 100644 index 0000000000..3a63ee7401 --- /dev/null +++ b/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; + } +});