Tweak thread creation & event adding to fix bugs around relations (#2369

) * Remove legacy code which caused threads to begin life with too many events * Update tests & behaviour
matrix-org · May 16, 2022 · ba1f6ff · ba1f6ff
1 parent 3e4f02b
commit ba1f6ff
Show file tree

Hide file tree

Showing 3 changed files with 17 additions and 7 deletions.
diff --git a/spec/unit/room.spec.ts b/spec/unit/room.spec.ts
@@ -1937,6 +1937,15 @@ describe("Room", function() {
             expect(() => room.createThread(rootEvent.getId(), rootEvent, [])).not.toThrow();
         });
 
+        it("creating thread from edited event should not conflate old versions of the event", () => {
+            const message = mkMessage();
+            const edit = mkEdit(message);
+            message.makeReplaced(edit);
+
+            const thread = room.createThread("$000", message, [], true);
+            expect(thread).toHaveLength(0);
+        });
+
         it("Edits update the lastReply event", async () => {
             room.client.supportsExperimentalThreads = () => true;
 
@@ -2036,17 +2045,15 @@ describe("Room", function() {
                 },
             });
 
-            let prom = emitPromise(room, ThreadEvent.New);
+            const prom = emitPromise(room, ThreadEvent.New);
             room.addLiveEvents([threadRoot, threadResponse1, threadResponse2, threadResponse2Reaction]);
             const thread = await prom;
 
             expect(thread).toHaveLength(2);
             expect(thread.replyToEvent.getId()).toBe(threadResponse2.getId());
 
-            prom = emitPromise(thread, ThreadEvent.Update);
             const threadResponse2ReactionRedaction = mkRedaction(threadResponse2Reaction);
             room.addLiveEvents([threadResponse2ReactionRedaction]);
-            await prom;
             expect(thread).toHaveLength(2);
             expect(thread.replyToEvent.getId()).toBe(threadResponse2.getId());
         });

diff --git a/src/models/room.ts b/src/models/room.ts
@@ -1659,8 +1659,10 @@ export class Room extends TypedEventEmitter<EmittedEvents, RoomEventHandlerMap>
         if (rootEvent) {
             const tl = this.getTimelineForEvent(rootEvent.getId());
             const relatedEvents = tl?.getTimelineSet().getAllRelationsEventForEvent(rootEvent.getId());
-            if (relatedEvents) {
-                events = events.concat(relatedEvents);
+            if (relatedEvents?.length) {
+                // Include all relations of the root event, given it'll be visible in both timelines,
+                // except `m.replace` as that will already be applied atop the event using `MatrixEvent::makeReplaced`
+                events = events.concat(relatedEvents.filter(e => !e.isRelation(RelationType.Replace)));
             }
         }
 

diff --git a/src/models/thread.ts b/src/models/thread.ts
@@ -221,9 +221,10 @@ export class Thread extends TypedEventEmitter<EmittedEvents, EventHandlerMap> {
             this._currentUserParticipated = true;
         }
 
-        // Add all annotations and replace relations to the timeline so that the relations are processed accordingly
         if ([RelationType.Annotation, RelationType.Replace].includes(event.getRelation()?.rel_type as RelationType)) {
-            this.addEventToTimeline(event, toStartOfTimeline);
+            // Apply annotations and replace relations to the relations of the timeline only
+            this.timelineSet.setRelationsTarget(event);
+            this.timelineSet.aggregateRelations(event);
             return;
         }