Aggregate relations regardless of whether event fits into the timeline (#3496)

spec/unit/event-timeline-set.spec.ts

@@ -14,6 +14,8 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
+import { mocked } from "jest-mock";
+
 import * as utils from "../test-utils/test-utils";
 import {
     DuplicateStrategy,
@@ -160,6 +162,33 @@ describe("EventTimelineSet", () => {
                 eventTimelineSet.addEventToTimeline(messageEvent, liveTimeline, true, false);
             }).not.toThrow();
         });
+
+        it("should aggregate relations which belong to unknown timeline without adding them to any timeline", () => {
+            // If threads are disabled all events go into the main timeline
+            mocked(client.supportsThreads).mockReturnValue(true);
+            const reactionEvent = utils.mkReaction(messageEvent, client, client.getSafeUserId(), roomId);
+
+            const liveTimeline = eventTimelineSet.getLiveTimeline();
+            expect(liveTimeline.getEvents().length).toStrictEqual(0);
+            eventTimelineSet.addEventToTimeline(reactionEvent, liveTimeline, {
+                toStartOfTimeline: true,
+            });
+            expect(liveTimeline.getEvents().length).toStrictEqual(0);
+
+            eventTimelineSet.addEventToTimeline(messageEvent, liveTimeline, {
+                toStartOfTimeline: true,
+            });
+            expect(liveTimeline.getEvents()).toHaveLength(1);
+            const [event] = liveTimeline.getEvents();
+            const reactions = eventTimelineSet.relations!.getChildEventsForEvent(
+                event.getId()!,
+                "m.annotation",
+                "m.reaction",
+            )!;
+            const relations = reactions.getRelations();
+            expect(relations).toHaveLength(1);
+            expect(relations[0].getId()).toBe(reactionEvent.getId());
+        });
     });
 
     describe("addEventToTimeline (thread timeline)", () => {

src/models/event-timeline-set.ts

@@ -721,13 +721,17 @@ export class EventTimelineSet extends TypedEventEmitter<EmittedEvents, EventTime
                 "in timelineSet(threadId=${this.thread?.id})`);
         }
 
+        const eventId = event.getId()!;
+        this.relations.aggregateParentEvent(event);
+        this.relations.aggregateChildEvent(event, this);
+
         // Make sure events don't get mixed in timelines they shouldn't be in (e.g. a
         // threaded message should not be in the main timeline).
         //
         // We can only run this check for timelines with a `room` because `canContain`
         // requires it
         if (this.room && !this.canContain(event)) {
-            let eventDebugString = `event=${event.getId()}`;
+            let eventDebugString = `event=${eventId}`;
             if (event.threadRootId) {
                 eventDebugString += `(belongs to thread=${event.threadRootId})`;
             }
@@ -738,7 +742,6 @@ export class EventTimelineSet extends TypedEventEmitter<EmittedEvents, EventTime
             return;
         }
 
-        const eventId = event.getId()!;
         timeline.addEvent(event, {
             toStartOfTimeline,
             roomState,
@@ -746,9 +749,6 @@ export class EventTimelineSet extends TypedEventEmitter<EmittedEvents, EventTime
         });
         this._eventIdToTimeline.set(eventId, timeline);
 
-        this.relations.aggregateParentEvent(event);
-        this.relations.aggregateChildEvent(event, this);
-
         const data: IRoomTimelineData = {
             timeline: timeline,
             liveEvent: !toStartOfTimeline && timeline == this.liveTimeline && !fromCache,
@@ -782,13 +782,17 @@ export class EventTimelineSet extends TypedEventEmitter<EmittedEvents, EventTime
                 "in timelineSet(threadId=${this.thread?.id})`);
         }
 
+        const eventId = event.getId()!;
+        this.relations.aggregateParentEvent(event);
+        this.relations.aggregateChildEvent(event, this);
+
         // Make sure events don't get mixed in timelines they shouldn't be in (e.g. a
         // threaded message should not be in the main timeline).
         //
         // We can only run this check for timelines with a `room` because `canContain`
         // requires it
         if (this.room && !this.canContain(event)) {
-            let eventDebugString = `event=${event.getId()}`;
+            let eventDebugString = `event=${eventId}`;
             if (event.threadRootId) {
                 eventDebugString += `(belongs to thread=${event.threadRootId})`;
             }
@@ -830,13 +834,9 @@ export class EventTimelineSet extends TypedEventEmitter<EmittedEvents, EventTime
         // If we got to the end of the loop, insertIndex points at the end of
         // the list.
 
-        const eventId = event.getId()!;
         timeline.insertEvent(event, insertIndex, roomState);
         this._eventIdToTimeline.set(eventId, timeline);
 
-        this.relations.aggregateParentEvent(event);
-        this.relations.aggregateChildEvent(event, this);
-
         const data: IRoomTimelineData = {
             timeline: timeline,
             liveEvent: timeline == this.liveTimeline,

src/models/room.ts

@@ -2103,7 +2103,6 @@ export class Room extends ReadReceipt<RoomEmittedEvents, RoomEventHandlerMap> {
         threadId?: string;
     } {
         if (!this.client?.supportsThreads()) {
-            logger.debug(`Room::eventShouldLiveIn: eventId=${event.getId()} client does not support threads`);
             return {
                 shouldLiveInRoom: true,
                 shouldLiveInThread: false,
@@ -2112,11 +2111,6 @@ export class Room extends ReadReceipt<RoomEmittedEvents, RoomEventHandlerMap> {
 
         // A thread root is always shown in both timelines
         if (event.isThreadRoot || roots?.has(event.getId()!)) {
-            if (event.isThreadRoot) {
-                logger.debug(`Room::eventShouldLiveIn: eventId=${event.getId()} isThreadRoot is true`);
-            } else {
-                logger.debug(`Room::eventShouldLiveIn: eventId=${event.getId()} is a known thread root`);
-            }
             return {
                 shouldLiveInRoom: true,
                 shouldLiveInThread: true,
@@ -2127,9 +2121,6 @@ export class Room extends ReadReceipt<RoomEmittedEvents, RoomEventHandlerMap> {
         // A thread relation (1st and 2nd order) is always only shown in a thread
         const threadRootId = event.threadRootId;
         if (threadRootId != undefined) {
-            logger.debug(
-                `Room::eventShouldLiveIn: eventId=${event.getId()} threadRootId=${threadRootId} is part of a thread`,
-            );
             return {
                 shouldLiveInRoom: false,
                 shouldLiveInThread: true,
@@ -2141,9 +2132,6 @@ export class Room extends ReadReceipt<RoomEmittedEvents, RoomEventHandlerMap> {
         let parentEvent: MatrixEvent | undefined;
         if (parentEventId) {
             parentEvent = this.findEventById(parentEventId) ?? events?.find((e) => e.getId() === parentEventId);
-            logger.debug(
-                `Room::eventShouldLiveIn: eventId=${event.getId()} parentEventId=${parentEventId} found=${!!parentEvent}`,
-            );
         }
 
         // Treat relations and redactions as extensions of their parents so evaluate parentEvent instead
@@ -2152,7 +2140,6 @@ export class Room extends ReadReceipt<RoomEmittedEvents, RoomEventHandlerMap> {
         }
 
         if (!event.isRelation()) {
-            logger.debug(`Room::eventShouldLiveIn: eventId=${event.getId()} not a relation`);
             return {
                 shouldLiveInRoom: true,
                 shouldLiveInThread: false,
@@ -2161,11 +2148,6 @@ export class Room extends ReadReceipt<RoomEmittedEvents, RoomEventHandlerMap> {
 
         // Edge case where we know the event is a relation but don't have the parentEvent
         if (roots?.has(event.relationEventId!)) {
-            logger.debug(
-                `Room::eventShouldLiveIn: eventId=${event.getId()} relationEventId=${
-                    event.relationEventId
-                } is a known root`,
-            );
             return {
                 shouldLiveInRoom: true,
                 shouldLiveInThread: true,
@@ -2176,7 +2158,6 @@ export class Room extends ReadReceipt<RoomEmittedEvents, RoomEventHandlerMap> {
         // We've exhausted all scenarios,
         // we cannot assume that it lives in the main timeline as this may be a relation for an unknown thread
         // adding the event in the wrong timeline causes stuck notifications and can break ability to send read receipts
-        logger.debug(`Room::eventShouldLiveIn: eventId=${event.getId()} belongs to an unknown timeline`);
         return {
             shouldLiveInRoom: false,
             shouldLiveInThread: false,