From 9e60f0d3f3c400a0076aab6e1b21394723f0070d Mon Sep 17 00:00:00 2001 From: Xavier Dubuc Date: Fri, 18 Sep 2020 10:09:04 +0000 Subject: [PATCH] [FIX] mail: use compute fields for identifying relations MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit (in message seen indicators) task-2335647 closes odoo/odoo#58375 X-original-commit: 3d4ff42f4474a5761c9e0737297d973992a2fcc2 Signed-off-by: Sébastien Theys (seb) --- .../src/components/message/message_tests.js | 8 +- .../message_seen_indicator_tests.js | 20 ++-- .../mail/static/src/models/message/message.js | 11 +-- .../message_seen_indicator.js | 95 ++++++++++++++----- .../messaging_notification_handler.js | 8 +- .../mail/static/src/models/thread/thread.js | 16 +++- 6 files changed, 108 insertions(+), 50 deletions(-) diff --git a/addons/mail/static/src/components/message/message_tests.js b/addons/mail/static/src/components/message/message_tests.js index dedc9434f2a..1737d7ec291 100644 --- a/addons/mail/static/src/components/message/message_tests.js +++ b/addons/mail/static/src/components/message/message_tests.js @@ -590,8 +590,8 @@ QUnit.test('do not show messaging seen indicator if before last seen by all mess const thread = this.env.models['mail.thread'].create({ id: 11, messageSeenIndicators: [['insert', { - id: this.env.models['mail.message_seen_indicator'].computeId(99, 11), - message: [['insert', { id: 99 }]], + channelId: 11, + messageId: 99, }]], model: 'mail.channel', }); @@ -670,8 +670,8 @@ QUnit.test('only show messaging seen indicator if authored by me, after last see }, ]]], messageSeenIndicators: [['insert', { - id: this.env.models['mail.message_seen_indicator'].computeId(100, 11), - message: [['insert', { id: 100 }]], + channelId: 11, + messageId: 100, }]], model: 'mail.channel', }); diff --git a/addons/mail/static/src/components/message_seen_indicator/message_seen_indicator_tests.js b/addons/mail/static/src/components/message_seen_indicator/message_seen_indicator_tests.js index 9eb61e6a100..fb9c6b8ba15 100644 --- a/addons/mail/static/src/components/message_seen_indicator/message_seen_indicator_tests.js +++ b/addons/mail/static/src/components/message_seen_indicator/message_seen_indicator_tests.js @@ -61,8 +61,8 @@ QUnit.test('rendering when just one has received the message', async function (a }, ]]], messageSeenIndicators: [['insert', { - id: this.env.models['mail.message_seen_indicator'].computeId(100, 1000), - message: [['insert', { id: 100 }]], + channelId: 1000, + messageId: 100, }]], }); const message = this.env.models['mail.message'].insert({ @@ -109,8 +109,8 @@ QUnit.test('rendering when everyone have received the message', async function ( }, ]]], messageSeenIndicators: [['insert', { - id: this.env.models['mail.message_seen_indicator'].computeId(100, 1000), - message: [['insert', { id: 100 }]], + channelId: 1000, + messageId: 100, }]], }); const message = this.env.models['mail.message'].insert({ @@ -158,8 +158,8 @@ QUnit.test('rendering when just one has seen the message', async function (asser }, ]]], messageSeenIndicators: [['insert', { - id: this.env.models['mail.message_seen_indicator'].computeId(100, 1000), - message: [['insert', { id: 100 }]], + channelId: 1000, + messageId: 100, }]], }); const message = this.env.models['mail.message'].insert({ @@ -207,8 +207,8 @@ QUnit.test('rendering when just one has seen & received the message', async func }, ]]], messageSeenIndicators: [['insert', { - id: this.env.models['mail.message_seen_indicator'].computeId(100, 1000), - message: [['insert', { id: 100 }]], + channelId: 1000, + messageId: 100, }]], }); const message = this.env.models['mail.message'].insert({ @@ -258,8 +258,8 @@ QUnit.test('rendering when just everyone has seen the message', async function ( }, ]]], messageSeenIndicators: [['insert', { - id: this.env.models['mail.message_seen_indicator'].computeId(100, 1000), - message: [['insert', { id: 100 }]], + channelId: 1000, + messageId: 100, }]], }); const message = this.env.models['mail.message'].insert({ 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 @@ -12,23 +12,6 @@ function factory(dependencies) { // Public //---------------------------------------------------------------------- - /** - * 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, }; }) ]