From b85bde9854072a94a99156ef18d59e6183e2d41c Mon Sep 17 00:00:00 2001
From: Xavier Dubuc <xdu@odoo.com>
Date: Wed, 3 Mar 2021 16:41:09 +0000
Subject: [PATCH] [FIX] mail: do not show falsy value in tracking values
 messages
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

task-2438828

closes odoo/odoo#67227

X-original-commit: 49585b8ee726af7edd113c3012b1168d24dc4d7c
Signed-off-by: Alexandre Kühn (aku) <aku@odoo.com>
---
 .../static/src/components/message/message.js  | 103 ++++--
 .../static/src/components/message/message.xml |   8 +-
 .../src/components/message/message_tests.js   | 340 +++++++++++++++++-
 .../mail/static/tests/helpers/mock_server.js  |   4 +
 4 files changed, 403 insertions(+), 52 deletions(-)

diff --git a/addons/mail/static/src/components/message/message.js b/addons/mail/static/src/components/message/message.js
index 52333f2cd5b..3f5314eef13 100644
--- a/addons/mail/static/src/components/message/message.js
+++ b/addons/mail/static/src/components/message/message.js
@@ -286,39 +286,76 @@ class Message extends Component {
         return this.message.tracking_value_ids.map(trackingValue => {
             const value = Object.assign({}, trackingValue);
             value.changed_field = _.str.sprintf(this.env._t("%s:"), value.changed_field);
-            if (value.field_type === 'datetime') {
-                if (value.old_value) {
-                    value.old_value =
-                        moment.utc(value.old_value).local().format('LLL');
-                }
-                if (value.new_value) {
-                    value.new_value =
-                        moment.utc(value.new_value).local().format('LLL');
-                }
-            } else if (value.field_type === 'date') {
-                if (value.old_value) {
-                    value.old_value =
-                        moment(value.old_value).local().format('LL');
-                }
-                if (value.new_value) {
-                    value.new_value =
-                        moment(value.new_value).local().format('LL');
-                }
-            } else if (value.field_type === 'boolean') {
-                if (value.old_value !== undefined){
-                    value.old_value = value.old_value ? this.env._t("True") : this.env._t("False");
-                }
-                if (value.new_value !== undefined){
-                    value.new_value = value.new_value ? this.env._t("True") : this.env._t("False");
-                }
-            } else if (value.field_type === 'monetary' && value.currency_id) {
-                const currency_id = this.env.session.currencies[value.currency_id]
-                if (value.old_value !== undefined) {
-                    value.old_value = format.monetary(value.old_value, null, { currency: currency_id, forceString: true });
-                }
-                if (value.new_value !== undefined) {
-                    value.new_value = format.monetary(value.new_value, null, { currency: currency_id, forceString: true });
-                }
+            /**
+             * Maps tracked field type to a JS formatter. Tracking values are
+             * not always stored in the same field type as their origin type.
+             * Field types that are not listed here are not supported by
+             * tracking in Python. Also see `create_tracking_values` in Python.
+             */
+            switch (value.field_type) {
+                case 'boolean':
+                    value.old_value = format.boolean(value.old_value, undefined, { forceString: true });
+                    value.new_value = format.boolean(value.new_value, undefined, { forceString: true });
+                    break;
+                /**
+                 * many2one formatter exists but is expecting id/name_get or data
+                 * object but only the target record name is known in this context.
+                 *
+                 * Selection formatter exists but requires knowing all
+                 * possibilities and they are not given in this context.
+                 */
+                case 'char':
+                case 'many2one':
+                case 'selection':
+                    value.old_value = format.char(value.old_value);
+                    value.new_value = format.char(value.new_value);
+                    break;
+                case 'date':
+                    if (value.old_value) {
+                        value.old_value = moment.utc(value.old_value);
+                    }
+                    if (value.new_value) {
+                        value.new_value = moment.utc(value.new_value);
+                    }
+                    value.old_value = format.date(value.old_value);
+                    value.new_value = format.date(value.new_value);
+                    break;
+                case 'datetime':
+                    if (value.old_value) {
+                        value.old_value = moment.utc(value.old_value);
+                    }
+                    if (value.new_value) {
+                        value.new_value = moment.utc(value.new_value);
+                    }
+                    value.old_value = format.datetime(value.old_value);
+                    value.new_value = format.datetime(value.new_value);
+                    break;
+                case 'float':
+                    value.old_value = format.float(value.old_value);
+                    value.new_value = format.float(value.new_value);
+                    break;
+                case 'integer':
+                    value.old_value = format.integer(value.old_value);
+                    value.new_value = format.integer(value.new_value);
+                    break;
+                case 'monetary':
+                    value.old_value = format.monetary(value.old_value, undefined, {
+                        currency: value.currency_id
+                            ? this.env.session.currencies[value.currency_id]
+                            : undefined,
+                        forceString: true,
+                    });
+                    value.new_value = format.monetary(value.new_value, undefined, {
+                        currency: value.currency_id
+                            ? this.env.session.currencies[value.currency_id]
+                            : undefined,
+                        forceString: true,
+                    });
+                    break;
+                case 'text':
+                    value.old_value = format.text(value.old_value);
+                    value.new_value = format.text(value.new_value);
+                    break;
             }
             return value;
         });
diff --git a/addons/mail/static/src/components/message/message.xml b/addons/mail/static/src/components/message/message.xml
index fb263ea6bb8..ffc376ee1e7 100644
--- a/addons/mail/static/src/components/message/message.xml
+++ b/addons/mail/static/src/components/message/message.xml
@@ -167,9 +167,11 @@
                                     <li>
                                         <div class="o_Message_trackingValue">
                                             <div class="o_Message_trackingValueFieldName o_Message_trackingValueItem" t-esc="value.changed_field"/>
-                                            <div class="o_Message_trackingValueOldValue o_Message_trackingValueItem" t-esc="value.old_value"/>
-                                            <t t-if="value.old_value !== value.new_value">
-                                                <div class="o_Message_trackingValueSeparator o_Message_trackingValueItem fa fa-long-arrow-right" title="Changed" role="img"/>
+                                            <t t-if="value.old_value">
+                                                <div class="o_Message_trackingValueOldValue o_Message_trackingValueItem" t-esc="value.old_value"/>
+                                            </t>
+                                            <div class="o_Message_trackingValueSeparator o_Message_trackingValueItem fa fa-long-arrow-right" title="Changed" role="img"/>
+                                            <t t-if="value.new_value">
                                                 <div class="o_Message_trackingValueNewValue o_Message_trackingValueItem" t-esc="value.new_value"/>
                                             </t>
                                         </div>
diff --git a/addons/mail/static/src/components/message/message_tests.js b/addons/mail/static/src/components/message/message_tests.js
index 99f7bf24aae..74b4c6ae09a 100644
--- a/addons/mail/static/src/components/message/message_tests.js
+++ b/addons/mail/static/src/components/message/message_tests.js
@@ -1060,8 +1060,8 @@ QUnit.test('basic rendering of tracking value (float type)', async function (ass
     );
     assert.strictEqual(
         document.querySelector('.o_Message_trackingValueOldValue').textContent,
-        "12.3",
-        "should display the correct old value (12.3)",
+        "12.30",
+        "should display the correct old value (12.30)",
     );
     assert.containsOnce(
         document.body,
@@ -1080,7 +1080,51 @@ QUnit.test('basic rendering of tracking value (float type)', async function (ass
     );
 });
 
-QUnit.test('rendering of tracked field with change of value from non-0 to 0', async function (assert) {
+QUnit.test('rendering of tracked field of type integer: from non-0 to 0', async function (assert) {
+    assert.expect(1);
+
+    await this.start();
+    const message = this.env.models['mail.message'].create({
+        id: 11,
+        tracking_value_ids: [{
+            changed_field: "Total",
+            field_type: "integer",
+            id: 6,
+            new_value: 0,
+            old_value: 1,
+        }],
+    });
+    await this.createMessageComponent(message);
+    assert.strictEqual(
+        document.querySelector('.o_Message_trackingValue').textContent,
+        "Total:10",
+        "should display the correct content of tracked field of type integer: from non-0 to 0 (Total: 1 -> 0)"
+    );
+});
+
+QUnit.test('rendering of tracked field of type integer: from 0 to non-0', async function (assert) {
+    assert.expect(1);
+
+    await this.start();
+    const message = this.env.models['mail.message'].create({
+        id: 11,
+        tracking_value_ids: [{
+            changed_field: "Total",
+            field_type: "integer",
+            id: 6,
+            new_value: 1,
+            old_value: 0,
+        }],
+    });
+    await this.createMessageComponent(message);
+    assert.strictEqual(
+        document.querySelector('.o_Message_trackingValue').textContent,
+        "Total:01",
+        "should display the correct content of tracked field of type integer: from 0 to non-0 (Total: 0 -> 1)"
+    );
+});
+
+QUnit.test('rendering of tracked field of type float: from non-0 to 0', async function (assert) {
     assert.expect(1);
 
     await this.start();
@@ -1097,12 +1141,12 @@ QUnit.test('rendering of tracked field with change of value from non-0 to 0', as
     await this.createMessageComponent(message);
     assert.strictEqual(
         document.querySelector('.o_Message_trackingValue').textContent,
-        "Total:10",
-        "should display the correct content of tracked field with change of value from non-0 to 0 (Total: 1 -> 0)"
+        "Total:1.000.00",
+        "should display the correct content of tracked field of type float: from non-0 to 0 (Total: 1.00 -> 0.00)"
     );
 });
 
-QUnit.test('rendering of tracked field with change of value from 0 to non-0', async function (assert) {
+QUnit.test('rendering of tracked field of type float: from 0 to non-0', async function (assert) {
     assert.expect(1);
 
     await this.start();
@@ -1119,12 +1163,56 @@ QUnit.test('rendering of tracked field with change of value from 0 to non-0', as
     await this.createMessageComponent(message);
     assert.strictEqual(
         document.querySelector('.o_Message_trackingValue').textContent,
-        "Total:01",
-        "should display the correct content of tracked field with change of value from 0 to non-0 (Total: 0 -> 1)"
+        "Total:0.001.00",
+        "should display the correct content of tracked field of type float: from 0 to non-0 (Total: 0.00 -> 1.00)"
     );
 });
 
-QUnit.test('rendering of tracked field with change of value from true to false', async function (assert) {
+QUnit.test('rendering of tracked field of type monetary: from non-0 to 0', async function (assert) {
+    assert.expect(1);
+
+    await this.start();
+    const message = this.env.models['mail.message'].create({
+        id: 11,
+        tracking_value_ids: [{
+            changed_field: "Total",
+            field_type: "monetary",
+            id: 6,
+            new_value: 0,
+            old_value: 1,
+        }],
+    });
+    await this.createMessageComponent(message);
+    assert.strictEqual(
+        document.querySelector('.o_Message_trackingValue').textContent,
+        "Total:1.000.00",
+        "should display the correct content of tracked field of type monetary: from non-0 to 0 (Total: 1.00 -> 0.00)"
+    );
+});
+
+QUnit.test('rendering of tracked field of type monetary: from 0 to non-0', async function (assert) {
+    assert.expect(1);
+
+    await this.start();
+    const message = this.env.models['mail.message'].create({
+        id: 11,
+        tracking_value_ids: [{
+            changed_field: "Total",
+            field_type: "monetary",
+            id: 6,
+            new_value: 1,
+            old_value: 0,
+        }],
+    });
+    await this.createMessageComponent(message);
+    assert.strictEqual(
+        document.querySelector('.o_Message_trackingValue').textContent,
+        "Total:0.001.00",
+        "should display the correct content of tracked field of type monetary: from 0 to non-0 (Total: 0.00 -> 1.00)"
+    );
+});
+
+QUnit.test('rendering of tracked field of type boolean: from true to false', async function (assert) {
     assert.expect(1);
 
     await this.start();
@@ -1142,11 +1230,11 @@ QUnit.test('rendering of tracked field with change of value from true to false',
     assert.strictEqual(
         document.querySelector('.o_Message_trackingValue').textContent,
         "Is Ready:TrueFalse",
-        "should display the correct content of tracked field with change of value from true to false (Is Ready: true -> false)"
+        "should display the correct content of tracked field of type boolean: from true to false (Is Ready: True -> False)"
     );
 });
 
-QUnit.test('rendering of tracked field with change of value from false to true', async function (assert) {
+QUnit.test('rendering of tracked field of type boolean: from false to true', async function (assert) {
     assert.expect(1);
 
     await this.start();
@@ -1164,11 +1252,11 @@ QUnit.test('rendering of tracked field with change of value from false to true',
     assert.strictEqual(
         document.querySelector('.o_Message_trackingValue').textContent,
         "Is Ready:FalseTrue",
-        "should display the correct content of tracked field with change of value from false to true (Is Ready: false -> true)"
+        "should display the correct content of tracked field of type boolean: from false to true (Is Ready: False -> True)"
     );
 });
 
-QUnit.test('rendering of tracked field with change of value from string to empty', async function (assert) {
+QUnit.test('rendering of tracked field of type char: from a string to empty string', async function (assert) {
     assert.expect(1);
 
     await this.start();
@@ -1186,11 +1274,11 @@ QUnit.test('rendering of tracked field with change of value from string to empty
     assert.strictEqual(
         document.querySelector('.o_Message_trackingValue').textContent,
         "Name:Marc",
-        "should display the correct content of tracked field with change of value from string to empty (Total: Marc ->)"
+        "should display the correct content of tracked field of type char: from a string to empty string (Name: Marc ->)"
     );
 });
 
-QUnit.test('rendering of tracked field with change of value from empty to string', async function (assert) {
+QUnit.test('rendering of tracked field of type char: from empty string to a string', async function (assert) {
     assert.expect(1);
 
     await this.start();
@@ -1208,7 +1296,227 @@ QUnit.test('rendering of tracked field with change of value from empty to string
     assert.strictEqual(
         document.querySelector('.o_Message_trackingValue').textContent,
         "Name:Marc",
-        "should display the correct content of tracked field with change of value from empty to string (Total: -> Marc)"
+        "should display the correct content of tracked field of type char: from empty string to a string (Name: -> Marc)"
+    );
+});
+
+QUnit.test('rendering of tracked field of type date: from no date to a set date', async function (assert) {
+    assert.expect(1);
+
+    await this.start();
+    const message = this.env.models['mail.message'].create({
+        id: 11,
+        tracking_value_ids: [{
+            changed_field: "Deadline",
+            field_type: "date",
+            id: 6,
+            new_value: "2018-12-14",
+            old_value: false,
+        }],
+    });
+    await this.createMessageComponent(message);
+    assert.strictEqual(
+        document.querySelector('.o_Message_trackingValue').textContent,
+        "Deadline:12/14/2018",
+        "should display the correct content of tracked field of type date: from no date to a set date (Deadline: -> 12/14/2018)"
+    );
+});
+
+QUnit.test('rendering of tracked field of type date: from a set date to no date', async function (assert) {
+    assert.expect(1);
+
+    await this.start();
+    const message = this.env.models['mail.message'].create({
+        id: 11,
+        tracking_value_ids: [{
+            changed_field: "Deadline",
+            field_type: "date",
+            id: 6,
+            new_value: false,
+            old_value: "2018-12-14",
+        }],
+    });
+    await this.createMessageComponent(message);
+    assert.strictEqual(
+        document.querySelector('.o_Message_trackingValue').textContent,
+        "Deadline:12/14/2018",
+        "should display the correct content of tracked field of type date: from a set date to no date (Deadline: 12/14/2018 ->)"
+    );
+});
+
+QUnit.test('rendering of tracked field of type datetime: from no date and time to a set date and time', async function (assert) {
+    assert.expect(1);
+
+    await this.start();
+    const message = this.env.models['mail.message'].create({
+        id: 11,
+        tracking_value_ids: [{
+            changed_field: "Deadline",
+            field_type: "datetime",
+            id: 6,
+            new_value: "2018-12-14 13:42:28",
+            old_value: false,
+        }],
+    });
+    await this.createMessageComponent(message);
+    assert.strictEqual(
+        document.querySelector('.o_Message_trackingValue').textContent,
+        "Deadline:12/14/2018 13:42:28",
+        "should display the correct content of tracked field of type datetime: from no date and time to a set date and time (Deadline: -> 12/14/2018 13:42:28)"
+    );
+});
+
+QUnit.test('rendering of tracked field of type datetime: from a set date and time to no date and time', async function (assert) {
+    assert.expect(1);
+
+    await this.start();
+    const message = this.env.models['mail.message'].create({
+        id: 11,
+        tracking_value_ids: [{
+            changed_field: "Deadline",
+            field_type: "datetime",
+            id: 6,
+            new_value: false,
+            old_value: "2018-12-14 13:42:28",
+        }],
+    });
+    await this.createMessageComponent(message);
+    assert.strictEqual(
+        document.querySelector('.o_Message_trackingValue').textContent,
+        "Deadline:12/14/2018 13:42:28",
+        "should display the correct content of tracked field of type datetime: from a set date and time to no date and time (Deadline: 12/14/2018 13:42:28 ->)"
+    );
+});
+
+QUnit.test('rendering of tracked field of type text: from some text to empty', async function (assert) {
+    assert.expect(1);
+
+    await this.start();
+    const message = this.env.models['mail.message'].create({
+        id: 11,
+        tracking_value_ids: [{
+            changed_field: "Name",
+            field_type: "text",
+            id: 6,
+            new_value: "",
+            old_value: "Marc",
+        }],
+    });
+    await this.createMessageComponent(message);
+    assert.strictEqual(
+        document.querySelector('.o_Message_trackingValue').textContent,
+        "Name:Marc",
+        "should display the correct content of tracked field of type text: from some text to empty (Name: Marc ->)"
+    );
+});
+
+QUnit.test('rendering of tracked field of type text: from empty to some text', async function (assert) {
+    assert.expect(1);
+
+    await this.start();
+    const message = this.env.models['mail.message'].create({
+        id: 11,
+        tracking_value_ids: [{
+            changed_field: "Name",
+            field_type: "text",
+            id: 6,
+            new_value: "Marc",
+            old_value: "",
+        }],
+    });
+    await this.createMessageComponent(message);
+    assert.strictEqual(
+        document.querySelector('.o_Message_trackingValue').textContent,
+        "Name:Marc",
+        "should display the correct content of tracked field of type text: from empty to some text (Name: -> Marc)"
+    );
+});
+
+QUnit.test('rendering of tracked field of type selection: from a selection to no selection', async function (assert) {
+    assert.expect(1);
+
+    await this.start();
+    const message = this.env.models['mail.message'].create({
+        id: 11,
+        tracking_value_ids: [{
+            changed_field: "State",
+            field_type: "selection",
+            id: 6,
+            new_value: "",
+            old_value: "ok",
+        }],
+    });
+    await this.createMessageComponent(message);
+    assert.strictEqual(
+        document.querySelector('.o_Message_trackingValue').textContent,
+        "State:ok",
+        "should display the correct content of tracked field of type selection: from a selection to no selection (State: ok ->)"
+    );
+});
+
+QUnit.test('rendering of tracked field of type selection: from no selection to a selection', async function (assert) {
+    assert.expect(1);
+
+    await this.start();
+    const message = this.env.models['mail.message'].create({
+        id: 11,
+        tracking_value_ids: [{
+            changed_field: "State",
+            field_type: "selection",
+            id: 6,
+            new_value: "ok",
+            old_value: "",
+        }],
+    });
+    await this.createMessageComponent(message);
+    assert.strictEqual(
+        document.querySelector('.o_Message_trackingValue').textContent,
+        "State:ok",
+        "should display the correct content of tracked field of type selection: from no selection to a selection (State: -> ok)"
+    );
+});
+
+QUnit.test('rendering of tracked field of type many2one: from having a related record to no related record', async function (assert) {
+    assert.expect(1);
+
+    await this.start();
+    const message = this.env.models['mail.message'].create({
+        id: 11,
+        tracking_value_ids: [{
+            changed_field: "Author",
+            field_type: "many2one",
+            id: 6,
+            new_value: "",
+            old_value: "Marc",
+        }],
+    });
+    await this.createMessageComponent(message);
+    assert.strictEqual(
+        document.querySelector('.o_Message_trackingValue').textContent,
+        "Author:Marc",
+        "should display the correct content of tracked field of type many2one: from having a related record to no related record (Author: Marc ->)"
+    );
+});
+
+QUnit.test('rendering of tracked field of type many2one: from no related record to having a related record', async function (assert) {
+    assert.expect(1);
+
+    await this.start();
+    const message = this.env.models['mail.message'].create({
+        id: 11,
+        tracking_value_ids: [{
+            changed_field: "Author",
+            field_type: "many2one",
+            id: 6,
+            new_value: "Marc",
+            old_value: "",
+        }],
+    });
+    await this.createMessageComponent(message);
+    assert.strictEqual(
+        document.querySelector('.o_Message_trackingValue').textContent,
+        "Author:Marc",
+        "should display the correct content of tracked field of type many2one: from no related record to having a related record (Author: -> Marc)"
     );
 });
 
diff --git a/addons/mail/static/tests/helpers/mock_server.js b/addons/mail/static/tests/helpers/mock_server.js
index 481b8d64f9c..9545d146962 100644
--- a/addons/mail/static/tests/helpers/mock_server.js
+++ b/addons/mail/static/tests/helpers/mock_server.js
@@ -1224,12 +1224,16 @@ MockServer.include({
             notifications = this._mockMailNotification_NotificationFormat(
                 notifications.map(notification => notification.id)
             );
+            const trackingValueIds = this._getRecords('mail.tracking.value', [
+                ['id', 'in', message.tracking_value_ids],
+            ]);
             const response = Object.assign({}, message, {
                 attachment_ids: formattedAttachments,
                 author_id: formattedAuthor,
                 history_partner_ids: historyPartnerIds,
                 needaction_partner_ids: needactionPartnerIds,
                 notifications,
+                tracking_value_ids: trackingValueIds,
             });
             if (message.subtype_id) {
                 const subtype = this._getRecords('mail.message.subtype', [