-
Notifications
You must be signed in to change notification settings - Fork 373
fix(llc, core): retain cached channel messages on offline access with unread messages #2299
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughThis update modifies message pagination and state management logic in the chat client and UI packages. It removes the clearing of cached messages when querying around a message or timestamp, especially for channels with unread messages accessed offline. Additional pagination fields based on message creation time are introduced for enhanced querying. Changes
Sequence Diagram(s)sequenceDiagram
participant UI
participant StreamChannelState
participant Channel
participant ChannelState
UI->>StreamChannelState: Access channel with unread messages (offline)
StreamChannelState->>Channel: Query messages at message/timestamp
Channel->>ChannelState: (No truncate) Update state with new data
ChannelState-->>StreamChannelState: Return updated messages (cached preserved)
StreamChannelState-->>UI: Render messages
Assessment against linked issues
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (6)
💤 Files with no reviewable changes (1)
🚧 Files skipped from review as they are similar to previous changes (5)
⏰ Context from checks skipped due to timeout of 90000ms (9)
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/stream_chat/CHANGELOG.md (1)
5-6: Refine wording for clarity and grammatical correctnessThe sentence currently reads a bit awkwardly (“Fixed cached messages are cleared …”) and breaks subject/verb agreement. Align it with the usual changelog style (“Fixed an issue where …”):
- - Fixed cached messages are cleared from channels with unread messages when accessed - offline. [[#2083]](https://github.com/GetStream/stream-chat-flutter/issues/2083) + - Fixed an issue where cached messages were cleared from channels with unread messages + when accessed offline. [[#2083]](https://github.com/GetStream/stream-chat-flutter/issues/2083)packages/stream_chat_flutter_core/CHANGELOG.md (1)
5-6: Mirror wording fix in core package changelogSame grammatical issue here—use “Fixed an issue where …” for consistency and readability:
- - Fixed cached messages are cleared from channels with unread messages when accessed - offline. [[#2083]](https://github.com/GetStream/stream-chat-flutter/issues/2083) + - Fixed an issue where cached messages were cleared from channels with unread messages + when accessed offline. [[#2083]](https://github.com/GetStream/stream-chat-flutter/issues/2083)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
packages/stream_chat/CHANGELOG.md(1 hunks)packages/stream_chat/lib/src/client/channel.dart(1 hunks)packages/stream_chat/lib/src/core/api/requests.dart(1 hunks)packages/stream_chat_flutter_core/CHANGELOG.md(1 hunks)packages/stream_chat_flutter_core/lib/src/stream_channel.dart(0 hunks)
💤 Files with no reviewable changes (1)
- packages/stream_chat_flutter_core/lib/src/stream_channel.dart
⏰ Context from checks skipped due to timeout of 90000ms (10)
- GitHub Check: analyze_legacy_versions
- GitHub Check: stream_chat_flutter
- GitHub Check: stream_chat_localizations
- GitHub Check: stream_chat
- GitHub Check: stream_chat_flutter_core
- GitHub Check: stream_chat_persistence
- GitHub Check: build (ios)
- GitHub Check: test
- GitHub Check: build (android)
- GitHub Check: analyze
🔇 Additional comments (2)
packages/stream_chat/lib/src/client/channel.dart (1)
1779-1790: LGTM! Well-implemented fix for message gap prevention.The conditional truncation logic effectively addresses the core issue from the PR. The implementation is clean and follows best practices:
- Modern switch expression syntax with appropriate pattern matching
- Clear comments explaining the reasoning for truncation
- Proper null-safe method calls
- Correct sequencing (truncate before state update)
This change should successfully prevent message gaps when querying around specific messages or timestamps while preserving cached messages in other scenarios.
packages/stream_chat/lib/src/core/api/requests.dart (1)
140-144: LGTM! Well-implemented timestamp-based pagination fields.The addition of the five new DateTime fields (
createdAtAfterOrEqual,createdAtAfter,createdAtBeforeOrEqual,createdAtBefore,createdAtAround) is properly integrated across the entire class. The implementation correctly:
- Includes all fields in the constructor,
copyWithmethod, and equality comparison- Uses consistent nullable DateTime types and proper JsonKey annotations
- Follows the established patterns for pagination parameters
- Supports the PR objective of improving message querying around specific timestamps for offline scenarios
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #2299 +/- ##
==========================================
+ Coverage 63.43% 63.45% +0.02%
==========================================
Files 408 408
Lines 25555 25561 +6
==========================================
+ Hits 16211 16221 +10
+ Misses 9344 9340 -4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/stream_chat/test/src/client/channel_test.dart (1)
2753-2866: Consider reducing code duplication between the two tests.While both tests serve distinct purposes (testing
idAroundvscreatedAtAround), there's significant structural similarity between them. Consider extracting common setup logic into a helper method to improve maintainability.Example refactor approach:
void testTruncationBehavior( String testName, PaginationParams pagination, String expectedTargetMessageId, ) { test(testName, () async { // Common setup logic final initialMessages = [ Message(id: 'msg1', text: 'Hello 1'), Message(id: 'msg2', text: 'Hello 2'), Message(id: 'msg3', text: 'Hello 3'), ]; // ... rest of common logic final res = await channel.query(messagesPagination: pagination); expect(res, isNotNull); expect(channel.state!.messages, hasLength(5)); expect(channel.state!.messages[2].id, expectedTargetMessageId); // ... common assertions }); } // Usage: testTruncationBehavior( 'should truncate state when querying around message id', const PaginationParams(idAround: 'target-message-id'), 'target-message-id', ); testTruncationBehavior( 'should truncate state when querying around created date', PaginationParams(createdAtAround: targetDate), 'target-message', );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/stream_chat/test/src/client/channel_test.dart(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (11)
- GitHub Check: stream_chat_flutter
- GitHub Check: format
- GitHub Check: build (ios)
- GitHub Check: analyze_legacy_versions
- GitHub Check: stream_chat_persistence
- GitHub Check: build (android)
- GitHub Check: stream_chat_localizations
- GitHub Check: stream_chat_flutter_core
- GitHub Check: analyze
- GitHub Check: test
- GitHub Check: stream_chat
🔇 Additional comments (2)
packages/stream_chat/test/src/client/channel_test.dart (2)
2753-2808: Well-structured test for idAround pagination functionality.This test properly verifies the truncation and replacement behavior when querying messages around a specific message ID. The test setup, mocking, and assertions are all appropriate and follow good testing practices.
2810-2866: Good test coverage for createdAtAround pagination.This test effectively verifies the timestamp-based pagination functionality. The test structure mirrors the previous test appropriately while testing a distinct feature.
75196d1 to
e2a1358
Compare
Submit a pull request
Linear: FLU-180
Github Issue: #2083
Screenshots / Videos
Screen.Recording.2025-07-03.at.14.48.58.mov
Screen.Recording.2025-07-03.at.14.31.17.mov
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Bug Fixes
Documentation