[FIX] mail: use compute fields for identifying relations

(in message seen indicators)

task-2335647

closes odoo/odoo#58373

X-original-commit: 3d4ff42f4474a5761c9e0737297d973992a2fcc2
Signed-off-by: Sébastien Theys (seb) <seb@odoo.com>

4 files changed, 94 insertions, 36 deletions

diff --git a/addons/mail/static/src/models/message/message.js b/addons/mail/static/src/models/message/message.js
index f375294666c..2c963632101 100644
--- a/addons/mail/static/src/models/message/message.js
+++ b/addons/mail/static/src/models/message/message.js
@@ -229,14 +229,9 @@ function factory(dependencies) {
                         // on `channel` channels for performance reasons
                         continue;
                     }
-                    thread.update({
-                        messageSeenIndicators: [[
-                            'insert',
-                            {
-                                id: this.env.models['mail.message_seen_indicator'].computeId(message.id, thread.id),
-                                message: [['link', message]],
-                            },
-                        ]],
+                    this.env.models['mail.message_seen_indicator'].insert({
+                        messageId: message.id,
+                        threadId: thread.id,
                     });
                 }
             }
diff --git a/addons/mail/static/src/models/message_seen_indicator/message_seen_indicator.js b/addons/mail/static/src/models/message_seen_indicator/message_seen_indicator.js
index bab1ef2046e..dd1848aaf7b 100644
--- a/addons/mail/static/src/models/message_seen_indicator/message_seen_indicator.js
+++ b/addons/mail/static/src/models/message_seen_indicator/message_seen_indicator.js
@@ -13,23 +13,6 @@ function factory(dependencies) {
         //----------------------------------------------------------------------
 
         /**
-         * FIXME replace by using messageId & channelId as identifying fields (task-2335647)
-         * 
-         * @static
-         * @param {mail.message|integer} message
-         * @param {mail.thread|integer} thread
-         * @returns {string|undefined}
-         */
-        static computeId(message, thread) {
-            if (message && thread) {
-                const messageId = typeof message !== 'object' ? message : message.id;
-                const threadId = typeof thread !== 'object' ? thread : thread.id;
-                return [messageId, threadId].join('-');
-            }
-            return undefined;
-        }
-
-        /**
          * @static
          * @param {mail.thread} [channel] the concerned thread
          */
@@ -73,7 +56,8 @@ function factory(dependencies) {
          * @override
          */
         static _createRecordLocalId(data) {
-            return `${this.modelName}_${data.id}`;
+            const { channelId, messageId } = data;
+            return `${this.modelName}_${channelId}_${messageId}`;
         }
 
         /**
@@ -239,11 +223,46 @@ function factory(dependencies) {
             }
             return [['replace', otherPartnersThatHaveSeen]];
         }
+
+        /**
+         * @private
+         * @returns {mail.message}
+         */
+        _computeMessage() {
+            return [['insert', { id: this.messageId }]];
+        }
+
+        /**
+         * @private
+         * @returns {mail.thread}
+         */
+        _computeThread() {
+            return [['insert', {
+                id: this.channelId,
+                model: 'mail.channel',
+            }]];
+        }
     }
 
     MessageSeenIndicator.modelName = 'mail.message_seen_indicator';
 
     MessageSeenIndicator.fields = {
+        /**
+         * The id of the channel this seen indicator is related to.
+         *
+         * Should write on this field to set relation between the channel and
+         * this seen indicator, not on `thread`.
+         *
+         * Reason for not setting the relation directly is the necessity to
+         * uniquely identify a seen indicator based on channel and message from data.
+         * Relational data are list of commands, which is problematic to deduce
+         * identifying records.
+         *
+         * TODO: task-2322536 (normalize relational data) & task-2323665
+         * (required fields) should improve and let us just use the relational
+         * fields.
+         */
+        channelId: attr(),
         hasEveryoneFetched: attr({
             compute: '_computeHasEveryoneFetched',
             default: false,
@@ -273,13 +292,36 @@ function factory(dependencies) {
                 'threadLastCurrentPartnerMessageSeenByEveryone',
             ],
         }),
-        message: many2one('mail.message'),
+        /**
+         * The message concerned by this seen indicator.
+         * This is automatically computed based on messageId field.
+         * @see messageId
+         */
+        message: many2one('mail.message', {
+            compute: '_computeMessage',
+            dependencies: [
+                'messageId',
+            ],
+        }),
         messageAuthor: many2one('mail.partner', {
             related: 'message.author',
         }),
-        messageId: attr({
-            related: 'message.id',
-        }),
+        /**
+         * The id of the message this seen indicator is related to.
+         *
+         * Should write on this field to set relation between the channel and
+         * this seen indicator, not on `message`.
+         *
+         * Reason for not setting the relation directly is the necessity to
+         * uniquely identify a seen indicator based on channel and message from data.
+         * Relational data are list of commands, which is problematic to deduce
+         * identifying records.
+         *
+         * TODO: task-2322536 (normalize relational data) & task-2323665
+         * (required fields) should improve and let us just use the relational
+         * fields.
+         */
+        messageId: attr(),
         partnersThatHaveFetched: many2many('mail.partner', {
             compute: '_computePartnersThatHaveFetched',
             dependencies: ['messageAuthor', 'messageId', 'threadPartnerSeenInfos'],
@@ -288,7 +330,16 @@ function factory(dependencies) {
             compute: '_computePartnersThatHaveSeen',
             dependencies: ['messageAuthor', 'messageId', 'threadPartnerSeenInfos'],
         }),
+        /**
+         * The thread concerned by this seen indicator.
+         * This is automatically computed based on channelId field.
+         * @see channelId
+         */
         thread: many2one('mail.thread', {
+            compute: '_computeThread',
+            dependencies: [
+                'channelId',
+            ],
             inverse: 'messageSeenIndicators'
         }),
         threadPartnerSeenInfos: one2many('mail.thread_partner_seen_info', {
diff --git a/addons/mail/static/src/models/messaging_notification_handler/messaging_notification_handler.js b/addons/mail/static/src/models/messaging_notification_handler/messaging_notification_handler.js
index 12976aaadad..8123a69e7a5 100644
--- a/addons/mail/static/src/models/messaging_notification_handler/messaging_notification_handler.js
+++ b/addons/mail/static/src/models/messaging_notification_handler/messaging_notification_handler.js
@@ -162,8 +162,8 @@ function factory(dependencies) {
             channel.update({
                 messageSeenIndicators: [['insert',
                     {
-                        id: this.env.models['mail.message_seen_indicator'].computeId(last_message_id, channel.id),
-                        message: [['insert', {id: last_message_id}]],
+                        channelId: channel.id,
+                        messageId: last_message_id,
                     }
                 ]],
             });
@@ -314,8 +314,8 @@ function factory(dependencies) {
                     // FIXME should no longer use computeId (task-2335647)
                     messageSeenIndicators: [['insert',
                         {
-                            id: this.env.models['mail.message_seen_indicator'].computeId(last_message_id, channel.id),
-                            message: [['link', lastMessage]],
+                            channelId: channel.id,
+                            messageId: lastMessage.id,
                         },
                     ]],
                 });
diff --git a/addons/mail/static/src/models/thread/thread.js b/addons/mail/static/src/models/thread/thread.js
index 8e7aee26bc9..09fcd46afed 100644
--- a/addons/mail/static/src/models/thread/thread.js
+++ b/addons/mail/static/src/models/thread/thread.js
@@ -221,6 +221,12 @@ function factory(dependencies) {
                 if (!data.seen_partners_info) {
                     data2.partnerSeenInfos = [['unlink-all']];
                 } else {
+                    /*
+                     * FIXME: not optimal to write on relation given the fact that the relation
+                     * will be (re)computed based on given fields.
+                     * (here channelId will compute partnerSeenInfo.thread))
+                     * task-2336946
+                     */
                     data2.partnerSeenInfos = [
                         ['insert-and-replace',
                             data.seen_partners_info.map(
@@ -245,12 +251,18 @@ function factory(dependencies) {
                             return currentSet;
                         }, new Set());
                         if (messageIds.size > 0) {
+                            /*
+                             * FIXME: not optimal to write on relation given the fact that the relation
+                             * will be (re)computed based on given fields.
+                             * (here channelId will compute messageSeenIndicator.thread))
+                             * task-2336946
+                             */
                             data2.messageSeenIndicators = [
                                 ['insert',
                                     [...messageIds].map(messageId => {
                                        return {
-                                           id: this.env.models['mail.message_seen_indicator'].computeId(messageId, data.id || this.id),
-                                           message: [['insert', { id: messageId }]],
+                                           channelId: data.id || this.id,
+                                           messageId,
                                        };
                                     })
                                 ]