[CIS-1583] Fix Thread not loading more replies (#2297)

* Use same strategy to load more messages in both MessageController and ChannelController

* Fix channel not loading more messages when in offline state

* Fix thread not loading more replies when in offline state

* Add Test Coverage

* Update CHANGELOG.md

CHANGELOG.md

@@ -13,6 +13,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 ### 🐞 Fixed
 - User mentions suggestions would not show when typing in a new line [#2253](https://github.com/GetStream/stream-chat-swift/pull/2253)
 - User mentions suggestions would stop showing when typing a space [#2253](https://github.com/GetStream/stream-chat-swift/pull/2253)
+- Fix Thread not loading more replies [#2297](https://github.com/GetStream/stream-chat-swift/pull/2297)
+- Fix Channel and Thread pagination not working when initialized offline [#2297](https://github.com/GetStream/stream-chat-swift/pull/2297)
 
 # [4.21.1](https://github.com/GetStream/stream-chat-swift/releases/tag/4.21.1)
 _September 06, 2022_

Sources/StreamChat/Controllers/MessageController/MessageController.swift

@@ -94,8 +94,8 @@ public class ChatMessageController: DataController, DelegateCallable, DataStoreP
         }
     }
 
-    /// Shows whether the controller has received first batch of replies from remote
-    private var loadedRepliesHead = false
+    /// The id of the last fetched reply
+    private var lastFetchedMessageId: MessageId?
 
     /// A Boolean value that returns wether pagination is finished
     public private(set) var hasLoadedAllPreviousReplies: Bool = false
@@ -296,8 +296,12 @@ public extension ChatMessageController {
             completion?(nil)
             return
         }
+
+        let lastLocalMessageId: () -> MessageId? = {
+            self.replies.last { !$0.isLocalOnly }?.id
+        }
 
-        let lastMessageId = messageId ?? (loadedRepliesHead ? replies.last?.id : nil)
+        let lastMessageId = messageId ?? lastFetchedMessageId ?? lastLocalMessageId()
 
         messageUpdater.loadReplies(
             cid: cid,
@@ -306,8 +310,8 @@ public extension ChatMessageController {
         ) { result in
             switch result {
             case let .success(payload):
-                self.loadedRepliesHead = true
                 self.hasLoadedAllPreviousReplies = payload.messages.count < limit
+                self.updateLastFetchedReplyId(with: payload)
                 self.callback { completion?(nil) }
             case let .failure(error):
                 self.callback { completion?(error) }
@@ -639,6 +643,11 @@ private extension ChatMessageController {
             return observer
         }
     }
+
+    func updateLastFetchedReplyId(with payload: MessageRepliesPayload) {
+        // Payload messages are ordered from oldest to newest
+        lastFetchedMessageId = payload.messages.first?.id
+    }
 }
 
 // MARK: - Delegate

Sources/StreamChatUI/ChatChannel/ChatChannelVC.swift

@@ -187,10 +187,6 @@ open class ChatChannelVC: _ViewController,
         _ vc: ChatMessageListVC,
         willDisplayMessageAt indexPath: IndexPath
     ) {
-        if channelController.state != .remoteDataFetched {
-            return
-        }
-
         guard messageListVC.listView.isTrackingOrDecelerating else {
             return
         }

Sources/StreamChatUI/ChatThread/ChatThreadVC.swift

@@ -216,10 +216,6 @@ open class ChatThreadVC: _ViewController,
         _ vc: ChatMessageListVC,
         willDisplayMessageAt indexPath: IndexPath
     ) {
-        if messageController.state != .remoteDataFetched {
-            return
-        }
-
         if indexPath.row < messages.count - 10 {
             return
         }

Tests/StreamChatTests/Controllers/MessageController/MessageController_Tests.swift

@@ -1074,7 +1074,6 @@ final class MessageController_Tests: XCTestCase {
     }
 
     func test_loadPreviousReplies_callsMessageUpdater_withCorrectValues() {
-        // Simulate `loadNextReplies` call
         controller.loadPreviousReplies()
 
         // Assert message updater is called with correct values
@@ -1083,34 +1082,34 @@ final class MessageController_Tests: XCTestCase {
         XCTAssertEqual(env.messageUpdater.loadReplies_pagination, .init(pageSize: 25))
     }
 
-    func test_loadPreviousReplies_noMessageIdPassed_properlyHandlesPagination() {
-        // Simulate `loadNextReplies` call
-        controller.loadPreviousReplies(
-            limit: 21,
-            completion: nil
-        )
-
-        // Pagination should not have a parameter because this is the first call
-        XCTAssertNil(env.messageUpdater.loadReplies_pagination?.parameter)
-
+    func test_loadPreviousReplies_noMessageIdPassed_noLastMessageFetched_usesLastMessageFromDB() {
+        // Create observers
+        controller.synchronize()
+
         // Call `loadPreviousReplies`, this time since the first batch was received already it should
         // pass the last message id
         env.messageUpdater.loadReplies_completion?(
             .success(
                 .init(
-                    messages: (0...30).map {
-                        _ in MessagePayload.dummy(messageId: .unique, authorUserId: .unique)
-                    }
+                    messages: []
                 )
             )
         )
         _ = controller.replies
         env.repliesObserver.items_mock = [
+            .mock(
+                id: "first message", cid: .unique, text: .unique, author: .unique
+            ),
             .mock(
                 id: "last message", cid: .unique, text: .unique, author: .unique
+            ),
+            // The last message used for pagination, needs to be in the server as well,
+            // so this one should not be used
+            .mock(
+                id: "last message only local", cid: .unique, text: .unique, author: .unique, localState: .pendingSync
             )
         ]
-        
+
         controller.loadPreviousReplies(
             limit: 21,
             completion: nil
@@ -1121,9 +1120,39 @@ final class MessageController_Tests: XCTestCase {
             .lessThan("last message")
         )
     }
+
+    func test_loadPreviousReplies_noMessageIdPassed_usesLastFetchedId() {
+        controller.loadPreviousReplies(
+            limit: 21,
+            completion: nil
+        )
+
+        // Pagination should not have a parameter because this is the first call
+        XCTAssertNil(env.messageUpdater.loadReplies_pagination?.parameter)
+
+        // The last fetched message id, is actually the first from the payload
+        var messages: [MessagePayload] = [MessagePayload.dummy(messageId: "last message", authorUserId: .unique)]
+        messages += (0...30).map { _ in
+            MessagePayload.dummy(messageId: .unique, authorUserId: .unique)
+        }
+
+        env.messageUpdater.loadReplies_completion?(
+            .success(.init(messages: messages))
+        )
+        _ = controller.replies
+
+        controller.loadPreviousReplies(
+            limit: 21,
+            completion: nil
+        )
+
+        XCTAssertEqual(
+            env.messageUpdater.loadReplies_pagination?.parameter,
+            .lessThan("last message")
+        )
+    }
 
     func test_loadPreviousReplies_messageIdPassed_properlyHandlesPagination() {
-        // Simulate `loadNextReplies` call
         controller.loadPreviousReplies(
             before: "last message",
             limit: 21,