-
Notifications
You must be signed in to change notification settings - Fork 373
feat(llc): add support for markUnreadByTimestamp #2460
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
This commit introduces the ability to mark a channel as unread from a specific point in time and enhances the existing `markUnread` functionality. - Added `markUnreadByTimestamp(DateTime timestamp)` to `Channel`, `StreamChatClient`, and the underlying API. This allows marking all messages after a given timestamp as unread. - Made the `messageId` parameter in `markUnread` optional. If not provided, the entire channel is marked as unread. - Updated comments and documentation for clarity. - Added comprehensive unit tests for the new and modified functionalities across the API, client, and channel layers.
|
Caution Review failedThe pull request is closed. WalkthroughAdds timestamp-based "mark unread" APIs: Changes
Sequence Diagram(s)sequenceDiagram
participant User as User Code
participant Channel as Channel
participant Client as StreamChatClient
participant ChannelAPI as ChannelApi
participant Server as Backend
User->>Channel: markUnreadByTimestamp(timestamp)
activate Channel
Channel->>Channel: check readEvents capability
Channel->>Client: markChannelUnreadByTimestamp(channelId, channelType, timestamp)
deactivate Channel
activate Client
Client->>ChannelAPI: markUnreadByTimestamp(channelId, channelType, timestamp)
deactivate Client
activate ChannelAPI
ChannelAPI->>Server: POST /channels/{type}/{id}/unread
note right of ChannelAPI: payload { "message_timestamp": "ISO8601 UTC" }
deactivate ChannelAPI
activate Server
Server-->>ChannelAPI: EmptyResponse
deactivate Server
ChannelAPI-->>Client: EmptyResponse
Client-->>Channel: EmptyResponse
Channel-->>User: EmptyResponse
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (7)
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.
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 (4)
packages/stream_chat/lib/src/client/client.dart (1)
1344-1372: Client unread helpers correctly delegate to ChannelApi
markChannelUnreadandmarkChannelUnreadByTimestampcleanly forward to the underlyingChannelApimethods and align with the documented semantics (whole-channel whenmessageIdis omitted, or by timestamp). The only minor nit is the asymmetry withmarkChannelRead, which uses a named optionalmessageId; consider converging on a single parameter style long‑term for API consistency.packages/stream_chat/lib/src/core/api/channel_api.dart (1)
326-355: Mark-unread API wiring looks correct and consistent
markUnreadandmarkUnreadByTimestamphit the expected/unreadendpoint, conditionally includemessage_idormessage_timestamp, and normalize timestamps to UTC ISO‑8601, matching the new tests. You might optionally clarify in the doc comment that omittingmessageIdmarks the entire channel as unread, mirroring the changelog wording, but behavior is already correct.packages/stream_chat/lib/src/client/channel.dart (2)
1644-1659:markUnreadAPI and implementation look correct; consider tightening the docsThe behavior (init check, capability guard, delegation to
_client.markChannelUnread) is consistent withmarkReadand the rest of the read APIs, and the optional positionalmessageIdaligns with the described change (whole-channel unread whennull).You might make the behavior a bit more explicit in the doc comment, e.g. note that omitting
messageIdmarks the entire channel as unread and passing a value marks from that message onwards. No functional changes needed.- /// Marks the channel as unread. - /// - /// Optionally provide a [messageId] to only mark messages from that ID - /// onwards as unread. + /// Marks the channel as unread for the current user. + /// + /// If [messageId] is omitted or `null`, the entire channel will be marked + /// as unread. Otherwise, only messages from the given [messageId] onwards + /// will be marked as unread.
1661-1675:markUnreadByTimestampmirrors existing patterns; small doc/detail suggestions onlyThe method correctly mirrors
markRead/markUnread/thread variants: it checks initialization, enforcescanUseReadReceipts, and delegates to_client.markChannelUnreadByTimestamp, which is what you want.Two small, non-blocking suggestions:
- Clarify in the comment whether
timestampcan be in local time and is converted to UTC by lower layers (to align with the REST contract).- Consider extracting a tiny helper for the repeated
canUseReadReceiptsguard shared by the five read/unread methods to reduce duplication, if this pattern continues to grow.- /// Marks the channel as unread by a given [timestamp]. - /// - /// All messages after the provided timestamp will be marked as unread. + /// Marks the channel as unread from a given [timestamp]. + /// + /// All messages created after the provided [timestamp] will be counted as + /// unread for the current user. + /// + /// The [timestamp] can be a local time; it is converted to UTC when sent + /// to the API.
📜 Review details
Configuration used: CodeRabbit 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)
packages/stream_chat/CHANGELOG.md(1 hunks)packages/stream_chat/lib/src/client/channel.dart(2 hunks)packages/stream_chat/lib/src/client/client.dart(1 hunks)packages/stream_chat/lib/src/core/api/channel_api.dart(1 hunks)packages/stream_chat/test/src/client/channel_test.dart(3 hunks)packages/stream_chat/test/src/client/client_test.dart(2 hunks)packages/stream_chat/test/src/core/api/channel_api_test.dart(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-09-25T08:19:01.469Z
Learnt from: xsahil03x
Repo: GetStream/stream-chat-flutter PR: 2394
File: packages/stream_chat_flutter/lib/src/message_action/message_actions_builder.dart:82-92
Timestamp: 2025-09-25T08:19:01.469Z
Learning: In the Stream Chat Flutter library, when deleting a message with MessageSendingStatus.failed or MessageSendingStatus.failed_update status, the _deleteMessage method in channel.dart automatically handles deletion locally via _deleteLocalMessage without making API calls, preventing 404 errors and deletingFailed states.
Applied to files:
packages/stream_chat/test/src/core/api/channel_api_test.dartpackages/stream_chat/lib/src/client/channel.dartpackages/stream_chat/lib/src/client/client.dartpackages/stream_chat/test/src/client/channel_test.dartpackages/stream_chat/lib/src/core/api/channel_api.dart
📚 Learning: 2025-09-25T08:19:01.469Z
Learnt from: xsahil03x
Repo: GetStream/stream-chat-flutter PR: 2394
File: packages/stream_chat_flutter/lib/src/message_action/message_actions_builder.dart:82-92
Timestamp: 2025-09-25T08:19:01.469Z
Learning: In the Stream Chat Flutter library, when deleting a message that hasn't been sent to the server yet (message.remoteCreatedAt == null) or is bounced with error, the _deleteMessage method in channel.dart automatically handles deletion locally via _deleteLocalMessage without making API calls, preventing 404 errors and deletingFailed states.
Applied to files:
packages/stream_chat/test/src/core/api/channel_api_test.dartpackages/stream_chat/lib/src/client/channel.dartpackages/stream_chat/lib/src/client/client.dartpackages/stream_chat/test/src/client/channel_test.dartpackages/stream_chat/lib/src/core/api/channel_api.dart
⏰ 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). (10)
- GitHub Check: analyze_legacy_versions
- GitHub Check: build (android)
- GitHub Check: test
- GitHub Check: build (ios)
- GitHub Check: analyze
- GitHub Check: stream_chat_localizations
- GitHub Check: stream_chat_flutter
- GitHub Check: stream_chat_flutter_core
- GitHub Check: stream_chat
- GitHub Check: stream_chat_persistence
🔇 Additional comments (6)
packages/stream_chat/CHANGELOG.md (1)
1-12: Changelog entry accurately reflects API behaviorThe description of
markUnreadByTimestampand the optionalmessageIdsemantics formarkUnread/markChannelUnreadmatches the implemented client and API methods and is clear for consumers.packages/stream_chat/test/src/client/client_test.dart (1)
2037-2102: New client tests thoroughly cover unread entry pointsThe three tests for
.markChannelUnread(with and withoutmessageId) and.markChannelUnreadByTimestampcorrectly assert delegation toapi.channel.markUnread/markUnreadByTimestampand ensure no extra interactions. This gives good coverage over the new optional parameter and timestamp paths.packages/stream_chat/test/src/core/api/channel_api_test.dart (1)
608-685: ChannelApi unread tests match the new REST payload contractsThe added tests for
markUnread(with and withoutmessageId) andmarkUnreadByTimestampverify the correct/unreadpath and distinguish the expected payload shapes, closely mirroring the implementation. This should catch regressions in how unread markers are serialized.packages/stream_chat/test/src/client/channel_test.dart (3)
6740-6771: LGTM! Excellent test coverage for optional messageId parameter.The test properly validates that
markUnread()can be called without parameters when the readEvents capability is present. The mock setup and verification correctly assert thatclient.markChannelUnreadis called with onlychannelIdandchannelType, confirming the optional parameter behavior.
6773-6806: LGTM! Good test coverage for backward compatibility.This test ensures that
markUnread(messageId)continues to work correctly when a messageId is explicitly provided, validating backward compatibility with the existing API usage. The mock properly includes the messageId parameter in the expected call.
6808-6864: LGTM! Comprehensive test coverage for the new markUnreadByTimestamp method.These tests properly validate the new timestamp-based unread marking functionality:
- First test ensures proper error handling when readEvents capability is missing
- Second test confirms successful execution when capability is present
- Both tests use consistent timestamp creation and follow established testing patterns
The implementation aligns with the PR objectives to add the ability to mark messages as unread from a specific timestamp.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #2460 +/- ##
==========================================
+ Coverage 64.61% 64.65% +0.04%
==========================================
Files 420 420
Lines 26210 26226 +16
==========================================
+ Hits 16936 16957 +21
+ Misses 9274 9269 -5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Submit a pull request
Fixes: FLU-341
Description of the pull request
This PR introduces the ability to mark a channel as unread from a specific point in time.
markUnreadByTimestamp(DateTime timestamp)toChannel,StreamChatClient, and the underlying API. This allows marking all messages after a given timestamp as unread.Summary by CodeRabbit
New Features
Documentation
Tests
✏️ Tip: You can customize this high-level summary in your review settings.