Skip to content

Fix thread list being ordered based on all updates #3458

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Jun 9, 2023
Merged

Fix thread list being ordered based on all updates #3458

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
80f835d
Add test for thread list stability around non-reply updates
t3chguy Jun 8, 2023
cb742c5
Fix thread list being ordered based on all updates
t3chguy Jun 8, 2023
0bbec69
Fix test
t3chguy Jun 8, 2023
932806f
Update spec/integ/matrix-client-event-timeline.spec.ts
t3chguy Jun 8, 2023
8553dc2
Iterate
t3chguy Jun 9, 2023
7be39e4
Merge remote-tracking branch 'origin/t3chguy/fix/25522' into t3chguy/…
t3chguy Jun 9, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
133 changes: 131 additions & 2 deletions spec/integ/matrix-client-event-timeline.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1453,7 +1453,7 @@ describe("MatrixClient event timelines", function () {
expect(room.getPendingEvents()).toHaveLength(1);
});

it("should handle thread updates by reordering the thread list", async () => {
it("should handle new thread replies by reordering the thread list", async () => {
// Test data for a second thread
const THREAD2_ROOT = utils.mkEvent({
room: roomId,
Expand Down Expand Up @@ -1568,7 +1568,8 @@ describe("MatrixClient event timelines", function () {
// Test adding a second event to the first thread
const thread = room.getThread(THREAD_ROOT.event_id!)!;
thread.initialEventsFetched = true;
const prom = emitPromise(room, ThreadEvent.Update);
const prom = emitPromise(room, ThreadEvent.NewReply);
respondToEvent(THREAD_ROOT_UPDATED);
respondToEvent(THREAD_ROOT_UPDATED);
respondToEvent(THREAD_ROOT_UPDATED);
respondToEvent(THREAD_ROOT_UPDATED);
Expand All @@ -1583,6 +1584,134 @@ describe("MatrixClient event timelines", function () {
THREAD_ROOT.event_id,
]);
});

it("should not reorder the thread list on other thread updates", async () => {
// Test data for a second thread
const THREAD2_ROOT = utils.mkEvent({
room: roomId,
user: userId,
type: "m.room.message",
content: {
body: "thread root",
msgtype: "m.text",
},
unsigned: {
"m.relations": {
"io.element.thread": {
//"latest_event": undefined,
count: 1,
current_user_participated: true,
},
},
},
event: false,
});

const THREAD2_REPLY = utils.mkEvent({
room: roomId,
user: userId,
type: "m.room.message",
content: {
"body": "thread2 reply",
"msgtype": "m.text",
"m.relates_to": {
// We can't use the const here because we change server support mode for test
rel_type: "io.element.thread",
event_id: THREAD_ROOT.event_id,
},
},
event: false,
});

// @ts-ignore we know this is a defined path for THREAD ROOT
THREAD2_ROOT.unsigned["m.relations"]["io.element.thread"].latest_event = THREAD2_REPLY;

const THREAD_REPLY_REACTION = utils.mkEvent({
room: roomId,
user: userId,
type: "m.reaction",
content: {
"m.relates_to": {
rel_type: RelationType.Annotation,
event_id: THREAD_REPLY.event_id,
key: "🪿",
},
},
event: true,
});
THREAD_REPLY_REACTION.localTimestamp += 1000;

// Modified thread root event containing latest thread reply in its unsigned
const THREAD_ROOT_UPDATED = {
...THREAD_ROOT,
unsigned: {
...THREAD_ROOT.unsigned,
"m.relations": {
...THREAD_ROOT.unsigned!["m.relations"],
"io.element.thread": {
...THREAD_ROOT.unsigned!["m.relations"]!["io.element.thread"],
count: 2,
latest_event: THREAD_REPLY,
},
},
},
};

// Response with test data for the thread list request
const threadsResponse = {
chunk: [THREAD2_ROOT, THREAD_ROOT],
state: [],
next_batch: RANDOM_TOKEN as string | null,
};

// @ts-ignore
client.clientOpts.threadSupport = true;
Thread.setServerSideSupport(FeatureSupport.Stable);
Thread.setServerSideListSupport(FeatureSupport.Stable);
Thread.setServerSideFwdPaginationSupport(FeatureSupport.Stable);

await client.stopClient(); // we don't need the client to be syncing at this time
const room = client.getRoom(roomId)!;

// Set up room threads
const timelineSets = await room!.createThreadsTimelineSets();
expect(timelineSets).not.toBeNull();
respondToThreads(threadsResponse);
respondToThreads(threadsResponse);
respondToEvent(THREAD_ROOT);
respondToEvent(THREAD2_ROOT);
respondToThread(THREAD_ROOT, [THREAD_REPLY]);
respondToThread(THREAD2_ROOT, [THREAD2_REPLY]);
await flushHttp(room.fetchRoomThreads());
const threadIds = room.getThreads().map((thread) => thread.id);
expect(threadIds).toContain(THREAD_ROOT.event_id);
expect(threadIds).toContain(THREAD2_ROOT.event_id);
const [allThreads] = timelineSets!;
const timeline = allThreads.getLiveTimeline()!;
// Test threads are in chronological order
expect(timeline.getEvents().map((it) => it.event.event_id)).toEqual([
THREAD_ROOT.event_id,
THREAD2_ROOT.event_id,
]);

// Test adding a second event to the first thread
const thread = room.getThread(THREAD_ROOT.event_id!)!;
thread.initialEventsFetched = true;
const prom = emitPromise(room, ThreadEvent.Update);
respondToEvent(THREAD_ROOT_UPDATED);
respondToEvent(THREAD_ROOT_UPDATED);
respondToEvent(THREAD_ROOT_UPDATED);
respondToEvent(THREAD2_ROOT);
await room.addLiveEvents([THREAD_REPLY_REACTION]);
await httpBackend.flushAllExpected();
await prom;
expect(thread.length).toBe(2);
// Test thread order is unchanged
expect(timeline!.getEvents().map((it) => it.event.event_id)).toEqual([
THREAD_ROOT.event_id,
THREAD2_ROOT.event_id,
]);
});
});

describe("without server compatibility", function () {
Expand Down
4 changes: 2 additions & 2 deletions src/models/room.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1957,7 +1957,7 @@ export class Room extends ReadReceipt<RoomEmittedEvents, RoomEventHandlerMap> {
}
}

this.on(ThreadEvent.Update, this.onThreadUpdate);
this.on(ThreadEvent.NewReply, this.onThreadReply);
this.on(ThreadEvent.Delete, this.onThreadDelete);
this.threadsReady = true;
}
Expand Down Expand Up @@ -2055,7 +2055,7 @@ export class Room extends ReadReceipt<RoomEmittedEvents, RoomEventHandlerMap> {
}
}

private onThreadUpdate(thread: Thread): void {
private onThreadReply(thread: Thread): void {
this.updateThreadRootEvents(thread, false, true);
}

Expand Down