Skip to content

fix: do not reset channel unread count on thread read - #1835

Open
mrpmohiburrahman wants to merge 1 commit into
GetStream:masterfrom
mrpmohiburrahman:fix/thread-read-resets-channel-unread
Open

fix: do not reset channel unread count on thread read#1835
mrpmohiburrahman wants to merge 1 commit into
GetStream:masterfrom
mrpmohiburrahman:fix/thread-read-resets-channel-unread

Conversation

@mrpmohiburrahman

Copy link
Copy Markdown

CLA

  • I have signed the Stream CLA (required).
  • Code changes are tested

Description of the changes, What, Why and How?

Fixes #1676.

What

channel.markRead({ thread_id }) marks a thread read, but the message.read event the
server echoes back was zeroing the channel unread count. Reading a thread reply made
still-unread channel messages disappear from the badge.

A message.read / message.read_locally carrying event.thread now early-outs of the
channel-scoped read branch in Channel._handleChannelEvent before any of it runs. The
offline DB gets the matching carve-out.

Why

The case 'message.read_locally': case 'message.read': branch is entirely channel-scoped
and ran unconditionally, including for thread-scoped read events. Nothing in it looked at
event.thread, so five channel-scoped writes fired for an event that only concerns a
thread:

write why it is wrong for a thread read
read[user].last_read = event.created_at advances the channel read cursor to the thread-read time
read[user].last_read_message_id stores a thread reply id as the channel's last-read message
read[user].unread_messages = 0 claims the channel has no unread messages
messageReceiptsTracker.onMessageRead(...) moves the read receipt marker behind channel "seen by" indicators
channelState.unreadCount = 0 the reported symptom

The issue suggests gating the last of those on isOwnEvent && !isThreadRead. That fixes
the visible number but leaves read[user].last_read advanced, and everything that
re-derives from that cursor reproduces the same wrong 0:

  • countUnread(lastRead) (src/channel.ts:1491) counts messages newer than last_read;
    with the cursor past the unread message it returns 0 regardless of the guard.
  • channel.truncated recomputes unreadCount = this.countUnread(...).
  • State hydration sets state.unreadCount = read[user].unread_messages, so a re-watch
    or reconnect restores the wrong 0.

So the guard belongs at the top of the case, not on one line inside it — one early-out
covers all five writes and is a smaller diff than the per-line version.

event.thread as the discriminator is what the SDK already uses: Thread.subscribeRepliesRead
(src/thread.ts:418) returns early unless event.thread is present and
event.thread.parent_message_id === this.id. Thread read state is applied there, off the
client event bus, so nothing is lost by the channel ignoring the event.

How

src/channel.ts — one early-out at the top of the case:

case 'message.read_locally':
case 'message.read':
  if (event.thread) break;

break exits the switch, not the method, so the post-switch watcher_count update still
runs. _callChannelListeners is dispatched separately by the client (src/client.ts:1419
vs 1425), so channel.on('message.read', …) subscribers still receive the event — only
channel state mutation is skipped. markReadLocally() never sets thread, so the
message.read_locally arm is inert.

src/offline-support/offline_support_api.ts — the same carve-out, message.read only:

if (type === 'message.read' && event.thread) return [];

handleRead is cid-keyed and channel-scoped, so without this a thread read persisted
unread_messages: 0 for the whole channel and hydration restored that 0 on restart —
the in-memory fix alone would not have survived an app restart with offline support on.

Scoped to message.read deliberately: notification.mark_read has no event.thread
check in its in-memory counterpart (StreamChat._handleClientEvent), so guarding it only
in the DB layer would let the two disagree across a restart.

Behaviour change worth flagging

The guard is not scoped to the connected user, so another user's thread read no longer
advances their channel read marker either. That is deliberate — read[userId].last_read is
the channel read cursor, and advancing it because someone read a thread reply would make
"seen by" claim they had seen channel messages they never opened. A wrong receipt seemed
worse than a missing one, but it does affect what downstream SDKs render, so flagging it
rather than burying it. Pinned by a test; happy to narrow it to the own-user path if you
would rather keep the previous behaviour there.

client.syncDeliveredCandidates([this]) also no longer fires on a thread read. It still
runs on message.new, message.delivered and after a channel query, and the delivery
report is a latest-delivered high-water mark, so the following one supersedes it.

Two assumptions, stated rather than buried

  1. That a thread read should leave the channel unread count alone is the reporter's reading
    of correct behaviour — no maintainer has commented on the issue, so it is not a ruling.
    If the intended semantics are different, say so and I will close this or rework it.
  2. The one premise not statically provable: that the server never sends event.thread on a
    channel-level read. src/thread.ts:418 already relies on it and
    test/unit/threads.test.ts:767 encodes it, but I could not verify it against the backend.

Out of scope

src/channel.ts:2238 (notification.mark_unread) looks thread-blind in the same way, but
it keys off event.thread_id rather than event.thread (src/thread.ts:336), so this
guard would not catch it. Unreported and a different code path — left alone to keep the
diff to what closes #1676. Happy to file a follow-up.

Tests

4 new tests, all watched failing with the guard removed and passing with it restored:

  • message.read carrying thread leaves unreadCount, last_read, last_read_message_id
    and unread_messages untouched, and does not call messageReceiptsTracker.onMessageRead.
  • The same for another user's thread read.
  • Offline: message.read carrying thread does not reach handleRead.
  • Offline: notification.mark_read carrying thread still does, pinning the narrow scope.

The pre-existing "should update channel read state produced for current user" test covers
the other direction — a message.read with no thread still zeroes unreadCount — and
stays green, so the guard does not over-fire.

baseline   78 files, 3077 passed, 1 skipped, 0 failed
this PR    78 files, 3081 passed, 1 skipped, 0 failed

yarn lint and yarn types both clean. yarn test-types not run locally — it needs live
API credentials; leaving it to CI.

Changelog

  • Fix channel.state.unreadCount being reset to 0 when marking a thread as read.

A `message.read` carrying `event.thread` is a thread read. It now early-outs of the
channel-scoped read branch in `Channel._handleChannelEvent` instead of advancing the
channel read cursor, clearing `unread_messages` and zeroing `unreadCount`. The offline
DB gets the matching carve-out so the wrong 0 is not persisted and re-hydrated.

Fixes GetStream#1676
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

message.read event for thread reads incorrectly resets channel unreadCount to 0

1 participant