Fix Giphy message not appearing in channel #6065
Conversation
SDK Size Comparison 📏
|
WalkthroughThis PR fixes a bug where a sent Giphy message may not appear in the channel by replacing three Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client
participant SendGiphyListener as Listener
participant ChannelLogic as ChannelStateLogic
participant QueryLogic as QueryThreadsLogic
participant ThreadLogic as ThreadStateLogic
Client->>Listener: onGiphySendResult(Result.Success(message))
activate Listener
Listener->>ChannelLogic: upsertMessage(message)
Note right of ChannelLogic `#DFF0D8`: channel state updated
Listener->>QueryLogic: upsertMessage(message)
Note right of QueryLogic `#DFF0D8`: active queries updated
Listener->>ThreadLogic: upsertMessage(message)
Note right of ThreadLogic `#DFF0D8`: thread state updated
deactivate Listener
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
📜 Recent review detailsConfiguration used: Repository UI Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
stream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/listener/internal/SendGiphyListenerState.kt (2)
24-26: Update the class-level KDoc to reflect the new behavior.The KDoc states "Handles removing ephemeral message from the state", but the implementation now upserts the Giphy message instead of deleting it. Update the documentation to accurately describe the current behavior.
🔎 Proposed fix
/** * [SendGiphyListener] implementation for [StatePlugin]. - * Handles removing ephemeral message from the state. + * Handles upserting Giphy message to the state. * * @param logic [LogicRegistry] */
32-36: Update the method KDoc to reflect the new behavior.The KDoc states "Removes ephemeral message from the state if the request was successful", but the implementation now upserts the Giphy message. Update the documentation to accurately describe the current behavior.
🔎 Proposed fix
/** - * Removes ephemeral message from the state if the request was successful. + * Upserts Giphy message to the state if the request was successful. * * @param cid The full channel id, i.e. "messaging:123". * @param result The API call result. */
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
CHANGELOG.mdstream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/listener/internal/SendGiphyListenerState.ktstream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/listener/internal/SendGiphyListenerStateTest.kt
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{kt,kts}
📄 CodeRabbit inference engine (AGENTS.md)
Format and apply Kotlin style with Spotless (4 spaces, no wildcard imports, licence headers)
Files:
stream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/listener/internal/SendGiphyListenerState.ktstream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/listener/internal/SendGiphyListenerStateTest.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/main/java/io/getstream/chat/android/state/plugin/listener/internal/SendGiphyListenerState.ktstream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/listener/internal/SendGiphyListenerStateTest.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/listener/internal/SendGiphyListenerStateTest.kt
🧠 Learnings (3)
📚 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/listener/internal/SendGiphyListenerStateTest.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-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/listener/internal/SendGiphyListenerStateTest.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/listener/internal/SendGiphyListenerStateTest.kt
🧬 Code graph analysis (1)
stream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/listener/internal/SendGiphyListenerStateTest.kt (1)
stream-chat-android-core/src/testFixtures/kotlin/io/getstream/chat/android/Mother.kt (1)
randomCID(101-101)
⏰ 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). (2)
- GitHub Check: compare-sdk-sizes / Compare SDK sizes
- GitHub Check: Build / compose apks
🔇 Additional comments (4)
CHANGELOG.md (1)
40-40: Changelog entry looks correct and consistentThe new Giphy fix line is in the right section, uses the same wording and link style as existing entries, and clearly describes the bug being fixed. No changes needed.
stream-chat-android-state/src/main/java/io/getstream/chat/android/state/plugin/listener/internal/SendGiphyListenerState.kt (1)
41-43: LGTM! The fix correctly addresses the Giphy rendering issue.Replacing
deleteMessagewithupsertMessageensures the Giphy message is properly maintained in the state across channel, query threads, and thread contexts. This prevents the delete-then-insert cycle that was causing the UI glitch.stream-chat-android-state/src/test/java/io/getstream/chat/android/state/plugin/listener/internal/SendGiphyListenerStateTest.kt (2)
43-70: LGTM! Comprehensive test coverage for the success path.The test correctly validates that
upsertMessageis called on channel state logic, all active query threads, and thread state logic when the Giphy send request succeeds. The use ofrunTestand backtick test naming follows the project's coding guidelines.
72-96: LGTM! Proper test coverage for the failure path.The test correctly validates that
upsertMessageis never called on any state logic when the Giphy send request fails. This ensures the fix doesn't inadvertently update state on error conditions.
|
🎯 Goal
Fixes a case where a newly sent Giphy message does not appear immediately in the channel.
Note: While this seems like a compose glitch, I wasn't able to figure out why the Giphy doesn't render. But we were anyway wrongly deleting the message from the state instead of upserting it, when we get a result from sending the Giphy. Fixing this seems to resolve the UI glitch as well, so my assumption is that the rendering fails because we rapidly delete and insert a new message (after receiving
message.newevent) with the same Giphy URL.🛠 Implementation details
Upsert the relevant Giphy message in the state instead of deleting it.
🎨 UI Changes
before.mp4
after.mp4
🧪 Testing
Summary by CodeRabbit
Bug Fixes
Tests
✏️ Tip: You can customize this high-level summary in your review settings.