Fix rooms with no messages appearing at the top of the room list (#31798)
* Extract and move timestamp function * Use new function in BaseRecencySorter * Remove deprecated method usage * Add jsdoc * Avoid unnecessary exports * Fix tests
This commit is contained in:
@@ -7,7 +7,7 @@ Please see LICENSE files in the repository root for full details.
|
|||||||
|
|
||||||
import type { Room } from "matrix-js-sdk/src/matrix";
|
import type { Room } from "matrix-js-sdk/src/matrix";
|
||||||
import type { Sorter, SortingAlgorithm } from ".";
|
import type { Sorter, SortingAlgorithm } from ".";
|
||||||
import { getLastTs } from "../../../room-list/algorithms/tag-sorting/RecentAlgorithm";
|
import { getLastTimestamp } from "./utils/getLastTimestamp";
|
||||||
|
|
||||||
export abstract class BaseRecencySorter implements Sorter {
|
export abstract class BaseRecencySorter implements Sorter {
|
||||||
public constructor(protected myUserId: string) {}
|
public constructor(protected myUserId: string) {}
|
||||||
@@ -29,7 +29,7 @@ export abstract class BaseRecencySorter implements Sorter {
|
|||||||
}
|
}
|
||||||
|
|
||||||
private getTs(room: Room, cache?: { [roomId: string]: number }): number {
|
private getTs(room: Room, cache?: { [roomId: string]: number }): number {
|
||||||
const ts = cache?.[room.roomId] ?? getLastTs(room, this.myUserId);
|
const ts = cache?.[room.roomId] ?? getLastTimestamp(room, this.myUserId);
|
||||||
if (cache) {
|
if (cache) {
|
||||||
cache[room.roomId] = ts;
|
cache[room.roomId] = ts;
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -0,0 +1,84 @@
|
|||||||
|
/*
|
||||||
|
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 { EventTimeline, EventType, type MatrixEvent, type Room } from "matrix-js-sdk/src/matrix";
|
||||||
|
|
||||||
|
import { EffectiveMembership, getEffectiveMembership } from "../../../../../utils/membership";
|
||||||
|
import * as Unread from "../../../../../Unread";
|
||||||
|
|
||||||
|
function shouldCauseReorder(event: MatrixEvent): boolean {
|
||||||
|
const type = event.getType();
|
||||||
|
const content = event.getContent();
|
||||||
|
const prevContent = event.getPrevContent();
|
||||||
|
|
||||||
|
// Never ignore membership changes
|
||||||
|
if (type === EventType.RoomMember && prevContent.membership !== content.membership) return true;
|
||||||
|
|
||||||
|
// Ignore display name changes
|
||||||
|
if (type === EventType.RoomMember && prevContent.displayname !== content.displayname) return false;
|
||||||
|
// Ignore avatar changes
|
||||||
|
if (type === EventType.RoomMember && prevContent.avatar_url !== content.avatar_url) return false;
|
||||||
|
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* For a given room, this function returns a timestamp that can be used for recency sorting.
|
||||||
|
* @param r room for which the timestamp is calculated
|
||||||
|
* @param userId mxId of the current user
|
||||||
|
* @returns timestamp
|
||||||
|
*/
|
||||||
|
export const getLastTimestamp = (r: Room, userId: string): number => {
|
||||||
|
const mainTimelineLastTs = ((): number => {
|
||||||
|
const timeline = r.getLiveTimeline().getEvents();
|
||||||
|
|
||||||
|
// MSC4186: Simplified Sliding Sync sets this.
|
||||||
|
// If it's present, sort by it.
|
||||||
|
const bumpStamp = r.getBumpStamp();
|
||||||
|
if (bumpStamp) {
|
||||||
|
return bumpStamp;
|
||||||
|
}
|
||||||
|
|
||||||
|
// If the room hasn't been joined yet, it probably won't have a timeline to
|
||||||
|
// parse. We'll still fall back to the timeline if this fails, but chances
|
||||||
|
// are we'll at least have our own membership event to go off of.
|
||||||
|
const effectiveMembership = getEffectiveMembership(r.getMyMembership());
|
||||||
|
if (effectiveMembership !== EffectiveMembership.Join) {
|
||||||
|
const membershipEvent = r
|
||||||
|
.getLiveTimeline()
|
||||||
|
.getState(EventTimeline.FORWARDS)
|
||||||
|
?.getStateEvents(EventType.RoomMember, userId);
|
||||||
|
if (membershipEvent && !Array.isArray(membershipEvent)) {
|
||||||
|
return membershipEvent.getTs();
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
for (let i = timeline.length - 1; i >= 0; --i) {
|
||||||
|
const ev = timeline[i];
|
||||||
|
if (!ev.getTs()) continue; // skip events that don't have timestamps (tests only?)
|
||||||
|
|
||||||
|
if (
|
||||||
|
(ev.getSender() === userId && shouldCauseReorder(ev)) ||
|
||||||
|
Unread.eventTriggersUnreadCount(r.client, ev)
|
||||||
|
) {
|
||||||
|
return ev.getTs();
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// we might only have events that don't trigger the unread indicator,
|
||||||
|
// in which case use the oldest event even if normally it wouldn't count.
|
||||||
|
// This is better than just assuming the last event was forever ago.
|
||||||
|
return timeline[0]?.getTs() ?? 0;
|
||||||
|
})();
|
||||||
|
|
||||||
|
const threadLastEventTimestamps = r.getThreads().map((thread) => {
|
||||||
|
const event = thread.replyToEvent ?? thread.rootEvent;
|
||||||
|
return event?.getTs() ?? 0;
|
||||||
|
});
|
||||||
|
|
||||||
|
return Math.max(mainTimelineLastTs, ...threadLastEventTimestamps);
|
||||||
|
};
|
||||||
@@ -106,7 +106,7 @@ describe("RoomListStoreV3", () => {
|
|||||||
// Let's pretend like a new timeline event came on the room in 37th index.
|
// Let's pretend like a new timeline event came on the room in 37th index.
|
||||||
const room = rooms[37];
|
const room = rooms[37];
|
||||||
const event = mkMessage({ room: room.roomId, user: `@foo${3}:matrix.org`, ts: 1000, event: true });
|
const event = mkMessage({ room: room.roomId, user: `@foo${3}:matrix.org`, ts: 1000, event: true });
|
||||||
room.timeline.push(event);
|
jest.spyOn(room.getLiveTimeline(), "getEvents").mockReturnValue([event]);
|
||||||
|
|
||||||
const payload = {
|
const payload = {
|
||||||
action: "MatrixActions.Room.timeline",
|
action: "MatrixActions.Room.timeline",
|
||||||
@@ -827,7 +827,7 @@ describe("RoomListStoreV3", () => {
|
|||||||
let ts = 1000;
|
let ts = 1000;
|
||||||
for (const room of [rooms[14], rooms[34]]) {
|
for (const room of [rooms[14], rooms[34]]) {
|
||||||
const event = mkMessage({ room: room.roomId, user: `@foo${3}:matrix.org`, ts: 1000, event: true });
|
const event = mkMessage({ room: room.roomId, user: `@foo${3}:matrix.org`, ts: 1000, event: true });
|
||||||
room.timeline.push(event);
|
jest.spyOn(room.getLiveTimeline(), "getEvents").mockReturnValue([event]);
|
||||||
|
|
||||||
const payload = {
|
const payload = {
|
||||||
action: "MatrixActions.Room.timeline",
|
action: "MatrixActions.Room.timeline",
|
||||||
|
|||||||
@@ -83,7 +83,7 @@ describe("RoomSkipList", () => {
|
|||||||
ts: totalRooms - i,
|
ts: totalRooms - i,
|
||||||
event: true,
|
event: true,
|
||||||
});
|
});
|
||||||
room.timeline.push(event);
|
jest.spyOn(room.getLiveTimeline(), "getEvents").mockReturnValue([event]);
|
||||||
skipList.reInsertRoom(room);
|
skipList.reInsertRoom(room);
|
||||||
expect(skipList.size).toEqual(rooms.length);
|
expect(skipList.size).toEqual(rooms.length);
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -14,7 +14,7 @@ export function getMockedRooms(client: MatrixClient, roomCount: number = 100): R
|
|||||||
const roomId = `!foo${i}:matrix.org`;
|
const roomId = `!foo${i}:matrix.org`;
|
||||||
const room = mkStubRoom(roomId, `Foo Room ${i}`, client);
|
const room = mkStubRoom(roomId, `Foo Room ${i}`, client);
|
||||||
const event = mkMessage({ room: roomId, user: `@foo${i}:matrix.org`, ts: i + 1, event: true });
|
const event = mkMessage({ room: roomId, user: `@foo${i}:matrix.org`, ts: i + 1, event: true });
|
||||||
room.timeline.push(event);
|
jest.spyOn(room.getLiveTimeline(), "getEvents").mockReturnValue([event]);
|
||||||
rooms.push(room);
|
rooms.push(room);
|
||||||
}
|
}
|
||||||
return rooms;
|
return rooms;
|
||||||
|
|||||||
@@ -0,0 +1,185 @@
|
|||||||
|
/*
|
||||||
|
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 { Room, type RoomState } from "matrix-js-sdk/src/matrix";
|
||||||
|
import { KnownMembership } from "matrix-js-sdk/src/types";
|
||||||
|
|
||||||
|
import { mkEvent, mkMessage, mkRoom, stubClient } from "../../../../../test-utils";
|
||||||
|
import { getLastTimestamp } from "../../../../../../src/stores/room-list-v3/skip-list/sorters/utils/getLastTimestamp";
|
||||||
|
|
||||||
|
describe("getLastTimestamp", () => {
|
||||||
|
it("should return last timestamp", () => {
|
||||||
|
const cli = stubClient();
|
||||||
|
const room = new Room("room123", cli, "@john:matrix.org");
|
||||||
|
|
||||||
|
const event1 = mkMessage({
|
||||||
|
room: room.roomId,
|
||||||
|
msg: "Hello world!",
|
||||||
|
user: "@alice:matrix.org",
|
||||||
|
ts: 5,
|
||||||
|
event: true,
|
||||||
|
});
|
||||||
|
const event2 = mkMessage({
|
||||||
|
room: room.roomId,
|
||||||
|
msg: "Howdy!",
|
||||||
|
user: "@bob:matrix.org",
|
||||||
|
ts: 10,
|
||||||
|
event: true,
|
||||||
|
});
|
||||||
|
|
||||||
|
room.getMyMembership = () => KnownMembership.Join;
|
||||||
|
|
||||||
|
room.addLiveEvents([event1], { addToState: true });
|
||||||
|
expect(getLastTimestamp(room, "@jane:matrix.org")).toBe(5);
|
||||||
|
expect(getLastTimestamp(room, "@john:matrix.org")).toBe(5);
|
||||||
|
|
||||||
|
room.addLiveEvents([event2], { addToState: true });
|
||||||
|
|
||||||
|
expect(getLastTimestamp(room, "@jane:matrix.org")).toBe(10);
|
||||||
|
expect(getLastTimestamp(room, "@john:matrix.org")).toBe(10);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("should return timestamp of membership event if user not joined to room", () => {
|
||||||
|
const cli = stubClient();
|
||||||
|
const room = mkRoom(cli, "!new:example.org");
|
||||||
|
// Mock a membership event
|
||||||
|
jest.spyOn(room.getLiveTimeline(), "getState").mockImplementation((_) => {
|
||||||
|
return {
|
||||||
|
getStateEvents: () =>
|
||||||
|
mkEvent({
|
||||||
|
type: "m.room.member",
|
||||||
|
user: "@john:matrix.org",
|
||||||
|
content: {},
|
||||||
|
ts: 500,
|
||||||
|
event: true,
|
||||||
|
}),
|
||||||
|
} as unknown as RoomState;
|
||||||
|
});
|
||||||
|
jest.spyOn(room, "getMyMembership").mockReturnValue(KnownMembership.Invite);
|
||||||
|
expect(getLastTimestamp(room, "@john:matrix.org")).toBe(500);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("should return bump stamp when using sliding sync", () => {
|
||||||
|
const cli = stubClient();
|
||||||
|
const room = new Room("room123", cli, "@john:matrix.org");
|
||||||
|
|
||||||
|
const event1 = mkMessage({
|
||||||
|
room: room.roomId,
|
||||||
|
msg: "Hello world!",
|
||||||
|
user: "@alice:matrix.org",
|
||||||
|
ts: 5,
|
||||||
|
event: true,
|
||||||
|
});
|
||||||
|
const event2 = mkMessage({
|
||||||
|
room: room.roomId,
|
||||||
|
msg: "Howdy!",
|
||||||
|
user: "@bob:matrix.org",
|
||||||
|
ts: 10,
|
||||||
|
event: true,
|
||||||
|
});
|
||||||
|
|
||||||
|
jest.spyOn(room, "getMyMembership").mockReturnValue(KnownMembership.Join);
|
||||||
|
jest.spyOn(room, "getBumpStamp").mockReturnValue(314);
|
||||||
|
room.addLiveEvents([event1, event2], { addToState: true });
|
||||||
|
expect(getLastTimestamp(room, "@john:matrix.org")).toBe(314);
|
||||||
|
});
|
||||||
|
|
||||||
|
describe("membership event special cases", () => {
|
||||||
|
it("should consider event if membership has changed", () => {
|
||||||
|
const cli = stubClient();
|
||||||
|
const room = new Room("room123", cli, "@john:matrix.org");
|
||||||
|
|
||||||
|
const event1 = mkMessage({
|
||||||
|
room: room.roomId,
|
||||||
|
msg: "Hello world!",
|
||||||
|
user: "@alice:matrix.org",
|
||||||
|
ts: 5,
|
||||||
|
event: true,
|
||||||
|
});
|
||||||
|
// Display name change that should be ignored during timestamp calculation
|
||||||
|
const event2 = mkEvent({
|
||||||
|
type: "m.room.member",
|
||||||
|
user: "@john:matrix.org",
|
||||||
|
content: {
|
||||||
|
membership: "leave",
|
||||||
|
},
|
||||||
|
prev_content: {
|
||||||
|
membership: "join",
|
||||||
|
},
|
||||||
|
ts: 400,
|
||||||
|
event: true,
|
||||||
|
});
|
||||||
|
|
||||||
|
jest.spyOn(room, "getMyMembership").mockReturnValue(KnownMembership.Join);
|
||||||
|
room.addLiveEvents([event1, event2], { addToState: true });
|
||||||
|
|
||||||
|
expect(getLastTimestamp(room, "@john:matrix.org")).toBe(400);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("should skip display name changes", () => {
|
||||||
|
const cli = stubClient();
|
||||||
|
const room = new Room("room123", cli, "@john:matrix.org");
|
||||||
|
|
||||||
|
const event1 = mkMessage({
|
||||||
|
room: room.roomId,
|
||||||
|
msg: "Hello world!",
|
||||||
|
user: "@alice:matrix.org",
|
||||||
|
ts: 5,
|
||||||
|
event: true,
|
||||||
|
});
|
||||||
|
// Display name change that should be ignored during timestamp calculation
|
||||||
|
const event2 = mkEvent({
|
||||||
|
type: "m.room.member",
|
||||||
|
user: "@john:matrix.org",
|
||||||
|
content: {
|
||||||
|
displayname: "bar",
|
||||||
|
},
|
||||||
|
prev_content: {
|
||||||
|
displayname: "foo",
|
||||||
|
},
|
||||||
|
ts: 500,
|
||||||
|
event: true,
|
||||||
|
});
|
||||||
|
|
||||||
|
jest.spyOn(room, "getMyMembership").mockReturnValue(KnownMembership.Join);
|
||||||
|
room.addLiveEvents([event1, event2], { addToState: true });
|
||||||
|
|
||||||
|
expect(getLastTimestamp(room, "@john:matrix.org")).toBe(5);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("should skip avatar changes", () => {
|
||||||
|
const cli = stubClient();
|
||||||
|
const room = new Room("room123", cli, "@john:matrix.org");
|
||||||
|
|
||||||
|
const event1 = mkMessage({
|
||||||
|
room: room.roomId,
|
||||||
|
msg: "Hello world!",
|
||||||
|
user: "@alice:matrix.org",
|
||||||
|
ts: 5,
|
||||||
|
event: true,
|
||||||
|
});
|
||||||
|
// Avatar url change that should be ignored during timestamp calculation
|
||||||
|
const event2 = mkEvent({
|
||||||
|
type: "m.room.member",
|
||||||
|
user: "@john:matrix.org",
|
||||||
|
content: {
|
||||||
|
avatar_url: "bar",
|
||||||
|
},
|
||||||
|
prev_content: {
|
||||||
|
avatar_url: "foo",
|
||||||
|
},
|
||||||
|
ts: 500,
|
||||||
|
event: true,
|
||||||
|
});
|
||||||
|
|
||||||
|
jest.spyOn(room, "getMyMembership").mockReturnValue(KnownMembership.Join);
|
||||||
|
room.addLiveEvents([event1, event2], { addToState: true });
|
||||||
|
|
||||||
|
expect(getLastTimestamp(room, "@john:matrix.org")).toBe(5);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
});
|
||||||
Reference in New Issue
Block a user