From 13f4d046f7af9faf4f737308eb612e5eaf89132b Mon Sep 17 00:00:00 2001
From: tsm-odoo <tsm@odoo.com>
Date: Thu, 18 Jan 2024 17:04:55 +0100
Subject: [PATCH] [FIX] mail: update html fields according to html insert
 option

Since [1], the `insert` method accepts the `html` option to
automatically cast record fields marked as `html` to markup. Before
this PR, this option was ignored after the first insertion if the
field value remained the same.

Steps to reproduce this issue:
- Insert a message without passing the `html` option: the body field is
an instance of string.
- Insert the same message setting the `html` option to true: the body
field of the message is still an instance of string while it should be
an instance of markup.

This PR fixes this issue.

[1]: https://github.com/odoo/odoo/pull/139501

closes odoo/odoo#149953

Signed-off-by: Matthieu Stockbauer (tsm) <tsm@odoo.com>
---
 addons/mail/static/src/core/common/record.js  | 29 ++++++++-----------
 addons/mail/static/tests/core/record_tests.js | 16 ++++++++++
 2 files changed, 28 insertions(+), 17 deletions(-)

diff --git a/addons/mail/static/src/core/common/record.js b/addons/mail/static/src/core/common/record.js
index 225c833ce6a..bbff9ae8df9 100644
--- a/addons/mail/static/src/core/common/record.js
+++ b/addons/mail/static/src/core/common/record.js
@@ -57,23 +57,18 @@ function updateAttr(record, fieldName, value) {
     const fieldDefinition = record.Model._fields.get(fieldName);
     // ensure each field write goes through the proxy exactly once to trigger reactives
     const targetRecord = record._proxyUsed.has(fieldName) ? record : record._proxy;
-    if (
-        fieldDefinition?.html &&
-        Record.trusted &&
-        typeof value === "string" &&
-        !(value instanceof Markup)
-    ) {
-        if (record[fieldName]?.toString() !== value) {
-            record._updateFields.add(fieldName);
-            targetRecord[fieldName] = markup(value);
-            record._updateFields.delete(fieldName);
-        }
-    } else {
-        if (record[fieldName] !== value) {
-            record._updateFields.add(fieldName);
-            targetRecord[fieldName] = value;
-            record._updateFields.delete(fieldName);
-        }
+    let shouldChange = record[fieldName] !== value;
+    let newValue = value;
+    if (fieldDefinition?.html && Record.trusted) {
+        shouldChange =
+            record[fieldName]?.toString() !== value?.toString() ||
+            !(record[fieldName] instanceof Markup);
+        newValue = typeof value === "string" ? markup(value) : value;
+    }
+    if (shouldChange) {
+        record._updateFields.add(fieldName);
+        targetRecord[fieldName] = newValue;
+        record._updateFields.delete(fieldName);
     }
 }
 
diff --git a/addons/mail/static/tests/core/record_tests.js b/addons/mail/static/tests/core/record_tests.js
index d93cd24afd7..e8f97ebedae 100644
--- a/addons/mail/static/tests/core/record_tests.js
+++ b/addons/mail/static/tests/core/record_tests.js
@@ -260,6 +260,22 @@ QUnit.test("Trusted insert on html field with { html: true }", async (assert) =>
     assert.strictEqual(world.body, "<p>world</p>");
 });
 
+QUnit.test("(Un)trusted insertion is applied even with same field value", async (assert) => {
+    (class Message extends Record {
+        static id = "id";
+        id;
+        body = Record.attr("", { html: true });
+    }).register();
+    const store = await start();
+    const rawMessage = { id: 1, body: "<p>hello</p>" };
+    let message = store.Message.insert(rawMessage);
+    assert.notOk(message.body instanceof markup("").constructor);
+    message = store.Message.insert(rawMessage, { html: true });
+    assert.ok(message.body instanceof markup("").constructor);
+    message = store.Message.insert(rawMessage);
+    assert.notOk(message.body instanceof markup("").constructor);
+});
+
 QUnit.test("Unshift preserves order", async (assert) => {
     (class Message extends Record {
         static id = "id";