From 543136e254f0f08854039bfa5f381be87f20ab1a Mon Sep 17 00:00:00 2001 From: Pol Quintana Date: Tue, 21 Nov 2023 12:47:00 +0100 Subject: [PATCH 1/6] Fix Mark As Unread in BG by introducing breaking changes --- .../ChannelController/ChannelController.swift | 160 ++++++++++-------- .../Repositories/ChannelRepository.swift | 12 +- .../StreamChat/Workers/ChannelUpdater.swift | 2 +- .../ChatChannel/ChatChannelVC.swift | 33 ++-- 4 files changed, 117 insertions(+), 90 deletions(-) diff --git a/Sources/StreamChat/Controllers/ChannelController/ChannelController.swift b/Sources/StreamChat/Controllers/ChannelController/ChannelController.swift index d7b31463a47..00602624ae2 100644 --- a/Sources/StreamChat/Controllers/ChannelController/ChannelController.swift +++ b/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() + getFirstUnreadMessageId(for: channel) } /// 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 + channel?.lastReadMessageId(userId: client.currentUserId) } /// 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) -> 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 + } +} diff --git a/Sources/StreamChat/Repositories/ChannelRepository.swift b/Sources/StreamChat/Repositories/ChannelRepository.swift index f532febcf91..01924fb9acc 100644 --- a/Sources/StreamChat/Repositories/ChannelRepository.swift +++ b/Sources/StreamChat/Repositories/ChannelRepository.swift @@ -47,16 +47,17 @@ class ChannelRepository { userId: UserId, from messageId: MessageId, lastReadMessageId: MessageId?, - completion: ((Error?) -> Void)? = nil + completion: ((Result) -> 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())) + } }) } } diff --git a/Sources/StreamChat/Workers/ChannelUpdater.swift b/Sources/StreamChat/Workers/ChannelUpdater.swift index a0ee0542b1e..a960e93effe 100644 --- a/Sources/StreamChat/Workers/ChannelUpdater.swift +++ b/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) -> Void)? = nil ) { channelRepository.markUnread( for: cid, diff --git a/Sources/StreamChatUI/ChatChannel/ChatChannelVC.swift b/Sources/StreamChatUI/ChatChannel/ChatChannelVC.swift index ac1047dc327..28cb9bbf72a 100644 --- a/Sources/StreamChatUI/ChatChannel/ChatChannelVC.swift +++ b/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,32 @@ private extension ChatChannelVC { ) } - func updateAllUnreadMessagesRelatedComponents() { - updateScrollToBottomButtonCount() - updateJumpToUnreadRelatedComponents() - updateUnreadMessagesBannerRelatedComponents() + func updateAllUnreadMessagesRelatedComponents(channel: ChatChannel? = nil) { + let firstUnreadMessageId = channelController.getFirstUnreadMessageId(for: channel) + 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 = channelController.getFirstUnreadMessageId(for: channel) ?? channelController.firstUnreadMessageId + let lastReadMessageId = channel?.lastReadMessageId(userId: client.currentUserId) ?? 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 = channelController.getFirstUnreadMessageId(for: channel) ?? channelController.firstUnreadMessageId + self.firstUnreadMessageId = firstUnreadMessageId messageListVC.updateUnreadMessagesSeparator(at: firstUnreadMessageId) } } From 652e303644d531561cef64b62d2b48befc91f218 Mon Sep 17 00:00:00 2001 From: Pol Quintana Date: Wed, 22 Nov 2023 15:15:01 +0100 Subject: [PATCH 2/6] Fix existing tests --- .../Repositories/ChannelRepository_Mock.swift | 6 ++-- .../Workers/ChannelUpdater_Mock.swift | 4 +-- .../ChannelController_Tests.swift | 36 +++++++++---------- .../ChannelRepository_Tests.swift | 16 ++++++--- .../Workers/ChannelUpdater_Tests.swift | 10 +++--- 5 files changed, 39 insertions(+), 33 deletions(-) diff --git a/TestTools/StreamChatTestTools/Mocks/StreamChat/Repositories/ChannelRepository_Mock.swift b/TestTools/StreamChatTestTools/Mocks/StreamChat/Repositories/ChannelRepository_Mock.swift index 539e2db6d72..123e9968e08 100644 --- a/TestTools/StreamChatTestTools/Mocks/StreamChat/Repositories/ChannelRepository_Mock.swift +++ b/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? + var markUnreadResult: Result? 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) -> Void)? = nil) { markUnreadCid = cid markUnreadUserId = userId markUnreadMessageId = messageId markUnreadLastReadMessageId = lastReadMessageId markUnreadResult.map { - completion?($0.error) + completion?($0) } } } diff --git a/TestTools/StreamChatTestTools/Mocks/StreamChat/Workers/ChannelUpdater_Mock.swift b/TestTools/StreamChatTestTools/Mocks/StreamChat/Workers/ChannelUpdater_Mock.swift index 1ef73fb417e..c85e9fc6b2c 100644 --- a/TestTools/StreamChatTestTools/Mocks/StreamChat/Workers/ChannelUpdater_Mock.swift +++ b/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) -> 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) -> Void)? = nil) { markUnread_cid = cid markUnread_userId = userId markUnread_messageId = messageId diff --git a/Tests/StreamChatTests/Controllers/ChannelController/ChannelController_Tests.swift b/Tests/StreamChatTests/Controllers/ChannelController/ChannelController_Tests.swift index d3e162435f9..649ebb11219 100644 --- a/Tests/StreamChatTests/Controllers/ChannelController/ChannelController_Tests.swift +++ b/Tests/StreamChatTests/Controllers/ChannelController/ChannelController_Tests.swift @@ -440,7 +440,7 @@ final class ChannelController_Tests: XCTestCase { let token = Token(rawValue: "", userId: userId, expiration: nil) controller.client.authenticationRepository.setMockToken(token) - try createChannel(oldestMessageId: oldestMessageId, newestMessageId: newestMessageId, channelReads: [channelRead]) + createChannel(oldestMessageId: oldestMessageId, newestMessageId: newestMessageId, channelReads: [channelRead]) try client.databaseContainer.writeSynchronously { try $0.saveCurrentUser(payload: .dummy(userId: userId, role: .user)) @@ -3958,8 +3958,8 @@ final class ChannelController_Tests: XCTestCase { func test_markUnread_whenChannelDoesNotExist() { var receivedError: Error? let expectation = self.expectation(description: "Mark Unread completes") - controller.markUnread(from: .unique) { error in - receivedError = error + controller.markUnread(from: .unique) { result in + receivedError = result.error expectation.fulfill() } @@ -3979,8 +3979,8 @@ final class ChannelController_Tests: XCTestCase { var receivedError: Error? let expectation = self.expectation(description: "Mark Unread completes") - controller.markUnread(from: .unique) { error in - receivedError = error + controller.markUnread(from: .unique) { result in + receivedError = result.error expectation.fulfill() } @@ -4034,8 +4034,8 @@ final class ChannelController_Tests: XCTestCase { var receivedError: Error? let expectation = self.expectation(description: "Mark Unread completes") - controller.markUnread(from: .unique) { error in - receivedError = error + controller.markUnread(from: .unique) { result in + receivedError = result.error expectation.fulfill() } @@ -4055,8 +4055,8 @@ final class ChannelController_Tests: XCTestCase { var receivedError: Error? let expectation = self.expectation(description: "Mark Unread completes") - controller.markUnread(from: .unique) { error in - receivedError = error + controller.markUnread(from: .unique) { result in + receivedError = result.error expectation.fulfill() } @@ -4076,12 +4076,12 @@ final class ChannelController_Tests: XCTestCase { var receivedError: Error? let expectation = self.expectation(description: "Mark Unread completes") - controller.markUnread(from: .unique) { error in - receivedError = error + controller.markUnread(from: .unique) { result in + receivedError = result.error expectation.fulfill() } let mockedError = TestError() - env.channelUpdater?.markUnread_completion?(mockedError) + env.channelUpdater?.markUnread_completion?(.failure(mockedError)) waitForExpectations(timeout: defaultTimeout) @@ -4100,13 +4100,13 @@ final class ChannelController_Tests: XCTestCase { var receivedError: Error? let messageId = MessageId.unique let expectation = self.expectation(description: "Mark Unread completes") - controller.markUnread(from: messageId) { error in - receivedError = error + controller.markUnread(from: messageId) { result in + receivedError = result.error expectation.fulfill() } let updater = try XCTUnwrap(env.channelUpdater) - updater.markUnread_completion?(nil) + updater.markUnread_completion?(.success(ChatChannel.mock(cid: .unique))) waitForExpectations(timeout: defaultTimeout) @@ -4130,13 +4130,13 @@ final class ChannelController_Tests: XCTestCase { var receivedError: Error? let expectation = self.expectation(description: "Mark Unread completes") - controller.markUnread(from: messageId) { error in - receivedError = error + controller.markUnread(from: messageId) { result in + receivedError = result.error expectation.fulfill() } let updater = try XCTUnwrap(env.channelUpdater) - updater.markUnread_completion?(nil) + updater.markUnread_completion?(.success(ChatChannel.mock(cid: .unique))) waitForExpectations(timeout: defaultTimeout) diff --git a/Tests/StreamChatTests/Repositories/ChannelRepository_Tests.swift b/Tests/StreamChatTests/Repositories/ChannelRepository_Tests.swift index a46f3aa68e1..b4d0a44d61a 100644 --- a/Tests/StreamChatTests/Repositories/ChannelRepository_Tests.swift +++ b/Tests/StreamChatTests/Repositories/ChannelRepository_Tests.swift @@ -72,15 +72,21 @@ final class ChannelRepository_Tests: XCTestCase { // MARK: - Mark as unread - func test_markUnread_successfulResponse() { + func test_markUnread_successfulResponse() throws { let cid = ChannelId.unique let userId = UserId.unique let messageId = MessageId.unique + try database.createCurrentUser() + try database.writeSynchronously { + try $0.saveChannel(payload: .dummy(channel: .dummy(cid: cid))) + } + database.writeSessionCounter = 0 + let expectation = self.expectation(description: "markUnread completes") var receivedError: Error? - repository.markUnread(for: cid, userId: userId, from: messageId, lastReadMessageId: .unique) { error in - receivedError = error + repository.markUnread(for: cid, userId: userId, from: messageId, lastReadMessageId: .unique) { result in + receivedError = result.error expectation.fulfill() } @@ -100,8 +106,8 @@ final class ChannelRepository_Tests: XCTestCase { let expectation = self.expectation(description: "markUnread completes") var receivedError: Error? - repository.markUnread(for: cid, userId: userId, from: messageId, lastReadMessageId: .unique) { error in - receivedError = error + repository.markUnread(for: cid, userId: userId, from: messageId, lastReadMessageId: .unique) { result in + receivedError = result.error expectation.fulfill() } diff --git a/Tests/StreamChatTests/Workers/ChannelUpdater_Tests.swift b/Tests/StreamChatTests/Workers/ChannelUpdater_Tests.swift index e5609715731..13554772ed8 100644 --- a/Tests/StreamChatTests/Workers/ChannelUpdater_Tests.swift +++ b/Tests/StreamChatTests/Workers/ChannelUpdater_Tests.swift @@ -1472,9 +1472,9 @@ final class ChannelUpdater_Tests: XCTestCase { let expectation = self.expectation(description: "markUnread completes") var receivedError: Error? - channelRepository.markUnreadResult = .success(()) - channelUpdater.markUnread(cid: .unique, userId: .unique, from: .unique, lastReadMessageId: .unique) { error in - receivedError = error + channelRepository.markUnreadResult = .success(.mock(cid: .unique)) + channelUpdater.markUnread(cid: .unique, userId: .unique, from: .unique, lastReadMessageId: .unique) { result in + receivedError = result.error expectation.fulfill() } @@ -1488,8 +1488,8 @@ final class ChannelUpdater_Tests: XCTestCase { var receivedError: Error? channelRepository.markUnreadResult = .failure(mockedError) - channelUpdater.markUnread(cid: .unique, userId: .unique, from: .unique, lastReadMessageId: .unique) { error in - receivedError = error + channelUpdater.markUnread(cid: .unique, userId: .unique, from: .unique, lastReadMessageId: .unique) { result in + receivedError = result.error expectation.fulfill() } From dcae26ad4f727b4fd929e4e2dde34f51fa53549d Mon Sep 17 00:00:00 2001 From: Pol Quintana Date: Wed, 22 Nov 2023 16:03:39 +0100 Subject: [PATCH 3/6] Add tests for ChatChannel.lastReadMessageId --- .../ChannelController/ChannelController.swift | 4 +-- .../Models/ChatChannel_Tests.swift | 30 +++++++++++++++++++ 2 files changed, 32 insertions(+), 2 deletions(-) diff --git a/Sources/StreamChat/Controllers/ChannelController/ChannelController.swift b/Sources/StreamChat/Controllers/ChannelController/ChannelController.swift index 00602624ae2..57bae8d71d6 100644 --- a/Sources/StreamChat/Controllers/ChannelController/ChannelController.swift +++ b/Sources/StreamChat/Controllers/ChannelController/ChannelController.swift @@ -125,7 +125,7 @@ public class ChatChannelController: DataController, DelegateCallable, DataStoreP /// The id of the message which the current user last read. public var lastReadMessageId: MessageId? { - channel?.lastReadMessageId(userId: client.currentUserId) + client.currentUserId.flatMap { channel?.lastReadMessageId(userId: $0) } } /// A boolean indicating if the user marked the channel as unread in the current session @@ -1678,7 +1678,7 @@ public extension ChatChannelController { } public extension ChatChannel { - func lastReadMessageId(userId: UserId?) -> MessageId? { + func lastReadMessageId(userId: UserId) -> MessageId? { guard let currentUserRead = reads.first(where: { $0.user.id == userId }) else { diff --git a/Tests/StreamChatTests/Models/ChatChannel_Tests.swift b/Tests/StreamChatTests/Models/ChatChannel_Tests.swift index f3d3aff79f1..7efaa4d41e1 100644 --- a/Tests/StreamChatTests/Models/ChatChannel_Tests.swift +++ b/Tests/StreamChatTests/Models/ChatChannel_Tests.swift @@ -285,6 +285,36 @@ final class ChatChannel_Tests: XCTestCase { XCTAssertEqual(channelWithoutCapability.isSlowMode, false) } + func test_lastReadMessageId_readsDontContainUser() { + let userId: UserId = "current" + let channel = ChatChannel.mock(cid: .unique, reads: [ + .init(lastReadAt: Date(), lastReadMessageId: .unique, unreadMessagesCount: 3, user: .mock(id: "other")) + ]) + + XCTAssertNil(channel.lastReadMessageId(userId: userId)) + } + + func test_lastReadMessageId_userReadDoesNotHaveLastRead() { + let userId: UserId = "current" + let channel = ChatChannel.mock(cid: .unique, reads: [ + .init(lastReadAt: Date(), lastReadMessageId: nil, unreadMessagesCount: 3, user: .mock(id: userId)), + .init(lastReadAt: Date(), lastReadMessageId: .unique, unreadMessagesCount: 3, user: .mock(id: "other")) + ]) + + XCTAssertNil(channel.lastReadMessageId(userId: userId)) + } + + func test_lastReadMessageId_userReadHasLastRead() { + let userId: UserId = "current" + let lastReadId = MessageId.unique + let channel = ChatChannel.mock(cid: .unique, reads: [ + .init(lastReadAt: Date(), lastReadMessageId: lastReadId, unreadMessagesCount: 3, user: .mock(id: userId)), + .init(lastReadAt: Date(), lastReadMessageId: .unique, unreadMessagesCount: 3, user: .mock(id: "other")) + ]) + + XCTAssertEqual(channel.lastReadMessageId(userId: userId), lastReadId) + } + private func setupChannel(withCapabilities capabilities: Set) -> ChatChannel { .mock( cid: .unique, From c0228db3b8fa8b079d1f485d1eb9721a11cdd0b6 Mon Sep 17 00:00:00 2001 From: Pol Quintana Date: Wed, 22 Nov 2023 16:16:58 +0100 Subject: [PATCH 4/6] Fix compilation issue --- Sources/StreamChatUI/ChatChannel/ChatChannelVC.swift | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Sources/StreamChatUI/ChatChannel/ChatChannelVC.swift b/Sources/StreamChatUI/ChatChannel/ChatChannelVC.swift index 28cb9bbf72a..7efc9c0bc13 100644 --- a/Sources/StreamChatUI/ChatChannel/ChatChannelVC.swift +++ b/Sources/StreamChatUI/ChatChannel/ChatChannelVC.swift @@ -579,7 +579,6 @@ private extension ChatChannelVC { } func updateAllUnreadMessagesRelatedComponents(channel: ChatChannel? = nil) { - let firstUnreadMessageId = channelController.getFirstUnreadMessageId(for: channel) updateScrollToBottomButtonCount(channel: channel) updateJumpToUnreadRelatedComponents(channel: channel) updateUnreadMessagesBannerRelatedComponents(channel: channel) @@ -592,7 +591,7 @@ private extension ChatChannelVC { func updateJumpToUnreadRelatedComponents(channel: ChatChannel? = nil) { let firstUnreadMessageId = channelController.getFirstUnreadMessageId(for: channel) ?? channelController.firstUnreadMessageId - let lastReadMessageId = channel?.lastReadMessageId(userId: client.currentUserId) ?? channelController.lastReadMessageId + let lastReadMessageId = client.currentUserId.flatMap { channel?.lastReadMessageId(userId: $0) } ?? channelController.lastReadMessageId messageListVC.updateJumpToUnreadMessageId( firstUnreadMessageId, From 588edb448133c25dc3760d37ea65119a1cd0ab00 Mon Sep 17 00:00:00 2001 From: Pol Quintana Date: Wed, 22 Nov 2023 17:53:05 +0100 Subject: [PATCH 5/6] Make sure we don't fallback into oldestRegularMessageId if channel is nil --- .../Controllers/ChannelController/ChannelController.swift | 6 +++--- Sources/StreamChatUI/ChatChannel/ChatChannelVC.swift | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/Sources/StreamChat/Controllers/ChannelController/ChannelController.swift b/Sources/StreamChat/Controllers/ChannelController/ChannelController.swift index 57bae8d71d6..784c18745b1 100644 --- a/Sources/StreamChat/Controllers/ChannelController/ChannelController.swift +++ b/Sources/StreamChat/Controllers/ChannelController/ChannelController.swift @@ -120,7 +120,7 @@ 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(for: channel) + channel.flatMap { getFirstUnreadMessageId(for: $0) } } /// The id of the message which the current user last read. @@ -1217,7 +1217,7 @@ public class ChatChannelController: DataController, DelegateCallable, DataStoreP updater.deleteImage(in: cid, url: url, completion: completion) } - public func getFirstUnreadMessageId(for channel: ChatChannel?) -> MessageId? { + 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 { @@ -1226,7 +1226,7 @@ public class ChatChannelController: DataController, DelegateCallable, DataStoreP return self?.messages.last(where: { $0.type == .regular || $0.type == .reply })?.id } - guard let currentUserRead = channel?.reads.first(where: { + guard let currentUserRead = channel.reads.first(where: { $0.user.id == client.currentUserId }) else { return oldestRegularMessage() diff --git a/Sources/StreamChatUI/ChatChannel/ChatChannelVC.swift b/Sources/StreamChatUI/ChatChannel/ChatChannelVC.swift index 7efc9c0bc13..f284a23ae93 100644 --- a/Sources/StreamChatUI/ChatChannel/ChatChannelVC.swift +++ b/Sources/StreamChatUI/ChatChannel/ChatChannelVC.swift @@ -590,7 +590,7 @@ private extension ChatChannelVC { } func updateJumpToUnreadRelatedComponents(channel: ChatChannel? = nil) { - let firstUnreadMessageId = channelController.getFirstUnreadMessageId(for: channel) ?? channelController.firstUnreadMessageId + let firstUnreadMessageId = channel.flatMap { channelController.getFirstUnreadMessageId(for: $0) } ?? channelController.firstUnreadMessageId let lastReadMessageId = client.currentUserId.flatMap { channel?.lastReadMessageId(userId: $0) } ?? channelController.lastReadMessageId messageListVC.updateJumpToUnreadMessageId( @@ -601,7 +601,7 @@ private extension ChatChannelVC { } func updateUnreadMessagesBannerRelatedComponents(channel: ChatChannel? = nil) { - let firstUnreadMessageId = channelController.getFirstUnreadMessageId(for: channel) ?? channelController.firstUnreadMessageId + let firstUnreadMessageId = channel.flatMap { channelController.getFirstUnreadMessageId(for: $0) } ?? channelController.firstUnreadMessageId self.firstUnreadMessageId = firstUnreadMessageId messageListVC.updateUnreadMessagesSeparator(at: firstUnreadMessageId) } From e9c1ef1f5512a990fc2198309ebe44296d97effc Mon Sep 17 00:00:00 2001 From: Pol Quintana Date: Thu, 23 Nov 2023 16:21:14 +0100 Subject: [PATCH 6/6] Update CHANGELOG --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index d4445a5eb18..68abb659c5c 100644 --- a/CHANGELOG.md +++ b/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` instead of `Error?` + # [4.43.0](https://github.com/GetStream/stream-chat-swift/releases/tag/4.43.0) _November 17, 2023_