[Background Mapping] Fix Mark as unread (#2906)

* Fix Mark As Unread in BG by introducing breaking changes

* Fix existing tests

* Add tests for ChatChannel.lastReadMessageId

* Fix compilation issue

* Make sure we don't fallback into oldestRegularMessageId if channel is nil

* Update CHANGELOG

CHANGELOG.md

@@ -3,9 +3,13 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 
 # Upcoming
 
+## StreamChatUI
 ### 🐞 Fixed
 - Fix skip slow mode capability not handled [#2904](https://github.com/GetStream/stream-chat-swift/pull/2904)
 
+### 🔄 Changed
+- `ChannelController.markUnread`'s `completion`'s argument is now a `(Result<ChatChannel, Error>` instead of `Error?`
+
 # [4.43.0](https://github.com/GetStream/stream-chat-swift/releases/tag/4.43.0)
 _November 17, 2023_
 

Sources/StreamChat/Controllers/ChannelController/ChannelController.swift

@@ -120,22 +120,12 @@ public class ChatChannelController: DataController, DelegateCallable, DataStoreP
     /// This is because we cannot calculate the accurate value until we have all he messages in memory.
     /// Paginate to get the most accurate value.
     public var firstUnreadMessageId: MessageId? {
-        getFirstUnreadMessageId()
+        channel.flatMap { getFirstUnreadMessageId(for: $0) }
     }
 
     /// The id of the message which the current user last read.
     public var lastReadMessageId: MessageId? {
-        guard let currentUserRead = channel?.reads.first(where: {
-            $0.user.id == client.currentUserId
-        }) else {
-            return nil
-        }
-
-        guard let lastReadMessageId = currentUserRead.lastReadMessageId else {
-            return nil
-        }
-
-        return lastReadMessageId
+        client.currentUserId.flatMap { channel?.lastReadMessageId(userId: $0) }
     }
 
     /// A boolean indicating if the user marked the channel as unread in the current session
@@ -909,22 +899,30 @@ public class ChatChannelController: DataController, DelegateCallable, DataStoreP
     /// - Parameters:
     ///   - messageId: The id of the first message id that will be marked as unread.
     ///   - completion: The completion will be called on a **callbackQueue** when the network request is finished.
-    public func markUnread(from messageId: MessageId, completion: ((Error?) -> Void)? = nil) {
+    public func markUnread(from messageId: MessageId, completion: ((Result<ChatChannel, Error>) -> Void)? = nil) {
         /// Perform action only if channel is already created on backend side and have a valid `cid`.
         guard let channel = channel else {
-            channelModificationFailed(completion)
+            let error = ClientError.ChannelNotCreatedYet()
+            log.error(error.localizedDescription)
+            callback {
+                completion?(.failure(error))
+            }
             return
         }
 
         /// Read events are not enabled for this channel
         guard channel.canReceiveReadEvents == true else {
-            channelFeatureDisabled(feature: "read events", completion: completion)
+            let error = ClientError.ChannelFeatureDisabled("Channel feature: read events is disabled for this channel.")
+            log.error(error.localizedDescription)
+            callback {
+                completion?(.failure(error))
+            }
             return
         }
 
         guard !markingRead, let currentUserId = client.currentUserId else {
             callback {
-                completion?(nil)
+                completion?(.success(channel))
             }
             return
         }
@@ -936,13 +934,13 @@ public class ChatChannelController: DataController, DelegateCallable, DataStoreP
             userId: currentUserId,
             from: messageId,
             lastReadMessageId: getLastReadMessageId(firstUnreadMessageId: messageId)
-        ) { [weak self] error in
+        ) { [weak self] result in
             self?.callback {
-                if error == nil {
+                if case .success = result {
                     self?.isMarkedAsUnread = true
                 }
                 self?.markingRead = false
-                completion?(error)
+                completion?(result)
             }
         }
     }
@@ -1219,6 +1217,60 @@ public class ChatChannelController: DataController, DelegateCallable, DataStoreP
         updater.deleteImage(in: cid, url: url, completion: completion)
     }
 
+    public func getFirstUnreadMessageId(for channel: ChatChannel) -> MessageId? {
+        // Return the oldest regular message if all messages are unread in the message list.
+        let oldestRegularMessage: () -> MessageId? = { [weak self] in
+            guard self?.hasLoadedAllPreviousMessages == true else {
+                return nil
+            }
+            return self?.messages.last(where: { $0.type == .regular || $0.type == .reply })?.id
+        }
+
+        guard let currentUserRead = channel.reads.first(where: {
+            $0.user.id == client.currentUserId
+        }) else {
+            return oldestRegularMessage()
+        }
+
+        // If there are no unreads, then return nil.
+        guard currentUserRead.unreadMessagesCount > 0 else {
+            return nil
+        }
+
+        // If there unreads but no `lastReadMessageId`, it means the whole message list is unread.
+        // So the top message (oldest one) is the first unread message id.
+        guard let lastReadMessageId = currentUserRead.lastReadMessageId else {
+            return oldestRegularMessage()
+        }
+
+        guard lastReadMessageId != messages.first?.id else {
+            return nil
+        }
+
+        guard let lastReadIndex = messages.firstIndex(where: { $0.id == lastReadMessageId }), lastReadIndex != 0 else {
+            // If there is a lastReadMessageId, and we loaded all messages, but can't find firstUnreadMessageId,
+            // then it means the lastReadMessageId is not reachable because the channel was truncated or hidden.
+            // So we return the oldest regular message already fetched.
+            if hasLoadedAllPreviousMessages {
+                return oldestRegularMessage()
+            }
+
+            return nil
+        }
+
+        let lookUpStartIndex = messages.index(before: lastReadIndex)
+
+        var id: MessageId?
+        for index in (0...lookUpStartIndex).reversed() {
+            let message = message(at: index)
+            guard message?.author.id != client.currentUserId, message?.deletedAt == nil else { continue }
+            id = message?.id
+            break
+        }
+
+        return id
+    }
+
     // MARK: - Internal
 
     func recoverWatchedChannel(completion: @escaping (Error?) -> Void) {
@@ -1507,60 +1559,6 @@ private extension ChatChannelController {
         return messageId(at: newLastReadMessageIndex)
     }
 
-    private func getFirstUnreadMessageId() -> MessageId? {
-        // Return the oldest regular message if all messages are unread in the message list.
-        let oldestRegularMessage: () -> MessageId? = { [weak self] in
-            guard self?.hasLoadedAllPreviousMessages == true else {
-                return nil
-            }
-            return self?.messages.last(where: { $0.type == .regular || $0.type == .reply })?.id
-        }
-
-        guard let currentUserRead = channel?.reads.first(where: {
-            $0.user.id == client.currentUserId
-        }) else {
-            return oldestRegularMessage()
-        }
-
-        // If there are no unreads, then return nil.
-        guard currentUserRead.unreadMessagesCount > 0 else {
-            return nil
-        }
-
-        // If there unreads but no `lastReadMessageId`, it means the whole message list is unread.
-        // So the top message (oldest one) is the first unread message id.
-        guard let lastReadMessageId = currentUserRead.lastReadMessageId else {
-            return oldestRegularMessage()
-        }
-
-        guard lastReadMessageId != messages.first?.id else {
-            return nil
-        }
-
-        guard let lastReadIndex = messages.firstIndex(where: { $0.id == lastReadMessageId }), lastReadIndex != 0 else {
-            // If there is a lastReadMessageId, and we loaded all messages, but can't find firstUnreadMessageId,
-            // then it means the lastReadMessageId is not reachable because the channel was truncated or hidden.
-            // So we return the oldest regular message already fetched.
-            if hasLoadedAllPreviousMessages {
-                return oldestRegularMessage()
-            }
-
-            return nil
-        }
-
-        let lookUpStartIndex = messages.index(before: lastReadIndex)
-
-        var id: MessageId?
-        for index in (0...lookUpStartIndex).reversed() {
-            let message = message(at: index)
-            guard message?.author.id != client.currentUserId, message?.deletedAt == nil else { continue }
-            id = message?.id
-            break
-        }
-
-        return id
-    }
-
     private func message(at index: Int) -> ChatMessage? {
         if !messages.indices.contains(index) {
             return nil
@@ -1678,3 +1676,19 @@ public extension ChatChannelController {
         }
     }
 }
+
+public extension ChatChannel {
+    func lastReadMessageId(userId: UserId) -> MessageId? {
+        guard let currentUserRead = reads.first(where: {
+            $0.user.id == userId
+        }) else {
+            return nil
+        }
+
+        guard let lastReadMessageId = currentUserRead.lastReadMessageId else {
+            return nil
+        }
+
+        return lastReadMessageId
+    }
+}

Sources/StreamChat/Repositories/ChannelRepository.swift

@@ -47,16 +47,17 @@ class ChannelRepository {
         userId: UserId,
         from messageId: MessageId,
         lastReadMessageId: MessageId?,
-        completion: ((Error?) -> Void)? = nil
+        completion: ((Result<ChatChannel, Error>) -> Void)? = nil
     ) {
         apiClient.request(
             endpoint: .markUnread(cid: cid, messageId: messageId, userId: userId)
         ) { [weak self] result in
             if let error = result.error {
-                completion?(error)
+                completion?(.failure(error))
                 return
             }
 
+            var channel: ChatChannel?
             self?.database.write({ session in
                 session.markChannelAsUnread(
                     for: cid,
@@ -66,8 +67,13 @@ class ChannelRepository {
                     lastReadAt: nil,
                     unreadMessagesCount: nil
                 )
+                channel = try session.channel(cid: cid)?.asModel()
             }, completion: { error in
-                completion?(error)
+                if let channel = channel, error == nil {
+                    completion?(.success(channel))
+                } else {
+                    completion?(.failure(error ?? ClientError.ChannelNotCreatedYet()))
+                }
             })
         }
     }

Sources/StreamChat/Workers/ChannelUpdater.swift

@@ -441,7 +441,7 @@ class ChannelUpdater: Worker {
         userId: UserId,
         from messageId: MessageId,
         lastReadMessageId: MessageId?,
-        completion: ((Error?) -> Void)? = nil
+        completion: ((Result<ChatChannel, Error>) -> Void)? = nil
     ) {
         channelRepository.markUnread(
             for: cid,

Sources/StreamChatUI/ChatChannel/ChatChannelVC.swift

@@ -391,8 +391,10 @@ open class ChatChannelVC: _ViewController,
             }
         case is MarkUnreadActionItem:
             dismiss(animated: true) { [weak self] in
-                self?.channelController.markUnread(from: message.id) { _ in
-                    self?.updateAllUnreadMessagesRelatedComponents()
+                self?.channelController.markUnread(from: message.id) { result in
+                    if case let .success(channel) = result {
+                        self?.updateAllUnreadMessagesRelatedComponents(channel: channel)
+                    }
                 }
             }
         default:
@@ -576,27 +578,31 @@ private extension ChatChannelVC {
         )
     }
 
-    func updateAllUnreadMessagesRelatedComponents() {
-        updateScrollToBottomButtonCount()
-        updateJumpToUnreadRelatedComponents()
-        updateUnreadMessagesBannerRelatedComponents()
+    func updateAllUnreadMessagesRelatedComponents(channel: ChatChannel? = nil) {
+        updateScrollToBottomButtonCount(channel: channel)
+        updateJumpToUnreadRelatedComponents(channel: channel)
+        updateUnreadMessagesBannerRelatedComponents(channel: channel)
     }
 
-    func updateScrollToBottomButtonCount() {
-        let channelUnreadCount = channelController.channel?.unreadCount ?? .noUnread
+    func updateScrollToBottomButtonCount(channel: ChatChannel? = nil) {
+        let channelUnreadCount = (channel ?? channelController.channel)?.unreadCount ?? .noUnread
         messageListVC.scrollToBottomButton.content = channelUnreadCount
     }
 
-    func updateJumpToUnreadRelatedComponents() {
+    func updateJumpToUnreadRelatedComponents(channel: ChatChannel? = nil) {
+        let firstUnreadMessageId = channel.flatMap { channelController.getFirstUnreadMessageId(for: $0) } ?? channelController.firstUnreadMessageId
+        let lastReadMessageId = client.currentUserId.flatMap { channel?.lastReadMessageId(userId: $0) } ?? channelController.lastReadMessageId
+
         messageListVC.updateJumpToUnreadMessageId(
-            channelController.firstUnreadMessageId,
-            lastReadMessageId: channelController.lastReadMessageId
+            firstUnreadMessageId,
+            lastReadMessageId: lastReadMessageId
         )
         messageListVC.updateJumpToUnreadButtonVisibility()
     }
 
-    func updateUnreadMessagesBannerRelatedComponents() {
-        firstUnreadMessageId = channelController.firstUnreadMessageId
+    func updateUnreadMessagesBannerRelatedComponents(channel: ChatChannel? = nil) {
+        let firstUnreadMessageId = channel.flatMap { channelController.getFirstUnreadMessageId(for: $0) } ?? channelController.firstUnreadMessageId
+        self.firstUnreadMessageId = firstUnreadMessageId
         messageListVC.updateUnreadMessagesSeparator(at: firstUnreadMessageId)
     }
 }

TestTools/StreamChatTestTools/Mocks/StreamChat/Repositories/ChannelRepository_Mock.swift

@@ -18,7 +18,7 @@ class ChannelRepository_Mock: ChannelRepository {
     var markUnreadUserId: UserId?
     var markUnreadMessageId: UserId?
     var markUnreadLastReadMessageId: UserId?
-    var markUnreadResult: Result<Void, Error>?
+    var markUnreadResult: Result<ChatChannel, Error>?
 
     override func markRead(cid: ChannelId, userId: UserId, completion: ((Error?) -> Void)? = nil) {
         markReadCid = cid
@@ -29,14 +29,14 @@ class ChannelRepository_Mock: ChannelRepository {
         }
     }
 
-    override func markUnread(for cid: ChannelId, userId: UserId, from messageId: MessageId, lastReadMessageId: MessageId?, completion: ((Error?) -> Void)? = nil) {
+    override func markUnread(for cid: ChannelId, userId: UserId, from messageId: MessageId, lastReadMessageId: MessageId?, completion: ((Result<ChatChannel, Error>) -> Void)? = nil) {
         markUnreadCid = cid
         markUnreadUserId = userId
         markUnreadMessageId = messageId
         markUnreadLastReadMessageId = lastReadMessageId
 
         markUnreadResult.map {
-            completion?($0.error)
+            completion?($0)
         }
     }
 }

TestTools/StreamChatTestTools/Mocks/StreamChat/Workers/ChannelUpdater_Mock.swift

@@ -81,7 +81,7 @@ final class ChannelUpdater_Mock: ChannelUpdater {
     @Atomic var markUnread_userId: UserId?
     @Atomic var markUnread_messageId: MessageId?
     @Atomic var markUnread_lastReadMessageId: MessageId?
-    @Atomic var markUnread_completion: ((Error?) -> Void)?
+    @Atomic var markUnread_completion: ((Result<ChatChannel, Error>) -> Void)?
 
     @Atomic var enableSlowMode_cid: ChannelId?
     @Atomic var enableSlowMode_cooldownDuration: Int?
@@ -359,7 +359,7 @@ final class ChannelUpdater_Mock: ChannelUpdater {
         markRead_completion = completion
     }
 
-    override func markUnread(cid: ChannelId, userId: UserId, from messageId: MessageId, lastReadMessageId: MessageId?, completion: ((Error?) -> Void)? = nil) {
+    override func markUnread(cid: ChannelId, userId: UserId, from messageId: MessageId, lastReadMessageId: MessageId?, completion: ((Result<ChatChannel, Error>) -> Void)? = nil) {
         markUnread_cid = cid
         markUnread_userId = userId
         markUnread_messageId = messageId