Skip to content

Conversation

@andremion
Copy link
Contributor

@andremion andremion commented Oct 23, 2025

🎯 Goal

  • Send message delivery receipts, supporting local data buffering, batched reporting, event handling, and UI indicators.
  • Marks messages as delivered on:
    • Single channel query result (first page only)
    • Channel list query result
    • Push notification received
    • WS message.new event

https://linear.app/stream/issue/AND-769

🛠 Implementation details

Client module

  • New persistence and repository for delivery receipts (MessageReceiptDao/Entity/Repository, ChatClientDatabase, DateConverter).
  • New delivery reporting pipeline (MessageReceiptManager, MessageReceiptReporter) with 1s buffer and max 100 confirmations per call.
  • message.delivered WS event support.
  • Introduced new Channel extensions: readsOf, deliveredReadsOf.
  • PrivacySettings.delivery_receipts support.
  • Channel read state extended with last_delivered_at and last_delivered_message_id.

Delivery receipts are:

  • Buffered and sent every 1s, max 100 per request.
  • Not sent for own messages, not sent if already read or already delivered.
  • Reconciled against reads (reads supersede delivered).

Privacy:

  • Suppressed if ownUser.privacySettings.deliveryReceipts.enabled == false.

UI:

  • Compose and XML indicators updated to support the new grey double tick (delivered)
  • Unified logic to hide the message status indicator when a message is deleted / ephemeral / sync status failed permanently.

🎨 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

  • Open the channel list from two devices connected with different users.
  • Send a message from user A to user B.
  • Long press on the new message from user A.
  • Click on Message Info.
  • The message should be Delivered.
  • Enter the chat from user B.
  • From user A, notice that the message should now be Read.

@andremion andremion added the core label Oct 23, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Oct 23, 2025

SDK Size Comparison 📏

SDK Before After Difference Status
stream-chat-android-client 3.23 MB 3.25 MB 0.02 MB 🟢
stream-chat-android-offline 3.45 MB 3.47 MB 0.02 MB 🟢
stream-chat-android-ui-components 10.54 MB 10.57 MB 0.03 MB 🟢
stream-chat-android-compose 12.76 MB 12.79 MB 0.03 MB 🟢

@andremion andremion force-pushed the AND-769-message-delivery-status branch 3 times, most recently from cbf13fa to 0b052a3 Compare October 27, 2025 15:21
@github-actions
Copy link
Contributor

github-actions bot commented Oct 27, 2025

DB Entities have been updated. Do we need to upgrade DB Version?
Modified Entities :

stream-chat-android-client/src/main/java/io/getstream/chat/android/client/persistence/db/entity/MessageReceiptEntity.kt
stream-chat-android-offline/src/main/java/io/getstream/chat/android/offline/repository/domain/channelconfig/internal/ChannelConfigEntity.kt

@andremion andremion force-pushed the AND-769-message-delivery-status branch 6 times, most recently from d6aebd9 to 89655f6 Compare November 4, 2025 10:23
@andremion andremion requested a review from Copilot November 4, 2025 14:17
Copy link
Contributor

Copilot AI left a 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 isMessageDelivered property 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.

Comment on lines +1407 to +1415
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 {
Copy link

Copilot AI Nov 4, 2025

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.

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

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.

@andremion andremion force-pushed the AND-769-message-delivery-status branch from 19d96be to 7e134c3 Compare November 4, 2025 14:44
@andremion andremion marked this pull request as ready for review November 4, 2025 14:44
@andremion andremion requested a review from a team as a code owner November 4, 2025 14:44
* @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? =
Copy link
Contributor

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?

Copy link
Contributor Author

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.

Copy link
Contributor

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()
Copy link
Contributor

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(
Copy link
Contributor

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)?

Copy link
Contributor Author

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.

Copy link
Contributor

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.

* @see [markChannelsAsDelivered] for the conditions to mark a message as delivered.
*/
suspend fun markMessageAsDelivered(message: Message) {
val currentUser = getCurrentUser() ?: run {
Copy link
Contributor

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,
Copy link
Contributor

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 the client database after fetching the channel(s). Later, when we call markMessageAsDelivered, 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...

Copy link
Contributor Author

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 😅

Copy link
Contributor

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,
Copy link
Contributor

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.

Copy link
Contributor Author

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 😓

Copy link
Contributor

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.

Copy link
Contributor Author

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(
Copy link
Contributor

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?

Copy link
Contributor Author

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

…`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.
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.
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.
@andremion andremion force-pushed the AND-769-message-delivery-status branch from 7e134c3 to ce8df3f Compare November 7, 2025 12:06
@sonarqubecloud
Copy link

sonarqubecloud bot commented Nov 7, 2025

Comment on lines +162 to +164
read.filter { read ->
read.user.id != message.user.id &&
(read.lastDeliveredAt ?: NEVER) >= message.getCreatedAtOrDefault(NEVER)
Copy link
Member

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants