Room list: remove unused sorting ManualAlgorithm (#32724)

* chore: remove unused `ManualAlgorithm` In the old room list ui, we can"t manually sort the rooms or the sections. Neither in a menu or by drag and drop * fix: add `undefined` to `tagRoom` signature since the code allows it * test: add tests to `RoomListActions` * chore: remuve unuseful comment * refactor: remove unused ``newIndex * doc: remove typing in tsdoc
2026-03-09 09:52:52 +00:00
parent 00dd0c48ba
commit 3e77974fa0
11 changed files with 205 additions and 167 deletions
@@ -0,0 +1,174 @@
+/*
+ * Copyright 2026 Element Creations Ltd.
+ *
+ * SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only OR LicenseRef-Element-Commercial
+ * Please see LICENSE files in the repository root for full details.
+ */
+
+import { type MatrixClient, type Room } from "matrix-js-sdk/src/matrix";
+import { mocked } from "jest-mock";
+
+import RoomListActions from "../../../src/actions/RoomListActions";
+import { DefaultTagID } from "../../../src/stores/room-list-v3/skip-list/tag";
+import Modal from "../../../src/Modal";
+import * as Rooms from "../../../src/Rooms";
+import { createTestClient, flushPromises, mkRoom } from "../../test-utils";
+
+jest.mock("../../../src/Modal");
+jest.mock("../../../src/Rooms");
+
+describe("RoomListActions", () => {
+    const ROOM_ID = "!room:example.org";
+
+    let client: MatrixClient;
+    let room: Room;
+    const dispatch = jest.fn();
+
+    beforeEach(() => {
+        client = createTestClient();
+        room = mkRoom(client, ROOM_ID);
+        mocked(Rooms.guessAndSetDMRoom).mockResolvedValue(undefined);
+    });
+
+    afterEach(() => {
+        jest.clearAllMocks();
+    });
+
+    describe("tagRoom", () => {
+        /**
+         * Invoke the async payload returned by tagRoom and wait for all promises to settle.
+         */
+        async function invokeTagRoom(
+            oldTag: Parameters<typeof RoomListActions.tagRoom>[2],
+            newTag: Parameters<typeof RoomListActions.tagRoom>[3],
+        ): Promise<void> {
+            const payload = RoomListActions.tagRoom(client, room, oldTag, newTag);
+
+            // Execute the async function embedded in the payload.
+            payload.fn(dispatch);
+
+            // Flush all microtasks / pending promises.
+            await flushPromises();
+        }
+
+        it("dispatches a pending action immediately with the optimistic update data", () => {
+            const payload = RoomListActions.tagRoom(client, room, DefaultTagID.Favourite, DefaultTagID.LowPriority);
+
+            payload.fn(dispatch);
+
+            expect(dispatch).toHaveBeenCalledWith(
+                expect.objectContaining({
+                    action: "RoomListActions.tagRoom.pending",
+                    request: { room, oldTag: DefaultTagID.Favourite, newTag: DefaultTagID.LowPriority },
+                }),
+            );
+        });
+
+        describe("DM tag handling", () => {
+            it.each([
+                [undefined, DefaultTagID.DM],
+                [DefaultTagID.DM, undefined],
+            ])(
+                "treats oldTag=%s and newTag=%s as a DM tag change and does not call setRoomTag or deleteRoomTag",
+                async (oldTag, newTag) => {
+                    await invokeTagRoom(oldTag, newTag as unknown as null);
+
+                    expect(Rooms.guessAndSetDMRoom).toHaveBeenCalledWith(room, newTag === DefaultTagID.DM);
+                    expect(client.deleteRoomTag).not.toHaveBeenCalled();
+                    expect(client.setRoomTag).not.toHaveBeenCalled();
+                },
+            );
+
+            it("opens an ErrorDialog and swallows the error if guessAndSetDMRoom rejects", async () => {
+                const error = new Error("DM tag error");
+                mocked(Rooms.guessAndSetDMRoom).mockRejectedValue(error);
+
+                await invokeTagRoom(undefined, DefaultTagID.DM);
+
+                expect(Modal.createDialog).toHaveBeenCalledWith(
+                    expect.anything(),
+                    expect.objectContaining({ description: error.message }),
+                );
+                // Error is swallowed — success is still dispatched.
+                expect(dispatch).toHaveBeenCalledWith(
+                    expect.objectContaining({ action: "RoomListActions.tagRoom.success" }),
+                );
+            });
+        });
+
+        describe("regular tag changes (non-DM)", () => {
+            it("deletes the old tag and adds the new tag when moving between two non-DM tags", async () => {
+                await invokeTagRoom(DefaultTagID.Favourite, DefaultTagID.LowPriority);
+
+                expect(client.deleteRoomTag).toHaveBeenCalledWith(ROOM_ID, DefaultTagID.Favourite);
+                expect(client.setRoomTag).toHaveBeenCalledWith(ROOM_ID, DefaultTagID.LowPriority);
+                expect(dispatch).toHaveBeenCalledWith(
+                    expect.objectContaining({ action: "RoomListActions.tagRoom.success" }),
+                );
+            });
+
+            it("only calls setRoomTag when there was no previous tag", async () => {
+                await invokeTagRoom(null, DefaultTagID.Favourite);
+
+                expect(client.deleteRoomTag).not.toHaveBeenCalled();
+                expect(client.setRoomTag).toHaveBeenCalledWith(ROOM_ID, DefaultTagID.Favourite);
+            });
+
+            it.each([null, DefaultTagID.DM])(
+                "only calls deleteRoomTag when moving from %s to another non-DM tag",
+                async (newTag) => {
+                    await invokeTagRoom(DefaultTagID.Favourite, newTag);
+
+                    expect(client.deleteRoomTag).toHaveBeenCalledWith(ROOM_ID, DefaultTagID.Favourite);
+                    expect(client.setRoomTag).not.toHaveBeenCalled();
+                },
+            );
+
+            it("makes no API calls when oldTag equals newTag", async () => {
+                await invokeTagRoom(DefaultTagID.Favourite, DefaultTagID.Favourite);
+
+                expect(client.deleteRoomTag).not.toHaveBeenCalled();
+                expect(client.setRoomTag).not.toHaveBeenCalled();
+            });
+
+            it("skips deleteRoomTag for the DM tag but still sets the new tag", async () => {
+                await invokeTagRoom(DefaultTagID.DM, DefaultTagID.Favourite);
+
+                expect(client.deleteRoomTag).not.toHaveBeenCalled();
+                expect(client.setRoomTag).toHaveBeenCalledWith(ROOM_ID, DefaultTagID.Favourite);
+            });
+
+            it("shows an ErrorDialog but still dispatches success when deleteRoomTag fails", async () => {
+                const error = new Error("delete failed");
+                jest.spyOn(client, "deleteRoomTag").mockRejectedValue(error);
+
+                await invokeTagRoom(DefaultTagID.Favourite, DefaultTagID.LowPriority);
+
+                expect(Modal.createDialog).toHaveBeenCalledWith(
+                    expect.anything(),
+                    expect.objectContaining({ description: error.message }),
+                );
+                // deleteRoomTag swallows the error, so Promise.all still resolves.
+                expect(dispatch).toHaveBeenCalledWith(
+                    expect.objectContaining({ action: "RoomListActions.tagRoom.success" }),
+                );
+            });
+
+            it("shows an ErrorDialog and dispatches failure when setRoomTag fails", async () => {
+                const error = new Error("set failed");
+                jest.spyOn(client, "setRoomTag").mockRejectedValue(error);
+
+                await invokeTagRoom(DefaultTagID.Favourite, DefaultTagID.LowPriority);
+
+                expect(Modal.createDialog).toHaveBeenCalledWith(
+                    expect.anything(),
+                    expect.objectContaining({ description: error.message }),
+                );
+                // setRoomTag rethrows, so Promise.all rejects → failure dispatched.
+                expect(dispatch).toHaveBeenCalledWith(
+                    expect.objectContaining({ action: "RoomListActions.tagRoom.failure" }),
+                );
+            });
+        });
+    });
+});
@@ -88,72 +88,6 @@ describe("ImportanceAlgorithm", () => {
        return algorithm;
    };

-    describe("When sortAlgorithm is manual", () => {
-        const sortAlgorithm = SortAlgorithm.Manual;
-        it("orders rooms by tag order without categorizing", () => {
-            jest.spyOn(RoomNotificationStateStore.instance, "getRoomState");
-            const algorithm = setupAlgorithm(sortAlgorithm);
-
-            // didn't check notif state
-            expect(RoomNotificationStateStore.instance.getRoomState).not.toHaveBeenCalled();
-            // sorted according to room tag order
-            expect(algorithm.orderedRooms).toEqual([roomC, roomA, roomB]);
-        });
-
-        describe("handleRoomUpdate", () => {
-            // XXX: This doesn't work because manual ordered rooms dont get categoryindices
-            // possibly related https://github.com/vector-im/element-web/issues/25099
-            it.skip("removes a room", () => {
-                const algorithm = setupAlgorithm(sortAlgorithm);
-
-                const shouldTriggerUpdate = algorithm.handleRoomUpdate(roomA, RoomUpdateCause.RoomRemoved);
-
-                expect(shouldTriggerUpdate).toBe(true);
-                expect(algorithm.orderedRooms).toEqual([roomC, roomB]);
-            });
-
-            // XXX: This doesn't work because manual ordered rooms dont get categoryindices
-            it.skip("adds a new room", () => {
-                const algorithm = setupAlgorithm(sortAlgorithm);
-
-                const shouldTriggerUpdate = algorithm.handleRoomUpdate(roomD, RoomUpdateCause.NewRoom);
-
-                expect(shouldTriggerUpdate).toBe(true);
-                expect(algorithm.orderedRooms).toEqual([roomC, roomB, roomD, roomE]);
-            });
-
-            it("does nothing and returns false for a timeline update", () => {
-                const algorithm = setupAlgorithm(sortAlgorithm);
-
-                const beforeRooms = algorithm.orderedRooms;
-                const shouldTriggerUpdate = algorithm.handleRoomUpdate(roomA, RoomUpdateCause.Timeline);
-
-                expect(shouldTriggerUpdate).toBe(false);
-                // strict equal
-                expect(algorithm.orderedRooms).toBe(beforeRooms);
-            });
-
-            it("does nothing and returns false for a read receipt update", () => {
-                const algorithm = setupAlgorithm(sortAlgorithm);
-
-                const beforeRooms = algorithm.orderedRooms;
-                const shouldTriggerUpdate = algorithm.handleRoomUpdate(roomA, RoomUpdateCause.ReadReceipt);
-
-                expect(shouldTriggerUpdate).toBe(false);
-                // strict equal
-                expect(algorithm.orderedRooms).toBe(beforeRooms);
-            });
-
-            it("throws for an unhandle update cause", () => {
-                const algorithm = setupAlgorithm(sortAlgorithm);
-
-                expect(() =>
-                    algorithm.handleRoomUpdate(roomA, "something unexpected" as unknown as RoomUpdateCause),
-                ).toThrow("Unsupported update cause: something unexpected");
-            });
-        });
-    });
-
    describe("When sortAlgorithm is alphabetical", () => {
        const sortAlgorithm = SortAlgorithm.Alphabetic;

@@ -61,7 +61,6 @@ describe("tagRoom()", () => {
                room,
                DefaultTagID.LowPriority, // remove
                DefaultTagID.Favourite, // add
-                0,
            );
        });

@@ -76,7 +75,6 @@ describe("tagRoom()", () => {
                room,
                DefaultTagID.Favourite, // remove
                DefaultTagID.LowPriority, // add
-                0,
            );
        });
    });
@@ -93,7 +91,6 @@ describe("tagRoom()", () => {
                room,
                DefaultTagID.Favourite, // remove
                null, // add
-                0,
            );
        });

@@ -108,7 +105,6 @@ describe("tagRoom()", () => {
                room,
                DefaultTagID.Favourite, // remove
                DefaultTagID.LowPriority, // add
-                0,
            );
        });
    });
@@ -124,7 +120,6 @@ describe("tagRoom()", () => {
                room,
                DefaultTagID.LowPriority, // remove
                DefaultTagID.Favourite, // add
-                0,
            );
        });

@@ -139,7 +134,6 @@ describe("tagRoom()", () => {
                room,
                DefaultTagID.LowPriority, // remove
                null, // add
-                0,
            );
        });
    });