fix(llc): handle per-channel notification.mark_read events again - #2891
Conversation
`ChannelClientState._listenReadEvents` subscribed to both `message.read` and `notification.mark_read` until the delivery-receipts refactor (#2429) split them and dropped the latter. `message.read` only reaches watchers, so from 9.20.0 on `markRead()` on a non-watched channel left `unreadCount` permanently stale — the unread pill and divider could not be cleared by any user action. Re-adds the event type to the existing listener, which since 9.19.0 also guards against thread-scoped reads and preserves delivery fields. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
9.27.0 is already tagged and published, so the entry belongs in a new Unreleased section rather than an existing released one. Matches the sibling v9 backport (#2889). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…93_notification-mark-read # Conflicts: # packages/stream_chat/CHANGELOG.md
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## v9 #2891 +/- ##
=====================================
Coverage ? 67.30%
=====================================
Files ? 431
Lines ? 27390
Branches ? 0
=====================================
Hits ? 18434
Misses ? 8956
Partials ? 0 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Submit a pull request
Linear: FLU-693
Fixes: #2886
CLA
Description of the pull request
Backport of master commit
a9b048261— "fix(llc): handle per-channel notification.mark_read events again" (#2890).v9 is affected: the regression landed in 9.20.0, so every v9 release from 9.20.0 onward carries it.
ChannelClientState._listenReadEventssubscribed to bothmessage.readandnotification.mark_readuntil the delivery-receipts refactor (#2429,3b5e410bb) split them into separate listeners and did not carrynotification.mark_readover.That distinction is load-bearing:
message.readis a channel event — delivered to watchers only.notification.mark_readis delivered on the reading user's own connection — so it reached the channel whether watched or not.Channel.markRead()writes no local state itself, and channels are easily non-watched (watchis silently downgraded when there is no connection id). So marking a non-watched channel as read succeeded server-side whileunreadCountstayed stale forever — the unread pill and divider survived the dismiss X,markReadWhenAtTheBottom, and every reopen, with no user action able to clear them.Port notes
Effectively a clean cherry-pick. v9's
_listenReadEventsis byte-identical to master's pre-fix version — including the thread guard and the delivery-field preservation — so the fix applies unchanged:_channel .on(EventType.messageRead, EventType.notificationMarkRead) .listen(Two mechanical differences from the master commit:
.fvmrc), where master is 120/tall. The single-line.on(...).listen(from master is 82 chars, so it wraps across three lines. Formatted with the pinned 3.6.2 SDK rather than a newer local one, which would have restyled the entire file.CHANGELOG.mdresolved manually into v9's## 9.27.0section (the only cherry-pick conflict).The test file applied without conflict.
Testing
Four tests in
channel_test.dart. On master I verified they actually catch the regression by reverting the one-line fix and re-running — three go red (the fourth is a negative test asserting thread-scoped reads are ignored, so it passes either way):Expected: <0> Actual: <10>Expected: <0> Actual: <10>No matching callsVerified on v9 with the pinned toolchain (Dart 3.6.2 / Flutter 3.27.4):
Out of scope
Same as the master PR:
totalUnreadCount/unreadChannelsgoing stale too, and the reporter shipped a client-level listener for it. That part isn't needed —ClientState.subscribeToEventsapplies both from any event carrying them.sync()replay re-incrementingunreadCountvia the non-idempotentaddNewMessage) is real and related, but a separate bug.Screenshots / Videos
No UI changes — beyond the unread pill and divider now clearing, which is the bug itself.
🤖 Generated with Claude Code