diff --git a/spec/unit/notifications.spec.ts b/spec/unit/notifications.spec.ts index aefe5777a53..a6ced4e17e9 100644 --- a/spec/unit/notifications.spec.ts +++ b/spec/unit/notifications.spec.ts @@ -134,7 +134,7 @@ describe("fixNotificationCountOnDecryption", () => { fixNotificationCountOnDecryption(mockClient, event); - expect(room.getUnreadNotificationCount(NotificationCountType.Total)).toBe(2); + expect(room.getUnreadNotificationCount(NotificationCountType.Total)).toBe(3); expect(room.getUnreadNotificationCount(NotificationCountType.Highlight)).toBe(1); }); @@ -154,7 +154,7 @@ describe("fixNotificationCountOnDecryption", () => { fixNotificationCountOnDecryption(mockClient, threadEvent); - expect(room.getThreadUnreadNotificationCount(THREAD_ID, NotificationCountType.Total)).toBe(1); + expect(room.getThreadUnreadNotificationCount(THREAD_ID, NotificationCountType.Total)).toBe(2); expect(room.getThreadUnreadNotificationCount(THREAD_ID, NotificationCountType.Highlight)).toBe(1); }); @@ -192,6 +192,31 @@ describe("fixNotificationCountOnDecryption", () => { expect(room.getThreadUnreadNotificationCount(THREAD_ID, NotificationCountType.Highlight)).toBe(0); }); + it("does not change the total room count when an event is marked as non-notifying", () => { + room.setThreadUnreadNotificationCount(THREAD_ID, NotificationCountType.Total, 0); + room.setUnreadNotificationCount(NotificationCountType.Total, 0); + room.setUnreadNotificationCount(NotificationCountType.Highlight, 0); + + event.getPushActions = jest.fn().mockReturnValue(mkPushAction(true, false)); + mockClient.getPushActionsForEvent = jest.fn().mockReturnValue(mkPushAction(false, false)); + + fixNotificationCountOnDecryption(mockClient, event); + expect(room.getUnreadNotificationCount(NotificationCountType.Total)).toBe(0); + expect(room.getUnreadNotificationCount(NotificationCountType.Highlight)).toBe(0); + }); + + it("does not change the total room count when a threaded event is marked as non-notifying", () => { + room.setThreadUnreadNotificationCount(THREAD_ID, NotificationCountType.Total, 0); + room.setThreadUnreadNotificationCount(THREAD_ID, NotificationCountType.Highlight, 0); + + threadEvent.getPushActions = jest.fn().mockReturnValue(mkPushAction(true, false)); + mockClient.getPushActionsForEvent = jest.fn().mockReturnValue(mkPushAction(false, false)); + + 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); diff --git a/src/client.ts b/src/client.ts index 88e17e9ac7b..c16ea6bdd4d 100644 --- a/src/client.ts +++ b/src/client.ts @@ -9483,64 +9483,77 @@ export class MatrixClient extends TypedEventEmitter 0) { + + let hasReadEvent; + if (isThreadEvent) { + const thread = room.getThread(event.threadRootId); + hasReadEvent = thread + ? thread.hasUserReadEvent(ourUserId, eventId) + : // If the thread object does not exist in the room yet, we don't + // want to calculate notification for this event yet. We have not + // restored the read receipts yet and can't accurately calculate + // notifications at this stage. + // + // This issue can likely go away when MSC3874 is implemented + true; + } else { + hasReadEvent = room.hasUserReadEvent(ourUserId, eventId); + } + + if (hasReadEvent) { + // If the event has been read, ignore it. + return; + } + + if (oldHighlight !== newHighlight || currentHighlightCount > 0) { // TODO: Handle mentions received while the client is offline // See also https://github.com/vector-im/element-web/issues/9069 - let hasReadEvent; + let newCount = currentHighlightCount; + if (newHighlight && !oldHighlight) newCount++; + if (!newHighlight && oldHighlight) newCount--; + if (isThreadEvent) { - const thread = room.getThread(event.threadRootId); - hasReadEvent = thread - ? thread.hasUserReadEvent(cli.getUserId()!, event.getId()!) - : // If the thread object does not exist in the room yet, we don't - // want to calculate notification for this event yet. We have not - // restored the read receipts yet and can't accurately calculate - // highlight notifications at this stage. - // - // This issue can likely go away when MSC3874 is implemented - true; + room.setThreadUnreadNotificationCount(event.threadRootId, NotificationCountType.Highlight, newCount); } else { - hasReadEvent = room.hasUserReadEvent(cli.getUserId()!, event.getId()!); + room.setUnreadNotificationCount(NotificationCountType.Highlight, newCount); } + } - if (!hasReadEvent) { - let newCount = currentCount; - if (newHighlight && !oldHighlight) newCount++; - if (!newHighlight && oldHighlight) newCount--; + // Total count is used to typically increment a room notification counter, but not loudly highlight it. + const currentTotalCount = room.getUnreadCountForEventContext(NotificationCountType.Total, event); - if (isThreadEvent) { - room.setThreadUnreadNotificationCount(event.threadRootId, NotificationCountType.Highlight, newCount); - } else { - room.setUnreadNotificationCount(NotificationCountType.Highlight, newCount); - } + // `notify` is used in practice for incrementing the total count + const newNotify = !!actions?.notify; - // Fix 'Mentions Only' rooms from not having the right badge count - const totalCount = - (isThreadEvent - ? room.getThreadUnreadNotificationCount(event.threadRootId, NotificationCountType.Total) - : room.getRoomUnreadNotificationCount(NotificationCountType.Total)) ?? 0; - - if (totalCount < newCount) { - if (isThreadEvent) { - room.setThreadUnreadNotificationCount(event.threadRootId, NotificationCountType.Total, newCount); - } else { - room.setUnreadNotificationCount(NotificationCountType.Total, newCount); - } - } + // The room total count is NEVER incremented by the server for encrypted rooms. We basically ignore + // the server here as it's always going to tell us to increment for encrypted events. + if (newNotify) { + if (isThreadEvent) { + room.setThreadUnreadNotificationCount( + event.threadRootId, + NotificationCountType.Total, + currentTotalCount + 1, + ); + } else { + room.setUnreadNotificationCount(NotificationCountType.Total, currentTotalCount + 1); } } } diff --git a/src/sync.ts b/src/sync.ts index 46b7367ca99..bfd6029f0b3 100644 --- a/src/sync.ts +++ b/src/sync.ts @@ -1299,18 +1299,29 @@ export class SyncApi { const accountDataEvents = this.mapSyncEventsFormat(joinObj.account_data); const encrypted = client.isRoomEncrypted(room.roomId); - // we do this first so it's correct when any of the events fire + // We store the server-provided value first so it's correct when any of the events fire. if (joinObj.unread_notifications) { - room.setUnreadNotificationCount( - NotificationCountType.Total, - joinObj.unread_notifications.notification_count ?? 0, - ); - - // We track unread notifications ourselves in encrypted rooms, so don't - // bother setting it here. We trust our calculations better than the - // server's for this case, and therefore will assume that our non-zero - // count is accurate. + /** + * We track unread notifications ourselves in encrypted rooms, so don't + * bother setting it here. We trust our calculations better than the + * server's for this case, and therefore will assume that our non-zero + * count is accurate. + * + * @see import("./client").fixNotificationCountOnDecryption + */ + if (!encrypted || joinObj.unread_notifications.notification_count === 0) { + // In an encrypted room, if the room has notifications enabled then it's typical for + // the server to flag all new messages as notifying. However, some push rules calculate + // events as ignored based on their event contents (e.g. ignoring msgtype=m.notice messages) + // so we want to calculate this figure on the client in all cases. + room.setUnreadNotificationCount( + NotificationCountType.Total, + joinObj.unread_notifications.notification_count ?? 0, + ); + } + if (!encrypted || room.getUnreadNotificationCount(NotificationCountType.Highlight) <= 0) { + // If the locally stored highlight count is zero, use the server provided value. room.setUnreadNotificationCount( NotificationCountType.Highlight, joinObj.unread_notifications.highlight_count ?? 0, @@ -1327,11 +1338,13 @@ export class SyncApi { // decryption room.resetThreadUnreadNotificationCount(Object.keys(unreadThreadNotifications)); for (const [threadId, unreadNotification] of Object.entries(unreadThreadNotifications)) { - room.setThreadUnreadNotificationCount( - threadId, - NotificationCountType.Total, - unreadNotification.notification_count ?? 0, - ); + if (!encrypted || unreadNotification.notification_count === 0) { + room.setThreadUnreadNotificationCount( + threadId, + NotificationCountType.Total, + unreadNotification.notification_count ?? 0, + ); + } const hasNoNotifications = room.getThreadUnreadNotificationCount(threadId, NotificationCountType.Highlight) <= 0;