-
Notifications
You must be signed in to change notification settings - Fork 222
Fix channel unread count cleared when a thread is marked as read #3710
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Fix channel unread count cleared when a thread is marked as read #3710
Conversation
WalkthroughThe changes update the channel read middleware to prevent thread read events from incorrectly resetting the channel's unread count. Tests are added to confirm this behavior, and the changelog is updated to document the fix. No exported or public API changes were made. Changes
Sequence Diagram(s)sequenceDiagram
participant EventMiddleware
participant EventPayload
participant ChannelReadState
EventMiddleware->>EventPayload: Receives read event
alt Event is thread read event
EventMiddleware-->>EventPayload: Check for thread details
EventMiddleware-->>EventMiddleware: Skip channel read reset
else Event is channel read event
EventMiddleware->>ChannelReadState: Reset unread count and update last read
end
Assessment against linked issues
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
CHANGELOG.md (1)
6-8
: Clarify the wording of the changelog entryTiny grammar tweak for readability – "being cleared" makes the sentence flow better.
- - Fix channel unread count cleared when a thread is marked as read [#3710](https://github.com/GetStream/stream-chat-swift/pull/3710) + - Fix channel unread count being cleared when a thread is marked as read [#3710](https://github.com/GetStream/stream-chat-swift/pull/3710)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
CHANGELOG.md
(1 hunks)Sources/StreamChat/WebSocketClient/EventMiddlewares/ChannelReadUpdaterMiddleware.swift
(3 hunks)Tests/StreamChatTests/WebSocketClient/EventMiddlewares/ChannelReadUpdaterMiddleware_Tests.swift
(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Test LLC (Debug)
- GitHub Check: Build Test App and Frameworks
- GitHub Check: Metrics
🔇 Additional comments (5)
Sources/StreamChat/WebSocketClient/EventMiddlewares/ChannelReadUpdaterMiddleware.swift (3)
38-40
: LGTM! Correctly prevents thread read events from resetting channel read state.This early return properly addresses the core issue where thread read events were incorrectly clearing the channel's unread count.
49-51
: LGTM! Consistent handling for both read event types.Good to see the same logic applied to both
MessageReadEventDTO
andNotificationMarkReadEventDTO
to ensure comprehensive coverage of thread read events.
90-92
: LGTM! Clean and focused helper method.The implementation correctly identifies thread read events by checking for both
threadDetails
andthreadPartial
, providing comprehensive coverage of thread-related event payloads.Tests/StreamChatTests/WebSocketClient/EventMiddlewares/ChannelReadUpdaterMiddleware_Tests.swift (2)
933-976
: LGTM! Comprehensive test coverage for MessageReadEvent with threads.The test properly validates that thread-related
MessageReadEvent
instances don't affect the channel's unread count or last read timestamp. Good use ofthreadDetails
in the event payload to simulate a realistic thread read scenario.
1048-1093
: LGTM! Complete test coverage for NotificationMarkReadEvent with threads.Excellent addition that ensures both
MessageReadEventDTO
andNotificationMarkReadEventDTO
are properly tested for thread event handling. The test maintains consistency with the existing test structure and validates the fix comprehensively.
SDK Size
|
SDK Performance
|
|
🔗 Issue Links
https://linear.app/stream/issue/IOS-936/marking-a-thread-read-clears-the-unread-count-of-the-channel
🎯 Goal
Fix channel unread count cleared when a thread is marked as read.
🛠 Implementation
The reason this happens is because our
ChannelReadUpdaterMiddleware
clears the unread count of a channel when themessage.read
event is triggered. The solution is to verify if the mark read event has a Thread or not. Having the same event for both Channels and Threads causes these problems. It would have been better if there were separate events.🧪 Manual Testing Notes
Steps:
☑️ Contributor Checklist
docs-content
repoSummary by CodeRabbit
Bug Fixes
Documentation
Tests