Add GroupedQueryChannels and grouped unread counts#6516
Open
VelikovPetar wants to merge 8 commits into
Open
Conversation
Co-Authored-By: Claude <noreply@anthropic.com>
Contributor
PR checklist ✅All required conditions are satisfied:
🎉 Great job! This PR is ready for review. |
Contributor
|
DB Entities have been updated. Do we need to upgrade DB Version? |
Contributor
SDK Size Comparison 📏
|
WalkthroughThis PR introduces a grouped channels feature for the Stream Chat Android SDK. It adds ChangesGrouped Channels Feature
Sequence DiagramsequenceDiagram
rect rgba(100, 150, 200, 0.5)
Note over ChannelListViewModel: Grouped mode initialization
ChannelListViewModel->>ChatClient: initGroupedQueryChannelsAsState(Grouped(groupKey))
ChatClient->>ChatClientStateCalls: initGroupedQueryChannelsState(identifier, factory)
ChatClientStateCalls->>StateRegistry: queryChannels(Grouped(groupKey))
StateRegistry-->>ChatClientStateCalls: QueryChannelsMutableState
ChatClientStateCalls->>QueryChannelsLogic: loadOfflineGroupedChannels()
QueryChannelsLogic-->>ChannelListViewModel: StateFlow~QueryChannelsState?~ (offline data)
end
rect rgba(100, 200, 150, 0.5)
Note over ChannelListViewModel: Remote fetch
ChannelListViewModel->>ChatClient: queryGroupedChannelsInternal(limit, groups, watch, presence)
ChatClient->>Plugin: onQueryGroupedChannelsRequest(...)
ChatClient->>MoshiChatApi: queryGroupedChannels(...)
MoshiChatApi->>ChannelApi: POST /channels/grouped
ChannelApi-->>MoshiChatApi: QueryGroupedChannelsResponse
MoshiChatApi-->>ChatClient: GroupedChannels
ChatClient->>Plugin: onQueryGroupedChannelsResult(result, ...)
Plugin->>QueryGroupedChannelsListenerState: onQueryGroupedChannelsResult(...)
QueryGroupedChannelsListenerState->>MutableGlobalState: setGroupedUnreadChannels(...)
QueryGroupedChannelsListenerState->>QueryChannelsLogic: applyGroupedResult(group, isFirstPage)
QueryChannelsLogic-->>ChannelListViewModel: updated channels via StateFlow
end
rect rgba(200, 150, 100, 0.5)
Note over EventHandlerSequential: Realtime event updates
EventHandlerSequential->>GroupedUnreadChannelsUpdater: calculateUpdatedCounts(batchId, event)
GroupedUnreadChannelsUpdater-->>EventHandlerSequential: updated Map~groupKey,unreadCount~
EventHandlerSequential->>MutableGlobalState: setGroupedUnreadChannels(...)
end
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
stream-chat-android-client/src/main/java/io/getstream/chat/android/client/api/state/ChatClientStateExtensions.kt (1)
224-235: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winTrack
aroundMessageIdwatch states too.This overload still returns the raw
getStateOrNull(...)flow, while the overload above wraps it inWatchedChannelStateFlowand registers it withstate.trackWatchedChannel(...). With the new reconnect logic relying on tracked watched-channel flows, channels opened viawatchChannelAsState(cid, messageLimit, aroundMessageId, ...)won't be re-watched after reconnect.Suggested fix
`@InternalStreamChatApi` `@JvmOverloads` public fun ChatClient.watchChannelAsState( cid: String, messageLimit: Int, aroundMessageId: String?, coroutineScope: CoroutineScope = CoroutineScope(DispatcherProvider.IO), ): StateFlow<ChannelState?> { StreamLog.i(TAG) { "[watchChannelAsState] cid: $cid, messageLimit: $messageLimit, aroundMessageId: $aroundMessageId" } - return getStateOrNull(coroutineScope) { + val flow = getStateOrNull(coroutineScope) { requestsAsState(coroutineScope).watchChannel(cid, messageLimit, chatClientConfig.userPresence, aroundMessageId) } + val watchedFlow = WatchedChannelStateFlow(flow, cid) + val watchedFlowRef = WeakReference(watchedFlow) + coroutineScope.launch { + runCatching { + clientState.initializationState.first { it == InitializationState.COMPLETE } + watchedFlowRef.get()?.let(state::trackWatchedChannel) + } + } + return watchedFlow }🤖 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/api/state/ChatClientStateExtensions.kt` around lines 224 - 235, The `watchChannelAsState(cid, messageLimit, aroundMessageId, coroutineScope)` overload in `ChatClientStateExtensions` should not return the raw `getStateOrNull(...)` flow; instead, wrap the result in `WatchedChannelStateFlow` like the other `watchChannelAsState` overload and register it with `state.trackWatchedChannel(...)`. Make sure the `aroundMessageId` variant uses the same watched-channel tracking path as the existing overload so reconnect logic can re-watch those channels automatically.stream-chat-android-client/api/stream-chat-android-client.api (1)
926-944: 🎯 Functional Correctness | 🟠 MajorKeep these public state interfaces backward-compatible.
GlobalStateandQueryChannelsStateare public Kotlin interfaces, and the new abstract properties in the API dump make external implementations brittle at runtime. Use a default-compatible path, such as a sub-interface or default-backed accessors, instead of adding required members.🤖 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/api/stream-chat-android-client.api` around lines 926 - 944, The public state interfaces are being made source/binary incompatible by adding required abstract members to GlobalState and QueryChannelsState. Update the API so these new state accessors are exposed through a backward-compatible approach, such as a new sub-interface or default-backed accessors on the existing interface, rather than adding mandatory abstract properties. Keep the existing interface contracts stable and preserve compatibility for external implementations that already implement GlobalState or QueryChannelsState.stream-chat-android-compose/src/test/kotlin/io/getstream/chat/android/compose/viewmodel/channels/ChannelListViewModelTest.kt (1)
1379-1389: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winTighten the grouped-state fixture.
Line 1389 stubs
stateRegistry.queryChannels(any()), so the newgroupKeytests still pass ifChannelListViewModel(groupKey = ...)looks up the standard query state or the wrong group key. Matching or capturing the expected grouped identifier here would make these tests actually protect the new state-wiring contract.Also applies to: 1392-1401
🤖 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/src/test/kotlin/io/getstream/chat/android/compose/viewmodel/channels/ChannelListViewModelTest.kt` around lines 1379 - 1389, The grouped-state fixture is too loose because stateRegistry.queryChannels(any()) will match any key, so the new groupKey coverage can pass even if ChannelListViewModel uses the wrong query state. Update the stubbing in ChannelListViewModelTest around queryChannelsState to match or capture the expected grouped identifier (the groupKey passed into ChannelListViewModel) and apply the same tightening to the related grouped-state setup later in the test so the fixtures verify the intended wiring.
🧹 Nitpick comments (6)
stream-chat-android-client/src/test/java/io/getstream/chat/android/client/api2/mapping/EventMappingTestArguments.kt (1)
542-556: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a null-counter mapping test case for
NotificationMarkUnreadEventDto.Lines 553-554 always pass non-null counts, so the fallback path at Lines 1230-1231 (
?: 0) isn’t directly exercised. Add one DTO/domain pair with null counts and include it inarguments().Proposed test addition
+ private val notificationMarkUnreadDtoWithNullCounts = notificationMarkUnreadDto.copy( + total_unread_count = null, + unread_channels = null, + ) + + private val notificationMarkUnreadWithNullCounts = notificationMarkUnread.copy( + totalUnreadCount = 0, + unreadChannels = 0, + ) ... fun arguments() = listOf( ... Arguments.of(notificationMarkUnreadDto, notificationMarkUnread), + Arguments.of(notificationMarkUnreadDtoWithNullCounts, notificationMarkUnreadWithNullCounts), ... )Also applies to: 1218-1233
🤖 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/api2/mapping/EventMappingTestArguments.kt` around lines 542 - 556, Add a null-counter mapping test for NotificationMarkUnreadEventDto because the existing notificationMarkUnreadDto only covers non-null unread counts and does not exercise the ?: 0 fallback in the mapper. Create an additional DTO/domain pair in EventMappingTestArguments with null values for the count fields (such as unread_messages, total_unread_count, unread_channels, and grouped_unread_channels), and include that pair in arguments(). Use the existing EventMappingTestArguments setup and NotificationMarkUnreadEventDto symbol to place the new case alongside the current mapping fixtures.stream-chat-android-client/src/test/java/io/getstream/chat/android/client/api2/MoshiChatApiTestArguments.kt (1)
449-479: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCover the missing-
unread_channelsfallback in the API-layer test inputs.
MoshiChatApi.queryGroupedChannelsconvertsunread_channelswith?: 0, but this argument set only exercises non-null values. The adapter test instream-chat-android-client/src/test/java/io/getstream/chat/android/client/parser2/QueryGroupedChannelsResponseAdapterTest.ktonly protects DTO parsing, so the domain-mapping fallback can still regress unnoticed.🤖 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/api2/MoshiChatApiTestArguments.kt` around lines 449 - 479, Add a test case in queryGroupedChannelsInput that exercises QueryGroupedChannelsResponse with unread_channels set to null, so MoshiChatApi.queryGroupedChannels’ fallback to 0 is covered. Keep the existing RetroSuccess/RetroError structure, but include a grouped channel response with unread_channels omitted or null in QueryGroupedChannelsGroup, and reference the queryGroupedChannelsInput helper so the API-layer mapping path is validated alongside the adapter test.stream-chat-android-client/src/main/java/io/getstream/chat/android/client/ChatClient.kt (1)
3352-3398: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd thread/state notes to the new grouped-query KDoc.
These new public APIs document parameters, but they still omit the thread expectations and state notes required for
stream-chat-android-clientpublic APIs. That contract matters here because the call now participates in plugin callbacks and grouped state restoration.As per coding guidelines, "Document public APIs with KDoc, including thread expectations and state notes."
🤖 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/ChatClient.kt` around lines 3352 - 3398, The new KDoc on ChatClient.queryGroupedChannels and queryGroupedChannelsInternal is missing the public API thread/state contract. Update the documentation to include the required thread expectations and state notes alongside the existing parameter descriptions, matching the style used by other public APIs in ChatClient. Ensure both overloads clearly state any main-thread or background-thread expectations and any relevant state/restore behavior.Source: Coding guidelines
stream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/state/event/handler/grouped/internal/GroupAwareChatEventHandlerFactory.kt (1)
19-24: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove the singleton fallback for
clientState.Every shown call site already passes
clientState, soChatClient.instance().clientStateonly adds a hidden global dependency. If this constructor is ever used without the explicit argument, grouped handlers can attach to the wrong client state in tests or multi-client flows.♻️ Proposed fix
-import io.getstream.chat.android.client.ChatClient import io.getstream.chat.android.client.api.event.ChatEventHandler import io.getstream.chat.android.client.api.event.ChatEventHandlerFactory import io.getstream.chat.android.client.setup.state.ClientState @@ internal class GroupAwareChatEventHandlerFactory( private val groupKey: String, private val resolver: ChannelGroupResolver = DefaultChannelGroupResolver(), - private val clientState: ClientState = ChatClient.instance().clientState, + private val clientState: ClientState, ) : ChatEventHandlerFactory(clientState) {Also applies to: 33-37
🤖 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/event/handler/grouped/internal/GroupAwareChatEventHandlerFactory.kt` around lines 19 - 24, Remove the singleton fallback for clientState in GroupAwareChatEventHandlerFactory and its grouped handler construction path. The constructor and factory should require the explicit ClientState already being passed by the call sites, instead of defaulting to ChatClient.instance().clientState. Update the GroupAwareChatEventHandlerFactory constructor and any related create/build logic so the grouped handlers always use the provided clientState and no hidden global dependency remains.stream-chat-android-client/src/test/java/io/getstream/chat/android/client/internal/state/plugin/listener/internal/QueryGroupedChannelsListenerStateTest.kt (1)
287-388: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a mixed first-page + paginated regression case.
These tests only cover responses where every requested group is either first-page or paginated. They won’t catch the case where one group carries a cursor and another is still a first-page refresh, which is where grouped unread merging diverges.
🤖 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/plugin/listener/internal/QueryGroupedChannelsListenerStateTest.kt` around lines 287 - 388, Add a mixed regression test in QueryGroupedChannelsListenerStateTest for onQueryGroupedChannelsResult where one requested group has a cursor (next or prev) and another remains a first-page refresh. Verify the paginated group is treated as not first page while the other still uses first-page behavior, and that grouped unread merging/update logic handles both cases in the same response. Reuse the existing listener, queryChannelsLogic, and GroupedChannelsGroupQuery setup patterns from the nearby tests to locate the correct assertions.stream-chat-android-client/src/test/java/io/getstream/chat/android/client/internal/state/internal/SyncManagerTest.kt (1)
93-93: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocument or remove the new suppression annotations.
These suppressions are newly added but not documented with rationale. Please add short justification comments (or refactor to avoid suppression).
Suggested patch
-@Suppress("LargeClass") +// Large reconnect coverage matrix is intentionally centralized in this test class. +@Suppress("LargeClass") `@OptIn`(InternalStreamChatApi::class) `@ExperimentalCoroutinesApi` internal class SyncManagerTest { @@ - `@Suppress`("LongMethod") + // Kept as a single scenario for readability of end-to-end reconnect assertions. + `@Suppress`("LongMethod") fun `dual-mode reconnect updateActiveChannels should query only cids not covered by grouped or standard`() = @@ - `@Suppress`("LongMethod") + // Kept as a single scenario for readability of end-to-end reconnect assertions. + `@Suppress`("LongMethod") fun `dual-mode reconnect should exclude tracked watched cids from updateActiveChannels`() =As per coding guidelines, "Use
@OptInannotations explicitly in Kotlin code; avoid suppressions unless documented".Also applies to: 818-819, 907-908
🤖 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/internal/SyncManagerTest.kt` at line 93, The new suppression annotations in SyncManagerTest are undocumented and should be justified or removed. Update the affected spots in SyncManagerTest, including the LargeClass suppression and the other newly added suppressions, by either refactoring to avoid them or adding brief inline comments explaining why each suppression is necessary. Keep the rationale near the relevant annotations so future readers understand the exception.Source: Coding guidelines
🤖 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/api/stream-chat-android-client.api`:
- Around line 1842-1856: MarkAllReadEvent now exposes a changed JVM
constructor/copy signature because groupedUnreadChannels was added, which can
break already-compiled callers. Update MarkAllReadEvent in the API surface to
preserve the old constructor and copy(...) descriptors for binary compatibility,
and introduce the new groupedUnreadChannels path via overloads or another
backward-compatible mechanism rather than replacing the existing signatures.
In
`@stream-chat-android-client/src/main/java/io/getstream/chat/android/client/api/state/QueryChannelsState.kt`:
- Around line 74-82: QueryChannelsState is exposing an internal
GroupedQueryConfig type through a public StateFlow, which leaks an internal
package into the SDK API surface. Update the QueryChannelsState contract so it
no longer returns
io.getstream.chat.android.client.internal.state.plugin.state.querychannels.GroupedQueryConfig
directly: either move the config model to a public package or replace
groupedQueryConfig with a public-facing representation/individual fields. Keep
the change additive and preserve binary compatibility by updating any related
state/query code paths that read or write groupedQueryConfig.
In
`@stream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/state/model/querychannels/pagination/internal/Mapper.kt`:
- Around line 26-33: The offline pagination mapping in
QueryChannelsRequest.toOfflinePaginationRequest() is dropping memberLimit before
it reaches AnyChannelPaginationRequest. Update the conversion path through
QueryChannelsPaginationRequest.toAnyChannelPaginationRequest() so memberLimit is
copied alongside messageLimit, keeping the offline query aligned with the
original QueryChannelsRequest.
In
`@stream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/state/plugin/listener/internal/QueryGroupedChannelsListenerState.kt`:
- Around line 68-76: The grouped unread counts update is currently gated on the
entire request being a first page, which causes mixed responses to skip unread
merging for groups that are still first-page refreshes. Update
QueryGroupedChannelsListenerState’s pagination check so each returned group is
evaluated independently, and only skip unread count merging for the specific
groups that have next/prev cursors while still applying
groupedUnreadChannelsUpdater.calculateUpdatedCounts and
globalState.setGroupedUnreadChannels for the eligible groups.
In
`@stream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/state/plugin/logic/querychannels/internal/QueryChannelsStateLogic.kt`:
- Around line 251-255: trackChannel is still writing the passed Channel snapshot
even when a fresher active ChannelState already exists. Update
QueryChannelsStateLogic.trackChannel to look up the current channel by cid from
the active state/raw state first and reuse that value when present, only falling
back to the channel argument when nothing is tracked yet. Keep the cids update
as-is, but make the setChannels call prefer the existing in-state channel over
the stale parameter.
In
`@stream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/state/plugin/state/querychannels/internal/QueryChannelsMutableState.kt`:
- Around line 87-90: The grouped QueryChannelsSpec in QueryChannelsMutableState
is using a placeholder QuerySortByField.descByName("last_updated"), which causes
grouped results to be re-sorted by the client instead of preserving server
order. Update the grouped branch in the identifier handling to avoid injecting a
fake sort and make the grouped path bypass the sortedChannels pipeline or
otherwise keep the response order intact. Use the
QueryChannelsIdentifier.Grouped and QueryChannelsSpec construction as the main
places to adjust this behavior.
In
`@stream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/state/sync/internal/SyncManager.kt`:
- Around line 419-425: The grouped-query recovery path in SyncManager should not
swallow failures from queryGroupedChannelsInternal by converting them to
emptySet(), because restoreActiveChannels then treats a failed refresh as
success. Update the grouped-query flow in
updateGroupedQueryChannels/queryGroupedChannelsInternal so errors are propagated
like the standard path, and let restoreActiveChannels abort or surface the
failure instead of continuing with stale grouped query state.
- Around line 445-449: The fallback in SyncManager.updateActiveChannels is being
skipped whenever hasGroupedQueries is true, which leaves some active
ChannelLogic states stale after reconnect. Update the recovery flow in
SyncManager so the fallback still runs when only grouped queries exist, and pass
groupedHandledCids as the cidsToExclude set instead of bypassing
updateActiveChannels entirely. Keep the existing standard-query behavior intact
by preserving the hasStandardQueries path and adjusting the conditional around
the active-channel recovery block.
In
`@stream-chat-android-client/src/test/java/io/getstream/chat/android/client/internal/state/event/handler/internal/EventHandlerSequentialTest.kt`:
- Around line 534-619: The grouped unread cases in
groupedUnreadChannelsArguments() are defined but never executed, so add a
`@ParameterizedTest` in EventHandlerSequentialTest that consumes this provider,
sets up MutableGlobalState.groupedUnreadChannels with the initial value, runs
the event handling flow used by the other sequential tests, and asserts the
final grouped unread map matches the expected result for each case. Use the
existing helpers and symbols like groupedUnreadChannelsArguments(),
MutableGlobalState, and the event handler test harness in this class to wire the
new parameterized coverage into the suite.
In
`@stream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/viewmodel/channels/ChannelListViewModel.kt`:
- Around line 1026-1032: The grouped search branch in ChannelListViewModel’s
QueryMode.Grouped path is missing the selected group constraint, so it can
return channels from other groups. Update the QueryChannelsRequest built there
to include the channel group predicate whenever the current group key is not the
sentinel value, and keep the existing optimizedChannelSearchFilter(searchQuery)
behavior intact. Use the QueryMode.Grouped branch and
optimizedChannelSearchFilter logic as the main locations to adjust so grouped
lists stay scoped to the channel’s group custom field.
- Around line 1155-1160: Grouped load-more failures are only logged in
loadMoreGroupedChannels and never reflected in channelsState, so the UI can miss
pagination errors. Update ChannelListViewModel’s grouped pagination path to set
loadingError when result.isSuccess is false and clear it when the load succeeds,
alongside the existing isLoadingMore reset. Make the state update in the same
place where loadMoreGroupedChannels handles result and channelsState.copy so
grouped pagination behaves like the standard pagination flow.
- Around line 242-249: The grouped ChannelListViewModel constructor currently
collides with the predefined-filter overload because
ChannelListViewModel(chatClient, "...") can match both when the trailing
parameters all have defaults. Update the grouped entry point in
ChannelListViewModel so it stays unambiguous, and align
ChannelListViewModelFactory to pass groupKey first so the grouped constructor is
clearly distinct from the predefined one.
In
`@stream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/viewmodel/channels/ChannelListViewModelFactory.kt`:
- Around line 135-150: The grouped ChannelListViewModelFactory constructor
currently defaults draftMessagesEnabled differently than the grouped
ChannelListViewModel constructor, so align the factory’s default with the
grouped ViewModel path. Update the grouped constructor in
ChannelListViewModelFactory to use the same default value as
ChannelListViewModel(groupKey, ...) and keep the forwarded draftMessagesEnabled
parameter unchanged so grouped mode behaves consistently regardless of
construction path.
---
Outside diff comments:
In `@stream-chat-android-client/api/stream-chat-android-client.api`:
- Around line 926-944: The public state interfaces are being made source/binary
incompatible by adding required abstract members to GlobalState and
QueryChannelsState. Update the API so these new state accessors are exposed
through a backward-compatible approach, such as a new sub-interface or
default-backed accessors on the existing interface, rather than adding mandatory
abstract properties. Keep the existing interface contracts stable and preserve
compatibility for external implementations that already implement GlobalState or
QueryChannelsState.
In
`@stream-chat-android-client/src/main/java/io/getstream/chat/android/client/api/state/ChatClientStateExtensions.kt`:
- Around line 224-235: The `watchChannelAsState(cid, messageLimit,
aroundMessageId, coroutineScope)` overload in `ChatClientStateExtensions` should
not return the raw `getStateOrNull(...)` flow; instead, wrap the result in
`WatchedChannelStateFlow` like the other `watchChannelAsState` overload and
register it with `state.trackWatchedChannel(...)`. Make sure the
`aroundMessageId` variant uses the same watched-channel tracking path as the
existing overload so reconnect logic can re-watch those channels automatically.
In
`@stream-chat-android-compose/src/test/kotlin/io/getstream/chat/android/compose/viewmodel/channels/ChannelListViewModelTest.kt`:
- Around line 1379-1389: The grouped-state fixture is too loose because
stateRegistry.queryChannels(any()) will match any key, so the new groupKey
coverage can pass even if ChannelListViewModel uses the wrong query state.
Update the stubbing in ChannelListViewModelTest around queryChannelsState to
match or capture the expected grouped identifier (the groupKey passed into
ChannelListViewModel) and apply the same tightening to the related grouped-state
setup later in the test so the fixtures verify the intended wiring.
---
Nitpick comments:
In
`@stream-chat-android-client/src/main/java/io/getstream/chat/android/client/ChatClient.kt`:
- Around line 3352-3398: The new KDoc on ChatClient.queryGroupedChannels and
queryGroupedChannelsInternal is missing the public API thread/state contract.
Update the documentation to include the required thread expectations and state
notes alongside the existing parameter descriptions, matching the style used by
other public APIs in ChatClient. Ensure both overloads clearly state any
main-thread or background-thread expectations and any relevant state/restore
behavior.
In
`@stream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/state/event/handler/grouped/internal/GroupAwareChatEventHandlerFactory.kt`:
- Around line 19-24: Remove the singleton fallback for clientState in
GroupAwareChatEventHandlerFactory and its grouped handler construction path. The
constructor and factory should require the explicit ClientState already being
passed by the call sites, instead of defaulting to
ChatClient.instance().clientState. Update the GroupAwareChatEventHandlerFactory
constructor and any related create/build logic so the grouped handlers always
use the provided clientState and no hidden global dependency remains.
In
`@stream-chat-android-client/src/test/java/io/getstream/chat/android/client/api2/mapping/EventMappingTestArguments.kt`:
- Around line 542-556: Add a null-counter mapping test for
NotificationMarkUnreadEventDto because the existing notificationMarkUnreadDto
only covers non-null unread counts and does not exercise the ?: 0 fallback in
the mapper. Create an additional DTO/domain pair in EventMappingTestArguments
with null values for the count fields (such as unread_messages,
total_unread_count, unread_channels, and grouped_unread_channels), and include
that pair in arguments(). Use the existing EventMappingTestArguments setup and
NotificationMarkUnreadEventDto symbol to place the new case alongside the
current mapping fixtures.
In
`@stream-chat-android-client/src/test/java/io/getstream/chat/android/client/api2/MoshiChatApiTestArguments.kt`:
- Around line 449-479: Add a test case in queryGroupedChannelsInput that
exercises QueryGroupedChannelsResponse with unread_channels set to null, so
MoshiChatApi.queryGroupedChannels’ fallback to 0 is covered. Keep the existing
RetroSuccess/RetroError structure, but include a grouped channel response with
unread_channels omitted or null in QueryGroupedChannelsGroup, and reference the
queryGroupedChannelsInput helper so the API-layer mapping path is validated
alongside the adapter test.
In
`@stream-chat-android-client/src/test/java/io/getstream/chat/android/client/internal/state/internal/SyncManagerTest.kt`:
- Line 93: The new suppression annotations in SyncManagerTest are undocumented
and should be justified or removed. Update the affected spots in
SyncManagerTest, including the LargeClass suppression and the other newly added
suppressions, by either refactoring to avoid them or adding brief inline
comments explaining why each suppression is necessary. Keep the rationale near
the relevant annotations so future readers understand the exception.
In
`@stream-chat-android-client/src/test/java/io/getstream/chat/android/client/internal/state/plugin/listener/internal/QueryGroupedChannelsListenerStateTest.kt`:
- Around line 287-388: Add a mixed regression test in
QueryGroupedChannelsListenerStateTest for onQueryGroupedChannelsResult where one
requested group has a cursor (next or prev) and another remains a first-page
refresh. Verify the paginated group is treated as not first page while the other
still uses first-page behavior, and that grouped unread merging/update logic
handles both cases in the same response. Reuse the existing listener,
queryChannelsLogic, and GroupedChannelsGroupQuery setup patterns from the nearby
tests to locate the correct assertions.
🪄 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
Run ID: 0961f914-409e-4a44-bff2-3ef5a0a718c1
📒 Files selected for processing (71)
stream-chat-android-client-test/src/main/java/io/getstream/chat/android/client/test/Mother.ktstream-chat-android-client/api/stream-chat-android-client.apistream-chat-android-client/src/main/java/io/getstream/chat/android/client/ChatClient.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/api/ChatApi.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/api/state/ChatClientStateExtensions.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/api/state/GlobalState.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/api/state/QueryChannelsState.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/api2/MoshiChatApi.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/api2/endpoint/ChannelApi.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/api2/mapping/EventMapping.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/api2/model/dto/EventDtos.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/api2/model/requests/QueryGroupedChannelsRequest.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/api2/model/response/QueryGroupedChannelsResponse.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/events/ChatEvent.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/offline/repository/database/internal/ChatDatabase.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/offline/repository/domain/queryChannels/internal/DatabaseQueryChannelsRepository.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/offline/repository/domain/queryChannels/internal/QueryChannelsEntity.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/state/event/handler/grouped/internal/ChannelGroupExtensions.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/state/event/handler/grouped/internal/ChannelGroupResolver.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/state/event/handler/grouped/internal/DefaultChannelGroupResolver.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/state/event/handler/grouped/internal/GroupAwareChatEventHandler.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/state/event/handler/grouped/internal/GroupAwareChatEventHandlerFactory.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/state/event/handler/grouped/internal/GroupedUnreadChannelsUpdater.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/model/querychannels/pagination/internal/Mapper.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/state/plugin/QueryChannelsIdentifier.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/internal/StatePlugin.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/state/plugin/listener/internal/QueryChannelsListenerState.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/state/plugin/listener/internal/QueryGroupedChannelsListenerState.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/state/plugin/logic/internal/LogicRegistry.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/state/plugin/logic/querychannels/internal/QueryChannelsLogic.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/global/internal/MutableGlobalState.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/state/plugin/state/internal/ChatClientStateCalls.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/state/plugin/state/internal/WatchedChannelStateFlow.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/state/plugin/state/querychannels/GroupedQueryConfig.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/state/plugin/state/querychannels/internal/QueryChannelsMutableState.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/state/sync/internal/SyncManager.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/plugin/Plugin.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/plugin/listeners/QueryGroupedChannelsListener.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/query/QueryChannelsSpec.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/ChatClientGroupedChannelsApiTests.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/EventChatJsonProvider.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/api2/MoshiChatApiTest.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/api2/MoshiChatApiTestArguments.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/api2/mapping/EventMappingTestArguments.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/internal/offline/Mother.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/internal/offline/repository/QueryChannelsImplRepositoryTest.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/internal/state/event/handler/grouped/internal/DefaultChannelGroupResolverTest.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/internal/state/event/handler/grouped/internal/GroupAwareChatEventHandlerTest.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/internal/state/event/handler/grouped/internal/GroupedUnreadChannelsUpdaterTest.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/internal/SyncManagerTest.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/internal/state/plugin/listener/internal/QueryGroupedChannelsListenerStateTest.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/internal/state/plugin/logic/internal/LogicRegistryTest.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/internal/state/plugin/logic/querychannels/internal/QueryChannelsLogicGroupedTest.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/internal/state/plugin/logic/querychannels/internal/QueryChannelsLogicTest.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/StateRegistryTest.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/internal/state/plugin/state/internal/ChatClientStateCallsTest.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/internal/state/plugin/state/querychannels/internal/QueryChannelsMutableStateTest.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/parser/EventArguments.ktstream-chat-android-client/src/test/java/io/getstream/chat/android/client/parser2/QueryGroupedChannelsResponseAdapterTest.ktstream-chat-android-compose/api/stream-chat-android-compose.apistream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/viewmodel/channels/ChannelListViewModel.ktstream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/viewmodel/channels/ChannelListViewModelFactory.ktstream-chat-android-compose/src/test/kotlin/io/getstream/chat/android/compose/viewmodel/channels/ChannelListViewModelTest.ktstream-chat-android-core/api/stream-chat-android-core.apistream-chat-android-core/src/main/java/io/getstream/chat/android/models/GroupedChannels.kt
aleksandar-apostolov
approved these changes
Jun 24, 2026
- Move GroupedQueryConfig to public api.state.querychannels package - Propagate memberLimit through offline pagination mapper - Add ParameterizedTest for grouped unread channel events - Reorder ChannelListViewModel grouped constructor to (groupKey, chatClient) to disambiguate from the predefined constructor - Set loadingError in loadMoreGroupedChannels for UI feedback on failures Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
andremion
approved these changes
Jun 25, 2026
| val total_unread_count: Int = 0, | ||
| val unread_channels: Int = 0, | ||
| val channel_message_count: Int? = null, | ||
| val grouped_unread_channels: Map<String, Int>? = null, |
Contributor
There was a problem hiding this comment.
Not a blocker: This DTO gets grouped_unread_channels and EventMapping maps it onto NewMessageEvent, but message.new also has a hand-written parser, NewMessageEventAdapter, that runs when fastEventParsing is on (registered in DirectEventParser). That adapter is not part of this PR and it does not read grouped_unread_channels, so it builds NewMessageEvent with the field as null. Once the direct parser succeeds, MoshiChatParser returns its result and skips the DTO path, so with fastEventParsing on the grouped counts sent on message.new get dropped.
It is a narrow case (only when both fastEventParsing and grouped channels are enabled), and it is not a crash. The counts still update from the notification events and from channel.updated. It also does not match the "both paths produce identical events" check in NewMessageEventParsingTest, which passes here only because jsonAllFields does not include the field yet.
Could we add the field to NewMessageEventAdapter, and to jsonAllFields so the parity test covers it? Happy to take it in a follow-up if you would rather keep this PR focused.
Contributor
Author
There was a problem hiding this comment.
Nice catch, I will add it here!
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Goal
Resolves: https://linear.app/stream/issue/AND-1260/port-groupedquerychannels-to-v7
Port PR #6437 (V6 squash commit
bfdc97edcf0754c4ba68bd638ba5a880841cfa04) fromv6todevelop.Adds support for the server-driven grouped-channels API (
POST /channels/grouped), where the backend partitions the channel list into named groups (e.g.direct,support) and returns per-group channels, pagination cursors, and unread counts. Surfaces those grouped unread counts on relevant chat events, and provides a ComposeChannelListViewModelpath that drives a UI off a group key without the consumer needing to know about filter/sort.This was not a clean cherry-pick —
develophas thestream-chat-android-stateandstream-chat-android-offlinemodules merged intostream-chat-android-client(PR #6069), so every file from those V6 modules required path + package rewriting before applying. Path mapping:stream-chat-android-state/src/.../state/X→stream-chat-android-client/src/.../internal/state/X(withclient.internal.state.*package)stream-chat-android-offline/src/.../offline/X→stream-chat-android-client/src/.../internal/offline/X(withclient.internal.offline.*package)StateRegistry,GlobalState,QueryChannelsState,ChatClientStateExtensions) live underclient/api/state/on develop, notclient/internal/state/plugin/.ChatEventHandler,ChatEventHandlerFactory,DefaultChatEventHandler,EventHandlingResult) live underclient/api/event/on develop.Implementation
ChatClient.queryGroupedChannels(limit, groups, watch, presence)returningGroupedChannels(per-groupchannels+unreadChannels+next/prevcursors). Per-group request options viaGroupedChannelsGroupQuery. Backed byPOST /channels/grouped(ChannelApi).QueryGroupedChannelsListener; theStatePluginimplementation merges returned per-group unread counts intoGlobalState.groupedUnreadChannelsand routes each returned group into a state keyed by a new sealedQueryChannelsIdentifier.Grouped(groupKey)variant (alongsideStandardandPredefined).QueryChannelsLogicbranches on identifier;applyGroupedResultreplaces channels on the first page (resettingchannelsOffsetdefensively) and appends on subsequent pages (driven off the request'snextcursor); persists per-group state under agroupKey-derived DB key.HasGroupedUnreadChannelsmarker onNewMessageEvent,NotificationMessageNewEvent,NotificationMarkReadEvent,NotificationMarkUnreadEvent,NotificationChannelDeletedEvent,NotificationChannelTruncatedEvent,MarkAllReadEvent.EventHandlerSequentialupdatesGlobalState.groupedUnreadChannelswhenever an inbound event carries the map.GroupedUnreadChannelsUpdaterhandles delta migration onchannel.updatedevents when a channel changes group.GroupAwareChatEventHandler+GroupAwareChatEventHandlerFactoryroute channels into the right grouped state based on the channel'sgroupcustom field viaDefaultChannelGroupResolver. Auto-installed byLogicRegistryforQueryChannelsIdentifier.Groupedqueries.StateRegistrytracks watched-channel state flows viaWatchedChannelStateFlowweak references soSyncManagercan re-watch them on reconnect without pinning lifecycle.SyncManagerre-issuesqueryGroupedChannelsfor active grouped logics on reconnect, reusing each group's capturedGroupedQueryConfig(limit, pageSize, watch, presence).updateActiveChannelsexcludes cids already covered by grouped/standard queries and watched flows.ChannelListViewModel(chatClient, groupKey, ...)constructor drives the UI off a group key. Init viachatClient.initGroupedQueryChannelsAsState(identifier)(no remote call). Load-more uses cursor pagination viaqueryGroupedChannelsInternal.groupKey: String? = nullas a 7th primary-ctor field with default. Pre-existing 2-arg and 6-arg constructors +copy()overloads preserved as manual secondary constructors / copy overloads to maintain binary compatibility with develop's previous published API.ChatDatabaseversion bumped 201 → 202 to reflect the newgroupKeycolumn onQueryChannelsEntity.UI Changes
No UI changes. New
ChannelListViewModel(chatClient, groupKey, ...)constructor is the new public surface for consumers wanting grouped channel lists.Testing
./gradlew :stream-chat-android-client:compileDebugKotlin :stream-chat-android-compose:compileDebugKotlin— clean./gradlew :stream-chat-android-client:detekt :stream-chat-android-compose:detekt— clean./gradlew :stream-chat-android-client:spotlessCheck :stream-chat-android-compose:spotlessCheck— clean./gradlew :stream-chat-android-client:apiDump :stream-chat-android-compose:apiDump :stream-chat-android-core:apiDump— regeneratedQueryChannelsLogicGroupedTest,QueryGroupedChannelsListenerStateTest,GroupAwareChatEventHandlerTest,GroupedUnreadChannelsUpdaterTest,DefaultChannelGroupResolverTest,ChatClientGroupedChannelsApiTests,QueryGroupedChannelsResponseAdapterTest,QueryChannelsLogicTest,QueryChannelsStateLogicTest,QueryChannelsMutableStateTest,StateRegistryTest,LogicRegistryTest,ChatClientStateCallsTest,SyncManagerTest,EventHandlerSequentialTest,QueryChannelsImplRepositoryTest,ChannelListViewModelTest,ChannelViewModelFactoryTest.Known follow-ups
A small number of net-new V6 tests were not ported in this PR because they depend on V6-only test helpers (notably the parameterized
groupedUnreadChannelsArgumentsand related random-event helpers inEventHandlerSequentialTest, plus therandomNotificationRemovedFromChannelEventhelper). The underlying production logic IS ported; only those specific test additions are deferred. Worth a follow-up PR to add them back.Summary by CodeRabbit
New Features
Bug Fixes
Tests