diff --git a/spec/integ/matrix-client-syncing.spec.ts b/spec/integ/matrix-client-syncing.spec.ts index dbf038d91..3e97c648b 100644 --- a/spec/integ/matrix-client-syncing.spec.ts +++ b/spec/integ/matrix-client-syncing.spec.ts @@ -1648,6 +1648,99 @@ describe("MatrixClient syncing", () => { }); }); + it("should zero total notifications for threads when absent from the notifications object", async () => { + syncData.rooms.join[roomOne][UNREAD_THREAD_NOTIFICATIONS.name] = { + [THREAD_ID]: { + highlight_count: 2, + notification_count: 5, + }, + }; + + httpBackend!.when("GET", "/sync").respond(200, syncData); + + client!.startClient(); + + await Promise.all([httpBackend!.flushAllExpected(), awaitSyncEvent()]); + + const room = client!.getRoom(roomOne); + + expect(room!.getThreadUnreadNotificationCount(THREAD_ID, NotificationCountType.Total)).toBe(5); + + syncData.rooms.join[roomOne][UNREAD_THREAD_NOTIFICATIONS.name] = {}; + + httpBackend!.when("GET", "/sync").respond(200, syncData); + + await Promise.all([httpBackend!.flushAllExpected(), awaitSyncEvent()]); + + expect(room!.getThreadUnreadNotificationCount(THREAD_ID, NotificationCountType.Total)).toBe(0); + }); + + it("should zero highlight notifications for threads in encrypted rooms", async () => { + syncData.rooms.join[roomOne][UNREAD_THREAD_NOTIFICATIONS.name] = { + [THREAD_ID]: { + highlight_count: 2, + notification_count: 5, + }, + }; + + httpBackend!.when("GET", "/sync").respond(200, syncData); + + client!.startClient(); + + await Promise.all([httpBackend!.flushAllExpected(), awaitSyncEvent()]); + + const room = client!.getRoom(roomOne); + + expect(room!.getThreadUnreadNotificationCount(THREAD_ID, NotificationCountType.Total)).toBe(5); + + syncData.rooms.join[roomOne][UNREAD_THREAD_NOTIFICATIONS.name] = { + [THREAD_ID]: { + highlight_count: 0, + notification_count: 0, + }, + }; + + httpBackend!.when("GET", "/sync").respond(200, syncData); + + await Promise.all([httpBackend!.flushAllExpected(), awaitSyncEvent()]); + + expect(room!.getThreadUnreadNotificationCount(THREAD_ID, NotificationCountType.Highlight)).toBe(0); + }); + + it("should not zero highlight notifications for threads in encrypted rooms", async () => { + syncData.rooms.join[roomOne][UNREAD_THREAD_NOTIFICATIONS.name] = { + [THREAD_ID]: { + highlight_count: 2, + notification_count: 5, + }, + }; + + httpBackend!.when("GET", "/sync").respond(200, syncData); + + client!.startClient(); + + await Promise.all([httpBackend!.flushAllExpected(), awaitSyncEvent()]); + + const room = client!.getRoom(roomOne); + room!.hasEncryptionStateEvent = jest.fn().mockReturnValue(true); + + expect(room!.getThreadUnreadNotificationCount(THREAD_ID, NotificationCountType.Total)).toBe(5); + + syncData.rooms.join[roomOne][UNREAD_THREAD_NOTIFICATIONS.name] = { + [THREAD_ID]: { + highlight_count: 0, + notification_count: 0, + }, + }; + + httpBackend!.when("GET", "/sync").respond(200, syncData); + + await Promise.all([httpBackend!.flushAllExpected(), awaitSyncEvent()]); + + expect(room!.getThreadUnreadNotificationCount(THREAD_ID, NotificationCountType.Total)).toBe(0); + expect(room!.getThreadUnreadNotificationCount(THREAD_ID, NotificationCountType.Highlight)).toBe(2); + }); + it("caches unknown threads receipts and replay them when the thread is created", async () => { const THREAD_ID = "$unknownthread:localhost"; diff --git a/spec/unit/room.spec.ts b/spec/unit/room.spec.ts index 36d9d0877..6ca454ef8 100644 --- a/spec/unit/room.spec.ts +++ b/spec/unit/room.spec.ts @@ -3548,7 +3548,7 @@ describe("Room", function () { expect(room.threadsAggregateNotificationType).toBe(NotificationCountType.Highlight); - room.resetThreadUnreadNotificationCount(); + room.resetThreadUnreadNotificationCountFromSync(); expect(room.threadsAggregateNotificationType).toBe(null); @@ -3565,16 +3565,6 @@ describe("Room", function () { expect(room.threadsAggregateNotificationType).toBe(NotificationCountType.Highlight); }); - it("partially resets room notifications", () => { - room.setThreadUnreadNotificationCount("123", NotificationCountType.Total, 666); - room.setThreadUnreadNotificationCount("456", NotificationCountType.Highlight, 123); - - room.resetThreadUnreadNotificationCount(["123"]); - - expect(room.getThreadUnreadNotificationCount("123", NotificationCountType.Total)).toBe(666); - expect(room.getThreadUnreadNotificationCount("456", NotificationCountType.Highlight)).toBe(0); - }); - it("emits event on notifications reset", () => { const cb = jest.fn(); @@ -3583,7 +3573,7 @@ describe("Room", function () { room.setThreadUnreadNotificationCount("123", NotificationCountType.Total, 666); room.setThreadUnreadNotificationCount("456", NotificationCountType.Highlight, 123); - room.resetThreadUnreadNotificationCount(); + room.resetThreadUnreadNotificationCountFromSync(); expect(cb).toHaveBeenLastCalledWith(); }); @@ -3605,10 +3595,10 @@ describe("Room", function () { }); it("lets you reset", () => { - room.setThreadUnreadNotificationCount("123", NotificationCountType.Highlight, 1); + room.setThreadUnreadNotificationCount("123", NotificationCountType.Total, 1); expect(room.hasThreadUnreadNotification()).toBe(true); - room.resetThreadUnreadNotificationCount(); + room.resetThreadUnreadNotificationCountFromSync(); expect(room.hasThreadUnreadNotification()).toBe(false); }); @@ -3636,12 +3626,38 @@ describe("Room", function () { it("allows reset", () => { room.setThreadUnreadNotificationCount("$123", NotificationCountType.Total, 1); room.setThreadUnreadNotificationCount("$456", NotificationCountType.Total, 1); + expect(room.threadsAggregateNotificationType).toBe(NotificationCountType.Total); + + room.resetThreadUnreadNotificationCountFromSync(); + + expect(room.threadsAggregateNotificationType).toBeNull(); + }); + + it("retains highlight for encrypted rooms on reset", () => { + room.hasEncryptionStateEvent = jest.fn().mockReturnValue(true); + + room.setThreadUnreadNotificationCount("$123", NotificationCountType.Total, 2); + room.setThreadUnreadNotificationCount("$456", NotificationCountType.Total, 1); room.setThreadUnreadNotificationCount("$123", NotificationCountType.Highlight, 1); expect(room.threadsAggregateNotificationType).toBe(NotificationCountType.Highlight); - room.resetThreadUnreadNotificationCount(); + room.resetThreadUnreadNotificationCountFromSync(); - expect(room.threadsAggregateNotificationType).toBeNull(); + expect(room.threadsAggregateNotificationType).toBe(NotificationCountType.Highlight); + }); + + it("resets highlight for unencrypted rooms on reset", () => { + room.hasEncryptionStateEvent = jest.fn().mockReturnValue(false); + + room.setThreadUnreadNotificationCount("$123", NotificationCountType.Total, 2); + room.setThreadUnreadNotificationCount("$456", NotificationCountType.Total, 1); + room.setThreadUnreadNotificationCount("$123", NotificationCountType.Highlight, 1); + expect(room.threadsAggregateNotificationType).toBe(NotificationCountType.Highlight); + + room.resetThreadUnreadNotificationCountFromSync(); + + expect(room.threadsAggregateNotificationType).toBe(null); + expect(room.getThreadUnreadNotificationCount("$123", NotificationCountType.Highlight)).toBe(0); }); }); diff --git a/src/models/room.ts b/src/models/room.ts index 456c6ba54..f36885809 100644 --- a/src/models/room.ts +++ b/src/models/room.ts @@ -1609,18 +1609,31 @@ export class Room extends ReadReceipt { } /** - * Resets the thread notifications for this room + * Resets the total thread notifications for all threads in this room to zero, + * excluding any threads whose IDs are given in `exceptThreadIds`. + * + * If the room is not encrypted, also resets the highlight notification count to zero + * for the same set of threads. + * + * This is intended for use from the sync code since we calculate highlight notification + * counts locally from decrypted messages. We want to partially trust the total from the + * server such that we clear notifications when read receipts arrive. The weird name is + * intended to reflect this. You probably do not want to use this. + * + * @param exceptThreadIds - The thread IDs to exclude from the reset. */ - public resetThreadUnreadNotificationCount(notificationsToKeep?: string[]): void { - if (notificationsToKeep) { - for (const [threadId] of this.threadNotifications) { - if (!notificationsToKeep.includes(threadId)) { - this.threadNotifications.delete(threadId); + public resetThreadUnreadNotificationCountFromSync(exceptThreadIds: string[] = []): void { + const isEncrypted = this.hasEncryptionStateEvent(); + + for (const [threadId, notifs] of this.threadNotifications) { + if (!exceptThreadIds.includes(threadId)) { + notifs.total = 0; + if (!isEncrypted) { + notifs.highlight = 0; } } - } else { - this.threadNotifications.clear(); } + this.emit(RoomEvent.UnreadNotifications); } diff --git a/src/sync.ts b/src/sync.ts index 8fe60698e..92015bd4e 100644 --- a/src/sync.ts +++ b/src/sync.ts @@ -1355,11 +1355,11 @@ export class SyncApi { const unreadThreadNotifications = joinObj[UNREAD_THREAD_NOTIFICATIONS.name] ?? joinObj[UNREAD_THREAD_NOTIFICATIONS.altName!]; if (unreadThreadNotifications) { - // Only partially reset unread notification - // We want to keep the client-generated count. Particularly important - // for encrypted room that refresh their notification count on event - // decryption - room.resetThreadUnreadNotificationCount(Object.keys(unreadThreadNotifications)); + // This mirrors the logic above for rooms: take the *total* notification count from + // the server for unencrypted rooms or is it's zero. Any threads not present in this + // object implicitly have zero notifications, so start by clearing the total counts + // for all such threads. + room.resetThreadUnreadNotificationCountFromSync(Object.keys(unreadThreadNotifications)); for (const [threadId, unreadNotification] of Object.entries(unreadThreadNotifications)) { if (!encrypted || unreadNotification.notification_count === 0) { room.setThreadUnreadNotificationCount( @@ -1380,7 +1380,7 @@ export class SyncApi { } } } else { - room.resetThreadUnreadNotificationCount(); + room.resetThreadUnreadNotificationCountFromSync(); } joinObj.timeline = joinObj.timeline || ({} as ITimeline);