-
Notifications
You must be signed in to change notification settings - Fork 371
feat(llc): Add message delivery receipts #2429
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
base: master
Are you sure you want to change the base?
Conversation
WalkthroughAdds end-to-end message delivery receipts: new MessageDelivery model and API, ChannelDeliveryReporter for batched reporting, delivery/read metadata in models, MessageRules validations, channel read/delivery helpers and capability checks, UI delivered-state support, and tests/mocks updated. Changes
Sequence Diagram(s)sequenceDiagram
participant Channel
participant Reporter as ChannelDeliveryReporter
participant API as ChannelApi
rect rgba(230,245,250,0.9)
Note over Channel,Reporter: Channel triggers delivery submission (on query/fetch or events)
Channel->>Reporter: submitForDelivery([this])
Reporter->>Reporter: throttle & batch (~1s)
Reporter->>API: markChannelsDelivered(deliveries)
API->>API: POST /channels/delivered
end
rect rgba(245,230,250,0.9)
Note over Channel: Delivery/read reconciliation on incoming events
Channel->>Channel: on messageRead / messageDelivered / typing
Channel->>Channel: reconcile reads/deliveries -> update Read/Event.lastDeliveredAt/Id
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Areas needing extra attention:
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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: 2
🧹 Nitpick comments (3)
packages/stream_chat/lib/src/event_type.dart (1)
181-182: Consider relocating this constant for better organization.The constant is functionally correct, but message-related events are grouped together earlier in the file (lines 15-49). Since
messageDeliveredis conceptually related tomessageRead(both are acknowledgment events), consider placing it near line 49 for improved logical grouping.Apply this diff to improve organization:
/// Event sent when reading a message static const String messageRead = 'message.read'; + + /// Event sent when a message is marked as delivered. + static const String messageDelivered = 'message.delivered'; /// Event sent when a channel is deleted static const String channelDeleted = 'channel.deleted';And remove the constant from its current location at the end of the file.
packages/stream_chat/lib/src/client/channel.dart (2)
9-9: Drop redundant MessageRules import.
MessageRulesis already available throughpackage:stream_chat/stream_chat.dart, so this direct import now triggers the analyzer’sunnecessary_importwarning. Please remove it to keep the lint pipeline clean.-import 'package:stream_chat/src/core/util/message_rules.dart';
2853-2868: Remove the unused_shouldUpdateChannelLastMessageAt.After migrating to
MessageRules.canUpdateChannelLastMessageAt, this helper is no longer referenced and the analyzer flags it asunused_element. Please delete the method to silence the warning and avoid dead code.- bool _shouldUpdateChannelLastMessageAt(Message message) { - if (message.isError) return false; - if (message.shadowed) return false; - if (message.isEphemeral) return false; - - final config = channelState.channel?.config; - if (message.isSystem && config?.skipLastMsgUpdateForSystemMsgs == true) { - return false; - } - - final currentUserId = _client.state.currentUser?.id; - if (currentUserId case final userId? when message.isNotVisibleTo(userId)) { - return false; - } - - return true; - }
📜 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 (17)
packages/stream_chat/lib/src/client/channel.dart(30 hunks)packages/stream_chat/lib/src/client/channel_delivery_reporter.dart(1 hunks)packages/stream_chat/lib/src/client/client.dart(8 hunks)packages/stream_chat/lib/src/core/api/channel_api.dart(3 hunks)packages/stream_chat/lib/src/core/models/channel_config.dart(2 hunks)packages/stream_chat/lib/src/core/models/channel_config.g.dart(2 hunks)packages/stream_chat/lib/src/core/models/channel_model.dart(1 hunks)packages/stream_chat/lib/src/core/models/event.dart(6 hunks)packages/stream_chat/lib/src/core/models/event.g.dart(2 hunks)packages/stream_chat/lib/src/core/models/message_delivery_info.dart(1 hunks)packages/stream_chat/lib/src/core/models/message_delivery_info.g.dart(1 hunks)packages/stream_chat/lib/src/core/models/read.dart(4 hunks)packages/stream_chat/lib/src/core/models/read.g.dart(1 hunks)packages/stream_chat/lib/src/core/util/extension.dart(1 hunks)packages/stream_chat/lib/src/core/util/message_rules.dart(1 hunks)packages/stream_chat/lib/src/event_type.dart(1 hunks)packages/stream_chat/lib/stream_chat.dart(2 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
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.
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.
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.
📚 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/lib/src/event_type.dartpackages/stream_chat/lib/src/core/models/channel_model.dartpackages/stream_chat/lib/src/core/util/message_rules.dartpackages/stream_chat/lib/src/core/models/message_delivery_info.dartpackages/stream_chat/lib/src/core/api/channel_api.dartpackages/stream_chat/lib/src/core/models/message_delivery_info.g.dartpackages/stream_chat/lib/src/client/client.dartpackages/stream_chat/lib/src/client/channel_delivery_reporter.dartpackages/stream_chat/lib/stream_chat.dartpackages/stream_chat/lib/src/client/channel.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/lib/src/event_type.dartpackages/stream_chat/lib/src/core/models/channel_model.dartpackages/stream_chat/lib/src/core/util/message_rules.dartpackages/stream_chat/lib/src/core/models/message_delivery_info.dartpackages/stream_chat/lib/src/core/api/channel_api.dartpackages/stream_chat/lib/src/core/models/message_delivery_info.g.dartpackages/stream_chat/lib/src/client/client.dartpackages/stream_chat/lib/stream_chat.dartpackages/stream_chat/lib/src/client/channel.dart
📚 Learning: 2025-08-08T14:27:59.621Z
Learnt from: xsahil03x
Repo: GetStream/stream-chat-flutter PR: 2348
File: packages/stream_chat_flutter_core/lib/src/stream_channel.dart:383-406
Timestamp: 2025-08-08T14:27:59.621Z
Learning: In stream_chat_flutter_core/lib/src/stream_channel.dart, threads (replies) do not support around-anchor loading. Thread replies are fetched as: initial latest page via StreamChannelState.getReplies(), and further pagination via StreamChannelState.queryReplies(parentId, direction: top|bottom). Anchored loads apply only to channel messages, not to threads.
Applied to files:
packages/stream_chat/lib/src/core/api/channel_api.dartpackages/stream_chat/lib/src/client/client.dartpackages/stream_chat/lib/src/client/channel.dart
🪛 GitHub Actions: legacy_version_analyze
packages/stream_chat/lib/src/core/models/event.dart
[info] 342-342: Missing documentation for a public member. Try adding documentation for the member.
packages/stream_chat/lib/src/client/channel.dart
[warning] 2852-2852: The declaration '_shouldUpdateChannelLastMessageAt' isn't referenced. Try removing the declaration of '_shouldUpdateChannelLastMessageAt'. - unused_element
[info] 9-9: The import of 'package:stream_chat/src/core/util/message_rules.dart' is unnecessary because all of the used elements are also provided by the import of 'package:stream_chat/stream_chat.dart'. Try removing the import directive. - unnecessary_import
⏰ 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). (7)
- GitHub Check: stream_chat_localizations
- GitHub Check: stream_chat_flutter_core
- GitHub Check: stream_chat_flutter
- GitHub Check: stream_chat
- GitHub Check: stream_chat_persistence
- GitHub Check: build (android)
- GitHub Check: test
🔇 Additional comments (1)
packages/stream_chat/lib/src/core/api/channel_api.dart (1)
401-406: Avoid pre-encoding the delivery payload
StreamHttpClient/Dio already handles JSON encoding (and keeps the structured payload available to interceptors). Feeding a pre-encoded string bypasses that and can flip the request totext/plain. Hand the map of receipts to_client.postand let the client serialize it. suggest_recommended_refactorraise_minor_issue- final response = await _client.post( - '/channels/delivered', - data: jsonEncode({'latest_delivered_messages': messages}), - ); + final response = await _client.post( + '/channels/delivered', + data: { + 'latest_delivered_messages': + messages.map((info) => info.toJson()).toList(), + }, + );
This commit introduces message delivery receipts, allowing clients to acknowledge when a message has been successfully delivered. Senders can now be notified that their messages have reached the recipient's device. **Key Changes:** * **Delivery Status in `Read` Model:** The `Read` model is extended to include `lastDeliveredAt` and `lastDeliveredMessageId`, enabling tracking of message delivery alongside read status. * **New Event Type:** A `message.delivered` event is introduced to broadcast delivery acknowledgments. * **`ChannelDeliveryReporter`:** A new `ChannelDeliveryReporter` class is added to manage, batch, and throttle the sending of delivery receipts to the backend, ensuring efficient network usage. * **Message Validation Logic:** A `MessageRules` utility class is created to centralize validation logic, determining if a message is eligible for delivery receipts, can be counted as unread, or can be sent. * **API and Model Updates:** New API endpoints (`markChannelsDelivered`) and corresponding data models (`MessageDeliveryInfo`) have been added to support this feature. * **Refactoring:** Logic for determining unread counts and message validity has been refactored from `Channel` and `ChannelClientState` into the new `MessageRules` class for better separation of concerns. Helper extensions have been added to `Read` and `ChannelClientState` to simplify querying for read and delivery statuses.
This change ensures that when the `ChannelDeliveryReporter` is canceled, any pending delivery candidates are cleared. The `cancel` method now clears the `_deliveryCandidates` list in addition to canceling the throttled reporting function. This prevents a potential memory leak and ensures a clean state when delivery reporting is stopped.
This commit introduces a new privacy setting to control delivery receipts. - A `DeliveryReceipts` class is added to `privacy_settings.dart`, allowing users to enable or disable the sending of delivery receipt events. - `PrivacySettings` is updated to include the new `deliveryReceipts` option. - `OwnUser` now includes an `isDeliveryReceiptsEnabled` getter to easily check the user's current setting. - Generated JSON serialization files have been updated accordingly.
ccad711 to
2e5b853
Compare
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
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: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/stream_chat/lib/src/client/client.dart (1)
789-801: Don’t block queryChannelsOnline on delivery submission; handle errors defensivelyAwaiting delivery submission can fail the entire channel query or delay UI unnecessarily. Submit after updating state, and don’t let errors bubble up.
Apply this refactor:
- final updateData = _mapChannelStateToChannel(channels); - // Submit delivery report for the channels fetched in this query. - await channelDeliveryReporter.submitForDelivery(updateData.value); + final updateData = _mapChannelStateToChannel(channels); @@ - this.state.addChannels(updateData.key); - return updateData.value; + this.state.addChannels(updateData.key); + // Best-effort, fire-and-forget. Do not block query result or throw. + unawaited( + channelDeliveryReporter + .submitForDelivery(updateData.value) + .catchError((e, stk) => logger.fine('Delivery submit failed', e, stk)), + ); + return updateData.value;
📜 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 (24)
packages/stream_chat/lib/src/client/channel.dart(36 hunks)packages/stream_chat/lib/src/client/channel_delivery_reporter.dart(1 hunks)packages/stream_chat/lib/src/client/client.dart(8 hunks)packages/stream_chat/lib/src/core/api/channel_api.dart(3 hunks)packages/stream_chat/lib/src/core/models/channel_config.dart(2 hunks)packages/stream_chat/lib/src/core/models/channel_config.g.dart(2 hunks)packages/stream_chat/lib/src/core/models/channel_model.dart(1 hunks)packages/stream_chat/lib/src/core/models/event.dart(6 hunks)packages/stream_chat/lib/src/core/models/event.g.dart(2 hunks)packages/stream_chat/lib/src/core/models/message_delivery.dart(1 hunks)packages/stream_chat/lib/src/core/models/message_delivery.g.dart(1 hunks)packages/stream_chat/lib/src/core/models/own_user.dart(1 hunks)packages/stream_chat/lib/src/core/models/privacy_settings.dart(3 hunks)packages/stream_chat/lib/src/core/models/privacy_settings.g.dart(3 hunks)packages/stream_chat/lib/src/core/models/read.dart(4 hunks)packages/stream_chat/lib/src/core/models/read.g.dart(1 hunks)packages/stream_chat/lib/src/core/util/extension.dart(1 hunks)packages/stream_chat/lib/src/core/util/message_rules.dart(1 hunks)packages/stream_chat/lib/src/event_type.dart(1 hunks)packages/stream_chat/lib/stream_chat.dart(3 hunks)packages/stream_chat/test/fixtures/read.json(1 hunks)packages/stream_chat/test/src/client/channel_test.dart(2 hunks)packages/stream_chat/test/src/core/models/read_test.dart(5 hunks)packages/stream_chat/test/src/mocks.dart(3 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/stream_chat/test/fixtures/read.json
🚧 Files skipped from review as they are similar to previous changes (11)
- packages/stream_chat/lib/src/core/models/channel_model.dart
- packages/stream_chat/lib/src/event_type.dart
- packages/stream_chat/lib/src/core/api/channel_api.dart
- packages/stream_chat/lib/src/core/models/event.dart
- packages/stream_chat/lib/src/core/models/event.g.dart
- packages/stream_chat/lib/src/core/models/read.dart
- packages/stream_chat/lib/src/client/channel_delivery_reporter.dart
- packages/stream_chat/lib/src/core/models/read.g.dart
- packages/stream_chat/lib/src/core/models/channel_config.dart
- packages/stream_chat/lib/src/core/models/channel_config.g.dart
- packages/stream_chat/lib/src/core/util/extension.dart
🧰 Additional context used
🧠 Learnings (3)
📚 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/lib/src/core/models/message_delivery.g.dartpackages/stream_chat/lib/stream_chat.dartpackages/stream_chat/lib/src/core/util/message_rules.dartpackages/stream_chat/lib/src/client/client.dartpackages/stream_chat/test/src/mocks.dartpackages/stream_chat/lib/src/core/models/message_delivery.dartpackages/stream_chat/lib/src/client/channel.dartpackages/stream_chat/test/src/client/channel_test.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/lib/src/core/models/message_delivery.g.dartpackages/stream_chat/lib/stream_chat.dartpackages/stream_chat/lib/src/core/util/message_rules.dartpackages/stream_chat/lib/src/client/client.dartpackages/stream_chat/test/src/mocks.dartpackages/stream_chat/lib/src/core/models/message_delivery.dartpackages/stream_chat/lib/src/client/channel.dartpackages/stream_chat/test/src/client/channel_test.dart
📚 Learning: 2025-08-08T14:27:59.621Z
Learnt from: xsahil03x
Repo: GetStream/stream-chat-flutter PR: 2348
File: packages/stream_chat_flutter_core/lib/src/stream_channel.dart:383-406
Timestamp: 2025-08-08T14:27:59.621Z
Learning: In stream_chat_flutter_core/lib/src/stream_channel.dart, threads (replies) do not support around-anchor loading. Thread replies are fetched as: initial latest page via StreamChannelState.getReplies(), and further pagination via StreamChannelState.queryReplies(parentId, direction: top|bottom). Anchored loads apply only to channel messages, not to threads.
Applied to files:
packages/stream_chat/lib/src/client/client.dartpackages/stream_chat/lib/src/client/channel.dart
🔇 Additional comments (31)
packages/stream_chat/lib/src/core/models/privacy_settings.g.dart (2)
19-22: LGTM! Consistent serialization pattern.The deserialization and serialization logic for
deliveryReceiptscorrectly mirrors the existing patterns fortypingIndicatorsandreadReceipts, including proper null handling and conditional serialization.Also applies to: 31-32
54-62: LGTM! Well-structured JSON helpers.The
DeliveryReceiptsJSON serialization helpers follow the exact same pattern asTypingIndicatorsandReadReceipts, with the defaultenabled: truebehavior appropriately maintained.packages/stream_chat/lib/src/core/models/own_user.dart (1)
210-216: LGTM! Consistent privacy extension implementation.The
isDeliveryReceiptsEnabledgetter correctly follows the established pattern of the existingisTypingIndicatorsEnabledandisReadReceiptsEnabledmethods, with appropriate null-safe default behavior (returnstruewhen not explicitly configured).packages/stream_chat/lib/src/core/models/privacy_settings.dart (2)
13-13: LGTM! Clean integration into PrivacySettings.The
deliveryReceiptsfield is correctly added to thePrivacySettingsmodel with proper constructor parameter, field declaration, documentation, and inclusion in the equality props.Also applies to: 27-28, 34-34
88-112: LGTM! Well-structured DeliveryReceipts model.The
DeliveryReceiptsclass follows the exact same pattern asTypingIndicatorsandReadReceipts, maintaining consistency across the privacy settings domain. The defaultenabled: truebehavior and clear documentation align with user expectations for receipt features.packages/stream_chat/lib/src/core/models/message_delivery.dart (1)
9-27: Model shape looks goodFields map correctly to 'cid' and 'id'; write-only serialization is appropriate.
packages/stream_chat/lib/src/core/models/message_delivery.g.dart (1)
1-13: Generated code — no review neededMapping matches the model.
packages/stream_chat/test/src/client/channel_test.dart (1)
4261-4274: Event type switch to messageRead looks correctThe change to EventType.messageRead aligns with recent event taxonomy.
Please add a focused test for EventType.messageDelivered that verifies updates of Read.lastDeliveredAt and Read.lastDeliveredMessageId on channel state. I can scaffold one if helpful.
packages/stream_chat/lib/stream_chat.dart (1)
20-21: Public exports LGTMExposing ChannelDeliveryReporter, MessageDelivery, and MessageRules matches the new API surface.
Confirm ChannelDeliveryReporter is intended as public API (not internal); if internal, consider moving behind an interface or keeping it package‑private.
Also applies to: 49-50, 71-72
packages/stream_chat/lib/src/client/client.dart (4)
237-241: Reporter lifecycle wiring looks goodLate initialization with a dedicated logger and callback is clean.
2136-2138: Good: reporter canceled on disconnectPrevents timers/tasks leaking across sessions.
2275-2283: All‑channels read reset logicSkipping when event.cid is present makes sense so only global notificationMarkRead zeroes all unread counts. Confirm per‑channel read/unread is still handled in Channel state listeners.
2386-2387: Map merge simplification LGTMUsing spread keeps previous channels intact and updates efficiently.
packages/stream_chat/test/src/mocks.dart (2)
102-106: Mock reporter wiring LGTMLazily providing a MockChannelDeliveryReporter enables tests to assert interactions without touching network.
177-196: Mock interface completenessStub methods are present and return completed futures; suitable for tests.
packages/stream_chat/lib/src/core/util/message_rules.dart (4)
5-27: LGTM! Clean utility class design.The non-instantiable utility class pattern with a private constructor is appropriate here. The
canUploadvalidation logic correctly checks for all valid message content types (text, attachments, quoted messages, and polls).
29-54: Well-implemented with backend alignment.The logic correctly mirrors the backend implementation (referenced in line 34) and handles all relevant edge cases including error states, shadowed/ephemeral messages, system message config, and visibility restrictions.
56-115: Comprehensive unread validation logic.The method thoroughly handles all edge cases for determining unread eligibility, including user preferences, channel capabilities, message types, visibility, and read state. The null handling for
currentUserRead(line 104) is appropriate - when there's no read state, new eligible messages should count as unread.
131-190: No issues found. The muted channel behavior is intentional and consistent.The code correctly implements a systematic design pattern: muting a channel suppresses all activity tracking, including delivery receipts. This is confirmed by:
- Both
canCountAsUnread()(line 77) andcanMarkAsDelivered()(line 146) consistently block muted channelsChannelDeliveryReporterexplicitly gates delivery submission oncanMarkAsDelivered()as the validation rule- The architecture treats mute as comprehensive activity suppression, not just user notification suppression
This is intentional design, not a bug.
packages/stream_chat/lib/src/client/channel.dart (12)
97-97: Good refactoring to helper method.Extracting the initialization logic into
_initStateimproves code organization and makes thefromStateconstructor cleaner.
736-736: Excellent refactoring to centralized validation.Using
MessageRules.canUploadeliminates code duplication and provides a single source of truth for message validation logic. This makes the codebase more maintainable.
1634-1687: Consistent capability checks throughout.The usage of
canUseReadReceiptsfor all read-related operations (markRead, markUnread, markThreadRead, markThreadUnread) provides consistent behavior and clear error messages.
1868-1872: Smart delivery reporting trigger.Submitting for delivery only when
messagesPagination == nullensures delivery receipts are sent for initial queries (fetching latest messages) but not during pagination. This prevents duplicate reporting and aligns with the expected behavior of delivery receipts.
2093-2098: Cleaner capability check logic.The simplified
_canSendTypingEventsgetter correctly validates both the channel capability (canUseTypingEvents) and user privacy setting (isTypingIndicatorsEnabled).
2162-2164: Good centralization of client references.Using
_clientgetter throughout the class improves consistency and makes the code more maintainable. The updated retry emoji (🔄) is also clearer.Also applies to: 2249-2253, 2257-2257
2861-2888: Well-integrated delivery receipt logic.The message.new event handling cleanly integrates delivery receipts:
- Lines 2872-2873 correctly identify thread-only messages
- Lines 2882-2884 use
MessageRules.canCountAsUnreadfor consistent validation- Line 2886 triggers delivery reporting for the channel
The unread count increment (line 2883) is safe in Dart's single-threaded event loop model.
3108-3191: Excellent read and delivery event integration.The event handling correctly manages the relationship between read and delivery states:
- messageRead events (lines 3118-3126): Preserve delivery info (
lastDeliveredAt,lastDeliveredMessageId) when updating read state- messageDelivered events (lines 3170-3178): Preserve read info when updating delivery state, using epoch zero as default for
lastReadwhen no current read exists- Reconciliation (lines 3133-3135, 3185-3187): Trigger delivery reconciliation after events from the current user
The symmetric handling ensures read and delivery states don't overwrite each other.
3220-3225: Safer null handling for last message.Using
lastOrNullinstead of direct access prevents exceptions when the messages list is empty, improving robustness.
3281-3339: Consistent read state access patterns.The refactoring to use
userReadOfhelper methods (lines 3288, 3294) andMessageRules.canCountAsUnread(line 3332) provides consistent read state access throughout the class. ThecountUnreadMentionsmethod now uses the same validation logic as other unread counting.
3621-3670: Excellent API for read and delivery queries.The
ChannelReadHelperextension provides a clean, well-documented interface for querying read and delivery state. The methods cover common use cases:
- User-specific reads (
userReadOf,userReadStreamOf)- Message-specific reads (
readsOf,readsOfStream)- Message-specific deliveries (
deliveriesOf,deliveriesOfStream)The clear documentation and consistent naming make this a valuable addition to the API surface.
3817-3864: Clean capability API with proper deprecation.The deprecation of
canReceiveReadEventsin favor ofcanUseReadReceiptsimproves clarity (lines 3817-3823), and the newcanUseDeliveryReceiptsgetter (lines 3861-3864) follows the same pattern for consistency. Both methods check the appropriateChannelCapabilityenum values.
| expect( | ||
| read.lastDeliveredAt, | ||
| DateTime.parse('2020-01-28T22:17:30.966485504Z'), | ||
| ); | ||
| expect( | ||
| read.lastDeliveredMessageId, | ||
| '8cc1301d-2d47-4305-945a-cd8e19b736d6', | ||
| ); |
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.
Solid coverage; add one missing assertion
Tests correctly cover serialization and copyWith for lastDeliveredAt. Add an assertion for lastDeliveredMessageId in the copyWith section to complete coverage.
Apply this minimal addition after Line 81:
+ expect(newRead.lastDeliveredMessageId, 'last_delivered_test');Also applies to: 31-33, 77-81
🤖 Prompt for AI Agents
In packages/stream_chat/test/src/core/models/read_test.dart around lines 15-22
and specifically add the missing assertion in the copyWith coverage after line
81 (also applicable to lines 31-33 and 77-81): the test currently asserts
lastDeliveredAt but omits verifying lastDeliveredMessageId after using copyWith
— add a single expect asserting that the
read.copyWith(...).lastDeliveredMessageId equals
'8cc1301d-2d47-4305-945a-cd8e19b736d6' to complete coverage.
The `deliveriesOf` method now correctly identifies a message as delivered if the user has already read it, even if `lastDeliveredAt` is null or precedes the message's creation time. This prevents scenarios where a message is marked as read but not considered delivered, ensuring more accurate delivery status tracking.
This commit introduces a new state to the `SendingIndicator` to show when a message has been delivered but not yet read. - A `checkAll` icon with a `textLowEmphasis` color is now displayed for delivered messages. - The `SendingIndicator` widget now accepts an `isMessageDelivered` boolean. - The `StreamSendingIndicatorBuilder` is updated to calculate `isMessageDelivered` based on the channel's state and pass it to the indicator.
This commit introduces a new "Message Info" screen, accessible from the message actions menu. This screen displays detailed delivery and read receipt information for a selected message. Key changes: - A new `MessageInfoSheet` widget has been created to display lists of users who have received and read the message. - An "Message Info" action has been added to the message long-press menu, which opens the new bottom sheet. - This feature is only enabled if delivery events are active for the channel.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
1 similar comment
✅ Actions performedReview triggered.
|
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
♻️ Duplicate comments (1)
packages/stream_chat/lib/src/core/models/read.dart (1)
106-146: Critical: Guard against null message timestamps (duplicate issue).This is the same critical null-safety issue flagged in the previous review.
message.createdAtis nullable for locally created or unsynced messages. CallingisBeforeon a null DateTime will throw a runtime exception.The issue occurs at:
- Line 112 in
readsOf- Lines 137 and 142 in
deliveriesOfDerive a non-null timestamp using
message.createdAt ?? message.createdLocallyAtand bail out early if both are null.Apply this diff to guard both methods:
List<Read> readsOf({required Message message}) { final sender = message.user; if (sender == null) return <Read>[]; + final messageTimestamp = message.createdAt ?? message.createdLocallyAt; + if (messageTimestamp == null) return <Read>[]; + return where((read) { if (read.user.id == sender.id) return false; - if (read.lastRead.isBefore(message.createdAt)) return false; + if (read.lastRead.isBefore(messageTimestamp)) return false; return true; }).toList(); } List<Read> deliveriesOf({required Message message}) { final sender = message.user; if (sender == null) return <Read>[]; + final messageTimestamp = message.createdAt ?? message.createdLocallyAt; + if (messageTimestamp == null) return <Read>[]; + return where((read) { if (read.user.id == sender.id) return false; final lastReadAt = read.lastRead; - if (!lastReadAt.isBefore(message.createdAt)) return true; + if (!lastReadAt.isBefore(messageTimestamp)) return true; final lastDeliveredAt = read.lastDeliveredAt; if (lastDeliveredAt == null) return false; - if (lastDeliveredAt.isBefore(message.createdAt)) return false; + if (lastDeliveredAt.isBefore(messageTimestamp)) return false; return true; }).toList(); }
🧹 Nitpick comments (1)
sample_app/lib/widgets/message_info_sheet.dart (1)
264-270: Consider fallback for empty user names.If
read.user.namecan be empty, consider providing a fallback (e.g., "Unknown User" or the user ID) to ensure a meaningful display.Expanded( child: Text( - read.user.name, + read.user.name.isEmpty ? 'Unknown User' : read.user.name, style: theme.textTheme.bodyBold, maxLines: 1, overflow: TextOverflow.ellipsis, ), ),
📜 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 (5)
packages/stream_chat/lib/src/core/models/read.dart(4 hunks)packages/stream_chat_flutter/lib/src/indicators/sending_indicator.dart(1 hunks)packages/stream_chat_flutter/lib/src/message_widget/sending_indicator_builder.dart(1 hunks)sample_app/lib/pages/channel_page.dart(2 hunks)sample_app/lib/widgets/message_info_sheet.dart(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📚 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_flutter/lib/src/indicators/sending_indicator.dartpackages/stream_chat/lib/src/core/models/read.dartpackages/stream_chat_flutter/lib/src/message_widget/sending_indicator_builder.dartsample_app/lib/widgets/message_info_sheet.dartsample_app/lib/pages/channel_page.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_flutter/lib/src/indicators/sending_indicator.dartpackages/stream_chat/lib/src/core/models/read.dartpackages/stream_chat_flutter/lib/src/message_widget/sending_indicator_builder.dartsample_app/lib/widgets/message_info_sheet.dartsample_app/lib/pages/channel_page.dart
📚 Learning: 2025-08-08T14:27:59.621Z
Learnt from: xsahil03x
Repo: GetStream/stream-chat-flutter PR: 2348
File: packages/stream_chat_flutter_core/lib/src/stream_channel.dart:383-406
Timestamp: 2025-08-08T14:27:59.621Z
Learning: In stream_chat_flutter_core/lib/src/stream_channel.dart, threads (replies) do not support around-anchor loading. Thread replies are fetched as: initial latest page via StreamChannelState.getReplies(), and further pagination via StreamChannelState.queryReplies(parentId, direction: top|bottom). Anchored loads apply only to channel messages, not to threads.
Applied to files:
packages/stream_chat_flutter/lib/src/message_widget/sending_indicator_builder.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). (1)
- GitHub Check: stream_chat_flutter
🔇 Additional comments (15)
packages/stream_chat/lib/src/core/models/read.dart (1)
96-99: LGTM: Clean null-safe implementation.The
userReadOfmethod correctly handles null userId and usesfirstWhereOrNullfor safe lookup.packages/stream_chat_flutter/lib/src/message_widget/sending_indicator_builder.dart (3)
66-67: LGTM!The refactoring to use the
readsOfhelper method improves code clarity and maintainability.
69-70: LGTM!The delivery tracking logic mirrors the existing read tracking pattern and integrates cleanly with the extended Read model.
75-75: LGTM!Correctly passes the delivery state to the indicator for rendering.
packages/stream_chat_flutter/lib/src/indicators/sending_indicator.dart (4)
10-16: LGTM!The new
isMessageDeliveredparameter and field are well-integrated with proper defaults for backward compatibility.Also applies to: 24-25
32-34: Good refactoring!Extracting the theme variables reduces duplicate lookups and improves code clarity.
43-49: LGTM!The delivery branch is correctly positioned to maintain proper precedence (read > delivered > completed > outgoing), and the visual distinction between delivered (textLowEmphasis) and read (accentPrimary) states is clear and intuitive.
39-39: LGTM!The color refactoring maintains consistent styling while using the extracted
colorThemereference throughout.Also applies to: 55-55, 63-63
sample_app/lib/widgets/message_info_sheet.dart (5)
1-16: Well-structured widget with clear purpose.The widget structure follows Flutter and Stream Chat conventions. The documentation clearly describes the feature's intent.
18-47: Modal presentation is well-configured.The use of
DraggableScrollableSheetwithStreamChannelwrapper ensures proper context propagation and good UX. Theme integration is appropriate.
138-173: Header implementation is clean and follows conventions.The header with close button uses appropriate theming and dismissal logic with
maybePop.
204-209: Workaround is properly documented.The
MediaQuery.removePaddingworkaround for the Flutter padding issue is well-documented with a link to the upstream issue. This is good practice for tracking technical debt.
73-74: Extension methods verified and properly handling edge cases.The
readsOfanddeliveriesOfextension methods are correctly defined inpackages/stream_chat/lib/src/core/models/read.dartand properly handle edge cases by returning empty lists when the message sender is null. The code insample_app/lib/widgets/message_info_sheet.dartcorrectly uses these methods and appropriately handles empty results with conditional rendering at line 77.sample_app/lib/pages/channel_page.dart (2)
8-8: Clean integration of the new feature.The import and integration of
MessageInfoSheetinto the message actions flow is well-structured and properly gated bychannelConfig?.deliveryEvents.
229-241: Message action integration is correct and well-gated.The "Message Info" action is properly conditional on
deliveryEventscapability, follows the established pattern for custom actions, and correctly dismisses the current dialog before showing the sheet.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #2429 +/- ##
==========================================
+ Coverage 63.99% 64.00% +0.01%
==========================================
Files 415 418 +3
Lines 25949 26118 +169
==========================================
+ Hits 16606 16717 +111
- Misses 9343 9401 +58 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
This commit introduces a new "Message Info" screen, accessible from the message actions menu. This screen displays detailed delivery and read receipt information for a selected message. Key changes: - A new `MessageInfoSheet` widget has been created to display lists of users who have received and read the message. - An "Message Info" action has been added to the message long-press menu, which opens the new bottom sheet. - This feature is only enabled if delivery events are active for the channel.
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/CHANGELOG.md (4)
9-9: API name/style consistency: prefer instance method notation.Use the same style as elsewhere in the changelog for client instance APIs.
-- Added `Client.markChannelsDelivered` method to submit delivery receipts. +- Added `client.markChannelsDelivered()` to submit delivery receipts.
10-11: List all newly added helpers (missinguserReadOf?).If
userReadOfshipped with this PR, include it for completeness; otherwise ignore.-- Added `deliveriesOf` and `readsOf` helper methods to `ReadIterableExtension` for querying read and - delivery statuses. +- Added `deliveriesOf`, `readsOf`, and `userReadOf` helper methods to `ReadIterableExtension` for + querying read and delivery statuses.
7-13: Capture new event and config flag explicitly.Changelog should mention the
message.deliveredevent and theChannelConfig.deliveryEventscapability flag introduced by this feature.- Added message delivery receipts support with `lastDeliveredAt` and `lastDeliveredMessageId` fields in `Read` model. - Added `client.markChannelsDelivered()` to submit delivery receipts. +- Added `message.delivered` event to broadcast delivery acknowledgments. - Added `deliveriesOf`, `readsOf`, and `userReadOf` helper methods to `ReadIterableExtension` for querying read and delivery statuses. - Added channel capability getters: `Channel.canUseDeliveryReceipts`, `Channel.canUseReadReceipts`, `Channel.canUseTypingEvents`. +- Added `ChannelConfig.deliveryEvents` flag to enable delivery receipt behavior.
12-13: Naming consistency note (optional).You’re introducing
canUse*while older getters usecanSend*. You’ve deprecated typing’s old name; consider aligning others in a future pass or documenting the naming strategy here.
📜 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 (2)
packages/stream_chat/CHANGELOG.md(1 hunks)packages/stream_chat_flutter/CHANGELOG.md(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/stream_chat_flutter/CHANGELOG.md
⏰ 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: stream_chat_flutter_core
- GitHub Check: stream_chat_persistence
- GitHub Check: stream_chat_localizations
- GitHub Check: analyze
- GitHub Check: stream_chat
- GitHub Check: stream_chat_flutter
- GitHub Check: build (ios)
- GitHub Check: build (android)
- GitHub Check: test
- GitHub Check: analyze_legacy_versions
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/core/models/privacy_settings_test.dart (1)
136-173: LGTM! Comprehensive test coverage for delivery receipts.The new test group for
DeliveryReceiptsis well-structured and provides thorough coverage including default values, JSON parsing, equality, and serialization.One minor observation: The
toJsontest (lines 167-172) is a valuable addition but is only present in theDeliveryReceiptsgroup. For consistency, consider adding similar tests toTypingIndicatorPrivacySettingsandReadReceiptsPrivacySettingsgroups.Additionally, there's noticeable code duplication across all three privacy settings test groups. While not blocking, consider refactoring with parameterized tests or shared test helpers to improve maintainability.
📜 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 ignored due to path filters (3)
packages/stream_chat_flutter/test/src/indicators/goldens/ci/sending_indicator_1.pngis excluded by!**/*.pngpackages/stream_chat_flutter/test/src/indicators/goldens/ci/sending_indicator_2.pngis excluded by!**/*.pngpackages/stream_chat_flutter/test/src/indicators/goldens/ci/sending_indicator_3.pngis excluded by!**/*.png
📒 Files selected for processing (7)
packages/stream_chat/test/src/client/channel_test.dart(8 hunks)packages/stream_chat/test/src/client/client_test.dart(1 hunks)packages/stream_chat/test/src/core/api/channel_api_test.dart(1 hunks)packages/stream_chat/test/src/core/models/own_user_test.dart(1 hunks)packages/stream_chat/test/src/core/models/privacy_settings_test.dart(5 hunks)packages/stream_chat/test/src/mocks.dart(3 hunks)packages/stream_chat_flutter/test/src/indicators/sending_indicator_test.dart(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/stream_chat/test/src/mocks.dart
🧰 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/client/channel_test.dartpackages/stream_chat_flutter/test/src/indicators/sending_indicator_test.dartpackages/stream_chat/test/src/client/client_test.dartpackages/stream_chat/test/src/core/api/channel_api_test.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/client/channel_test.dartpackages/stream_chat_flutter/test/src/indicators/sending_indicator_test.dartpackages/stream_chat/test/src/client/client_test.dartpackages/stream_chat/test/src/core/api/channel_api_test.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: build (ios)
- GitHub Check: build (android)
- GitHub Check: test
- GitHub Check: analyze
- GitHub Check: stream_chat_flutter
- GitHub Check: stream_chat_localizations
- GitHub Check: stream_chat_persistence
- GitHub Check: stream_chat_flutter_core
- GitHub Check: stream_chat
- GitHub Check: analyze_legacy_versions
🔇 Additional comments (14)
packages/stream_chat/test/src/core/models/own_user_test.dart (1)
648-689: LGTM! Comprehensive test coverage for delivery receipts.The new test cases for
isDeliveryReceiptsEnabledare well-structured and provide complete coverage of all code paths:
- Default behavior when null (returns true)
- Explicit enabled state
- Explicit disabled state
- Edge case where privacy settings exists but delivery receipts field is null
The tests mirror the existing patterns for
isTypingIndicatorsEnabledandisReadReceiptsEnabled, maintaining consistency across the test suite.packages/stream_chat_flutter/test/src/indicators/sending_indicator_test.dart (2)
49-109: LGTM! Golden tests comprehensively cover delivery states.The golden tests now cover all visual states of the sending indicator:
- Read state (sending_indicator_0)
- Delivered state (sending_indicator_1)
- Sent but not delivered/read state (sending_indicator_2)
- Sending state (sending_indicator_3)
The updated test description on line 50-51 and the addition of
isMessageDelivered: trueon line 60 clearly distinguish the delivered state test. The new golden test (lines 71-89) appropriately tests the intermediate sent state without delivery or read flags.
111-210: Excellent test coverage for delivery and read states.The three widget tests thoroughly validate the indicator behavior:
- Delivered state test (lines 111-142): Correctly verifies
checkAllicon withtextLowEmphasiscolor for delivered messages- Read state test (lines 144-175): Correctly verifies
checkAllicon withaccentPrimarycolor for read messages- Precedence test (lines 177-210): Critically validates that read status takes priority over delivered when both are true, ensuring correct visual hierarchy
All tests are properly wrapped with theme context and include clear assertions. The inline comment on line 204 enhances readability.
packages/stream_chat/test/src/core/models/privacy_settings_test.dart (4)
8-23: LGTM!The delivery receipts integration into the "all fields" test is correct and consistent with the existing pattern for typing indicators and read receipts.
25-33: LGTM!The null field handling for delivery receipts is correct and consistent with the pattern established for other privacy settings.
35-46: LGTM!The partial fields test correctly validates that delivery receipts remain null when not present in the JSON.
48-69: LGTM!The equality test correctly includes delivery receipts in the comparison, ensuring the new field participates in equality semantics.
packages/stream_chat/test/src/client/client_test.dart (1)
2920-2940: LGTM! Well-structured test for the new delivery receipts API.The test correctly verifies that
StreamChatClient.markChannelsDelivereddelegates to the underlying channel API with the proper delivery information. The test structure is consistent with similar tests in the file (e.g.,markAllRead).packages/stream_chat/test/src/core/api/channel_api_test.dart (1)
836-841: ****The test verification is correct. The actual implementation in
markChannelsDelivered()usesjsonEncode()when posting to/channels/delivered, as confirmed by the grep output showing:final response = await _client.post( '/channels/delivered', data: jsonEncode({ 'latest_delivered_messages': deliveries, }), );While other endpoints pass raw data structures, this endpoint specifically uses JSON encoding. The test verification accurately reflects the implementation's behavior.
Likely an incorrect or invalid review comment.
packages/stream_chat/test/src/client/channel_test.dart (5)
204-209: LGTM! Mock setup is consistent.The channelDeliveryReporter mock is properly configured in all relevant test groups to handle delivery submission calls that are now triggered by the new delivery receipts functionality.
Also applies to: 540-544, 932-935
4271-4300: EventType update aligns with delivery receipts feature.The change from
EventType.notificationMarkReadtoEventType.messageReadis consistent with the PR's event model refactoring. The test logic correctly validates that read state is created for new users.
4381-4478: Excellent test coverage for delivery/read interaction.These tests properly validate that:
- Delivery information persists when read events update (avoiding data loss)
- Delivery reconciliation is triggered for current user read events
The test structure follows existing patterns and uses appropriate mocking.
4480-4663: Comprehensive coverage for message.delivered events.The test suite properly validates all aspects of delivery event handling:
- Delivery state creation and updates
- Preservation of existing read information
- Reconciliation triggering for current user events
The tests mirror the read event test structure, maintaining consistency across the test file.
5342-5625: Well-structured tests for ChannelReadHelper extensions.The test group provides thorough coverage of the new helper methods:
- Proper validation of filtering logic (sender exclusion, timestamp checks)
- Both synchronous and stream variants are tested
- Edge cases like null values and non-existent users are handled
- Stream tests properly use
expectLaterwithemitsInOrderThe test structure is clean and follows existing patterns in the 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 (1)
packages/stream_chat/test/src/client/channel_test.dart (1)
204-208: Consider adding error handling tests for delivery reporter.While the mock setup is correct, the tests don't cover scenarios where
channelDeliveryReporter.submitForDeliveryorreconcileDeliverymight fail. Consider adding tests to verify that delivery reporter failures are handled gracefully and don't break the main channel functionality.Also applies to: 3597-3601, 6010-6014
📜 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 (1)
packages/stream_chat/test/src/client/channel_test.dart(10 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/client/channel_test.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/client/channel_test.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: test
- GitHub Check: stream_chat_localizations
- GitHub Check: build (ios)
- GitHub Check: build (android)
- GitHub Check: stream_chat_flutter
- GitHub Check: analyze
- GitHub Check: stream_chat_flutter_core
- GitHub Check: stream_chat_persistence
- GitHub Check: stream_chat
🔇 Additional comments (8)
packages/stream_chat/test/src/client/channel_test.dart (8)
204-208: Mock setup looks good.The mock for
channelDeliveryReporter.submitForDeliveryis appropriately configured. Note that this same mock is repeated in other test groups (lines 3597-3601, 6010-6014), which is necessary due to test group scoping.
3146-3202: LGTM! Delivery submission logic is correctly tested.The tests appropriately verify that:
- Delivery is submitted when fetching latest messages (no pagination parameters)
- Delivery is NOT submitted when paginating through older messages
This distinction is important for avoiding unnecessary delivery receipts when scrolling through message history.
3988-4009: Good coverage for message receipt delivery submission.The test correctly verifies that delivery submission is triggered when a new message is received.
4349-4380: Event type correction looks appropriate.The change from
EventType.notificationMarkReadtoEventType.messageReadaligns better with the test name and appears to be part of the delivery receipts refactoring. Ensure this event type change is consistent with backend event definitions.
4460-4521: Excellent test coverage for preserving delivery info.The test correctly verifies that when a
message.readevent is processed, the existinglastDeliveredAtandlastDeliveredMessageIdfields are preserved. This is important to ensure delivery information isn't lost when read events update the read state.
4523-4557: Reconciliation logic is properly tested.Good use of
addTearDownto restore the original user state and prevent test pollution. The verification confirms that delivery reconciliation is triggered when the current user's read event is received.
4559-4650: Good coverage for message delivered events.Both tests appropriately verify:
- Updating existing read state with delivery information
- Creating new read state with delivery information when user doesn't exist
The use of
distantPastfor defaultlastReadvalues is a good pattern.
5421-5703: Excellent comprehensive test coverage for ChannelReadHelper.The tests thoroughly cover all helper methods with appropriate test cases including:
- Basic functionality
- Edge cases (null userId, non-existent users)
- Stream behavior
- Filtering logic (excluding senders, checking timestamps)
- Implicit delivery through read receipts
The test quality is high with proper setup, assertions, and teardown.
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 (3)
packages/stream_chat/test/src/client/channel_delivery_reporter_test.dart (3)
39-224: Excellent test coverage for submission filtering.The test suite thoroughly validates all the filtering conditions: channels without CID/message/capability, own messages, already-read/delivered messages, candidate updates, and throttling behavior.
Consider adding a test for submitting an empty channel list to verify graceful handling:
test('should handle empty channel list', () async { await reporter.submitForDelivery([]); await delay(150); expect(capturedDeliveries, isEmpty); });
226-327: Solid reconciliation test coverage.The tests properly validate that the reporter reconciles delivery candidates when channels are read/delivered and retains valid candidates.
The test at lines 322-326 lacks an assertion. If the intent is to verify it doesn't throw, consider adding an explicit expectation or a comment:
test('should handle channels without cid', () async { final channel = Channel(client, 'test-type', null); // Should not throw expect(() => reporter.reconcileDelivery([channel]), returnsNormally); });
512-628: Well-designed test helpers.The helper functions are reusable, clearly named, and provide flexible test object creation with builder patterns and optional parameters.
The logger at line 516 prints all records to console, which might produce noisy test output. Consider making logging configurable or using a lower level for routine test runs:
Logger _createLogger(String name, {Level level = Level.OFF}) { final logger = Logger.detached(name)..level = level; logger.onRecord.listen(print); return logger; }
📜 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 (1)
packages/stream_chat/test/src/client/channel_delivery_reporter_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 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/client/channel_delivery_reporter_test.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 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/client/channel_delivery_reporter_test.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: build (android)
- GitHub Check: test
- GitHub Check: build (ios)
- GitHub Check: analyze
- GitHub Check: analyze_legacy_versions
- GitHub Check: stream_chat_flutter_core
- GitHub Check: stream_chat_localizations
- GitHub Check: stream_chat_flutter
- GitHub Check: stream_chat
- GitHub Check: stream_chat_persistence
🔇 Additional comments (4)
packages/stream_chat/test/src/client/channel_delivery_reporter_test.dart (4)
1-37: LGTM! Well-structured test setup.The test initialization follows best practices: fallback values registered for custom types, consistent setup/teardown, and proper resource cleanup via
reporter.cancel().
329-376: LGTM! Comprehensive cancellation coverage.The tests validate both single-channel cancellation and handling of non-existent channels, with clear verification that only targeted channels are affected.
378-465: Excellent batching and error handling tests.The batch limit test properly validates the 100-channel constraint, and the error-handling test demonstrates graceful recovery with a retry pattern.
467-508: LGTM! Complete cancellation coverage.The tests properly verify that
cancel()clears all state and prevents both queued candidates and pending throttled calls from executing.
Submit a pull request
Fixes: FLU-224
Description of the pull request
This PR introduces message delivery receipts, allowing clients to acknowledge when a message has been successfully delivered. Senders can now be notified that their messages have reached the recipient's device.
Key Changes:
ReadModel: TheReadmodel is extended to includelastDeliveredAtandlastDeliveredMessageId, enabling tracking of message delivery alongside read status.message.deliveredevent is introduced to broadcast delivery acknowledgments.ChannelDeliveryReporter: A newChannelDeliveryReporterclass is added to manage, batch, and throttle the sending of delivery receipts to the backend, ensuring efficient network usage.MessageRulesutility class is created to centralize validation logic, determining if a message is eligible for delivery receipts, can be counted as unread, or can be sent.markChannelsDelivered) and corresponding data models (MessageDeliveryInfo) have been added to support this feature.ChannelandChannelClientStateinto the newMessageRulesclass for better separation of concerns. Helper extensions have been added toReadandChannelClientStateto simplify querying for read and delivery statuses.Screenshots / Videos
Screen.Recording.2025-11-07.at.14.06.55.mov
Summary by CodeRabbit
New Features
Improvements
Tests