ChannelLogic refactor (part 1) and event handling bug fixes
#6038
Conversation
… events and mark deprecated properties
SDK Size Comparison 📏
|
VelikovPetar
changed the title
ChannelLogic refactor (part 1)ChannelLogic refactor (part 1) and event handling bug fixes
WalkthroughAdds ChannelEventHandler and ChannelLogicImpl, converts ChannelLogic to an interface, changes stateLogic from function to property, renames toggleHidden→setHidden, simplifies typing handling, adds ThreadInfo test factories, updates many listener/logic tests, and appends a changelog entry. Changes
Sequence Diagram(s)%%{init: {"themeVariables": {"actorBorder":"#2b2b2b", "actorBkg":"#f6f8fa", "signal":"#8b5cf6"}}}%%
sequenceDiagram
autonumber
participant Caller as Caller
participant Logic as ChannelLogicImpl
participant Handler as ChannelEventHandler
participant StateLogic as ChannelStateLogic
participant Mutable as ChannelMutableState
Caller->>Logic: handleEvent(event)
Logic->>Handler: handle(event)
alt new message / notification
Handler->>StateLogic: upsertMessage(message)
StateLogic->>Mutable: upsertMessage(message)
else message update
Handler->>StateLogic: updateMessage(message)
StateLogic->>Mutable: updateMessage(message)
else message delete
Handler->>StateLogic: deleteMessage(message)
StateLogic->>Mutable: deleteMessage(message)
else visibility change
Handler->>StateLogic: setHidden(flag)
StateLogic->>Mutable: setHidden(flag)
else typing / read / presence
Handler->>StateLogic: updateTypingEvent / updateRead / updateDelivered(...)
StateLogic->>Mutable: apply updates
end
Mutable-->>StateLogic: state changed
StateLogic-->>Logic: acknowledge
Logic-->>Caller: done
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Repository UI Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used📓 Path-based instructions (3)**/*.{kt,kts}📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.kt📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/src/test/**/*.kt📄 CodeRabbit inference engine (AGENTS.md)
Files:
🧠 Learnings (1)📚 Learning: 2025-12-17T15:00:07.506ZApplied to files:
⏰ 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)
🔇 Additional comments (4)
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: 4
🧹 Nitpick comments (4)
stream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelEventHandler.kt (1)
75-86: Consider documenting thegetCurrentUserIdparameter.The KDoc documents
cidandstateLogicbut omitsgetCurrentUserId. As per coding guidelines, public APIs should include complete KDoc.🔎 Suggested documentation improvement:
/** * Class responsible for updating the local database based on channel-related events. * * @param cid The channel identifier. * @param stateLogic The [ChannelStateLogic] instance used to update the channel state. - * @param getCurrentUserId Returns the currently logged in user ID. + * @param getCurrentUserId A function that returns the currently logged in user ID, or null if not logged in. */CHANGELOG.md (1)
37-39: Polish wording for clarity in the new fixed itemsThe current bullets are understandable but could read more clearly in English. Consider tightening them slightly:
Suggested text tweaks
- Fix `notification.thread_message_new` event upserting the message in the channel even when not sent to channel. [#6038](...) + Fix `notification.thread_message_new` event upserting the message in the channel even when it was not sent to the channel. [#6038](...) - Fix `message.updated` and `message.deleted` events unhiding the channel. [#6038](...) + Fix `message.updated` and `message.deleted` events unintentionally unhiding the channel. [#6038](...) - Fix inserting instead of updating a message breaking pagination. [#6038](...) + Fix inserting a message instead of updating it, which was breaking pagination. [#6038](...)stream-chat-android-client-test/src/main/java/io/getstream/chat/android/client/test/Mother.kt (1)
296-296: Consider defaulting thread to null for backward compatibility.The default
thread = randomThreadInfo()means tests that don't explicitly setthreadwill now receive a randomThreadInfo. This could cause unexpected behavior in existing tests that assume a channel-level read event (no thread).For example, in
WhenHandleEvent.ktline 245, the test explicitly setsthread = nullto work around this. Consider usingthread: ThreadInfo? = nullas the default to maintain backward compatibility.🔎 Suggested change:
public fun randomNotificationMarkReadEvent( createdAt: Date = Date(), user: User = randomUser(), cid: String = randomCID(), channelType: String = randomString(), channelId: String = randomString(), totalUnreadCount: Int = randomInt(), unreadChannels: Int = randomInt(), lastReadMessageId: String? = randomString(), threadId: String? = randomString(), - thread: ThreadInfo? = randomThreadInfo(), + thread: ThreadInfo? = null, unreadThreads: Int? = randomInt(), unreadThreadMessages: Int? = randomInt(), ): NotificationMarkReadEvent = NotificationMarkReadEvent(stream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelLogicImpl.kt (1)
226-268: Consider adding error handling for gap-filling coroutine.The
coroutineScope.launchblock lacks error handling. If the recursivefillTheGapcall orrunChannelQueryOnlinefails, the error is silently swallowed. Consider adding aCoroutineExceptionHandleror logging failures.🔎 Suggested improvement:
private fun fillTheGap( messageLimit: Int, loadedMessages: List<Message>, requestedMessages: List<Message>, ) { if (loadedMessages.isEmpty() || requestedMessages.isEmpty() || messageLimit <= 0) return coroutineScope.launch { + try { val loadedMessageIds = loadedMessages .filter { it.getCreatedAtOrNull() != null } .sortedBy { it.getCreatedAtOrDefault(NEVER) } .map { it.id } // ... rest of the logic - }?.onSuccess { fillTheGap(messageLimit, loadedMessages, it.messages) } + }?.onSuccess { fillTheGap(messageLimit, loadedMessages, it.messages) } + } catch (e: Exception) { + logger.e(e) { "[fillTheGap] Failed to fill message gap" } + } } }
📜 Review details
Configuration used: defaults
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 (26)
CHANGELOG.md(1 hunks)stream-chat-android-client-test/src/main/java/io/getstream/chat/android/client/test/Mother.kt(4 hunks)stream-chat-android-client/src/main/java/io/getstream/chat/android/client/channel/state/ChannelState.kt(2 hunks)stream-chat-android-core/src/testFixtures/kotlin/io/getstream/chat/android/Mother.kt(2 hunks)stream-chat-android-state/src/main/java/io/getstream/chat/android/state/event/handler/internal/utils/ChatEventUtils.kt(1 hunks)stream-chat-android-state/src/main/java/io/getstream/chat/android/state/extensions/ChatClient.kt(4 hunks)stream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/listener/internal/EditMessageListenerState.kt(2 hunks)stream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/listener/internal/HideChannelListenerState.kt(3 hunks)stream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/listener/internal/PushPreferencesListenerState.kt(1 hunks)stream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/listener/internal/QueryChannelListenerState.kt(1 hunks)stream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/listener/internal/SendGiphyListenerState.kt(1 hunks)stream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelEventHandler.kt(1 hunks)stream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelLogic.kt(2 hunks)stream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelLogicImpl.kt(1 hunks)stream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelStateLogic.kt(3 hunks)stream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/logic/internal/LogicRegistry.kt(4 hunks)stream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/state/StateRegistry.kt(1 hunks)stream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/state/channel/internal/ChannelMutableState.kt(3 hunks)stream-chat-android-state/src/test/java/io/getstream/chat/android/state/channel/controller/WhenHandleEvent.kt(8 hunks)stream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/listener/internal/DeleteMessageListenerStateTest.kt(1 hunks)stream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/listener/internal/EditMessageListenerStateTest.kt(5 hunks)stream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/listener/internal/HideChannelListenerStateTest.kt(4 hunks)stream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/listener/internal/PushPreferencesListenerStateTest.kt(1 hunks)stream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelEventHandlerTest.kt(1 hunks)stream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelLogicTest.kt(1 hunks)stream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelStateLogicTest.kt(1 hunks)
🧰 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/listener/internal/EditMessageListenerState.ktstream-chat-android-state/src/test/java/io/getstream/chat/android/state/channel/controller/WhenHandleEvent.ktstream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/listener/internal/QueryChannelListenerState.ktstream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/listener/internal/EditMessageListenerStateTest.ktstream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/logic/internal/LogicRegistry.ktstream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/listener/internal/PushPreferencesListenerState.ktstream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/listener/internal/HideChannelListenerState.ktstream-chat-android-state/src/main/java/io/getstream/chat/android/state/extensions/ChatClient.ktstream-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/listener/internal/PushPreferencesListenerStateTest.ktstream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelEventHandler.ktstream-chat-android-state/src/main/java/io/getstream/chat/android/state/event/handler/internal/utils/ChatEventUtils.ktstream-chat-android-core/src/testFixtures/kotlin/io/getstream/chat/android/Mother.ktstream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelLogicTest.ktstream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelLogicImpl.ktstream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/state/StateRegistry.ktstream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/listener/internal/DeleteMessageListenerStateTest.ktstream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelLogic.ktstream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelEventHandlerTest.ktstream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/listener/internal/SendGiphyListenerState.ktstream-chat-android-client-test/src/main/java/io/getstream/chat/android/client/test/Mother.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/channel/state/ChannelState.ktstream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/listener/internal/HideChannelListenerStateTest.ktstream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/state/channel/internal/ChannelMutableState.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/listener/internal/EditMessageListenerState.ktstream-chat-android-state/src/test/java/io/getstream/chat/android/state/channel/controller/WhenHandleEvent.ktstream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/listener/internal/QueryChannelListenerState.ktstream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/listener/internal/EditMessageListenerStateTest.ktstream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/logic/internal/LogicRegistry.ktstream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/listener/internal/PushPreferencesListenerState.ktstream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/listener/internal/HideChannelListenerState.ktstream-chat-android-state/src/main/java/io/getstream/chat/android/state/extensions/ChatClient.ktstream-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/listener/internal/PushPreferencesListenerStateTest.ktstream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelEventHandler.ktstream-chat-android-state/src/main/java/io/getstream/chat/android/state/event/handler/internal/utils/ChatEventUtils.ktstream-chat-android-core/src/testFixtures/kotlin/io/getstream/chat/android/Mother.ktstream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelLogicTest.ktstream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelLogicImpl.ktstream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/state/StateRegistry.ktstream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/listener/internal/DeleteMessageListenerStateTest.ktstream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelLogic.ktstream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelEventHandlerTest.ktstream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/listener/internal/SendGiphyListenerState.ktstream-chat-android-client-test/src/main/java/io/getstream/chat/android/client/test/Mother.ktstream-chat-android-client/src/main/java/io/getstream/chat/android/client/channel/state/ChannelState.ktstream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/listener/internal/HideChannelListenerStateTest.ktstream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/state/channel/internal/ChannelMutableState.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.ktstream-chat-android-state/src/test/java/io/getstream/chat/android/state/channel/controller/WhenHandleEvent.ktstream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/listener/internal/EditMessageListenerStateTest.ktstream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/listener/internal/PushPreferencesListenerStateTest.ktstream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelLogicTest.ktstream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/listener/internal/DeleteMessageListenerStateTest.ktstream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelEventHandlerTest.ktstream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/listener/internal/HideChannelListenerStateTest.kt
🧠 Learnings (4)
📚 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.ktstream-chat-android-state/src/test/java/io/getstream/chat/android/state/channel/controller/WhenHandleEvent.ktstream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/listener/internal/EditMessageListenerStateTest.ktstream-chat-android-state/src/main/java/io/getstream/chat/android/state/extensions/ChatClient.ktstream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/listener/internal/PushPreferencesListenerStateTest.ktstream-chat-android-core/src/testFixtures/kotlin/io/getstream/chat/android/Mother.ktstream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelLogicTest.ktstream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/listener/internal/DeleteMessageListenerStateTest.ktstream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelEventHandlerTest.ktstream-chat-android-client-test/src/main/java/io/getstream/chat/android/client/test/Mother.ktstream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/listener/internal/HideChannelListenerStateTest.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.ktstream-chat-android-state/src/test/java/io/getstream/chat/android/state/channel/controller/WhenHandleEvent.ktstream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/listener/internal/EditMessageListenerStateTest.ktstream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelLogicTest.ktstream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelEventHandlerTest.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.ktstream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/listener/internal/EditMessageListenerStateTest.ktstream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/listener/internal/PushPreferencesListenerStateTest.ktstream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelEventHandlerTest.ktstream-chat-android-client-test/src/main/java/io/getstream/chat/android/client/test/Mother.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/channel/controller/WhenHandleEvent.ktstream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/listener/internal/EditMessageListenerStateTest.ktstream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelEventHandlerTest.kt
🧬 Code graph analysis (4)
stream-chat-android-state/src/test/java/io/getstream/chat/android/state/channel/controller/WhenHandleEvent.kt (1)
stream-chat-android-client-test/src/main/java/io/getstream/chat/android/client/test/Mother.kt (1)
randomNotificationMarkReadEvent(286-314)
stream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelLogicImpl.kt (1)
stream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelLogic.kt (2)
handleEvent(171-171)updateDataForChannel(150-157)
stream-chat-android-client-test/src/main/java/io/getstream/chat/android/client/test/Mother.kt (1)
stream-chat-android-core/src/testFixtures/kotlin/io/getstream/chat/android/Mother.kt (1)
randomThreadInfo(1008-1036)
stream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/state/channel/internal/ChannelMutableState.kt (3)
stream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelLogic.kt (1)
upsertMessage(131-131)stream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelStateLogic.kt (1)
upsertMessage(334-341)stream-chat-android-client/src/main/java/io/getstream/chat/android/client/channel/ChannelMessagesUpdateLogic.kt (2)
upsertMessage(23-40)upsertMessage(26-26)
⏰ 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: API check
- GitHub Check: compare-sdk-sizes / Compare SDK sizes
- GitHub Check: Unit tests
- GitHub Check: Debug build
- GitHub Check: Build / compose apks
🔇 Additional comments (45)
stream-chat-android-state/src/main/java/io/getstream/chat/android/state/extensions/ChatClient.kt (4)
337-337: LGTM! Clean refactoring to new loader method.The change from the old loader to
loadBefore(messageId = null, limit = messageLimit)is correct. Passingnullas the messageId appropriately indicates loading from the oldest available messages.
358-358: LGTM! Correct delegation to new loader method.The change to
loadAfter(messageId = baseMessageId, limit = messageLimit)correctly delegates to the new loader implementation, maintaining the same behavior.
383-383: LGTM! Appropriate refactoring to new loader.The change to
loadAround(messageId)correctly aligns with the new loader naming convention while preserving the function's behavior.
477-477: LGTM! Internal refactoring to new loader.The change from calling
loadMessagesAroundIdto callingloadAround(messageId)directly is a clean refactoring that maintains the same behavior while eliminating an unnecessary indirection.stream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/listener/internal/QueryChannelListenerState.kt (1)
79-79: Centralized error handling is properly implemented.The delegation to
logic.channel(...).onQueryChannelResult(request, result)successfully centralizes result handling. The implementation inChannelLogicImplhandles bothResult.SuccessandResult.Failurecases, delegating tochannelStateLogic.propagateChannelQuery()andchannelStateLogic.propagateQueryError()respectively, maintaining full behavioral consistency with the previous explicit implementation.stream-chat-android-state/src/main/java/io/getstream/chat/android/state/event/handler/internal/utils/ChatEventUtils.kt (1)
72-79: LGTM!The added comment clarifies why
lastReadMessageIdis explicitly set tonullforMarkAllReadEvent, which is helpful for maintainability.stream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/listener/internal/EditMessageListenerState.kt (2)
65-67: LGTM!The property access pattern change is consistent with the refactored
ChannelLogicinterface.
48-50: ThestateLogicaccess patterns are correctly implemented.ChannelLogicexposesstateLogicas a property (accessed without parentheses), whileThreadLogicexposesstateLogicas a function (called with parentheses). The code correctly reflects these different interface definitions.stream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/state/channel/internal/ChannelMutableState.kt (3)
213-214: LGTM!The deprecation annotation clearly communicates that
oldMessageswill be removed in future versions, giving consumers time to migrate.
428-430: LGTM!The simplified
updateTypingEventmethod with a single parameter is cleaner than the previous multi-argument version.
540-552: Key fix for pagination integrity.This guarded update correctly distinguishes between upsert (insert or update) and update-only operations. By skipping updates for non-existent messages, this prevents
message.updatedandmessage.deletedevents from inadvertently inserting out-of-order messages that would break pagination.stream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelEventHandler.kt (4)
149-172: Core bug fix: UsingupdateMessageinstead ofupsertMessagefor update/delete events.This correctly distinguishes between events that should insert messages (
message.new,notification.message_new,notification.thread_message_new) and those that should only update existing messages (message.updated,message.deleted). This prevents pagination breakage when updates introduce out-of-order messages.
174-183: Fix fornotification.thread_message_newhandling.Correctly guards upsert with
showInChannelcheck, ensuring thread-only replies don't appear in the main channel message list. The hidden state update is also correctly guarded by!shadowed.
300-321: Well-designed message update helpers.The
upsertMessagehelper preservescreatedLocallyAtfor proper message sorting and retainsownReactions. TheupdateMessagehelper correctly returns early if the message doesn't exist, preventing phantom message insertions.
243-253: Thread-scoped read events correctly filtered.Read events are properly filtered to exclude thread-scoped reads (
event.thread == null), ensuring channel-level read state isn't affected by thread reads.stream-chat-android-client/src/main/java/io/getstream/chat/android/client/channel/state/ChannelState.kt (2)
63-64: LGTM!The deprecation annotation on the public interface aligns with the implementation in
ChannelMutableState.kt.
135-136: LGTM!Good catch fixing the documentation typo from "performatic" to "performant".
stream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/state/StateRegistry.kt (1)
56-56: LGTM: Idiomatic constructor declaration.Removing the explicit
constructorkeyword is standard Kotlin style when there are no annotations or visibility modifiers on the primary constructor itself.stream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/listener/internal/PushPreferencesListenerStateTest.kt (1)
42-42: LGTM: Mock setup correctly reflects the API change.The mock configuration properly reflects the change from
stateLogic()function tostateLogicproperty, consistent with the broader ChannelLogic refactor.stream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/listener/internal/PushPreferencesListenerState.kt (1)
53-53: LGTM: Property access aligns with the refactored API.The change from function call to property access is correct and consistent with the ChannelLogic interface evolution throughout this PR.
stream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/logic/internal/LogicRegistry.kt (1)
32-32: LGTM! Clean refactoring to interface-based design.The changes correctly implement the ChannelLogic interface pattern:
- Import of ChannelLogicImpl (Line 32)
- Instantiation using ChannelLogicImpl while returning ChannelLogic interface (Lines 119-127)
- Property-based access to stateLogic instead of function calls (Lines 135, 207)
This aligns with the PR's goal of extracting ChannelLogic as an interface with ChannelLogicImpl as the concrete implementation.
Also applies to: 119-127, 135-136, 207-208
stream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/listener/internal/HideChannelListenerState.kt (1)
57-57: LGTM! API improvements applied correctly.The changes correctly apply the refactored API:
- Property access for stateLogic (Lines 57, 74)
- Renamed method from
toggleHidden()tosetHidden()(Lines 57, 85) - the new name is more explicit about the operationAlso applies to: 74-74, 85-85
stream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/listener/internal/EditMessageListenerStateTest.kt (1)
50-50: LGTM! Test mocks updated for property-based access.All mock setups correctly changed from
on(it.stateLogic())toon(it.stateLogic)to match the new property-based API.Also applies to: 89-89, 129-129, 169-169, 209-209
stream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelStateLogicTest.kt (1)
210-210: LGTM! Test assertion updated for simplified typing API.The verification correctly updated from
updateTypingEvents(...)(plural, two arguments) toupdateTypingEvent(...)(singular, one argument), reflecting the API simplification in ChannelMutableState.stream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/listener/internal/DeleteMessageListenerStateTest.kt (1)
45-45: LGTM! Mock setup updated for property-based access.The mock correctly changed from
on(it.stateLogic())toon(it.stateLogic)to align with the new property-based API.stream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/listener/internal/SendGiphyListenerState.kt (1)
41-41: LGTM! Null-safe property access applied correctly.The null-safe chain correctly updated from
?.stateLogic()?to?.stateLogic?while maintaining proper null handling.stream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/listener/internal/HideChannelListenerStateTest.kt (1)
40-40: LGTM! Test updated for both API changes.The test correctly reflects both refactoring changes:
- Mock setup uses property access:
on(it.stateLogic)(Line 40)- Verifications use renamed method:
setHidden(true/false)(Lines 51, 63, 75)Also applies to: 51-51, 63-63, 75-75
stream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelLogicTest.kt (1)
54-60: LGTM! Test correctly uses concrete implementation.The test properly instantiates
ChannelLogicImplwith all required dependencies while keeping thesutvariable typed asChannelLogicinterface. This is good practice for testing the implementation while maintaining interface-based design.stream-chat-android-state/src/test/java/io/getstream/chat/android/state/channel/controller/WhenHandleEvent.kt (6)
91-94: LGTM: Mock delegation for upsertMessage is correctly configured.The stub bridges
channelStateLogic.upsertMessage()tochannelMutableState.upsertMessage(), ensuring the mock propagates messages to state for tests that verify message presence.
105-110: LGTM: Updated to use ChannelLogicImpl.The test correctly instantiates the new
ChannelLogicImplconcrete implementation, aligning with the refactor that extractsChannelLogicas an interface.
134-134: LGTM: Reflects API rename from toggleHidden to setHidden.The verification correctly uses the renamed method
setHidden(false).
153-154: Correct distinction between upsert and update operations.The test now verifies that:
- Initial message insertion calls
upsertMessage(times(1))- Subsequent update event calls
updateMessage(not upsert)This aligns with the PR's bug fix to prevent pagination breakage when updates introduced out-of-order messages.
177-181: LGTM: Poll preservation tests updated to verify updateMessage.Both poll preservation tests correctly verify that message updates (with or without polls) use
updateMessagerather thanupsertMessage, consistent with the event handling refinement.Also applies to: 205-209
245-245: LGTM: Explicit thread = null for channel-level read events.Setting
thread = nullexplicitly clarifies this is a channel-level (not thread-level) read event, making the test intent clearer.stream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelStateLogic.kt (3)
191-198: LGTM: New updateMessage method for in-place message updates.This method correctly delegates to
mutableState.updateMessage(), providing a distinct path fromupsertMessage(). This supports the PR's fix to prevent pagination breakage by not inserting out-of-order messages during updates.
582-586: LGTM: Renamed toggleHidden to setHidden with improved documentation.The method name
setHiddenis more accurate since it sets a specific value rather than toggling. The parameter documentation is also improved from "Boolean." to "Whether the channel is hidden."
303-308: Simplification of typing state update is correct.The
updateTypingEventmethod inChannelMutableStateproperly handles internal state management by updating the_typingmutable state flow. The parameterrawTypingEventsis not needed since thetypingEventobject contains all required information for state updates. Both channel-local and global typing state are correctly updated viamutableState.updateTypingEvent()andglobalMutableState.tryEmitTypingEvent().stream-chat-android-client-test/src/main/java/io/getstream/chat/android/client/test/Mother.kt (1)
71-71: LGTM: Imports added for ThreadInfo support.The imports correctly bring in
ThreadInfoandrandomThreadInfoto support the new parameter inrandomNotificationMarkReadEvent.Also applies to: 90-90
stream-chat-android-core/src/testFixtures/kotlin/io/getstream/chat/android/Mother.kt (2)
1008-1036: LGTM: Well-structured randomThreadInfo factory function.The factory follows the established pattern of other
random*functions in this file:
- All parameters have sensible defaults using existing random helpers
- Parameters correctly map to
ThreadInfoconstructor- Public visibility is appropriate for test fixtures
As per coding guidelines for test files, this helper correctly uses
publicvisibility for shared test utilities.
62-62: LGTM: Import added for ThreadInfo model.stream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelLogicImpl.kt (5)
43-58: LGTM: Well-documented class with clear responsibilities.The KDoc clearly explains the purpose and dependencies. The class properly implements
ChannelLogicinterface and encapsulates channel state management.
60-62: Initialization order is correct but subtle.The initialization relies on
mutableStatebeing initialized first (line 60), theneventHandlerusescid(line 61) which delegates tomutableState.cid. This works because Kotlin initializes properties in declaration order.
172-183: LGTM: Event handling delegated to ChannelEventHandler.The refactor correctly delegates event handling to the new
ChannelEventHandler, keepingChannelLogicImplfocused on channel operations while centralizing event-to-state mapping.
100-118: LGTM: Watch method with proper loading guard.The loading guard at line 103 prevents duplicate watch requests, which protects against UI bugs that could DDOS the API. Good defensive programming.
213-216: Singleton architecture is intentional per Stream Chat design; testing requires mocking tools like MockK.
ChatClient.instance()is the documented, intended pattern for this SDK—ChatClient is a singleton: you'll create it once and re-use it across your application. The coupling is architecturally justified for state consistency, avoiding duplicate connections, and simplifying resource management. Testing this requires mock tools (MockK, Mockito with extensions for Kotlin finals), which is standard practice in this codebase. The code meets all Kotlin style guidelines: proper indentation, no wildcard imports, license header present, and KDoc documentation included.
...in/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelEventHandler.kt
Outdated
Show resolved
Hide resolved
...in/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelEventHandler.kt
Outdated
Show resolved
Hide resolved
.../src/main/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelLogic.kt
Outdated
Show resolved
Hide resolved
...ava/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelEventHandlerTest.kt
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
stream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelEventHandler.kt (1)
318-327: Potential inconsistency betweengetMessageandmutableState.getMessageById.The
getMessagehelper on line 326 only looks up messages fromvisibleMessages, while other code paths (lines 150, 155, 331, 336) usemutableState.getMessageById(). This inconsistency could causeupdateMessageto skip updates for messages that exist in the full message state but are not currently visible (e.g., during search or pagination).Consider using
mutableState.getMessageById()for consistency:🔎 Proposed fix
private fun getMessage(id: String): Message? { - return mutableState.visibleMessages.value[id]?.copy() + return mutableState.getMessageById(id)?.copy() }
📜 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 (2)
stream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelEventHandler.kt(1 hunks)stream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelEventHandlerTest.kt(1 hunks)
🧰 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/ChannelEventHandlerTest.ktstream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelEventHandler.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/ChannelEventHandlerTest.ktstream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelEventHandler.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/ChannelEventHandlerTest.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/ChannelEventHandlerTest.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/ChannelEventHandlerTest.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/ChannelEventHandlerTest.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/ChannelEventHandlerTest.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/**/*.kt : Compose components should follow noun-based naming (e.g., `MessageList`, `ChannelListHeader`)
Applied to files:
stream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelEventHandler.kt
🧬 Code graph analysis (2)
stream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelEventHandlerTest.kt (2)
stream-chat-android-core/src/testFixtures/kotlin/io/getstream/chat/android/Mother.kt (9)
randomCID(102-102)randomString(95-99)randomMessage(303-400)randomUser(183-229)randomPoll(866-908)randomDate(708-708)randomChannel(424-484)randomReaction(241-267)randomMember(652-678)stream-chat-android-client-test/src/main/java/io/getstream/chat/android/client/test/Mother.kt (6)
randomNewMessageEvent(516-542)randomNotificationMessageNewEvent(434-454)randomMemberAddedEvent(390-408)randomNotificationRemovedFromChannelEvent(664-682)randomMessageReadEvent(246-264)randomChannelUserBannedEvent(702-721)
stream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelEventHandler.kt (3)
stream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelLogic.kt (2)
upsertMessage(115-115)getMessage(108-108)stream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelStateLogic.kt (2)
upsertMessage(334-341)updateMessage(196-198)stream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/state/channel/internal/ChannelMutableState.kt (2)
upsertMessage(536-538)updateMessage(545-552)
⏰ 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). (6)
- GitHub Check: Build / compose apks
- GitHub Check: compare-sdk-sizes / Compare SDK sizes
- GitHub Check: API check
- GitHub Check: Debug build
- GitHub Check: Unit tests
- GitHub Check: ktlint
🔇 Additional comments (5)
stream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelEventHandlerTest.kt (5)
99-120: LGTM!Test class setup is well-structured with proper mocking. The use of backtick test names follows coding guidelines, and the
@Suppress("LargeClass")annotation is appropriate given the comprehensive coverage needed.
122-228: LGTM!Comprehensive test coverage for
NewMessageEventandMessageUpdatedEventscenarios, including edge cases like preservingcreatedLocallyAtfor current user messages, poll enrichment, and handling non-existing messages.
229-324: LGTM!Good coverage of
MessageDeletedEvent(hard vs soft delete) andNotificationMessageNewEvent(inside search vs not) scenarios. The tests correctly verify the distinction betweenupsertMessageandupdateMessagecalls per PR objectives.
326-938: LGTM!Excellent comprehensive coverage across all event types: thread messages, reactions, members, watchers, channel updates, typing, read/delivery receipts, invitations, bans, and polls. The tests correctly verify edge cases like thread-specific reads not updating channel read state (lines 704-720).
940-1096: LGTM!Complete test coverage for reminder events, user presence, mute updates, and message deletion scenarios. The tests properly verify edge cases like operations on non-existing messages.
# Conflicts: # stream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelLogic.kt
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
stream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/state/channel/internal/ChannelMutableState.kt (1)
213-214: Consider enhancing the deprecation annotation with alevelparameter.The deprecation message is clear, but Kotlin's
@Deprecatedannotation supports alevelparameter (DeprecationLevel.WARNING,ERROR, orHIDDEN) to indicate the deprecation severity. For a newly deprecated API,WARNING(the default) is appropriate, but explicitly specifying it improves clarity and makes future escalation easier to track.🔎 Suggested enhancement
- @Deprecated("This property is not used anymore and will be removed in future versions.") + @Deprecated( + message = "This property is not used anymore and will be removed in future versions.", + level = DeprecationLevel.WARNING, + ) override val oldMessages: StateFlow<List<Message>> = messagesTransformation(_oldMessages!!.mapState { it.values })
📜 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 (7)
CHANGELOG.mdstream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelEventHandler.ktstream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelLogicImpl.ktstream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelStateLogic.ktstream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/state/channel/internal/ChannelMutableState.ktstream-chat-android-state/src/test/java/io/getstream/chat/android/state/channel/controller/WhenHandleEvent.ktstream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelEventHandlerTest.kt
🚧 Files skipped from review as they are similar to previous changes (1)
- CHANGELOG.md
🧰 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/ChannelEventHandlerTest.ktstream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelEventHandler.ktstream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelStateLogic.ktstream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelLogicImpl.ktstream-chat-android-state/src/test/java/io/getstream/chat/android/state/channel/controller/WhenHandleEvent.ktstream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/state/channel/internal/ChannelMutableState.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/ChannelEventHandlerTest.ktstream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelEventHandler.ktstream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelStateLogic.ktstream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelLogicImpl.ktstream-chat-android-state/src/test/java/io/getstream/chat/android/state/channel/controller/WhenHandleEvent.ktstream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/state/channel/internal/ChannelMutableState.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/ChannelEventHandlerTest.ktstream-chat-android-state/src/test/java/io/getstream/chat/android/state/channel/controller/WhenHandleEvent.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/ChannelEventHandlerTest.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/ChannelEventHandlerTest.ktstream-chat-android-state/src/test/java/io/getstream/chat/android/state/channel/controller/WhenHandleEvent.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/ChannelEventHandlerTest.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/ChannelEventHandlerTest.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/**/*.kt : Compose components should follow noun-based naming (e.g., `MessageList`, `ChannelListHeader`)
Applied to files:
stream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelEventHandler.kt
🧬 Code graph analysis (3)
stream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelEventHandler.kt (3)
stream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/state/channel/internal/ChannelMutableState.kt (1)
upsertMessage(536-538)stream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelStateLogic.kt (1)
upsertMessage(325-332)stream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelLogic.kt (1)
upsertMessage(115-115)
stream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelLogicImpl.kt (1)
stream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelLogic.kt (2)
handleEvent(155-155)updateDataForChannel(134-141)
stream-chat-android-state/src/test/java/io/getstream/chat/android/state/channel/controller/WhenHandleEvent.kt (1)
stream-chat-android-client-test/src/main/java/io/getstream/chat/android/client/test/Mother.kt (2)
randomMessageUpdateEvent(456-472)randomNotificationMarkReadEvent(286-314)
🔇 Additional comments (28)
stream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/state/channel/internal/ChannelMutableState.kt (1)
428-430: LGTM!The simplified
updateTypingEventfunction is a clean improvement. The rename from plural to singular correctly reflects the single-parameter signature, and the null-safe access to_typing?.valueproperly handles the post-destroy()case.stream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelStateLogic.kt (3)
294-300: LGTM! Simplified typing state update.The change from
updateTypingEvents(...)toupdateTypingEvent(typingEvent)simplifies the API by delegating the raw typing events handling to the internal implementation. This aligns with the broader refactoring to consolidate event handling.
573-577: Good API rename fromtoggleHiddentosetHidden.The rename is more semantically accurate since this method sets the hidden state to a specific value rather than toggling it. The updated parameter documentation clearly describes the purpose.
928-930: Typo fix looks good.The method name correction from
udpateMessageCounttoupdateMessageCountfixes the typo.stream-chat-android-state/src/test/java/io/getstream/chat/android/state/channel/controller/WhenHandleEvent.kt (5)
89-95: Mock delegation setup is correct.The
upsertMessagemock delegation ensures that messages are properly propagated to thechannelMutableStateduring tests, which is necessary for the test assertions to work correctly with the newChannelLogicImplstructure.
105-111: Test updated to useChannelLogicImpl.The test correctly instantiates
ChannelLogicImplinstead of the now-interfaceChannelLogic, aligning with the refactoring that extractsChannelLogicas an interface.
134-135: Verify call updated for renamed API.The change from
toggleHidden(false)tosetHidden(false)correctly reflects the API rename inChannelStateLogic.
149-155: Message update test expectations refined.The test now properly verifies that both the original message and the updated message from the event are upserted separately, reflecting the correct event handling flow.
244-250: Read event test correctly setsthread = null.This ensures the test covers channel-level read events (not thread reads), which should update the channel read state. This aligns with the event handling logic that only updates channel reads when
thread == null.stream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelEventHandlerTest.kt (7)
99-120: Well-structured test setup.The test class properly sets up mocks for
ChannelStateLogicandChannelMutableState, and initializes theChannelEventHandlerwith the required dependencies. The@Suppress("LargeClass")annotation is appropriate given the comprehensive event coverage.
122-138: Good test forcreatedLocallyAtpreservation logic.This test verifies that for messages from the current user, the
createdLocallyAttimestamp is preserved from the existing message to ensure proper sorting. This is an important edge case for message synchronization.
181-214: Comprehensive poll enrichment tests.These tests verify the important behavior that when a
MessageUpdatedEventarrives without a poll, the original poll is preserved from the existing message. This prevents data loss during message updates.
229-275: Message deletion tests cover both hard and soft delete scenarios.Good coverage of both
hardDelete = true(message removed from state) andhardDelete = false(message upserted with deleted state) behaviors.
637-664: Thread-aware read event handling is correctly tested.These tests verify that
MessageReadEventonly updates the channel read state whenthread == null, preventing thread reads from affecting channel-level read state.
884-944: Reminder event tests verify proper message update flow.The tests correctly verify that reminder events update the message's reminder field, and handle the edge case where the message doesn't exist in state.
326-346: Test correctly verifies the handler's actual behavior.The test appropriately validates that
NotificationThreadMessageNewEventtriggers a message upsert. The handler implementation contains only this single operation (line 174:is NotificationThreadMessageNewEvent -> upsertMessage(event.message)), and the test reflects that. No additional visibility or channel logic exists to verify.Likely an incorrect or invalid review comment.
stream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelEventHandler.kt (7)
75-86: Well-documented internal class.The KDoc properly documents the class purpose and constructor parameters, including the callback for obtaining the current user ID.
96-116: Clean event routing structure.The
handlemethod properly routes events to eitherhandleCidEventfor channel-specific events or handles global events (user presence, read all, mutes, message deletion) directly. Unhandled events are safely ignored.
149-162:MessageUpdatedEventhandling preserves poll and enriches reply.The logic correctly:
- Retrieves the original message to preserve the poll if missing from the event
- Enriches the
replyTofield from state if available- This addresses the PR objective of fixing message update events
However, note that
ownReactionsis preserved in theupsertMessagehelper but not explicitly here.
164-172:MessageDeletedEventcorrectly differentiates hard vs soft delete.Hard deletes remove the message from state, while soft deletes upsert the message with deletion metadata. Message count is updated when provided.
174-178: Reaction events upsert messages without visibility changes.This aligns with the PR objective that reaction events should not change channel visibility. The events simply update the message with the new reaction state.
291-305:upsertMessagehelper preserves local state appropriately.The helper:
- Preserves
createdLocallyAtfor current user messages when requested- Preserves
ownReactionsfrom the existing message to prevent reaction state loss- Calls both
upsertMessageanddelsertPinnedMessagefor proper pinned message handling
307-309: Defensive copy ingetMessage.The
copy()call ensures the returned message is a defensive copy, preventing external modifications from affecting the internal state.stream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelLogicImpl.kt (5)
43-72: Well-structured implementation class.The class properly implements the
ChannelLogicinterface with:
- Clear constructor parameters with KDoc
- Proper initialization of
ChannelEventHandlerwith dependencies- Read-only accessors for
cid,state, andstateLogicThe
TooManyFunctionsandLargeClasssuppressions are acceptable given the comprehensive functionality.
100-118: Watch method prevents duplicate requests.The check for
mutableState.loading.valueprevents concurrent watch requests, which protects against UI bugs that could DDOS the API. The error message is clear and informative.
172-183: Event handling delegates toChannelEventHandler.The
handleEventmethod logs the event and delegates to the extractedChannelEventHandler. This is a clean separation of concerns from the refactoring.
185-201: Query coordination between offline and online sources.The
runChannelQuerymethod:
- Captures loaded messages before the query
- Attempts offline query first
- Runs online query and triggers gap-filling on success
- Falls back to offline result if online fails
This provides good offline-first behavior with eventual consistency.
265-288: Offline query correctly skips newer/around pagination.The method returns
nullfor newer or around-id queries since local data may be stale. This ensures fresh data from the backend for these scenarios.
.../main/java/io/getstream/chat/android/state/plugin/logic/channel/internal/ChannelLogicImpl.kt
Show resolved
Hide resolved
|
🎯 Goal
Part 1 of the
ChannelLogic/JumpToMessagerefactoring.Sets the baseline on which the new and optimised channel logic will be built:
ChannelLogicinterface with the common operations (to be implemented by the current and the new implementation)ChannelLogicinterface (now namedChannelLogicImpl)ChannelLogicto a separateChannelEventHandlerclassAdditionally, fixes
message.updated/message.deletedshowing the channel (this event doesn't update the channel visibility)🛠 Implementation details
ChannelLogicand its implementationChannelLogicImplChannelEventHandler🎨 UI Changes
NA
🧪 Testing
Test various scenarios related to channel / message list: pagination, reactions, quoting, pinning, updating, threading - everything should work as expected.
Summary by CodeRabbit
Bug Fixes
Documentation
Improvements
Tests
✏️ Tip: You can customize this high-level summary in your review settings.