Support local unread counts for channels with read events disabled - #6609
Conversation
…server-data merge
PR checklist ✅All required conditions are satisfied:
🎉 Great job! This PR is ready for review. |
|
@coderabbitai review |
✅ Action performedReview finished.
|
SDK Size Comparison 📏
|
Walkthrough
ChangesLocal unread-count support
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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 |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
stream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/state/plugin/state/channel/internal/ChannelStateImpl.kt (1)
1188-1213: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winMark-read outcomes are not in parity between the two channel-state implementations. The same mark-read decision table is implemented twice, and the two copies disagree on the empty-message case and on the
unreadMessagescheck. A channel behaves differently depending on which state implementation is active.
stream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/state/plugin/state/channel/internal/ChannelStateImpl.kt#L1188-L1213: add|| currentUserRead.unreadMessages > 0to the final condition, and align the empty-message-list outcome with the legacy implementation.stream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/state/plugin/state/channel/internal/ChannelStateLegacyImpl.kt#L595-L624: keep theunreadMessagescheck, and align the empty-message-list outcome with the chosen behavior. Add a test for each branch in both implementations.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@stream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/state/plugin/state/channel/internal/ChannelStateImpl.kt` around lines 1188 - 1213, Align mark-read decision logic across ChannelStateImpl.markRead and ChannelStateLegacyImpl.markRead: include currentUserRead.unreadMessages > 0 in the final condition, and make both empty-message branches return the same chosen outcome. Add coverage for each decision branch in both implementations. Update ChannelStateImpl.kt lines 1188-1213 and ChannelStateLegacyImpl.kt lines 595-624; tests should cover both implementations and all affected branches.
🧹 Nitpick comments (3)
stream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/state/plugin/logic/channel/internal/ChannelLogicImpl.kt (1)
250-265: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winLocal-read persistence is implemented twice. Both channel-logic implementations read the current reads, skip empty lists, and launch
upsertChannelReadsonMarkReadResult.HandledLocally. Neither handles a persistence failure, so a failed upsert silently loses the local reset.
stream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/state/plugin/logic/channel/internal/ChannelLogicImpl.kt#L250-L265: extract the persistence rule into one shared helper and log a failure from the launched coroutine.stream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/state/plugin/logic/channel/internal/legacy/ChannelLogicLegacyImpl.kt#L203-L215: call the shared helper instead of repeating the block inline.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@stream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/state/plugin/logic/channel/internal/ChannelLogicImpl.kt` around lines 250 - 265, Local-read persistence is duplicated and silently ignores upsert failures. In ChannelLogicImpl.kt lines 250-265, extract the reads-empty check and coroutine upsert into a shared helper that logs failures from the launched coroutine; in ChannelLogicLegacyImpl.kt lines 203-215, replace the inline persistence block with a call to that helper while preserving the HandledLocally behavior.stream-chat-android-compose-sample/src/main/java/io/getstream/chat/android/compose/sample/feature/channel/list/ChannelsActivity.kt (1)
109-140: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the duplicated channel-list filter/sort construction into a shared helper.
Both activities build the same
ChannelListViewModelFactorywith identical KDoc, the sameisLocalUnreadCountEnabledbranch, and the sameFilters/QuerySortByFieldconfiguration. Any future change to this filter (for example, adding another channel type) must be made in both places, and the two copies can drift.
stream-chat-android-compose-sample/src/main/java/io/getstream/chat/android/compose/sample/feature/channel/list/ChannelsActivity.kt#L109-L140: replace this block with a call to a shared helper function that takeschatClientandsettingsand returns the builtChannelListViewModelFactory.stream-chat-android-compose-sample/src/main/java/io/getstream/chat/android/compose/sample/ui/chats/ChatsActivity.kt#L137-L168: replace this block with the same shared helper call.♻️ Suggested shared helper
fun buildSampleChannelListViewModelFactory( chatClient: ChatClient, settings: CustomSettings, ): ChannelListViewModelFactory { val currentUserId = chatClient.getCurrentUser()?.id ?: "" return if (settings.isLocalUnreadCountEnabled) { ChannelListViewModelFactory( chatClient = chatClient, querySort = QuerySortByField<Channel>().desc("pinned_at").desc("last_updated"), filters = Filters.and( Filters.`in`("type", listOf("messaging", "livestream")), Filters.`in`("members", listOf(currentUserId)), Filters.or(Filters.notExists("draft"), Filters.eq("draft", false)), ), chatEventHandlerFactory = CustomChatEventHandlerFactory(), ) } else { ChannelListViewModelFactory( chatClient = chatClient, predefinedFilterName = "android_sample_filter", filterValues = mapOf( "channel_type" to "messaging", "user_id" to currentUserId, ), chatEventHandlerFactory = CustomChatEventHandlerFactory(), ) } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@stream-chat-android-compose-sample/src/main/java/io/getstream/chat/android/compose/sample/feature/channel/list/ChannelsActivity.kt` around lines 109 - 140, Extract the duplicated channel-list factory construction into a shared buildSampleChannelListViewModelFactory helper accepting ChatClient and CustomSettings, preserving the existing local-unread filter/sort and predefined-filter branches. In stream-chat-android-compose-sample/src/main/java/io/getstream/chat/android/compose/sample/feature/channel/list/ChannelsActivity.kt lines 109-140 and stream-chat-android-compose-sample/src/main/java/io/getstream/chat/android/compose/sample/ui/chats/ChatsActivity.kt lines 137-168, replace each duplicated block with a call to this helper using the activity’s chatClient and settings.stream-chat-android-client/src/test/java/io/getstream/chat/android/client/internal/state/event/handler/internal/EventHandlerSequentialTest.kt (1)
424-473: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for the
NotificationMessageNewEventbranch.These two tests cover
persistLocallyTrackedReadsonly throughrandomNewMessageEvent. The productionwhenblock inpersistLocallyTrackedReadsalso matchesNotificationMessageNewEvent. Add a test usingrandomNotificationMessageNewEventto confirm that branch also triggersrepos.upsertChannelReads.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@stream-chat-android-client/src/test/java/io/getstream/chat/android/client/internal/state/event/handler/internal/EventHandlerSequentialTest.kt` around lines 424 - 473, Add a test alongside the existing local unread tracking tests that invokes persistLocallyTrackedReads through randomNotificationMessageNewEvent, with local unread tracking enabled and locally tracked reads configured, then verify repos.upsertChannelReads receives the channel ID and reads. Keep the setup and assertions consistent with the existing randomNewMessageEvent coverage.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@stream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/offline/repository/domain/channel/internal/DatabaseChannelRepository.kt`:
- Around line 71-98: Move all fallback DB/user resolution out of
cacheMutex.withLock: preload the required channel rows and resolved reads before
the lock in preserveLocallyTrackedReads and the upsertChannelReads flow,
including selectChannel. Pass the preloaded in-memory data into the locked merge
so cacheMutex only protects synchronous cache updates; preserve existing merge
behavior and avoid any channelDao.select or getUser calls while the lock is
held.
In
`@stream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/state/plugin/listener/internal/ChannelMarkReadListenerState.kt`:
- Around line 45-55: Update the HandledLocally branch in
ChannelMarkReadListenerState to return Result.Success(Unit) after refreshing
active query-channel state. Preserve the existing refreshChannelState call and
ensure callers treat a locally completed mark-read as successful rather than
returning Error.GenericError.
---
Outside diff comments:
In
`@stream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/state/plugin/state/channel/internal/ChannelStateImpl.kt`:
- Around line 1188-1213: Align mark-read decision logic across
ChannelStateImpl.markRead and ChannelStateLegacyImpl.markRead: include
currentUserRead.unreadMessages > 0 in the final condition, and make both
empty-message branches return the same chosen outcome. Add coverage for each
decision branch in both implementations. Update ChannelStateImpl.kt lines
1188-1213 and ChannelStateLegacyImpl.kt lines 595-624; tests should cover both
implementations and all affected branches.
---
Nitpick comments:
In
`@stream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/state/plugin/logic/channel/internal/ChannelLogicImpl.kt`:
- Around line 250-265: Local-read persistence is duplicated and silently ignores
upsert failures. In ChannelLogicImpl.kt lines 250-265, extract the reads-empty
check and coroutine upsert into a shared helper that logs failures from the
launched coroutine; in ChannelLogicLegacyImpl.kt lines 203-215, replace the
inline persistence block with a call to that helper while preserving the
HandledLocally behavior.
In
`@stream-chat-android-client/src/test/java/io/getstream/chat/android/client/internal/state/event/handler/internal/EventHandlerSequentialTest.kt`:
- Around line 424-473: Add a test alongside the existing local unread tracking
tests that invokes persistLocallyTrackedReads through
randomNotificationMessageNewEvent, with local unread tracking enabled and
locally tracked reads configured, then verify repos.upsertChannelReads receives
the channel ID and reads. Keep the setup and assertions consistent with the
existing randomNewMessageEvent coverage.
In
`@stream-chat-android-compose-sample/src/main/java/io/getstream/chat/android/compose/sample/feature/channel/list/ChannelsActivity.kt`:
- Around line 109-140: Extract the duplicated channel-list factory construction
into a shared buildSampleChannelListViewModelFactory helper accepting ChatClient
and CustomSettings, preserving the existing local-unread filter/sort and
predefined-filter branches. In
stream-chat-android-compose-sample/src/main/java/io/getstream/chat/android/compose/sample/feature/channel/list/ChannelsActivity.kt
lines 109-140 and
stream-chat-android-compose-sample/src/main/java/io/getstream/chat/android/compose/sample/ui/chats/ChatsActivity.kt
lines 137-168, replace each duplicated block with a call to this helper using
the activity’s chatClient and settings.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 742011a2-2f22-40a3-b7b8-24b4e4b64927
📒 Files selected for processing (37)
stream-chat-android-client/api/stream-chat-android-client.apistream-chat-android-client/src/main/java/io/getstream/chat/android/client/api/ChatClientConfig.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/api/state/StateRegistry.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/offline/repository/domain/channel/internal/DatabaseChannelRepository.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/offline/repository/factory/internal/DatabaseRepositoryFactory.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/state/event/handler/internal/EventHandlerSequential.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/state/plugin/factory/StreamStatePluginFactory.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/state/plugin/listener/internal/ChannelMarkReadListenerState.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/state/plugin/logic/channel/internal/ChannelLogic.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/state/plugin/logic/channel/internal/ChannelLogicImpl.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/state/plugin/logic/channel/internal/legacy/ChannelEventHandlerLegacyImpl.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/state/plugin/logic/channel/internal/legacy/ChannelLogicLegacyImpl.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/state/plugin/logic/channel/internal/legacy/ChannelStateLogic.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/state/plugin/logic/querychannels/internal/QueryChannelsStateLogic.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/state/plugin/state/channel/internal/ChannelStateImpl.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/state/plugin/state/channel/internal/ChannelStateLegacyImpl.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/state/plugin/state/channel/internal/MarkReadResult.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/persistance/repository/ChannelRepository.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/internal/offline/repository/domain/channel/internal/ChannelRepositoryImplTest.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/internal/state/event/TotalUnreadCountTest.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/internal/state/event/handler/internal/EventHandlerSequentialTest.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/internal/state/event/handler/internal/EventHandlerSequentialUserMessagesDeletedTest.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/internal/state/plugin/listener/internal/ChannelMarkReadListenerStateTest.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/internal/state/plugin/logic/channel/internal/ChannelLogicImplTest.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/internal/state/plugin/logic/channel/internal/legacy/ChannelStateLogicTest.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/internal/state/plugin/logic/querychannels/internal/QueryChannelsStateLogicTest.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/internal/state/plugin/state/channel/internal/ChannelStateImplLocalUnreadCountTest.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/internal/state/plugin/state/channel/internal/ChannelStateImplReadReceiptsTest.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/internal/state/plugin/state/channel/internal/ChannelStateLegacyImplLocalUnreadCountTest.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/internal/state/plugin/state/channel/internal/ChannelStateLegacyImplTest.ktstream-chat-android-compose-sample/src/main/java/io/getstream/chat/android/compose/sample/ChatHelper.ktstream-chat-android-compose-sample/src/main/java/io/getstream/chat/android/compose/sample/data/CustomSettings.ktstream-chat-android-compose-sample/src/main/java/io/getstream/chat/android/compose/sample/feature/channel/list/ChannelsActivity.ktstream-chat-android-compose-sample/src/main/java/io/getstream/chat/android/compose/sample/ui/chats/ChatsActivity.ktstream-chat-android-compose-sample/src/main/java/io/getstream/chat/android/compose/sample/ui/login/CustomLoginActivity.ktstream-chat-android-compose-sample/src/main/java/io/getstream/chat/android/compose/sample/ui/login/UserLoginActivity.ktstream-chat-android-compose-sample/src/main/res/values/strings.xml
a5e0cc0 to
e77e987
Compare
…count # Conflicts: # stream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/state/plugin/logic/channel/internal/legacy/ChannelStateLogic.kt # stream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/state/plugin/state/channel/internal/ChannelStateImpl.kt # stream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/state/plugin/state/channel/internal/ChannelStateLegacyImpl.kt # stream-chat-android-client/src/test/java/io/getstream/chat/android/client/internal/state/plugin/state/channel/internal/ChannelStateLegacyImplTest.kt
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
…ious unread separator
…ls in updateLastMessage
8051d03 to
a0be421
Compare
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
stream-chat-android-client/src/main/java/io/getstream/chat/android/client/extensions/internal/Channel.kt (1)
86-103: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winUse
isLocalUnreadCountEnabledfor the locally tracked read guard.
readEventsEnabled = falseis not the opt-in flag, so this branch must also checkclientConfig.isLocalUnreadCountEnabledbefore skipping the current-user read update. Otherwise non-opted-in channels with server-side read events disabled still get their own user read copied inupdateLastMessage, bypassing the per-channel legacy/local-track guard.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@stream-chat-android-client/src/main/java/io/getstream/chat/android/client/extensions/internal/Channel.kt` around lines 86 - 103, Update the current-user read branch in Channel’s newReads mapping to skip the update only when both readEventsEnabled is false and clientConfig.isLocalUnreadCountEnabled is true. Use the existing isLocalUnreadCountEnabled configuration symbol, preserving the normal read.copy path for channels not opted into local unread tracking.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In
`@stream-chat-android-client/src/main/java/io/getstream/chat/android/client/extensions/internal/Channel.kt`:
- Around line 86-103: Update the current-user read branch in Channel’s newReads
mapping to skip the update only when both readEventsEnabled is false and
clientConfig.isLocalUnreadCountEnabled is true. Use the existing
isLocalUnreadCountEnabled configuration symbol, preserving the normal read.copy
path for channels not opted into local unread tracking.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: e507346b-5545-461b-8f18-6cf66f739589
📒 Files selected for processing (3)
stream-chat-android-client/src/main/java/io/getstream/chat/android/client/extensions/internal/Channel.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/state/plugin/state/channel/internal/ChannelStateImpl.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/state/plugin/state/channel/internal/ChannelStateLegacyImpl.kt
🚧 Files skipped from review as they are similar to previous changes (2)
- stream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/state/plugin/state/channel/internal/ChannelStateLegacyImpl.kt
- stream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/state/plugin/state/channel/internal/ChannelStateImpl.kt
22d8ede to
c5e87fc
Compare
|



Goal
Closes AND-1358
Provide an unread mechanism for channels where server-side read events are disabled (e.g. livestreams). Android port of stream-chat-swift#4102.
Adds an opt-in
ChatClientConfig.isLocalUnreadCountEnabledflag (defaultfalse). When on, channels withreadEventsEnabled = falseget a per-channel unread count tracked on-device. The user-level total unread count is unaffected.Scope: best-effort, live-events only
By design this is a best-effort count of live events, not an authoritative unread count. These channels turn off server-side read tracking, so there is no read state to reconcile against — which means:
This matches the intended behavior of the original feature, confirmed with the iOS team (author of stream-chat-swift#4102). Making it accurate across restarts/offline would require server-tracked reads, which is exactly what
read_events = falsedisables.Implementation
markReadon a locally tracked channel resets the count in state without a network request (markRead()returns a newMarkReadResultdescribing whether a remote request is still needed) and refreshes the channel-list rows. It advanceslastReadjust past the last message, mirroring the server's mark-read, so the message list doesn't render a spurious unread separator for the user's own next message.updateLastMessagealso leaves the local read untouched, so the on-device count is the single writer (it would otherwise increment the persisted read with weaker guards).ChannelRepository.upsertChannelReads(default no-op) that bypasses the server-data merge, so the last-known count survives an app restart.Demo app
The compose sample gets a "Local unread count" toggle (advanced login options). When enabled it also includes livestream channels in the channel list so the feature is testable.
Testing
spotlessCheck,detekt,apiCheck(only additive: the newChatClientConfig/StateRegistrytrailing flag; source-compatible), and the full clienttestDebugUnitTestsuite pass.Manual testing:
/readrequest.Best-effort by design (see Scope): messages arriving while the app is backgrounded/killed, offline, or before a fresh install are not counted.
Summary by CodeRabbit
New Features
Bug Fixes