Fix synthesizeReceipt (#2916)

spec/unit/notifications.spec.ts

@@ -37,7 +37,7 @@ let event: MatrixEvent;
 let threadEvent: MatrixEvent;
 
 const ROOM_ID = "!roomId:example.org";
-let THREAD_ID;
+let THREAD_ID: string;
 
 function mkPushAction(notify, highlight): IActionsObject {
     return {
@@ -76,7 +76,7 @@ describe("fixNotificationCountOnDecryption", () => {
             event: true,
         }, mockClient);
 
-        THREAD_ID = event.getId();
+        THREAD_ID = event.getId()!;
         threadEvent = mkEvent({
             type: EventType.RoomMessage,
             content: {
@@ -108,6 +108,16 @@ describe("fixNotificationCountOnDecryption", () => {
         expect(room.getUnreadNotificationCount(NotificationCountType.Highlight)).toBe(1);
     });
 
+    it("does not change the room count when there's no unread count", () => {
+        room.setUnreadNotificationCount(NotificationCountType.Total, 0);
+        room.setUnreadNotificationCount(NotificationCountType.Highlight, 0);
+
+        fixNotificationCountOnDecryption(mockClient, event);
+
+        expect(room.getRoomUnreadNotificationCount(NotificationCountType.Total)).toBe(0);
+        expect(room.getRoomUnreadNotificationCount(NotificationCountType.Highlight)).toBe(0);
+    });
+
     it("changes the thread count to highlight on decryption", () => {
         expect(room.getThreadUnreadNotificationCount(THREAD_ID, NotificationCountType.Total)).toBe(1);
         expect(room.getThreadUnreadNotificationCount(THREAD_ID, NotificationCountType.Highlight)).toBe(0);
@@ -118,6 +128,16 @@ describe("fixNotificationCountOnDecryption", () => {
         expect(room.getThreadUnreadNotificationCount(THREAD_ID, NotificationCountType.Highlight)).toBe(1);
     });
 
+    it("does not change the room count when there's no unread count", () => {
+        room.setThreadUnreadNotificationCount(THREAD_ID, NotificationCountType.Total, 0);
+        room.setThreadUnreadNotificationCount(THREAD_ID, NotificationCountType.Highlight, 0);
+
+        fixNotificationCountOnDecryption(mockClient, event);
+
+        expect(room.getThreadUnreadNotificationCount(THREAD_ID, NotificationCountType.Total)).toBe(0);
+        expect(room.getThreadUnreadNotificationCount(THREAD_ID, NotificationCountType.Highlight)).toBe(0);
+    });
+
     it("emits events", () => {
         const cb = jest.fn();
         room.on(RoomEvent.UnreadNotifications, cb);

spec/unit/read-receipt.spec.ts

@@ -20,6 +20,7 @@ import { MAIN_ROOM_TIMELINE, ReceiptType } from '../../src/@types/read_receipts'
 import { MatrixClient } from "../../src/client";
 import { Feature, ServerSupport } from '../../src/feature';
 import { EventType } from '../../src/matrix';
+import { synthesizeReceipt } from '../../src/models/read-receipt';
 import { encodeUri } from '../../src/utils';
 import * as utils from "../test-utils/test-utils";
 
@@ -175,4 +176,20 @@ describe("Read receipt", () => {
             await flushPromises();
         });
     });
+
+    describe("synthesizeReceipt", () => {
+        it.each([
+            { event: roomEvent, destinationId: MAIN_ROOM_TIMELINE },
+            { event: threadEvent, destinationId: threadEvent.threadRootId! },
+        ])("adds the receipt to $destinationId", ({ event, destinationId }) => {
+            const userId = "@bob:example.org";
+            const receiptType = ReceiptType.Read;
+
+            const fakeReadReceipt = synthesizeReceipt(userId, event, receiptType);
+
+            const content = fakeReadReceipt.getContent()[event.getId()!][receiptType][userId];
+
+            expect(content.thread_id).toEqual(destinationId);
+        });
+    });
 });

src/client.ts

@@ -5836,8 +5836,14 @@ export class MatrixClient extends TypedEventEmitter<EmittedEvents, ClientEventHa
             ).then(async (res) => {
                 const mapper = this.getEventMapper();
                 const matrixEvents = res.chunk.map(mapper);
-                for (const event of matrixEvents) {
-                    await eventTimeline.getTimelineSet()?.thread?.processEvent(event);
+
+                // Process latest events first
+                for (const event of matrixEvents.slice().reverse()) {
+                    await thread?.processEvent(event);
+                    const sender = event.getSender()!;
+                    if (!backwards || thread?.getEventReadUpTo(sender) === null) {
+                        room.addLocalEchoReceipt(sender, event, ReceiptType.Read);
+                    }
                 }
 
                 const newToken = res.next_batch;
@@ -9344,19 +9350,16 @@ export function fixNotificationCountOnDecryption(cli: MatrixClient, event: Matri
     if (!room || !cli.getUserId()) return;
 
     const isThreadEvent = !!event.threadRootId && !event.isThreadRoot;
-    const currentCount = (isThreadEvent
-        ? room.getThreadUnreadNotificationCount(
-            event.threadRootId,
-            NotificationCountType.Highlight,
-        )
-        : room.getUnreadNotificationCount(NotificationCountType.Highlight)) ?? 0;
+
+    const totalCount = room.getUnreadCountForEventContext(NotificationCountType.Total, event);
+    const currentCount = room.getUnreadCountForEventContext(NotificationCountType.Highlight, event);
 
     // Ensure the unread counts are kept up to date if the event is encrypted
     // We also want to make sure that the notification count goes up if we already
     // have encrypted events to avoid other code from resetting 'highlight' to zero.
     const oldHighlight = !!oldActions?.tweaks?.highlight;
     const newHighlight = !!actions?.tweaks?.highlight;
-    if (oldHighlight !== newHighlight || currentCount > 0) {
+    if ((oldHighlight !== newHighlight || currentCount > 0) && totalCount > 0) {
         // TODO: Handle mentions received while the client is offline
         // See also https://github.com/vector-im/element-web/issues/9069
         const hasReadEvent = isThreadEvent
@@ -9381,7 +9384,7 @@ export function fixNotificationCountOnDecryption(cli: MatrixClient, event: Matri
             // Fix 'Mentions Only' rooms from not having the right badge count
             const totalCount = (isThreadEvent
                 ? room.getThreadUnreadNotificationCount(event.threadRootId, NotificationCountType.Total)
-                : room.getUnreadNotificationCount(NotificationCountType.Total)) ?? 0;
+                : room.getRoomUnreadNotificationCount(NotificationCountType.Total)) ?? 0;
 
             if (totalCount < newCount) {
                 if (isThreadEvent) {

src/models/read-receipt.ts

@@ -33,7 +33,7 @@ export function synthesizeReceipt(userId: string, event: MatrixEvent, receiptTyp
                 [receiptType]: {
                     [userId]: {
                         ts: event.getTs(),
-                        threadId: event.threadRootId ?? MAIN_ROOM_TIMELINE,
+                        thread_id: event.threadRootId ?? MAIN_ROOM_TIMELINE,
                     },
                 },
             },

src/models/room.ts

@@ -1183,7 +1183,7 @@ export class Room extends ReadReceipt<RoomEmittedEvents, RoomEventHandlerMap> {
      *                  for this type.
      */
     public getUnreadNotificationCount(type = NotificationCountType.Total): number {
-        let count = this.notificationCounts[type] ?? 0;
+        let count = this.getRoomUnreadNotificationCount(type);
         if (this.client.canSupport.get(Feature.ThreadUnreadNotifications) !== ServerSupport.Unsupported) {
             for (const threadNotification of this.threadNotifications.values()) {
                 count += threadNotification[type] ?? 0;
@@ -1192,6 +1192,27 @@ export class Room extends ReadReceipt<RoomEmittedEvents, RoomEventHandlerMap> {
         return count;
     }
 
+    /**
+     * Get the notification for the event context (room or thread timeline)
+     */
+    public getUnreadCountForEventContext(type = NotificationCountType.Total, event: MatrixEvent): number {
+        const isThreadEvent = !!event.threadRootId && !event.isThreadRoot;
+
+        return (isThreadEvent
+            ? this.getThreadUnreadNotificationCount(event.threadRootId, type)
+            : this.getRoomUnreadNotificationCount(type)) ?? 0;
+    }
+
+    /**
+     * Get one of the notification counts for this room
+     * @param {String} type The type of notification count to get. default: 'total'
+     * @return {Number} The notification count, or undefined if there is no count
+     *                  for this type.
+     */
+    public getRoomUnreadNotificationCount(type = NotificationCountType.Total): number {
+        return this.notificationCounts[type] ?? 0;
+    }
+
     /**
      * @experimental
      * Get one of the notification counts for a thread
@@ -1758,20 +1779,17 @@ export class Room extends ReadReceipt<RoomEmittedEvents, RoomEventHandlerMap> {
             let latestMyThreadsRootEvent: MatrixEvent | undefined;
             const roomState = this.getLiveTimeline().getState(EventTimeline.FORWARDS);
             for (const rootEvent of threadRoots) {
-                this.threadsTimelineSets[0]?.addLiveEvent(rootEvent, {
+                const opts = {
                     duplicateStrategy: DuplicateStrategy.Ignore,
                     fromCache: false,
                     roomState,
-                });
+                };
+                this.threadsTimelineSets[0]?.addLiveEvent(rootEvent, opts);
 
                 const threadRelationship = rootEvent
                     .getServerAggregatedRelation<IThreadBundledRelationship>(THREAD_RELATION_TYPE.name);
                 if (threadRelationship?.current_user_participated) {
-                    this.threadsTimelineSets[1]?.addLiveEvent(rootEvent, {
-                        duplicateStrategy: DuplicateStrategy.Ignore,
-                        fromCache: false,
-                        roomState,
-                    });
+                    this.threadsTimelineSets[1]?.addLiveEvent(rootEvent, opts);
                     latestMyThreadsRootEvent = rootEvent;
                 }
             }

src/models/thread.ts

@@ -123,7 +123,7 @@ export class Thread extends ReadReceipt<EmittedEvents, EventHandlerMap> {
         this.room.on(MatrixEventEvent.BeforeRedaction, this.onBeforeRedaction);
         this.room.on(RoomEvent.Redaction, this.onRedaction);
         this.room.on(RoomEvent.LocalEchoUpdated, this.onEcho);
-        this.timelineSet.on(RoomEvent.Timeline, this.onEcho);
+        this.timelineSet.on(RoomEvent.Timeline, this.onTimelineEvent);
 
         // even if this thread is thought to be originating from this client, we initialise it as we may be in a
         // gappy sync and a thread around this event may already exist.
@@ -192,6 +192,18 @@ export class Thread extends ReadReceipt<EmittedEvents, EventHandlerMap> {
         }
     };
 
+    private onTimelineEvent = (
+        event: MatrixEvent,
+        room: Room | undefined,
+        toStartOfTimeline: boolean | undefined,
+    ): void => {
+        // Add a synthesized receipt when paginating forward in the timeline
+        if (!toStartOfTimeline) {
+            room!.addLocalEchoReceipt(event.getSender()!, event, ReceiptType.Read);
+        }
+        this.onEcho(event);
+    };
+
     private onEcho = async (event: MatrixEvent): Promise<void> => {
         if (event.threadRootId !== this.id) return; // ignore echoes for other timelines
         if (this.lastEvent === event) return;