diff --git a/NextcloudTalk/Chat/BaseChatViewController.swift b/NextcloudTalk/Chat/BaseChatViewController.swift index eb1b22675..5dc61b27c 100644 --- a/NextcloudTalk/Chat/BaseChatViewController.swift +++ b/NextcloudTalk/Chat/BaseChatViewController.swift @@ -684,6 +684,7 @@ import Toast let isAtBottom = self.shouldScrollOnNewMessages() let keyDate = self.dateSections[indexPath.section] updatedMessage.isGroupMessage = message.isGroupMessage && message.actorType != "bots" && updatedMessage.lastEditTimestamp == 0 + updatedMessage.copyPendingReactions(from: message) self.messages[keyDate]?[indexPath.row] = updatedMessage // Check if there are any messages that reference our message as a parent -> these need to be reloaded as well @@ -3138,6 +3139,16 @@ import Toast guard let (indexPath, message) = self.indexPathAndMessage(forMessageId: message.messageId) else { return } + // .added and .removed only confirm a pending reaction, there is nothing to draw or to + // create if the server state already landed (e.g. chat relay reaction system message) + if state == .added || state == .removed { + if message.hasTemporaryReaction(reaction) { + message.setOrUpdateTemporaryReaction(reaction, state: state) + } + + return + } + message.setOrUpdateTemporaryReaction(reaction, state: state) CATransaction.begin() diff --git a/NextcloudTalk/Chat/NCChatMessage.swift b/NextcloudTalk/Chat/NCChatMessage.swift index c5a419c13..ee06f049e 100644 --- a/NextcloudTalk/Chat/NCChatMessage.swift +++ b/NextcloudTalk/Chat/NCChatMessage.swift @@ -288,6 +288,29 @@ import SwiftyAttributes } } + /// Copies the reactions of `message` the server has not confirmed yet. A message built from the + /// database has none of its own, so without this our reaction disappears until the server sends it. + public func copyPendingReactions(from message: NCChatMessage) { + let ownReactions = self.reactionsSelfArray() + + let pendingReactions = message.temporaryReactions().compactMap({ $0 as? NCChatReaction }).filter { reaction in + switch reaction.state { + case .adding, .added: + return !ownReactions.contains(reaction.reaction) + case .removing, .removed: + return ownReactions.contains(reaction.reaction) + default: + return false + } + } + + self.temporaryReactions().addObjects(from: pendingReactions) + } + + public func hasTemporaryReaction(_ reaction: String) -> Bool { + return temporaryReactions().compactMap({ $0 as? NCChatReaction }).contains { $0.reaction == reaction } + } + public func setOrUpdateTemporaryReaction(_ reaction: String, state: NCChatReactionState) { if let updateReaction = temporaryReactions().compactMap({ $0 as? NCChatReaction }).first(where: { $0.reaction == reaction }) { updateReaction.reaction = reaction @@ -513,9 +536,6 @@ import SwiftyAttributes managedChatMessage.systemMessage = chatMessage.systemMessage managedChatMessage.isReplyable = chatMessage.isReplyable managedChatMessage.messageType = chatMessage.messageType - // Reactions we already know keep their position, new ones are appended - managedChatMessage.reactionsJSONString = NCChatMessage.reactionsJSONString(for: chatMessage.storedReactions(), - keepingOrderOf: managedChatMessage.storedReactions()) managedChatMessage.expirationTimestamp = chatMessage.expirationTimestamp managedChatMessage.isMarkdownMessage = chatMessage.isMarkdownMessage managedChatMessage.lastEditActorId = chatMessage.lastEditActorId @@ -529,6 +549,10 @@ import SwiftyAttributes managedChatMessage.pinnedAt = chatMessage.pinnedAt if !isRoomLastMessage { + // Reactions we already know keep their position, new ones are appended. Both fields are + // written together, the room's last message has counts but never our own reactions. + managedChatMessage.reactionsJSONString = NCChatMessage.reactionsJSONString(for: chatMessage.storedReactions(), + keepingOrderOf: managedChatMessage.storedReactions()) managedChatMessage.reactionsSelfJSONString = chatMessage.reactionsSelfJSONString // Only update the thread data if there is any data (e.g. omit chat relay messages without thread data) diff --git a/NextcloudTalkTests/Unit/Chat/UnitNCChatMessageTest.swift b/NextcloudTalkTests/Unit/Chat/UnitNCChatMessageTest.swift index c9b841ce0..91828b7d7 100644 --- a/NextcloudTalkTests/Unit/Chat/UnitNCChatMessageTest.swift +++ b/NextcloudTalkTests/Unit/Chat/UnitNCChatMessageTest.swift @@ -67,6 +67,70 @@ final class UnitNCChatMessageTest: TestBaseRealm { XCTAssertEqual(reactionOrder([["😀": 1, "👍": 1, "❤️": 1]]), ["❤️", "👍", "😀"]) } + /// A message as it comes back from the database + private func storedMessage(reactions: [String: Int], ownReactions: [String]) -> NCChatMessage { + let message = NCChatMessage() + let pairs = reactions.map { [$0.key, $0.value] as [Any] } + message.reactionsJSONString = String(data: try! JSONSerialization.data(withJSONObject: pairs), encoding: .utf8) + message.reactionsSelfJSONString = String(data: try! JSONSerialization.data(withJSONObject: ownReactions), encoding: .utf8) + + return message + } + + // An update in between replaces the message with the stored one, which doesn't know the reaction yet + func testReactionAddedButNotEchoedBackYetIsKept() throws { + let shownMessage = NCChatMessage() + shownMessage.setOrUpdateTemporaryReaction("👍", state: .adding) + + let updatedMessage = storedMessage(reactions: [:], ownReactions: []) + updatedMessage.copyPendingReactions(from: shownMessage) + + XCTAssertEqual(updatedMessage.reactionsArray().map { $0.reaction }, ["👍"], + "A reaction the server has not echoed back yet must stay on the message") + XCTAssertTrue(updatedMessage.reactionsArray().first?.userReacted == true) + } + + func testReactionRemovedButNotEchoedBackYetStaysRemoved() throws { + let shownMessage = NCChatMessage() + shownMessage.setOrUpdateTemporaryReaction("👍", state: .removing) + + // The server still has our reaction + let updatedMessage = storedMessage(reactions: ["👍": 1], ownReactions: ["👍"]) + updatedMessage.copyPendingReactions(from: shownMessage) + + XCTAssertTrue(updatedMessage.reactionsArray().isEmpty, + "A reaction we removed must not come back until the server says it is still there") + } + + // Once confirmed the message carries the reaction itself, keeping it too would count it twice + func testEchoedBackReactionIsNoLongerKept() throws { + let shownMessage = NCChatMessage() + shownMessage.setOrUpdateTemporaryReaction("👍", state: .added) + + let updatedMessage = storedMessage(reactions: ["👍": 2], ownReactions: ["👍"]) + updatedMessage.copyPendingReactions(from: shownMessage) + + XCTAssertEqual(updatedMessage.temporaryReactions().count, 0) + XCTAssertEqual(updatedMessage.reactionsArray().first?.count, 2, "The reaction must not be counted twice") + } + + // Taking only the counts would leave our reaction unmarked, and counted again as a temporary one + func testRoomLastMessageUpdateDoesNotDesyncReactionsFromOurOwn() throws { + // We reacted, and are waiting for the server to confirm it + let shownMessage = NCChatMessage() + shownMessage.setOrUpdateTemporaryReaction("👍", state: .added) + + // A room update arrives first: it counts our reaction, but carries no reactionsSelf + let message = storedMessage(reactions: [:], ownReactions: []) + let roomLastMessage = storedMessage(reactions: ["👍": 1], ownReactions: []) + NCChatMessage.update(message, with: roomLastMessage, isRoomLastMessage: true) + + message.copyPendingReactions(from: shownMessage) + + XCTAssertEqual(message.reactionsArray().first?.count, 1, + "Our reaction must not be counted both by the room update and as a temporary one") + } + // Messages stored before reactions were kept in order hold a JSON object instead of pairs func testReactionsStoredInTheOldFormatAreStillRead() throws { let message = NCChatMessage()