Fix thread replies updating the channel unread count #6056
Conversation
SDK Size Comparison 📏
|
WalkthroughThis pull request fixes an issue where thread replies were incorrectly updating the channel unread count. The Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
CHANGELOG.md (1)
35-38: Clarify that the unread update was incorrect (minor wording nit).To better reflect the bug being fixed, consider wording like “Fix thread replies incorrectly updating channel unread count” so it’s clear this was an erroneous increment, not a neutral behavior change.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
CHANGELOG.mdstream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelStateLogic.ktstream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelStateLogicTest.kt
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{kt,kts}
📄 CodeRabbit inference engine (AGENTS.md)
Format and apply Kotlin style with Spotless (4 spaces, no wildcard imports, licence headers)
Files:
stream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelStateLogicTest.ktstream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelStateLogic.kt
**/*.kt
📄 CodeRabbit inference engine (AGENTS.md)
**/*.kt: Use@OptInannotations explicitly; avoid suppressions unless documented
Document public APIs with KDoc, including thread expectations and state notes
Files:
stream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelStateLogicTest.ktstream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelStateLogic.kt
**/src/test/**/*.kt
📄 CodeRabbit inference engine (AGENTS.md)
**/src/test/**/*.kt: Use backtick test names (for example:funmessage list filters muted channels()) for readability
Use deterministic tests withrunTest+ virtual time for concurrency-sensitive logic (uploads, sync, message state)
Keep helper extensions private/internal in test files
Files:
stream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelStateLogicTest.kt
🧠 Learnings (5)
📚 Learning: 2025-12-17T15:00:07.506Z
Learnt from: CR
Repo: GetStream/stream-chat-android PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-17T15:00:07.506Z
Learning: Applies to **/stream-chat-android-ui-components/**/*Test.kt : Record Shot baselines when behaviour changes in XML kit UI tests
Applied to files:
stream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelStateLogicTest.kt
📚 Learning: 2025-12-17T15:00:07.506Z
Learnt from: CR
Repo: GetStream/stream-chat-android PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-17T15:00:07.506Z
Learning: Applies to **/src/test/**/*.kt : Use deterministic tests with `runTest` + virtual time for concurrency-sensitive logic (uploads, sync, message state)
Applied to files:
stream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelStateLogicTest.kt
📚 Learning: 2025-12-17T15:00:07.506Z
Learnt from: CR
Repo: GetStream/stream-chat-android PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-17T15:00:07.506Z
Learning: Applies to **/stream-chat-android-compose/**/*Test.kt : Add Paparazzi snapshots for Compose UI regressions and run `verifyPaparazziDebug`
Applied to files:
stream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelStateLogicTest.kt
📚 Learning: 2025-12-17T15:00:07.506Z
Learnt from: CR
Repo: GetStream/stream-chat-android PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-17T15:00:07.506Z
Learning: Applies to **/src/test/**/*.kt : Use backtick test names (for example: `fun `message list filters muted channels`()`) for readability
Applied to files:
stream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelStateLogicTest.kt
📚 Learning: 2025-12-17T15:00:07.506Z
Learnt from: CR
Repo: GetStream/stream-chat-android PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-17T15:00:07.506Z
Learning: Applies to **/src/test/**/*.kt : Keep helper extensions private/internal in test files
Applied to files:
stream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelStateLogicTest.kt
🧬 Code graph analysis (1)
stream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelStateLogicTest.kt (1)
stream-chat-android-core/src/testFixtures/kotlin/io/getstream/chat/android/Mother.kt (4)
randomChannelUserRead(485-501)randomString(94-98)randomMessage(302-399)randomUser(182-228)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Test compose (2)
- GitHub Check: Test compose (1)
- GitHub Check: Test compose (0)
- GitHub Check: Debug build
- GitHub Check: Unit tests
🔇 Additional comments (16)
stream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelStateLogic.kt (7)
866-870: Good use of deduplication cache.The early-exit guard for already-processed messages prevents duplicate unread count updates. The cache-based approach is efficient.
871-882: Thread reply filtering logic looks correct.The guard correctly skips unread count updates for thread replies not shown in the channel (
parentId != null && !showInChannel). This addresses the core issue described in the PR objectives.
883-898: LGTM: Guards for current user and message types.The guards for messages from the current user, shadowed messages, and silent messages are correctly implemented. These ensure that only relevant messages increment the unread count.
899-904: Outdated event guard prevents race conditions.This guard correctly prevents stale events from overwriting more recent local state. This is a good defensive measure against out-of-order event delivery.
905-914: Unread count update logic is sound.The method correctly increments the unread count and updates the
lastReceivedEventDateonly when all guards pass. The finalprocessedMessageIds.put()ensures idempotency.
878-878: No issues found. The property namemessage.parentIdis correct for the Message model in the Stream Chat Android SDK.
78-78: Cache size of 100 is adequate for per-channel message deduplication.The
processedMessageIdscache is scoped per-channel instance and serves to deduplicate read event processing. A cache size of 100 entries is reasonable and consistent with common Android practices. Even if cache entries are evicted during rapid message arrival, the resulting duplicate read update is idempotent and has minimal performance impact. No production issues or overflow scenarios have been reported in the codebase.stream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelStateLogicTest.kt (9)
72-72: Appropriate use of suppression annotation.The
@Suppress("LargeClass")annotation is justified given the comprehensive test coverage added. The test class has grown due to the expanded guard conditions in the implementation.
443-473: Excellent test for deduplication logic.The test correctly verifies that processing the same message twice only increments the unread count once. This validates the
processedMessageIdscache behavior.As per coding guidelines, the backtick test name is readable and descriptive.
476-503: LGTM: Muted channel test.The test correctly verifies that muted channels don't update unread counts. The setup and assertions are clear.
505-533: Critical test case for the PR objective.This test validates the core fix: thread replies not shown in channel (
parentId != null && showInChannel = false) should not increment the unread count. This directly addresses the bug described in the PR.
535-620: Comprehensive coverage of message type guards.The tests for current user messages, shadowed messages, and silent messages are well-structured and verify the correct behavior. Each test follows the same pattern for consistency.
622-649: Good test for race condition protection.This test verifies that outdated events (older than the current read state) don't incorrectly update the unread count. This prevents issues with out-of-order event delivery.
651-683: Happy path test validates correct increment.This test verifies that a regular message from another user correctly increments the unread count and updates the
lastReceivedEventDate. The expected state is explicitly constructed and verified.
685-718: Important edge case: thread replies shown in channel.This test verifies that thread replies with
showInChannel = trueDO increment the unread count, which is the correct behavior. This distinguishes between the two thread reply scenarios (shown vs. not shown in channel).
720-739: Defensive test for null read state.This test ensures the method handles the edge case where no current read state exists without crashing. The method correctly skips the update when
currentReadis null.
|
gpunto
deleted the
bug/AND-968_fix_thread_replies_increasing_unread_count
branch
🎯 Goal
The cannel unread count should not be updated for thread replies (unless the reply is also sent to channel).
🛠 Implementation details
Fix the case where
message.parentMessageId != nullis increasing the channel unread count.🎨 UI Changes
before.mov
after.mov
🧪 Testing
Summary by CodeRabbit
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.