Don't reset unread count when adding a synthetic receipt

Fixes #3684 and there are lots more details about why we chose this solution in that issue.
matrix-org · Sep 6, 2023 · 3b485c3 · 3b485c3
1 parent 88ec0e3
commit 3b485c3
Show file tree

Hide file tree

Showing 2 changed files with 82 additions and 1 deletion.
diff --git a/spec/unit/room.spec.ts b/spec/unit/room.spec.ts
@@ -1453,6 +1453,87 @@ describe("Room", function () {
         }
 
         describe("addReceipt", function () {
+            describe("resets the unread count", () => {
+                const event1 = utils.mkMessage({ room: roomId, user: userA, msg: "1", event: true });
+                const event2 = utils.mkMessage({ room: roomId, user: userA, msg: "2", event: true });
+
+                it("should reset the unread count when our non-synthetic receipt points to the latest event", () => {
+                    // Given a room with 2 events, and an unread count set.
+                    room.client.isInitialSyncComplete = jest.fn().mockReturnValue(true);
+                    room.timeline = [event1, event2];
+                    room.setUnread(NotificationCountType.Total, 45);
+                    room.setUnread(NotificationCountType.Highlight, 57);
+                    // Sanity check:
+                    expect(room.getUnreadNotificationCount(NotificationCountType.Total)).toEqual(45);
+                    expect(room.getUnreadNotificationCount(NotificationCountType.Highlight)).toEqual(57);
+
+                    // When I receive a receipt for me for the last event
+                    const receipt = mkReceipt(roomId, [mkRecord(event2.getId()!, "m.read", userA, 123)]);
+                    room.addReceipt(receipt);
+
+                    // Then the count is set to 0
+                    expect(room.getUnreadNotificationCount(NotificationCountType.Total)).toEqual(0);
+                    expect(room.getUnreadNotificationCount(NotificationCountType.Highlight)).toEqual(0);
+                });
+
+                it("should not reset the unread count when someone else's receipt points to the latest event", () => {
+                    // Given a room with 2 events, and an unread count set.
+                    room.client.isInitialSyncComplete = jest.fn().mockReturnValue(true);
+                    room.timeline = [event1, event2];
+                    room.setUnread(NotificationCountType.Total, 45);
+                    room.setUnread(NotificationCountType.Highlight, 57);
+                    // Sanity check:
+                    expect(room.getUnreadNotificationCount(NotificationCountType.Total)).toEqual(45);
+                    expect(room.getUnreadNotificationCount(NotificationCountType.Highlight)).toEqual(57);
+
+                    // When I receive a receipt for someone else for the last event
+                    const receipt = mkReceipt(roomId, [mkRecord(event2.getId()!, "m.read", userB, 123)]);
+                    room.addReceipt(receipt);
+
+                    // Then the count is unchanged because it's not my receipt
+                    expect(room.getUnreadNotificationCount(NotificationCountType.Total)).toEqual(45);
+                    expect(room.getUnreadNotificationCount(NotificationCountType.Highlight)).toEqual(57);
+                });
+
+                it("should not reset the unread count when our non-synthetic receipt points to an earlier event", () => {
+                    // Given a room with 2 events, and an unread count set.
+                    room.client.isInitialSyncComplete = jest.fn().mockReturnValue(true);
+                    room.timeline = [event1, event2];
+                    room.setUnread(NotificationCountType.Total, 45);
+                    room.setUnread(NotificationCountType.Highlight, 57);
+                    // Sanity check:
+                    expect(room.getUnreadNotificationCount(NotificationCountType.Total)).toEqual(45);
+                    expect(room.getUnreadNotificationCount(NotificationCountType.Highlight)).toEqual(57);
+
+                    // When I receive a receipt for me for an earlier event
+                    const receipt = mkReceipt(roomId, [mkRecord(event1.getId()!, "m.read", userA, 123)]);
+                    room.addReceipt(receipt);
+
+                    // Then the count is unchanged because it wasn't the latest event
+                    expect(room.getUnreadNotificationCount(NotificationCountType.Total)).toEqual(45);
+                    expect(room.getUnreadNotificationCount(NotificationCountType.Highlight)).toEqual(57);
+                });
+
+                it("should not reset the unread count when our a synthetic receipt points to the latest event", () => {
+                    // Given a room with 2 events, and an unread count set.
+                    room.client.isInitialSyncComplete = jest.fn().mockReturnValue(true);
+                    room.timeline = [event1, event2];
+                    room.setUnread(NotificationCountType.Total, 45);
+                    room.setUnread(NotificationCountType.Highlight, 57);
+                    // Sanity check:
+                    expect(room.getUnreadNotificationCount(NotificationCountType.Total)).toEqual(45);
+                    expect(room.getUnreadNotificationCount(NotificationCountType.Highlight)).toEqual(57);
+
+                    // When I receive a synthetic receipt for me for the last event
+                    const receipt = mkReceipt(roomId, [mkRecord(event2.getId()!, "m.read", userA, 123)]);
+                    room.addReceipt(receipt, true);
+
+                    // Then the count is unchanged because the receipt was synthetic
+                    expect(room.getUnreadNotificationCount(NotificationCountType.Total)).toEqual(45);
+                    expect(room.getUnreadNotificationCount(NotificationCountType.Highlight)).toEqual(57);
+                });
+            });
+
             it("should store the receipt so it can be obtained via getReceiptsForEvent", function () {
                 const ts = 13787898424;
                 room.addReceipt(mkReceipt(roomId, [mkRecord(eventToAck.getId()!, "m.read", userB, ts)]));

diff --git a/src/models/room.ts b/src/models/room.ts
@@ -2952,7 +2952,7 @@ export class Room extends ReadReceipt<RoomEmittedEvents, RoomEventHandlerMap> {
                         // from synapse
                         // This needs to be done after the initial sync as we do not want this
                         // logic to run whilst the room is being initialised
-                        if (this.client.isInitialSyncComplete() && userId === this.client.getUserId()) {
+                        if (!synthetic && this.client.isInitialSyncComplete() && userId === this.client.getUserId()) {
                             const lastEvent = receiptDestination.timeline[receiptDestination.timeline.length - 1];
                             if (lastEvent && eventId === lastEvent.getId() && userId === lastEvent.getSender()) {
                                 receiptDestination.setUnread(NotificationCountType.Total, 0);