-
Notifications
You must be signed in to change notification settings - Fork 305
Introduce message delivery receipts #5979
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: develop
Are you sure you want to change the base?
Conversation
SDK Size Comparison 📏
|
cbf13fa to
0b052a3
Compare
|
DB Entities have been updated. Do we need to upgrade DB Version? |
d6aebd9 to
89655f6
Compare
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.
Pull Request Overview
This pull request adds delivery status indicators for messages in the Stream Chat SDK. It introduces a new "delivered" state between "sent" and "read" states, allowing the UI to display when messages have been delivered to recipients but not yet read.
Key changes:
- Added
isMessageDeliveredproperty to message and channel state classes - Introduced new delivery status icon and UI indicators
- Updated database schema version to support delivery events persistence
- Added new drawable resource for the delivered state (double checkmark in grey)
Reviewed Changes
Copilot reviewed 116 out of 122 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| stream-chat-android-core/src/main/java/io/getstream/chat/android/models/ChannelUserRead.kt | Added lastDeliveredAt and lastDeliveredMessageId properties |
| stream-chat-android-core/src/main/java/io/getstream/chat/android/models/Config.kt | Added deliveryEventsEnabled configuration flag |
| stream-chat-android-core/src/main/java/io/getstream/chat/android/PrivacySettings.kt | Added delivery receipts privacy settings |
| stream-chat-android-ui-components/src/main/res/drawable/stream_ui_ic_check_double_grey.xml | New drawable for delivered status indicator |
| stream-chat-android-ui-components/src/main/kotlin/io/getstream/chat/android/ui/feature/messages/list/adapter/MessageListItem.kt | Added isMessageDelivered property to MessageItem |
| stream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/components/channels/MessageReadStatusIcon.kt | Updated to support delivered status icon |
| stream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/theme/ChatComponentFactory.kt | Added new MessageFooterStatusIndicator overload with params |
| stream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/database/internal/ChatDatabase.kt | Incremented database version to 96 |
| stream-chat-android-state/src/main/java/io/getstream/chat/android/state/event/handler/internal/utils/ChatEventUtils.kt | Added MessageDeliveredEvent to ChannelUserRead conversion |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (params.messageItem.isMessageDelivered) { | ||
| MessageReadStatusIcon( | ||
| modifier = params.modifier, | ||
| message = params.messageItem.message, | ||
| isMessageRead = params.messageItem.isMessageRead, | ||
| isMessageDelivered = params.messageItem.isMessageDelivered, | ||
| readCount = params.messageItem.messageReadBy.size, | ||
| ) | ||
| } else { |
Copilot
AI
Nov 4, 2025
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.
The conditional logic at line 1407 checks isMessageDelivered, but the message could be read (which implies delivered). The condition should check for both read and delivered states to ensure MessageReadStatusIcon is always called with the isMessageDelivered parameter, maintaining consistency. Consider simplifying this to always pass the isMessageDelivered parameter.
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.
This is done to prevent breaking changes in the component factory.
If isMessageDelivered == true, that means we can call the new overloaded function; otherwise, we have to call the deprecated one.
stream-chat-android-compose-sample/src/main/res/values/strings.xml
Outdated
Show resolved
Hide resolved
19d96be to
7e134c3
Compare
| * @param userId The ID of the user whose read state is to be retrieved. | ||
| * @return The [ChannelUserRead] object representing the user's read state, or null if not found. | ||
| */ | ||
| public fun Channel.userRead(userId: UserId): ChannelUserRead? = |
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.
Just wondering if need to expose these new methods as public - do you think they would be useful for integrators, without the risk of us having to change the implementation/Public API?
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.
I would say so. But I'm fine with making it internal and exposing it on demand.
I will change it then.
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.
I don't really have a very strong opinion here - if you think this would be a stable API, we can leave it exposed.
| public fun Channel.readsOf(message: Message): List<ChannelUserRead> = | ||
| read.filter { read -> | ||
| read.user.id != message.user.id && | ||
| read.lastRead >= message.getCreatedAtOrThrow() |
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.
I would use the non-throwing method here, like getCreatedAtOrDefault()
| /** | ||
| * A plugin that marks messages as delivered when channels are queried. | ||
| */ | ||
| internal class MessageDeliveredPlugin( |
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.
Do you think it would be possible to avoid introducing new Plugins, as it is something that we want to completely remove in the future. I think that this introduces another layer of complexity over the ChatClient. Wouldn't it be possible to achieve the same thing by calling the appropriate MessageReceiptManager methods from the corresponding ChatClient methods (queryChannels/queryChannel)?
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.
I wouldn’t say the long-term goal is to remove plugins entirely, but rather to make them internal and non-optional.
In this case, regardless of the approach, the implementation would likely live outside ChatClient — which is already close to 5K LOC.
Keeping it as an internal plugin for now helps isolate responsibilities and prevents adding more logic directly to ChatClient.
We can revisit and consolidate it later as part of the broader plugin refactoring.
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.
I think that we are reducing not more than 3 lines at best in the client by introducing the Plugin - two invocations of the MessageReceiptManager. I am generally against the introduction of new Plugins, because they are executing crucial business logic as a side-effect, and it's really difficult to locate that logic, unless you explicitly know about the existence of that specific Plugin.
But nevertheless, let's keep it like this, but I definitely agree that should revisit this as part of the planned refactor.
.../main/java/io/getstream/chat/android/compose/ui/components/channels/MessageReadStatusIcon.kt
Show resolved
Hide resolved
stream-chat-android-core/src/main/java/io/getstream/chat/android/models/User.kt
Show resolved
Hide resolved
| * @see [markChannelsAsDelivered] for the conditions to mark a message as delivered. | ||
| */ | ||
| suspend fun markMessageAsDelivered(message: Message) { | ||
| val currentUser = getCurrentUser() ?: run { |
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.
Would this work for the case when push notifications are received, if the app is killed? In that case, we wouldn't have a connected user to check against.
| internal class MessageReceiptManager( | ||
| private val now: () -> Date, | ||
| private val getCurrentUser: () -> User?, | ||
| private val repositoryFacade: RepositoryFacade, |
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.
Can we somehow avoid the dependency on the OfflinePlugin RepositoryFacade?
To be honest I am mostly concerned about the case where the OfflinePlugin is not present, and we have to make calls to getMessage/getChannel, which if happens often enough, we could end up in a rate limit error. (Note the case where a customer doesn't have the OfflinePlugin, and is in a channel where multiple message.new events are received in short amount of time - we could end up firing getChannel lots of times)
Can we somehow remove the need for the retrieveChannel/retrieveMessage methods in the manager?
I am thinking in the direction of:
- Instead of depending on the
OfflinePlugin/HTTP calls, why don't we store some channel config in theclientdatabase after fetching the channel(s). Later, when we callmarkMessageAsDelivered, we can check that stored config, to make sure we should mark the message.
This whole logic should be easier when the state/db is moved down to the client...
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.
I see. In that case, we need to store more than just the channel config. We use message data and channel read, too.
I will include Message, Channel, ChannelUserRead, and ChannelConfig in the client database then.
Expects an even bigger PR 😅
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.
Hmm but my question still stands: Isn't hitting rate limit errors a real possibility?
Side note: We can also access some of the State plugin data from Client via:
logicRegistry
?.channelStateLogic("type", "id")
?.listenForChannelState()
?.toChannel() // ?.getMessageById(id)Perhaps this can be utilised to check for existing channel data (but this still wouldn't cover the case when the client is used without the state plugin)
|
|
||
| internal data class MessageReceipt( | ||
| val messageId: String, | ||
| val type: String, |
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.
Is the type needed? As far as I can see, if is always set to delivery.
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.
The goal is to support other types of message receipt, like read, in the future.
The current read implementation is quite confusing and hard to maintain 😓
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.
I am just afraid that this will become legacy code, if we don't actually update this to support the different receipts.
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.
Let's remove it then
| /** | ||
| * Repository that aggregates all internal repositories used by [ChatClient]. | ||
| */ | ||
| internal class ChatClientRepository( |
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.
I am not sure whether we really need this class - wouldn't it just introduce another layer of complexity?
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.
The goal is to avoid having many constructor parameters when a class depends on multiple repositories.
For example, when the manager depends on MessageRepository, ChannelRepository, and MessageReceiptRepository, we would have to pass 3 dependencies instead of 1
...oid-client/src/main/java/io/getstream/chat/android/client/receipts/MessageReceiptReporter.kt
Show resolved
Hide resolved
…`clear` function The `MessageReceiptRepository.getAllByType` function now accepts a `limit` parameter and returns a `Flow<List<MessageReceipt>>` instead of a `suspend` function returning `List<MessageReceipt>`. A new `clear()` function has been added to `MessageReceiptRepository` to delete all receipts from the database.
The `DeliveryReceiptsManager` has been renamed to `MessageReceiptManager`. Instead of making a direct API call to mark messages as delivered, the manager now creates `MessageReceipt` objects and upserts them into the local `MessageReceiptRepository`. This change also refactors the corresponding tests to verify the local database interaction rather than the API call.
…rving the local database for delivery receipts and reporting them to the backend. This reporter collects receipts in batches and sends them periodically to reduce network traffic. After a successful API call, the reported receipts are removed from the local database. New tests are included to verify this functionality.
…essage receipt management
Moves the `MessageReceiptRepository` and related classes from the `stream-chat-android-offline` module to the `stream-chat-android-client` module. This refactoring centralizes persistence logic within the client module. The moved classes have been renamed and their package structure updated. Method names within the repository have been made more specific (e.g., `upsert` is now `upsertMessageReceipts`). The `MessageReceiptRepository` is now an internal interface with a companion object factory.
This commit adds support for the `message.delivered` event, which is triggered when a message is marked as delivered. The following changes are included: - A new `MessageDeliveredEvent` data class. - New fields (`last_delivered_at`, `last_delivered_message_id`) to the `ChannelUserRead` DTO and model. - Logic to parse and map the `message.delivered` event from the backend DTO to the domain model.
Added a `deliveryEventsEnabled` flag to the `Config` model. This allows disabling message delivery events on a per-channel basis. Delivery receipts will not be stored or sent if this flag is disabled in the channel configuration.
This commit refactors the `MessageReceiptManager` to enhance the logic for marking messages as delivered. Key changes include: - Renaming `markMessagesAsDelivered(messages: List<Message>)` to `markMessageAsDelivered(message: Message)` and making it the public API. The old function is now private and handles batch processing. - Before marking a message as delivered, the manager now fetches the corresponding channel from the local repository or the API to ensure all conditions are met. - The responsibility of filtering messages that can be marked as delivered is moved into a new private function `canMarkMessageAsDelivered`. - Test cases have been updated to reflect these changes and improve coverage.
The `MessageReceiptManager` is refactored to use `suspend` functions instead of launching new coroutines from a provided `CoroutineScope`. This simplifies the class by removing the need for an injected scope and improves testability. Key changes: - `markChannelsAsDelivered` and `markMessageAsDelivered` are now `suspend` functions. - The `CoroutineScope` dependency has been removed from `MessageReceiptManager` and its initialization in `ChatClient`. - `MessageDeliveredPlugin` is updated to use `onSuccessSuspend` to call the new suspend functions. - `ChatNotificationsImpl` now launches a coroutine to call the suspend function `markMessageAsDelivered`. - Tests are updated to use `verifyBlocking` for testing suspend functions and to provide the necessary `CoroutineScope` where required.
…ugin and ChatClient
When `markMessageAsDelivered` is called with a `messageId`, the SDK now fetches the message from the local repository. If the message is not found locally, it falls back to fetching it from the API. This ensures that a delivery receipt can be sent for a push notification even if the message is not yet in the local database.
This change prevents sending message delivery receipts for messages that are either shadowed or sent by a user who has been muted by the current user.
7e134c3 to
ce8df3f
Compare
|
| read.filter { read -> | ||
| read.user.id != message.user.id && | ||
| (read.lastDeliveredAt ?: NEVER) >= message.getCreatedAtOrDefault(NEVER) |
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.
You should probably want to also check for lastRead and promote it to delivered for supporting old messages where lastDeliveredAt is not present.



🎯 Goal
message.neweventhttps://linear.app/stream/issue/AND-769
🛠 Implementation details
Client module
message.deliveredWS event support.readsOf,deliveredReadsOf.PrivacySettings.delivery_receiptssupport.last_delivered_atandlast_delivered_message_id.Delivery receipts are:
Privacy:
ownUser.privacySettings.deliveryReceipts.enabled== false.UI:
🎨 UI Changes
1-to-1 chat
Screen.Recording.2025-11-04.at.08.43.56.mov
Group chat
Screen.Recording.2025-11-04.at.10.06.56.mov
🧪 Testing