From 54d868b39730a59688f3d92276edfd93b9e50681 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Wed, 21 Jun 2023 14:00:38 +0100 Subject: [PATCH 1/4] Aggregate relations regardless of whether event fits into the timeline Relations are not timeline specific --- src/models/event-timeline-set.ts | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/models/event-timeline-set.ts b/src/models/event-timeline-set.ts index a003f136239..cc41e543c44 100644 --- a/src/models/event-timeline-set.ts +++ b/src/models/event-timeline-set.ts @@ -721,13 +721,17 @@ export class EventTimelineSet extends TypedEventEmitter Date: Wed, 21 Jun 2023 14:05:40 +0100 Subject: [PATCH 2/4] Remove debug logging --- src/models/room.ts | 19 ------------------- 1 file changed, 19 deletions(-) diff --git a/src/models/room.ts b/src/models/room.ts index 713e5470e31..a73c750bad3 100644 --- a/src/models/room.ts +++ b/src/models/room.ts @@ -2103,7 +2103,6 @@ export class Room extends ReadReceipt { 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 { // 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 { // 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 { 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 { } 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 { // 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 { // 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, From 6d5e15fd2403f1e8e27a711d4cd76c83031c6c22 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Thu, 22 Jun 2023 10:50:49 +0100 Subject: [PATCH 3/4] Add test --- spec/unit/event-timeline-set.spec.ts | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/spec/unit/event-timeline-set.spec.ts b/spec/unit/event-timeline-set.spec.ts index b5445a03397..f317dd56562 100644 --- a/spec/unit/event-timeline-set.spec.ts +++ b/spec/unit/event-timeline-set.spec.ts @@ -28,6 +28,7 @@ import { } from "../../src"; import { Thread } from "../../src/models/thread"; import { ReEmitter } from "../../src/ReEmitter"; +import { mocked } from "jest-mock"; describe("EventTimelineSet", () => { const roomId = "!foo:bar"; @@ -160,6 +161,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)", () => { From 3bd2b9b7ee85636eaa0afd741272e3ba216c4272 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Thu, 22 Jun 2023 10:54:41 +0100 Subject: [PATCH 4/4] Delint --- spec/unit/event-timeline-set.spec.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/spec/unit/event-timeline-set.spec.ts b/spec/unit/event-timeline-set.spec.ts index f317dd56562..a817127569c 100644 --- a/spec/unit/event-timeline-set.spec.ts +++ b/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, @@ -28,7 +30,6 @@ import { } from "../../src"; import { Thread } from "../../src/models/thread"; import { ReEmitter } from "../../src/ReEmitter"; -import { mocked } from "jest-mock"; describe("EventTimelineSet", () => { const roomId = "!foo:bar";