diff --git a/src/channel.ts b/src/channel.ts index 497febdd4d..c8fb3a589a 100644 --- a/src/channel.ts +++ b/src/channel.ts @@ -2007,6 +2007,10 @@ export class Channel { // delivery-report network sync below is skipped for it. case 'message.read_locally': case 'message.read': + // Thread read events are handled indiscriminately within the reactive `thread` object, we can + // skip them here in order to ensure the channel does not get read incidentally when it should + // not. + if (event.thread) break; if (event.user?.id && event.created_at) { const previousReadState = channelState.read[event.user.id]; channelState.read[event.user.id] = { diff --git a/src/offline-support/offline_support_api.ts b/src/offline-support/offline_support_api.ts index fdebd645eb..f87369c07c 100644 --- a/src/offline-support/offline_support_api.ts +++ b/src/offline-support/offline_support_api.ts @@ -1050,6 +1050,10 @@ export abstract class AbstractOfflineDB implements OfflineDBApi { } if (type === 'message.read' || type === 'notification.mark_read') { + // We make sure not to update channel reads (which is what's stored in + // the offline DB in any case) whenever we receive a read event for a + // a thread specifically. + if (event.thread) return []; return this.handleRead({ event, unreadMessages: 0, execute }); } diff --git a/test/unit/channel.test.js b/test/unit/channel.test.js index 35fb1e6f0e..4e2da5a3db 100644 --- a/test/unit/channel.test.js +++ b/test/unit/channel.test.js @@ -991,6 +991,91 @@ describe('Channel _handleChannelEvent', function () { initialReadState.last_delivered_message_id, ); }); + // Tests against this issue: https://github.com/GetStream/stream-chat-js/issues/1676 + it('should not touch channel read state for a thread read', () => { + channel.state.unreadCount = initialCountUnread; + channel.state.read[user.id] = initialReadState; + const onMessageRead = vi.spyOn(channel.messageReceiptsTracker, 'onMessageRead'); + const event = { + ...messageReadEvent, + thread: { parent_message_id: 'parent-message-id' }, + }; + + channel._handleChannelEvent(event); + + expect(channel.state.unreadCount).toBe(initialCountUnread); + expect(new Date(channel.state.read[user.id].last_read).getTime()).toBe( + new Date(initialReadState.last_read).getTime(), + ); + expect(channel.state.read[user.id].last_read_message_id).toBe( + initialReadState.last_read_message_id, + ); + expect(channel.state.read[user.id].unread_messages).toBe(initialCountUnread); + expect(onMessageRead).not.toHaveBeenCalled(); + }); + + it('should not touch channel read state for another user’s thread read', () => { + const anotherUser = { id: 'another-user' }; + channel.state.unreadCount = initialCountUnread; + channel.state.read[anotherUser.id] = initialReadState; + const onMessageRead = vi.spyOn(channel.messageReceiptsTracker, 'onMessageRead'); + const event = { + ...messageReadEvent, + user: anotherUser, + thread: { parent_message_id: 'parent-message-id' }, + }; + + channel._handleChannelEvent(event); + + expect(channel.state.unreadCount).toBe(initialCountUnread); + expect(new Date(channel.state.read[anotherUser.id].last_read).getTime()).toBe( + new Date(initialReadState.last_read).getTime(), + ); + expect(channel.state.read[anotherUser.id].last_read_message_id).toBe( + initialReadState.last_read_message_id, + ); + expect(channel.state.read[anotherUser.id].unread_messages).toBe(initialCountUnread); + expect(onMessageRead).not.toHaveBeenCalled(); + }); + + // Skipping the case also skips the delivery sync at its tail. Pinned because it is a + // deliberate behaviour change, not an oversight: the next `message.new` / + // `message.delivered` / channel query supersedes the report anyway. + it('should not sync delivery report candidates on a thread read', () => { + const syncDeliveredCandidates = vi.spyOn(client, 'syncDeliveredCandidates'); + const event = { + ...messageReadEvent, + thread: { parent_message_id: 'parent-message-id' }, + }; + + channel._handleChannelEvent(event); + + expect(syncDeliveredCandidates).not.toHaveBeenCalled(); + }); + + // The stored fields above are only half of it: `countUnread()` re-derives the count from + // `read[user].last_read`, so an advanced cursor reproduces the reported 0 even with + // `unreadCount` guarded. This pins the value the integrator actually reads. + it('should keep lastRead() and countUnread() consistent after a thread read', () => { + // An unread channel message sent after the current user's last channel read but before + // the thread read - it must keep counting as unread once the thread is read. + channel.state.addMessagesSorted([ + generateMsg({ date: new Date(1800).toISOString(), user: otherUser }), + ]); + channel.state.read[user.id] = { + ...initialReadState, + last_read: new Date(initialReadState.last_read), + }; + const event = { + ...messageReadEvent, + thread: { parent_message_id: 'parent-message-id' }, + }; + + channel._handleChannelEvent(event); + + expect(channel.lastRead().getTime()).toBe(1500); + expect(channel.countUnread(channel.lastRead())).toBe(1); + }); }); describe('message.delivered', () => { diff --git a/test/unit/offline-support/offline_support_api.test.ts b/test/unit/offline-support/offline_support_api.test.ts index da553c5037..f2ca3cb2d1 100644 --- a/test/unit/offline-support/offline_support_api.test.ts +++ b/test/unit/offline-support/offline_support_api.test.ts @@ -1915,6 +1915,35 @@ describe('OfflineSupportApi', () => { expect(result).toEqual([]); }); }); + + describe('thread reads', () => { + const threadEvent = (type: string) => + ({ + ...dummyEvent, + type, + thread: { parent_message_id: 'parent-message-id' }, + }) as unknown as Event; + + it('is a no-op for message.read carrying a thread', async () => { + const event = threadEvent('message.read'); + + const result = await offlineDb.handleEvent({ event }); + + expect(offlineDb.handleRead).not.toHaveBeenCalled(); + expect(result).toEqual([]); + }); + + // `handleRead` writes `unread_messages: 0` for this event type too, so a + // thread-scoped `notification.mark_read` would zero the whole channel just the same. + it('is a no-op for notification.mark_read carrying a thread', async () => { + const event = threadEvent('notification.mark_read'); + + const result = await offlineDb.handleEvent({ event }); + + expect(offlineDb.handleRead).not.toHaveBeenCalled(); + expect(result).toEqual([]); + }); + }); }); });