RoomListStore: Support specific sorting requirements for muted rooms (#29665)

* Sort muted rooms to the bottom of the room list

* Re-insert room on mute/unmute

* Write tests

* Fix broken playwright test

Muted rooms are at the bottom, so we need to scroll.

playwright/e2e/left-panel/room-list-panel/room-list.spec.ts

@@ -108,6 +108,11 @@ test.describe("Room list", () => {
             // Remove hover on the room list item
             await roomListView.hover();
 
+            // Scroll to the bottom of the list
+            await page.getByRole("grid", { name: "Room list" }).evaluate((e) => {
+                e.scrollTop = e.scrollHeight;
+            });
+
             // The room decoration should have the muted icon
             await expect(roomItem.getByTestId("notification-decoration")).toBeVisible();
 

src/stores/room-list-v3/RoomListStoreV3.ts

@@ -34,6 +34,7 @@ import { LowPriorityFilter } from "./skip-list/filters/LowPriorityFilter";
 import { type Sorter, SortingAlgorithm } from "./skip-list/sorters";
 import { SettingLevel } from "../../settings/SettingLevel";
 import { MARKED_UNREAD_TYPE_STABLE, MARKED_UNREAD_TYPE_UNSTABLE } from "../../utils/notifications";
+import { getChangedOverrideRoomMutePushRules } from "../room-list/utils/roomMute";
 
 /**
  * These are the filters passed to the room skip list.
@@ -179,22 +180,7 @@ export class RoomListStoreV3Class extends AsyncStoreWithClient<EmptyObject> {
             }
 
             case "MatrixActions.accountData": {
-                if (payload.event_type !== EventType.Direct) return;
+                this.handleAccountDataPayload(payload);
-                const dmMap = payload.event.getContent();
-                let needsEmit = false;
-                for (const userId of Object.keys(dmMap)) {
-                    const roomIds = dmMap[userId];
-                    for (const roomId of roomIds) {
-                        const room = this.matrixClient.getRoom(roomId);
-                        if (!room) {
-                            logger.warn(`${roomId} was found in DMs but the room is not in the store`);
-                            continue;
-                        }
-                        this.roomSkipList.addRoom(room);
-                        needsEmit = true;
-                    }
-                }
-                if (needsEmit) this.emit(LISTS_UPDATE_EVENT);
                 break;
             }
 
@@ -230,6 +216,47 @@ export class RoomListStoreV3Class extends AsyncStoreWithClient<EmptyObject> {
         }
     }
 
+    /**
+     * This method deals with the two types of account data payloads that we care about.
+     */
+    private handleAccountDataPayload(payload: ActionPayload): void {
+        const eventType = payload.event_type;
+        let needsEmit = false;
+        switch (eventType) {
+            // When we're told about new DMs, insert the associated dm rooms.
+            case EventType.Direct: {
+                const dmMap = payload.event.getContent();
+                for (const userId of Object.keys(dmMap)) {
+                    const roomIds = dmMap[userId];
+                    for (const roomId of roomIds) {
+                        const room = this.matrixClient!.getRoom(roomId);
+                        if (!room) {
+                            logger.warn(`${roomId} was found in DMs but the room is not in the store`);
+                            continue;
+                        }
+                        this.roomSkipList!.addRoom(room);
+                        needsEmit = true;
+                    }
+                }
+                break;
+            }
+            case EventType.PushRules: {
+                // When a room becomes muted/unmuted, re-insert that room.
+                const possibleMuteChangeRoomIds = getChangedOverrideRoomMutePushRules(payload);
+                if (!possibleMuteChangeRoomIds) return;
+                const rooms = possibleMuteChangeRoomIds
+                    .map((id) => this.matrixClient?.getRoom(id))
+                    .filter((room) => !!room);
+                for (const room of rooms) {
+                    this.roomSkipList!.addRoom(room);
+                    needsEmit = true;
+                }
+                break;
+            }
+        }
+        if (needsEmit) this.emit(LISTS_UPDATE_EVENT);
+    }
+
     /**
      * Create the correct sorter depending on the persisted user preference.
      * @param myUserId The user-id of our user.

src/stores/room-list-v3/skip-list/sorters/RecencySorter.ts

@@ -8,6 +8,7 @@ Please see LICENSE files in the repository root for full details.
 import type { Room } from "matrix-js-sdk/src/matrix";
 import { type Sorter, SortingAlgorithm } from ".";
 import { getLastTs } from "../../../room-list/algorithms/tag-sorting/RecentAlgorithm";
+import { RoomNotificationStateStore } from "../../../notifications/RoomNotificationStateStore";
 
 export class RecencySorter implements Sorter {
     public constructor(private myUserId: string) {}
@@ -18,6 +19,13 @@ export class RecencySorter implements Sorter {
     }
 
     public comparator(roomA: Room, roomB: Room, cache?: any): number {
+        // Check mute status first; muted rooms should be at the bottom
+        const isRoomAMuted = RoomNotificationStateStore.instance.getRoomState(roomA).muted;
+        const isRoomBMuted = RoomNotificationStateStore.instance.getRoomState(roomB).muted;
+        if (isRoomAMuted && !isRoomBMuted) return 1;
+        if (isRoomBMuted && !isRoomAMuted) return -1;
+
+        // Then check recency; recent rooms should be at the top
         const roomALastTs = this.getTs(roomA, cache);
         const roomBLastTs = this.getTs(roomB, cache);
         return roomBLastTs - roomALastTs;

test/unit-tests/stores/room-list-v3/RoomListStoreV3-test.ts

@@ -27,6 +27,7 @@ import DMRoomMap from "../../../../src/utils/DMRoomMap";
 import { SortingAlgorithm } from "../../../../src/stores/room-list-v3/skip-list/sorters";
 import SettingsStore from "../../../../src/settings/SettingsStore";
 import * as utils from "../../../../src/utils/notifications";
+import * as roomMute from "../../../../src/stores/room-list/utils/roomMute";
 
 describe("RoomListStoreV3", () => {
     async function getRoomListStore() {
@@ -635,4 +636,83 @@ describe("RoomListStoreV3", () => {
             });
         });
     });
+
+    describe("Muted rooms", () => {
+        async function getRoomListStoreWithMutedRooms() {
+            const client = stubClient();
+            const rooms = getMockedRooms(client);
+
+            // Let's say that rooms 34, 84, 64, 14, 57 are muted
+            const mutedIndices = [34, 84, 64, 14, 57];
+            const mutedRooms = mutedIndices.map((i) => rooms[i]);
+            jest.spyOn(RoomNotificationStateStore.instance, "getRoomState").mockImplementation((room) => {
+                const state = {
+                    muted: mutedRooms.includes(room),
+                } as unknown as RoomNotificationState;
+                return state;
+            });
+
+            client.getVisibleRooms = jest.fn().mockReturnValue(rooms);
+            jest.spyOn(AsyncStoreWithClient.prototype, "matrixClient", "get").mockReturnValue(client);
+            const store = new RoomListStoreV3Class(dispatcher);
+            await store.start();
+            return { client, rooms, mutedIndices, mutedRooms, store, dispatcher };
+        }
+
+        it("Muted rooms are sorted to the bottom of the list", async () => {
+            const { store, mutedRooms, client } = await getRoomListStoreWithMutedRooms();
+            const lastFiveRooms = store.getSortedRooms().slice(95);
+            const expectedRooms = new RecencySorter(client.getSafeUserId()).sort(mutedRooms);
+            // We expect the muted rooms to be at the bottom sorted by recency
+            expect(lastFiveRooms).toEqual(expectedRooms);
+        });
+
+        it("Muted rooms are sorted within themselves", async () => {
+            const { store, rooms } = await getRoomListStoreWithMutedRooms();
+
+            // Let's say that rooms 14 and 34 get new messages in that order
+            let ts = 1000;
+            for (const room of [rooms[14], rooms[34]]) {
+                const event = mkMessage({ room: room.roomId, user: `@foo${3}:matrix.org`, ts: 1000, event: true });
+                room.timeline.push(event);
+
+                const payload = {
+                    action: "MatrixActions.Room.timeline",
+                    event,
+                    isLiveEvent: true,
+                    isLiveUnfilteredRoomTimelineEvent: true,
+                    room,
+                };
+                dispatcher.dispatch(payload, true);
+                ts = ts + 1;
+            }
+
+            const lastFiveRooms = store.getSortedRooms().slice(95);
+            // The order previously would  have been 84, 64, 57, 34, 14
+            // Expected new order is 34, 14, 84, 64, 57
+            const expectedRooms = [rooms[34], rooms[14], rooms[84], rooms[64], rooms[57]];
+            expect(lastFiveRooms).toEqual(expectedRooms);
+        });
+
+        it("Muted room is correctly sorted when unmuted", async () => {
+            const { store, mutedRooms, rooms, client } = await getRoomListStoreWithMutedRooms();
+
+            // Let's say that muted room 64 becomes un-muted.
+            const unmutedRoom = rooms[64];
+            jest.spyOn(roomMute, "getChangedOverrideRoomMutePushRules").mockImplementation(() => [unmutedRoom.roomId]);
+            client.getRoom = jest.fn().mockReturnValue(unmutedRoom);
+            const payload = {
+                action: "MatrixActions.accountData",
+                event_type: EventType.PushRules,
+            };
+            mutedRooms.splice(2, 1);
+            dispatcher.dispatch(payload, true);
+
+            const lastFiveRooms = store.getSortedRooms().slice(95);
+            // We expect room at index 64 to no longer be at the bottom
+            expect(lastFiveRooms).not.toContain(unmutedRoom);
+            // Room 64 should go to index 34 since we're sorting by recency
+            expect(store.getSortedRooms()[34]).toEqual(unmutedRoom);
+        });
+    });
 });

test/unit-tests/stores/room-list-v3/skip-list/RoomSkipList-test.ts

@@ -9,6 +9,7 @@ import { shuffle } from "lodash";
 
 import type { Room } from "matrix-js-sdk/src/matrix";
 import type { Sorter } from "../../../../../src/stores/room-list-v3/skip-list/sorters";
+import type { RoomNotificationState } from "../../../../../src/stores/notifications/RoomNotificationState";
 import { mkMessage, stubClient } from "../../../../test-utils";
 import { RoomSkipList } from "../../../../../src/stores/room-list-v3/skip-list/RoomSkipList";
 import { RecencySorter } from "../../../../../src/stores/room-list-v3/skip-list/sorters/RecencySorter";
@@ -16,6 +17,7 @@ import { AlphabeticSorter } from "../../../../../src/stores/room-list-v3/skip-li
 import { getMockedRooms } from "./getMockedRooms";
 import SpaceStore from "../../../../../src/stores/spaces/SpaceStore";
 import { MetaSpace } from "../../../../../src/stores/spaces";
+import { RoomNotificationStateStore } from "../../../../../src/stores/notifications/RoomNotificationStateStore";
 
 describe("RoomSkipList", () => {
     function generateSkipList(roomCount?: number): {
@@ -36,6 +38,12 @@ describe("RoomSkipList", () => {
         jest.spyOn(SpaceStore.instance, "isRoomInSpace").mockImplementation((space) => space === MetaSpace.Home);
         jest.spyOn(SpaceStore.instance, "activeSpace", "get").mockImplementation(() => MetaSpace.Home);
         jest.spyOn(SpaceStore.instance, "storeReadyPromise", "get").mockImplementation(() => Promise.resolve());
+        jest.spyOn(RoomNotificationStateStore.instance, "getRoomState").mockImplementation(() => {
+            const state = {
+                mute: false,
+            } as unknown as RoomNotificationState;
+            return state;
+        });
     });
 
     it("Rooms are in sorted order after initial seed", () => {