TAC: Order rooms by most recent after notification level (#12329)

* Order room by thread timestamp

* Fix key errors in test

* Update jest snapshots

* Update snapshots

* Rename alpha/beta to numbers

* Add playwright test

playwright/e2e/spaces/threads-activity-centre/index.ts

@@ -30,30 +30,30 @@ import { ElementAppPage } from "../../../pages/ElementAppPage";
  * - Invite the bot to both rooms and ensure that it has joined
  */
 export const test = base.extend<{
-    roomAlphaName?: string;
-    roomAlpha: { name: string; roomId: string };
-    roomBetaName?: string;
-    roomBeta: { name: string; roomId: string };
+    room1Name?: string;
+    room1: { name: string; roomId: string };
+    room2Name?: string;
+    room2: { name: string; roomId: string };
     msg: MessageBuilder;
     util: Helpers;
 }>({
     displayName: "Mae",
     botCreateOpts: { displayName: "Other User" },
 
-    roomAlphaName: "Room Alpha",
-    roomAlpha: async ({ roomAlphaName: name, app, user, bot }, use) => {
+    room1Name: "Room 1",
+    room1: async ({ room1Name: name, app, user, bot }, use) => {
         const roomId = await app.client.createRoom({ name, invite: [bot.credentials.userId] });
         await use({ name, roomId });
     },
-    roomBetaName: "Room Beta",
-    roomBeta: async ({ roomBetaName: name, app, user, bot }, use) => {
+    room2Name: "Room 2",
+    room2: async ({ room2Name: name, app, user, bot }, use) => {
         const roomId = await app.client.createRoom({ name, invite: [bot.credentials.userId] });
         await use({ name, roomId });
     },
     msg: async ({ page, app, util }, use) => {
         await use(new MessageBuilder(page, app, util));
     },
-    util: async ({ roomAlpha, roomBeta, page, app, bot }, use) => {
+    util: async ({ room1, room2, page, app, bot }, use) => {
         await use(new Helpers(page, app, bot));
     },
 });
@@ -337,23 +337,27 @@ export class Helpers {
      * @param room1
      * @param room2
      * @param msg - MessageBuilder
+     * @param hasMention - whether to include a mention in the first message
      */
     async populateThreads(
         room1: { name: string; roomId: string },
         room2: { name: string; roomId: string },
         msg: MessageBuilder,
+        hasMention = true,
     ) {
-        await this.receiveMessages(room2, [
-            "Msg1",
-            msg.threadedOff("Msg1", {
-                "body": "User",
-                "format": "org.matrix.custom.html",
-                "formatted_body": "<a href='https://matrix.to/#/@user:localhost'>User</a>",
-                "m.mentions": {
-                    user_ids: ["@user:localhost"],
-                },
-            }),
-        ]);
+        if (hasMention) {
+            await this.receiveMessages(room2, [
+                "Msg1",
+                msg.threadedOff("Msg1", {
+                    "body": "User",
+                    "format": "org.matrix.custom.html",
+                    "formatted_body": "<a href='https://matrix.to/#/@user:localhost'>User</a>",
+                    "m.mentions": {
+                        user_ids: ["@user:localhost"],
+                    },
+                }),
+            ]);
+        }
         await this.receiveMessages(room2, ["Msg2", msg.threadedOff("Msg2", "Resp2")]);
         await this.receiveMessages(room1, ["Msg3", msg.threadedOff("Msg3", "Resp3")]);
     }

playwright/e2e/spaces/threads-activity-centre/threadsActivityCentre.spec.ts

@@ -35,7 +35,7 @@ test.describe("Threads Activity Centre", () => {
         await expect(util.getSpacePanel()).toMatchScreenshot("tac-button-expanded.png");
     });
 
-    test("should not show indicator when there is no thread", async ({ roomAlpha: room1, util }) => {
+    test("should not show indicator when there is no thread", async ({ room1, util }) => {
         // No indicator should be shown
         await util.assertNoTacIndicator();
 
@@ -46,23 +46,15 @@ test.describe("Threads Activity Centre", () => {
         await util.assertNoTacIndicator();
     });
 
-    test("should show a notification indicator when there is a message in a thread", async ({
-        roomAlpha: room1,
-        util,
-        msg,
-    }) => {
+    test("should show a notification indicator when there is a message in a thread", async ({ room1, util, msg }) => {
         await util.goTo(room1);
         await util.receiveMessages(room1, ["Msg1", msg.threadedOff("Msg1", "Resp1")]);
 
         // The indicator should be shown
         await util.assertNotificationTac();
     });
 
-    test("should show a highlight indicator when there is a mention in a thread", async ({
-        roomAlpha: room1,
-        util,
-        msg,
-    }) => {
+    test("should show a highlight indicator when there is a mention in a thread", async ({ room1, util, msg }) => {
         await util.goTo(room1);
         await util.receiveMessages(room1, [
             "Msg1",
@@ -80,7 +72,7 @@ test.describe("Threads Activity Centre", () => {
         await util.assertHighlightIndicator();
     });
 
-    test("should show the rooms with unread threads", async ({ roomAlpha: room1, roomBeta: room2, util, msg }) => {
+    test("should show the rooms with unread threads", async ({ room1, room2, util, msg }) => {
         await util.goTo(room2);
         await util.populateThreads(room1, room2, msg);
         // The indicator should be shown
@@ -97,7 +89,7 @@ test.describe("Threads Activity Centre", () => {
         await expect(util.getTacPanel()).toMatchScreenshot("tac-panel-mix-unread.png");
     });
 
-    test("should update with a thread is read", async ({ roomAlpha: room1, roomBeta: room2, util, msg }) => {
+    test("should update with a thread is read", async ({ room1, room2, util, msg }) => {
         await util.goTo(room2);
         await util.populateThreads(room1, room2, msg);
 
@@ -120,6 +112,17 @@ test.describe("Threads Activity Centre", () => {
         await expect(util.getTacPanel()).toMatchScreenshot("tac-panel-notification-unread.png");
     });
 
+    test("should order by recency after notification level", async ({ room1, room2, util, msg }) => {
+        await util.goTo(room2);
+        await util.populateThreads(room1, room2, msg, false);
+
+        await util.openTac();
+        await util.assertRoomsInTac([
+            { room: room1.name, notificationLevel: "notification" },
+            { room: room2.name, notificationLevel: "notification" },
+        ]);
+    });
+
     test("should block the Spotlight to open when the TAC is opened", async ({ util, page }) => {
         const toggleSpotlight = () => page.keyboard.press(`${CommandOrControl}+k`);
 

src/components/views/spaces/threads-activity-centre/useUnreadThreadRooms.ts

@@ -89,12 +89,12 @@ function computeUnreadThreadRooms(mxClient: MatrixClient, msc3946ProcessDynamicP
     const visibleRooms = mxClient.getVisibleRooms(msc3946ProcessDynamicPredecessor);
 
     let greatestNotificationLevel = NotificationLevel.None;
-    const rooms = [];
+    const rooms: Result["rooms"] = [];
 
     for (const room of visibleRooms) {
         // We only care about rooms with unread threads
         if (VisibilityProvider.instance.isRoomVisible(room) && doesRoomHaveUnreadThreads(room)) {
-            // Get the greatest notification level of all rooms
+            // Get the greatest notification level of all threads
             const notificationLevel = getThreadNotificationLevel(room);
 
             // If the room has an activity notification or less, we ignore it
@@ -110,20 +110,35 @@ function computeUnreadThreadRooms(mxClient: MatrixClient, msc3946ProcessDynamicP
         }
     }
 
-    const sortedRooms = rooms.sort((a, b) => sortRoom(a.notificationLevel, b.notificationLevel));
+    const sortedRooms = rooms.sort((a, b) => sortRoom(a, b));
     return { greatestNotificationLevel, rooms: sortedRooms };
 }
 
+/**
+ * Store the room and its thread notification level
+ */
+type RoomData = Result["rooms"][0];
+
 /**
  * Sort notification level by the most important notification level to the least important
  * Highlight > Notification > Activity
- * @param notificationLevelA - notification level of room A
- * @param notificationLevelB - notification level of room B
+ * If the notification level is the same, we sort by the most recent thread
+ * @param roomDataA - room and notification level of room A
+ * @param roomDataB - room and notification level of room B
  * @returns {number}
  */
-function sortRoom(notificationLevelA: NotificationLevel, notificationLevelB: NotificationLevel): number {
+function sortRoom(roomDataA: RoomData, roomDataB: RoomData): number {
+    const { notificationLevel: notificationLevelA, room: roomA } = roomDataA;
+    const { notificationLevel: notificationLevelB, room: roomB } = roomDataB;
+
+    const timestampA = roomA.getLastThread()?.events.at(-1)?.getTs();
+    const timestampB = roomB.getLastThread()?.events.at(-1)?.getTs();
+
     // NotificationLevel is a numeric enum, so we can compare them directly
     if (notificationLevelA > notificationLevelB) return -1;
     else if (notificationLevelB > notificationLevelA) return 1;
-    else return 0;
+    // Display most recent first
+    else if (!timestampA) return 1;
+    else if (!timestampB) return -1;
+    else return timestampB - timestampA;
 }

test/components/views/spaces/ThreadsActivityCentre-test.tsx

@@ -59,16 +59,23 @@ describe("ThreadsActivityCentre", () => {
     });
     roomWithActivity.name = "Just activity";
 
-    const roomWithNotif = new Room("!room:server", cli, cli.getSafeUserId(), {
+    const roomWithNotif = new Room("!room2:server", cli, cli.getSafeUserId(), {
         pendingEventOrdering: PendingEventOrdering.Detached,
     });
     roomWithNotif.name = "A notification";
 
-    const roomWithHighlight = new Room("!room:server", cli, cli.getSafeUserId(), {
+    const roomWithHighlight = new Room("!room3:server", cli, cli.getSafeUserId(), {
         pendingEventOrdering: PendingEventOrdering.Detached,
     });
     roomWithHighlight.name = "This is a real highlight";
 
+    const getDefaultThreadArgs = (room: Room) => ({
+        room: room,
+        client: cli,
+        authorId: "@foo:bar",
+        participantUserIds: ["@fee:bar"],
+    });
+
     beforeAll(async () => {
         jest.spyOn(MatrixClientPeg, "get").mockReturnValue(cli);
         jest.spyOn(MatrixClientPeg, "safeGet").mockReturnValue(cli);
@@ -77,26 +84,15 @@ describe("ThreadsActivityCentre", () => {
         jest.spyOn(dmRoomMap, "getUserIdForRoomId");
         jest.spyOn(DMRoomMap, "shared").mockReturnValue(dmRoomMap);
 
-        await populateThread({
-            room: roomWithActivity,
-            client: cli,
-            authorId: "@foo:bar",
-            participantUserIds: ["@fee:bar"],
-        });
+        await populateThread(getDefaultThreadArgs(roomWithActivity));
 
-        const notifThreadInfo = await populateThread({
-            room: roomWithNotif,
-            client: cli,
-            authorId: "@foo:bar",
-            participantUserIds: ["@fee:bar"],
-        });
+        const notifThreadInfo = await populateThread(getDefaultThreadArgs(roomWithNotif));
         roomWithNotif.setThreadUnreadNotificationCount(notifThreadInfo.thread.id, NotificationCountType.Total, 1);
 
         const highlightThreadInfo = await populateThread({
-            room: roomWithHighlight,
-            client: cli,
-            authorId: "@foo:bar",
-            participantUserIds: ["@fee:bar"],
+            ...getDefaultThreadArgs(roomWithHighlight),
+            // timestamp
+            ts: 5,
         });
         roomWithHighlight.setThreadUnreadNotificationCount(
             highlightThreadInfo.thread.id,
@@ -181,6 +177,52 @@ describe("ThreadsActivityCentre", () => {
         expect(screen.getByRole("menu")).toMatchSnapshot();
     });
 
+    it("should order the room with the same notification level by most recent", async () => {
+        // Generate two new rooms with threads
+        const secondRoomWithHighlight = new Room("!room4:server", cli, cli.getSafeUserId(), {
+            pendingEventOrdering: PendingEventOrdering.Detached,
+        });
+        secondRoomWithHighlight.name = "This is a second real highlight";
+
+        const secondHighlightThreadInfo = await populateThread({
+            ...getDefaultThreadArgs(secondRoomWithHighlight),
+            // timestamp
+            ts: 1,
+        });
+        secondRoomWithHighlight.setThreadUnreadNotificationCount(
+            secondHighlightThreadInfo.thread.id,
+            NotificationCountType.Highlight,
+            1,
+        );
+
+        const thirdRoomWithHighlight = new Room("!room5:server", cli, cli.getSafeUserId(), {
+            pendingEventOrdering: PendingEventOrdering.Detached,
+        });
+        thirdRoomWithHighlight.name = "This is a third real highlight";
+
+        const thirdHighlightThreadInfo = await populateThread({
+            ...getDefaultThreadArgs(thirdRoomWithHighlight),
+            // timestamp
+            ts: 7,
+        });
+        thirdRoomWithHighlight.setThreadUnreadNotificationCount(
+            thirdHighlightThreadInfo.thread.id,
+            NotificationCountType.Highlight,
+            1,
+        );
+
+        cli.getVisibleRooms = jest
+            .fn()
+            .mockReturnValue([roomWithHighlight, secondRoomWithHighlight, thirdRoomWithHighlight]);
+
+        renderTAC();
+        await userEvent.click(getTACButton());
+
+        // The room should be ordered by the most recent thread
+        // thirdHighlightThreadInfo (timestamp 7) > highlightThreadInfo (timestamp 5) > secondHighlightThreadInfo (timestamp 1)
+        expect(screen.getByRole("menu")).toMatchSnapshot();
+    });
+
     it("should block Ctrl/CMD + k shortcut", async () => {
         cli.getVisibleRooms = jest.fn().mockReturnValue([roomWithHighlight]);