Skip to content

Commit

Permalink
TAC: Order rooms by most recent after notification level (matrix-org#…
Browse files Browse the repository at this point in the history
…12329)

* Order room by thread timestamp

* Fix key errors in test

* Update jest snapshots

* Update snapshots

* Rename alpha/beta to numbers

* Add playwright test
  • Loading branch information
florianduros authored Mar 15, 2024
1 parent 94bd798 commit e247d31
Show file tree
Hide file tree
Showing 7 changed files with 297 additions and 60 deletions.
44 changes: 24 additions & 20 deletions playwright/e2e/spaces/threads-activity-centre/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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));
},
});
Expand Down Expand Up @@ -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")]);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand All @@ -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",
Expand All @@ -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
Expand All @@ -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);

Expand All @@ -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`);

Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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;
}
78 changes: 60 additions & 18 deletions test/components/views/spaces/ThreadsActivityCentre-test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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,
Expand Down Expand Up @@ -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]);

Expand Down
Loading

0 comments on commit e247d31

Please sign in to comment.