fix: Reactions flickering when adding one

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

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)

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