From e4b7217fc36c517405886210db5b2447c4ab101e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alexandre=20K=C3=BChn?= <aku@odoo.com>
Date: Thu, 28 Sep 2023 13:22:39 +0200
Subject: [PATCH] [FIX] mail: remove message reaction was not working
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Follow-up PR of https://github.com/odoo/odoo/pull/136308

Commit above simplified code in discuss models so code applies
on records than properties on records. For example, instead of
juggling between persona local id and persona, some code can simply
keep logic on persona without leaking local id.

However the improvements are iterative, and more improvements are
needed to stop using local id. Some code still need local ids.
Commit above mistakenly converts a local id to persona on some code
that still work in terms of local id. As a result, message reactions
were not working properly, notably when adding a new reaction using
click on an existing message reaction of someone else.

There was also a bug in implementation in mock server which made the
test not working. This commit fixes this issue too.

Task-3523101

closes odoo/odoo#136940

Signed-off-by: Alexandre Kühn (aku) <aku@odoo.com>
---
 .../static/src/core/common/message_reactions_model.js    | 9 +++++----
 .../tests/helpers/mock_server/models/mail_message.js     | 5 ++++-
 addons/mail/static/tests/message/message_tests.js        | 2 ++
 3 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/addons/mail/static/src/core/common/message_reactions_model.js b/addons/mail/static/src/core/common/message_reactions_model.js
index 4110fc7331e..7ab774dd964 100644
--- a/addons/mail/static/src/core/common/message_reactions_model.js
+++ b/addons/mail/static/src/core/common/message_reactions_model.js
@@ -13,9 +13,10 @@ export class MessageReactions extends Record {
      * @returns {import("models").MessageReactions}
      */
     static insert(data) {
-        let reaction = this.store.Message.get(data.message.id)?.reactions.find(
-            ({ content }) => content === data.content
-        );
+        if (data.message && !(data.message instanceof Record)) {
+            data.message = this.store.Message.insert(data.message);
+        }
+        let reaction = data.message.reactions.find(({ content }) => content === data.content);
         if (!reaction) {
             /** @type {import("models").MessageReactions} */
             reaction = this.preinsert(data);
@@ -46,7 +47,7 @@ export class MessageReactions extends Record {
             count: data.count,
             content: data.content,
             message: data.message,
-            personas: reaction.personas.filter((p) => !personasToUnlink.has(p)),
+            personas: reaction.personas.filter((p) => !personasToUnlink.has(p.localId)),
         });
         return reaction;
     }
diff --git a/addons/mail/static/tests/helpers/mock_server/models/mail_message.js b/addons/mail/static/tests/helpers/mock_server/models/mail_message.js
index 11bf3b576ad..65eb1a299ec 100644
--- a/addons/mail/static/tests/helpers/mock_server/models/mail_message.js
+++ b/addons/mail/static/tests/helpers/mock_server/models/mail_message.js
@@ -78,7 +78,10 @@ patch(MockServer.prototype, {
                         guests: [],
                         message: { id: messageId },
                         partners: [
-                            ["ADD", { id: this.pyEnv.currentPartnerId, name: currentPartner.name }],
+                            [
+                                action === "add" ? "ADD" : "DELETE",
+                                { id: this.pyEnv.currentPartnerId, name: currentPartner.name },
+                            ],
                         ],
                     },
                 ],
diff --git a/addons/mail/static/tests/message/message_tests.js b/addons/mail/static/tests/message/message_tests.js
index e9ec4f7f4ba..17d1c32e481 100644
--- a/addons/mail/static/tests/message/message_tests.js
+++ b/addons/mail/static/tests/message/message_tests.js
@@ -531,6 +531,8 @@ QUnit.test("Two users reacting with the same emoji", async () => {
     await contains(".o-mail-MessageReaction", { text: "😅2" });
     await click(".o-mail-MessageReaction");
     await contains(".o-mail-MessageReaction", { text: "😅1" });
+    await click(".o-mail-MessageReaction");
+    await contains(".o-mail-MessageReaction", { text: "😅2" });
 });
 
 QUnit.test("Reaction summary", async () => {