Skip to content

Commit 8293011

Browse files
justjannejustjanne
authored
Fix #23916: Prevent edits of the last message in a thread getting lost (#2951)
* Fix issue where the root event of a thread had to be loaded in a complicated way * Fix issue where edits to the last event of a thread would get lost * Fix issue where thread reply count would desync * Refactor relations pagination mocking for tests
1 parent 4c5f416 commit 8293011

File tree

5 files changed

+70
-126
lines changed

5 files changed

+70
-126
lines changed

‎spec/integ/matrix-client-event-timeline.spec.ts

Lines changed: 29 additions & 85 deletions
Original file line numberDiff line numberDiff line change
@@ -27,12 +27,11 @@ import {
2727
MatrixEvent,
2828
PendingEventOrdering,
2929
Room,
30-
RoomEvent,
3130
} from "../../src/matrix";
3231
import { logger } from "../../src/logger";
33-
import { encodeUri } from "../../src/utils";
32+
import { encodeParams, encodeUri, QueryDict, replaceParam } from "../../src/utils";
3433
import { TestClient } from "../TestClient";
35-
import { FeatureSupport, Thread, THREAD_RELATION_TYPE } from "../../src/models/thread";
34+
import { FeatureSupport, Thread, THREAD_RELATION_TYPE, ThreadEvent } from "../../src/models/thread";
3635
import { emitPromise } from "../test-utils/test-utils";
3736

3837
const userId = "@alice:localhost";
@@ -47,6 +46,18 @@ const withoutRoomId = (e: Partial<IEvent>): Partial<IEvent> => {
4746
return copy;
4847
};
4948

49+
/**
50+
* Our httpBackend only allows matching calls if we have the exact same query, in the exact same order
51+
* This method allows building queries with the exact same parameter order as the fetchRelations method in client
52+
* @param params query parameters
53+
*/
54+
const buildRelationPaginationQuery = (params: QueryDict): string => {
55+
if (Thread.hasServerSideFwdPaginationSupport === FeatureSupport.Experimental) {
56+
params = replaceParam("dir", "org.matrix.msc3715.dir", params);
57+
}
58+
return "?" + encodeParams(params).toString();
59+
};
60+
5061
const USER_MEMBERSHIP_EVENT = utils.mkMembership({
5162
room: roomId,
5263
mship: "join",
@@ -595,42 +606,6 @@ describe("MatrixClient event timelines", function () {
595606
.respond(200, function () {
596607
return THREAD_ROOT;
597608
});
598-
599-
httpBackend
600-
.when(
601-
"GET",
602-
"/_matrix/client/v1/rooms/!foo%3Abar/relations/" +
603-
encodeURIComponent(THREAD_ROOT.event_id!) +
604-
"/" +
605-
encodeURIComponent(THREAD_RELATION_TYPE.name) +
606-
"?dir=b&limit=1",
607-
)
608-
.respond(200, function () {
609-
return {
610-
original_event: THREAD_ROOT,
611-
chunk: [THREAD_REPLY],
612-
// no next batch as this is the oldest end of the timeline
613-
};
614-
});
615-
616-
const thread = room.createThread(THREAD_ROOT.event_id!, undefined, [], false);
617-
await httpBackend.flushAllExpected();
618-
const timelineSet = thread.timelineSet;
619-
620-
const timelinePromise = client.getEventTimeline(timelineSet, THREAD_REPLY.event_id!);
621-
const timeline = await timelinePromise;
622-
623-
expect(timeline!.getEvents().find((e) => e.getId() === THREAD_ROOT.event_id!)).toBeTruthy();
624-
expect(timeline!.getEvents().find((e) => e.getId() === THREAD_REPLY.event_id!)).toBeTruthy();
625-
});
626-
627-
it("should handle thread replies with server support by fetching a contiguous thread timeline", async () => {
628-
// @ts-ignore
629-
client.clientOpts.experimentalThreadSupport = true;
630-
Thread.setServerSideSupport(FeatureSupport.Experimental);
631-
await client.stopClient(); // we don't need the client to be syncing at this time
632-
const room = client.getRoom(roomId)!;
633-
634609
httpBackend
635610
.when("GET", "/rooms/!foo%3Abar/event/" + encodeURIComponent(THREAD_ROOT.event_id!))
636611
.respond(200, function () {
@@ -644,7 +619,7 @@ describe("MatrixClient event timelines", function () {
644619
encodeURIComponent(THREAD_ROOT.event_id!) +
645620
"/" +
646621
encodeURIComponent(THREAD_RELATION_TYPE.name) +
647-
"?dir=b&limit=1",
622+
buildRelationPaginationQuery({ dir: Direction.Backward, limit: 1 }),
648623
)
649624
.respond(200, function () {
650625
return {
@@ -656,15 +631,14 @@ describe("MatrixClient event timelines", function () {
656631
const thread = room.createThread(THREAD_ROOT.event_id!, undefined, [], false);
657632
await httpBackend.flushAllExpected();
658633
const timelineSet = thread.timelineSet;
659-
660634
httpBackend
661635
.when("GET", "/rooms/!foo%3Abar/event/" + encodeURIComponent(THREAD_ROOT.event_id!))
662636
.respond(200, function () {
663637
return THREAD_ROOT;
664638
});
639+
await flushHttp(emitPromise(thread, ThreadEvent.Update));
665640

666-
const timelinePromise = client.getEventTimeline(timelineSet, THREAD_REPLY.event_id!);
667-
const [timeline] = await Promise.all([timelinePromise, httpBackend.flushAllExpected()]);
641+
const timeline = await client.getEventTimeline(timelineSet, THREAD_REPLY.event_id!);
668642

669643
const eventIds = timeline!.getEvents().map((it) => it.getId());
670644
expect(eventIds).toContain(THREAD_ROOT.event_id);
@@ -1273,7 +1247,7 @@ describe("MatrixClient event timelines", function () {
12731247
event_id: THREAD_ROOT.event_id,
12741248
},
12751249
},
1276-
event: false,
1250+
event: true,
12771251
});
12781252

12791253
// Test data for the first thread, with the second reply
@@ -1286,7 +1260,7 @@ describe("MatrixClient event timelines", function () {
12861260
"io.element.thread": {
12871261
...THREAD_ROOT.unsigned!["m.relations"]!["io.element.thread"],
12881262
count: 2,
1289-
latest_event: THREAD_REPLY2,
1263+
latest_event: THREAD_REPLY2.event,
12901264
},
12911265
},
12921266
},
@@ -1314,12 +1288,13 @@ describe("MatrixClient event timelines", function () {
13141288
respondToThreads(threadsResponse);
13151289
respondToThreads(threadsResponse);
13161290
respondToEvent(THREAD_ROOT);
1317-
respondToEvent(THREAD_ROOT);
1318-
respondToEvent(THREAD2_ROOT);
13191291
respondToEvent(THREAD2_ROOT);
13201292
respondToThread(THREAD_ROOT, [THREAD_REPLY]);
13211293
respondToThread(THREAD2_ROOT, [THREAD2_REPLY]);
13221294
await flushHttp(room.fetchRoomThreads());
1295+
const threadIds = room.getThreads().map((thread) => thread.id);
1296+
expect(threadIds).toContain(THREAD_ROOT.event_id);
1297+
expect(threadIds).toContain(THREAD2_ROOT.event_id);
13231298
const [allThreads] = timelineSets!;
13241299
const timeline = allThreads.getLiveTimeline()!;
13251300
// Test threads are in chronological order
@@ -1330,12 +1305,15 @@ describe("MatrixClient event timelines", function () {
13301305

13311306
// Test adding a second event to the first thread
13321307
const thread = room.getThread(THREAD_ROOT.event_id!)!;
1333-
const prom = emitPromise(allThreads!, RoomEvent.Timeline);
1334-
await thread.addEvent(client.getEventMapper()(THREAD_REPLY2), false);
1308+
const prom = emitPromise(room, ThreadEvent.NewReply);
1309+
respondToEvent(THREAD_ROOT_UPDATED);
13351310
respondToEvent(THREAD_ROOT_UPDATED);
13361311
respondToEvent(THREAD_ROOT_UPDATED);
1312+
respondToEvent(THREAD2_ROOT);
1313+
room.addLiveEvents([THREAD_REPLY2]);
13371314
await httpBackend.flushAllExpected();
13381315
await prom;
1316+
expect(thread.length).toBe(2);
13391317
// Test threads are in chronological order
13401318
expect(timeline!.getEvents().map((it) => it.event.event_id)).toEqual([
13411319
THREAD2_ROOT.event_id,
@@ -1652,24 +1630,6 @@ describe("MatrixClient event timelines", function () {
16521630
const thread = room.getThread(THREAD_ROOT.event_id!)!;
16531631
const timelineSet = thread.timelineSet;
16541632

1655-
const buildParams = (direction: Direction, token: string): string => {
1656-
if (Thread.hasServerSideFwdPaginationSupport === FeatureSupport.Experimental) {
1657-
return `?from=${token}&org.matrix.msc3715.dir=${direction}`;
1658-
} else {
1659-
return `?dir=${direction}&from=${token}`;
1660-
}
1661-
};
1662-
1663-
httpBackend
1664-
.when("GET", "/rooms/!foo%3Abar/context/" + encodeURIComponent(THREAD_ROOT.event_id!))
1665-
.respond(200, {
1666-
start: "start_token",
1667-
events_before: [],
1668-
event: THREAD_ROOT,
1669-
events_after: [],
1670-
state: [],
1671-
end: "end_token",
1672-
});
16731633
httpBackend
16741634
.when("GET", "/rooms/!foo%3Abar/event/" + encodeURIComponent(THREAD_ROOT.event_id!))
16751635
.respond(200, function () {
@@ -1692,27 +1652,11 @@ describe("MatrixClient event timelines", function () {
16921652
encodeURIComponent(THREAD_ROOT.event_id!) +
16931653
"/" +
16941654
encodeURIComponent(THREAD_RELATION_TYPE.name) +
1695-
buildParams(Direction.Backward, "start_token"),
1655+
buildRelationPaginationQuery({ dir: Direction.Backward, limit: 1 }),
16961656
)
16971657
.respond(200, function () {
16981658
return {
1699-
original_event: THREAD_ROOT,
1700-
chunk: [],
1701-
};
1702-
});
1703-
httpBackend
1704-
.when(
1705-
"GET",
1706-
"/_matrix/client/v1/rooms/!foo%3Abar/relations/" +
1707-
encodeURIComponent(THREAD_ROOT.event_id!) +
1708-
"/" +
1709-
encodeURIComponent(THREAD_RELATION_TYPE.name) +
1710-
buildParams(Direction.Forward, "end_token"),
1711-
)
1712-
.respond(200, function () {
1713-
return {
1714-
original_event: THREAD_ROOT,
1715-
chunk: [THREAD_REPLY],
1659+
chunk: [THREAD_ROOT],
17161660
};
17171661
});
17181662
const timeline = await flushHttp(client.getEventTimeline(timelineSet, THREAD_ROOT.event_id!));

‎spec/test-utils/thread.ts

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -138,9 +138,6 @@ export const mkThread = ({
138138
}
139139

140140
const thread = room.createThread(rootEvent.getId() ?? "", rootEvent, events, true);
141-
// So that we do not have to mock the thread loading
142-
thread.initialEventsFetched = true;
143-
thread.addEvents(events, true);
144141

145142
return { thread, rootEvent, events };
146143
};

‎spec/unit/room.spec.ts

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2475,11 +2475,11 @@ describe("Room", function () {
24752475

24762476
let prom = emitPromise(room, ThreadEvent.New);
24772477
room.addLiveEvents([randomMessage, threadRoot, threadResponse]);
2478-
const thread = await prom;
2478+
const thread: Thread = await prom;
24792479
await emitPromise(room, ThreadEvent.Update);
24802480

2481-
expect(thread.replyToEvent.event).toEqual(threadResponse.event);
2482-
expect(thread.replyToEvent.getContent().body).toBe(threadResponse.getContent().body);
2481+
expect(thread.replyToEvent!.event).toEqual(threadResponse.event);
2482+
expect(thread.replyToEvent!.getContent().body).toBe(threadResponse.getContent().body);
24832483

24842484
room.client.fetchRoomEvent = (eventId: string) =>
24852485
Promise.resolve({
@@ -2490,7 +2490,7 @@ describe("Room", function () {
24902490
[THREAD_RELATION_TYPE.name]: {
24912491
latest_event: {
24922492
...threadResponse.event,
2493-
content: threadResponseEdit.event.content,
2493+
content: threadResponseEdit.getContent()["m.new_content"],
24942494
},
24952495
count: 2,
24962496
current_user_participated: true,
@@ -2502,7 +2502,7 @@ describe("Room", function () {
25022502
prom = emitPromise(room, ThreadEvent.Update);
25032503
room.addLiveEvents([threadResponseEdit]);
25042504
await prom;
2505-
expect(thread.replyToEvent.getContent().body).toBe(threadResponseEdit.getContent()["m.new_content"].body);
2505+
expect(thread.replyToEvent!.getContent().body).toBe(threadResponseEdit.getContent()["m.new_content"].body);
25062506
});
25072507

25082508
it("Redactions to thread responses decrement the length", async () => {
@@ -2847,7 +2847,7 @@ describe("Room", function () {
28472847
expect(room.eventShouldLiveIn(reply2, events, roots).shouldLiveInThread).toBeFalsy();
28482848
});
28492849

2850-
it("should aggregate relations in thread event timeline set", () => {
2850+
it("should aggregate relations in thread event timeline set", async () => {
28512851
Thread.setServerSideSupport(FeatureSupport.Stable);
28522852
const threadRoot = mkMessage();
28532853
const rootReaction = mkReaction(threadRoot);
@@ -2856,9 +2856,10 @@ describe("Room", function () {
28562856

28572857
const events = [threadRoot, rootReaction, threadResponse, threadReaction];
28582858

2859+
const prom = emitPromise(room, ThreadEvent.New);
28592860
room.addLiveEvents(events);
2860-
2861-
const thread = threadRoot.getThread()!;
2861+
const thread = await prom;
2862+
expect(thread).toBe(threadRoot.getThread());
28622863
expect(thread.rootEvent).toBe(threadRoot);
28632864

28642865
const rootRelations = thread.timelineSet.relations

‎src/models/room.ts

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2090,9 +2090,7 @@ export class Room extends ReadReceipt<RoomEmittedEvents, RoomEventHandlerMap> {
20902090
// If we jump to an event in a thread where neither the event, nor the root,
20912091
// nor any thread event are loaded yet, we'll load the event as well as the thread root, create the thread,
20922092
// and pass the event through this.
2093-
for (const event of events) {
2094-
thread.setEventMetadata(event);
2095-
}
2093+
thread.addEvents(events, false);
20962094

20972095
// If we managed to create a thread and figure out its `id` then we can use it
20982096
this.threads.set(thread.id, thread);

0 commit comments

Comments
 (0)