[MERGE] mail: fix performance issues with channel seen feature

Revision on https://github.com/odoo/odoo/commit/844fb2b51ea7cfa557b19466c0a53dde19433e7a

There were some performance issue with this newly added feature.
This was particularly the case on channels with multiple users, therefore
we have decided to only enable it on direct chats for the moment.

Also, we avoid unnecessary writing in DB and sending notification on longpoll
if the seen information of a user is already up-to-date.

closes odoo/odoo#31074

7 files changed, 153 insertions, 85 deletions

diff --git a/addons/mail/models/mail_channel.py b/addons/mail/models/mail_channel.py
index 180a995a68f..37b4d222c26 100644
--- a/addons/mail/models/mail_channel.py
+++ b/addons/mail/models/mail_channel.py
@@ -730,7 +730,11 @@ class Channel(models.Model):
         self.ensure_one()
         if self.channel_message_ids.ids:
             last_message_id = self.channel_message_ids.ids[0] # zero is the index of the last message
-            self.env['mail.channel.partner'].search([('channel_id', 'in', self.ids), ('partner_id', '=', self.env.user.partner_id.id)]).write({
+            channel_partner = self.env['mail.channel.partner'].search([('channel_id', 'in', self.ids), ('partner_id', '=', self.env.user.partner_id.id)])
+            if channel_partner.seen_message_id.id == last_message_id:
+                # last message seen by user is already up-to-date
+                return
+            channel_partner.write({
                 'seen_message_id': last_message_id,
                 'fetched_message_id': last_message_id,
             })
@@ -739,7 +743,11 @@ class Channel(models.Model):
                 'last_message_id': last_message_id,
                 'partner_id': self.env.user.partner_id.id,
             }
-            self.env['bus.bus'].sendmany([[(self._cr.dbname, 'mail.channel', self.id), data]])
+            if self.channel_type == 'chat':
+                self.env['bus.bus'].sendmany([[(self._cr.dbname, 'mail.channel', self.id), data]])
+            else:
+                data['channel_id'] = self.id
+                self.env['bus.bus'].sendone((self._cr.dbname, 'res.partner', self.env.user.partner_id.id), data)
             return last_message_id
 
     @api.multi
@@ -750,8 +758,14 @@ class Channel(models.Model):
         for channel in self:
             if not channel.channel_message_ids.ids:
                 return
+            if channel.channel_type != 'chat':
+                return
             last_message_id = channel.channel_message_ids.ids[0] # zero is the index of the last message
-            self.env['mail.channel.partner'].search([('channel_id', '=', channel.id), ('partner_id', '=', self.env.user.partner_id.id)]).write({
+            channel_partner = self.env['mail.channel.partner'].search([('channel_id', '=', channel.id), ('partner_id', '=', self.env.user.partner_id.id)])
+            if channel_partner.fetched_message_id.id == last_message_id:
+                # last message fetched by user is already up-to-date
+                return
+            channel_partner.write({
                 'fetched_message_id': last_message_id,
             })
             data = {
diff --git a/addons/mail/static/src/js/models/threads/channel.js b/addons/mail/static/src/js/models/threads/channel.js
index a2316edb7be..9f0f37264db 100644
--- a/addons/mail/static/src/js/models/threads/channel.js
+++ b/addons/mail/static/src/js/models/threads/channel.js
@@ -1,7 +1,6 @@
 odoo.define('mail.model.Channel', function (require) {
 "use strict";
 
-var ChannelSeenMixin = require('mail.model.ChannelSeenMixin');
 var SearchableThread = require('mail.model.SearchableThread');
 var ThreadTypingMixin = require('mail.model.ThreadTypingMixin');
 var mailUtils = require('mail.utils');
@@ -17,7 +16,7 @@ var time = require('web.time');
  * Any piece of code in JS that make use of channels must ideally interact with
  * such objects, instead of direct data from the server.
  */
-var Channel = SearchableThread.extend(ChannelSeenMixin, ThreadTypingMixin, {
+var Channel = SearchableThread.extend(ThreadTypingMixin, {
     /**
      * @override
      * @param {Object} params
@@ -50,7 +49,6 @@ var Channel = SearchableThread.extend(ChannelSeenMixin, ThreadTypingMixin, {
     init: function (params) {
         var self = this;
         this._super.apply(this, arguments);
-        ChannelSeenMixin.init.apply(this, arguments);
         ThreadTypingMixin.init.apply(this, arguments);
 
         var data = params.data;
@@ -406,6 +404,21 @@ var Channel = SearchableThread.extend(ChannelSeenMixin, ThreadTypingMixin, {
         }, { shadow: true });
     },
     /**
+     * @private
+     * @returns {$.Promise<integer>} resolved with ID of last seen message
+     */
+    _notifySeen: function () {
+        var self = this;
+        return this._rpc({
+            model: 'mail.channel',
+            method: 'channel_seen',
+            args: [[this._id]],
+        }, { shadow: true }).then(function (lastSeenMessageID) {
+            self._lastSeenMessageID = lastSeenMessageID;
+            return lastSeenMessageID;
+        });
+    },
+    /**
      * Prepare and send a message to the server on this channel.
      *
      * @override
diff --git a/addons/mail/static/src/js/models/threads/dm_chat.js b/addons/mail/static/src/js/models/threads/dm_chat.js
index 5e69dbeff8c..0f9d3a32294 100644
--- a/addons/mail/static/src/js/models/threads/dm_chat.js
+++ b/addons/mail/static/src/js/models/threads/dm_chat.js
@@ -1,13 +1,14 @@
 odoo.define('mail.model.DMChat', function (require) {
 "use strict";
 
+var ChannelSeenMixin = require('mail.model.ChannelSeenMixin');
 var TwoUserChannel = require('mail.model.TwoUserChannel');
 
 /**
  * Any piece of code in JS that make use of DMs must ideally interact with
  * such objects, instead of direct data from the server.
  */
-var DMChat = TwoUserChannel.extend({
+var DMChat = TwoUserChannel.extend(ChannelSeenMixin, {
     /**
      * @override
      * @param {Object} params
@@ -21,6 +22,7 @@ var DMChat = TwoUserChannel.extend({
      */
     init: function (params) {
         this._super.apply(this, arguments);
+        ChannelSeenMixin.init.apply(this, arguments);
 
         var data = params.data;
 
diff --git a/addons/mail/static/src/js/models/threads/mixins/channel_seen_mixin.js b/addons/mail/static/src/js/models/threads/mixins/channel_seen_mixin.js
index 022994673d2..20a5bba5407 100644
--- a/addons/mail/static/src/js/models/threads/mixins/channel_seen_mixin.js
+++ b/addons/mail/static/src/js/models/threads/mixins/channel_seen_mixin.js
@@ -213,26 +213,6 @@ var ChannelSeenMixin = {
     },
 
     //--------------------------------------------------------------------------
-    // Private
-    //--------------------------------------------------------------------------
-
-    /**
-     * @private
-     * @returns {$.Promise<integer>} resolved with ID of last seen message
-     */
-    _notifySeen: function () {
-        var self = this;
-        return this._rpc({
-            model: 'mail.channel',
-            method: 'channel_seen',
-            args: [[this._id]],
-        }, { shadow: true }).then(function (lastSeenMessageID) {
-            self._lastSeenMessageID = lastSeenMessageID;
-            return lastSeenMessageID;
-        });
-    },
-
-    //--------------------------------------------------------------------------
     // Handlers
     //--------------------------------------------------------------------------
 
diff --git a/addons/mail/static/src/js/services/mail_notification_manager.js b/addons/mail/static/src/js/services/mail_notification_manager.js
index b59d7b91434..4aa768e4c9d 100644
--- a/addons/mail/static/src/js/services/mail_notification_manager.js
+++ b/addons/mail/static/src/js/services/mail_notification_manager.js
@@ -80,7 +80,9 @@ MailManager.include({
         if (!channel) {
             return;
         }
-        channel.updateSeenPartnersInfo(data);
+        if (channel.hasSeenFeature()) {
+            channel.updateSeenPartnersInfo(data);
+        }
     },
     /**
      * Called when a new or updated message is received on a channel
@@ -126,7 +128,9 @@ MailManager.include({
         if (!channel) {
             return;
         }
-        channel.updateSeenPartnersInfo(data);
+        if (channel.hasSeenFeature()) {
+            channel.updateSeenPartnersInfo(data);
+        }
         if (session.partner_id !== data.partner_id) {
             return;
         }
@@ -236,6 +240,27 @@ MailManager.include({
         }
     },
     /**
+     * Called when receiving a multi_user_channel seen notification. Only
+     * the current user is notified. This must be handled as if this is a
+     * channel seen notification.
+     *
+     * Note that this is a 'res.partner' notification because only the current
+     * user is notified on channel seen. This is a consequence from disabling
+     * the seen feature on multi_user_channel, because we still need to get
+     * the last seen message ID in order to display the "New Messages" separator
+     * in Discuss.
+     *
+     * @private
+     * @param {Object} data
+     * @param {integer} data.channel_id
+     * @param {string} data.info 'channel_seen'
+     * @param {integer} data.last_message_id
+     * @param {integer} data.partner_id
+     */
+    _handlePartnerChannnelSeenNotification: function (data) {
+        this._handleChannelSeenNotification(data.channel_id, data);
+    },
+    /**
      * Add or remove failure when receiving a failure update message
      *
      * @private
@@ -387,6 +412,8 @@ MailManager.include({
             this._handlePartnerMailFailureNotification(data);
         } else if (data.type === 'user_connection') {
             this._handlePartnerUserConnectionNotification(data);
+        } else if (data.info === 'channel_seen') {
+            this._handlePartnerChannnelSeenNotification(data);
         } else {
             this._handlePartnerChannelNotification(data);
         }
diff --git a/addons/mail/static/tests/discuss_seen_indicator_tests.js b/addons/mail/static/tests/discuss_seen_indicator_tests.js
index 41fd6a508d6..4db61a56ed1 100644
--- a/addons/mail/static/tests/discuss_seen_indicator_tests.js
+++ b/addons/mail/static/tests/discuss_seen_indicator_tests.js
@@ -80,10 +80,11 @@ QUnit.test('2 members: 1 myself message not received initially', function (asser
     assert.expect(2);
     var done = assert.async();
 
-    var general = {
+    var dmChat = {
         id: 1,
-        channel_type: "channel",
-        name: "general",
+        channel_type: "chat",
+        direct_partner: [{ id: 2, name: 'Someone', im_status: '' }],
+        name: 'DM',
         members: [{
             id: 2,
             name: "Someone",
@@ -106,12 +107,12 @@ QUnit.test('2 members: 1 myself message not received initially', function (asser
 
     this.data.initMessaging = {
         channel_slots: {
-            channel_channel: [general],
+            channel_direct_message: [dmChat],
         },
     };
     this.data['mail.channel'] = {
         fields: {},
-        records: [general],
+        records: [dmChat],
     };
     this.data['mail.message'].records = [{
         author_id: [3, "Me"],
@@ -130,10 +131,10 @@ QUnit.test('2 members: 1 myself message not received initially', function (asser
         services: this.services,
         session: { partner_id: 3 },
     }).then(function (discuss) {
-        // click on general
-        var $general = discuss.$('.o_mail_discuss_sidebar')
+        // click on DM chat
+        var $dmChat = discuss.$('.o_mail_discuss_sidebar')
                         .find('.o_mail_discuss_item[data-thread-id=1]');
-        testUtils.dom.click($general);
+        testUtils.dom.click($dmChat);
 
         assert.isVisible(discuss.$('.o_thread_message[data-message-id=100]'));
         assert.containsNone(discuss.$('.o_mail_thread_message_seen_icon[data-message-id=100]'));
@@ -147,10 +148,11 @@ QUnit.test('2 members: 1 myself message received by everyone initially', functio
     assert.expect(4);
     var done = assert.async();
 
-    var general = {
+    var dmChat = {
         id: 1,
-        channel_type: "channel",
-        name: "general",
+        channel_type: "chat",
+        direct_partner: [{ id: 2, name: 'Someone', im_status: '' }],
+        name: 'DM',
         members: [{
             id: 2,
             name: "Someone",
@@ -173,12 +175,12 @@ QUnit.test('2 members: 1 myself message received by everyone initially', functio
 
     this.data.initMessaging = {
         channel_slots: {
-            channel_channel: [general],
+            channel_direct_message: [dmChat],
         },
     };
     this.data['mail.channel'] = {
         fields: {},
-        records: [general],
+        records: [dmChat],
     };
     this.data['mail.message'].records = [{
         author_id: [3, "Me"],
@@ -197,10 +199,10 @@ QUnit.test('2 members: 1 myself message received by everyone initially', functio
         services: this.services,
         session: { partner_id: 3 },
     }).then(function (discuss) {
-        // click on general
-        var $general = discuss.$('.o_mail_discuss_sidebar')
+        // click on DM chat
+        var $dmChat = discuss.$('.o_mail_discuss_sidebar')
                         .find('.o_mail_discuss_item[data-thread-id=1]');
-        testUtils.dom.click($general);
+        testUtils.dom.click($dmChat);
         var $seenIcon = discuss.$('.o_mail_thread_message_seen_icon[data-message-id=100]');
         assert.isVisible($seenIcon, "should display seen icon on message");
         assert.containsOnce($seenIcon, '.fa-check', "should display a single check");
@@ -219,10 +221,11 @@ QUnit.test('2 members: 1 myself message seen by everyone initially', function (a
     assert.expect(4);
     var done = assert.async();
 
-    var general = {
+    var dmChat = {
         id: 1,
-        channel_type: "channel",
-        name: "general",
+        channel_type: "chat",
+        direct_partner: [{ id: 2, name: 'Someone', im_status: '' }],
+        name: 'DM',
         members: [{
             id: 2,
             name: "Someone",
@@ -245,12 +248,12 @@ QUnit.test('2 members: 1 myself message seen by everyone initially', function (a
 
     this.data.initMessaging = {
         channel_slots: {
-            channel_channel: [general],
+            channel_direct_message: [dmChat],
         },
     };
     this.data['mail.channel'] = {
         fields: {},
-        records: [general],
+        records: [dmChat],
     };
     this.data['mail.message'].records = [{
         author_id: [3, "Me"],
@@ -269,10 +272,10 @@ QUnit.test('2 members: 1 myself message seen by everyone initially', function (a
         services: this.services,
         session: { partner_id: 3 },
     }).then(function (discuss) {
-        // click on general
-        var $general = discuss.$('.o_mail_discuss_sidebar')
+        // click on DM chat
+        var $dmChat = discuss.$('.o_mail_discuss_sidebar')
                         .find('.o_mail_discuss_item[data-thread-id=1]');
-        testUtils.dom.click($general);
+        testUtils.dom.click($dmChat);
         var $seenIcon = discuss.$('.o_mail_thread_message_seen_icon[data-message-id=100]');
         assert.isVisible($seenIcon, "should display seen icon on message");
         assert.containsN($seenIcon, '.fa-check', 2, "should display 2 checks");
@@ -292,10 +295,11 @@ QUnit.test('2 members: 1 myself message received by everyone (initially not rece
     var done = assert.async();
 
     var self = this;
-    var general = {
+    var dmChat = {
         id: 1,
-        channel_type: "channel",
-        name: "general",
+        channel_type: "chat",
+        direct_partner: [{ id: 2, name: 'Someone', im_status: '' }],
+        name: 'DM',
         members: [{
             id: 2,
             name: "Someone",
@@ -318,12 +322,12 @@ QUnit.test('2 members: 1 myself message received by everyone (initially not rece
 
     this.data.initMessaging = {
         channel_slots: {
-            channel_channel: [general],
+            channel_direct_message: [dmChat],
         },
     };
     this.data['mail.channel'] = {
         fields: {},
-        records: [general],
+        records: [dmChat],
     };
     this.data['mail.message'].records = [{
         author_id: [3, "Me"],
@@ -342,10 +346,10 @@ QUnit.test('2 members: 1 myself message received by everyone (initially not rece
         services: this.services,
         session: { partner_id: 3 },
     }).then(function (discuss) {
-        // click on general
-        var $general = discuss.$('.o_mail_discuss_sidebar')
+        // click on DM chat
+        var $dmChat = discuss.$('.o_mail_discuss_sidebar')
                         .find('.o_mail_discuss_item[data-thread-id=1]');
-        testUtils.dom.click($general);
+        testUtils.dom.click($dmChat);
 
         assert.isVisible(discuss.$('.o_thread_message[data-message-id=100]'));
         assert.containsNone(discuss.$('.o_mail_thread_message_seen_icon[data-message-id=100]'));
@@ -377,10 +381,11 @@ QUnit.test('2 members: 1 myself message seen by everyone (initially not received
     var done = assert.async();
 
     var self = this;
-    var general = {
+    var dmChat = {
         id: 1,
-        channel_type: "channel",
-        name: "general",
+        channel_type: "chat",
+        direct_partner: [{ id: 2, name: 'Someone', im_status: '' }],
+        name: 'DM',
         members: [{
             id: 2,
             name: "Someone",
@@ -403,12 +408,12 @@ QUnit.test('2 members: 1 myself message seen by everyone (initially not received
 
     this.data.initMessaging = {
         channel_slots: {
-            channel_channel: [general],
+            channel_direct_message: [dmChat],
         },
     };
     this.data['mail.channel'] = {
         fields: {},
-        records: [general],
+        records: [dmChat],
     };
     this.data['mail.message'].records = [{
         author_id: [3, "Me"],
@@ -427,10 +432,10 @@ QUnit.test('2 members: 1 myself message seen by everyone (initially not received
         services: this.services,
         session: { partner_id: 3 },
     }).then(function (discuss) {
-        // click on general
-        var $general = discuss.$('.o_mail_discuss_sidebar')
+        // click on DM chat
+        var $dmChat = discuss.$('.o_mail_discuss_sidebar')
                         .find('.o_mail_discuss_item[data-thread-id=1]');
-        testUtils.dom.click($general);
+        testUtils.dom.click($dmChat);
 
         assert.isVisible(discuss.$('.o_thread_message[data-message-id=100]'));
         assert.containsNone(discuss.$('.o_mail_thread_message_seen_icon[data-message-id=100]'));
@@ -462,10 +467,11 @@ QUnit.test('2 members: 1 other message seen by me (initially not received)', fun
     var done = assert.async();
 
     var self = this;
-    var general = {
+    var dmChat = {
         id: 1,
-        channel_type: "channel",
-        name: "general",
+        channel_type: "chat",
+        direct_partner: [{ id: 2, name: 'Someone', im_status: '' }],
+        name: 'DM',
         members: [{
             id: 2,
             name: "Someone",
@@ -488,12 +494,12 @@ QUnit.test('2 members: 1 other message seen by me (initially not received)', fun
 
     this.data.initMessaging = {
         channel_slots: {
-            channel_channel: [general],
+            channel_direct_message: [dmChat],
         },
     };
     this.data['mail.channel'] = {
         fields: {},
-        records: [general],
+        records: [dmChat],
     };
     this.data['mail.message'].records = [{
         author_id: [2, "Other"],
@@ -512,10 +518,10 @@ QUnit.test('2 members: 1 other message seen by me (initially not received)', fun
         services: this.services,
         session: { partner_id: 3 },
     }).then(function (discuss) {
-        // click on general
-        var $general = discuss.$('.o_mail_discuss_sidebar')
+        // click on DM chat
+        var $dmChat = discuss.$('.o_mail_discuss_sidebar')
                         .find('.o_mail_discuss_item[data-thread-id=1]');
-        testUtils.dom.click($general);
+        testUtils.dom.click($dmChat);
 
         assert.isVisible(discuss.$('.o_thread_message[data-message-id=100]'));
         assert.containsNone(discuss.$('.o_mail_thread_message_seen_icon[data-message-id=100]'));
@@ -535,7 +541,11 @@ QUnit.test('2 members: 1 other message seen by me (initially not received)', fun
     });
 });
 
-QUnit.test('3 members: 1 myself message received by some initially', function (assert) {
+QUnit.skip('3 members: 1 myself message received by some initially', function (assert) {
+    // IMPORTANT: This test make sense if this feature is on multi user channels,
+    // which was the case in case at some point. It has been disabled for
+    // performance reasons. We are unsure if this will come back in the future,
+    // so we keep to tests in 'skip' mode in the mean time.
     assert.expect(4);
     var done = assert.async();
 
@@ -624,7 +634,12 @@ QUnit.test('3 members: 1 myself message received by some initially', function (a
     });
 });
 
-QUnit.test('several members: 1 myself message seen by everyone (initially not received)', function (assert) {
+QUnit.skip('several members: 1 myself message seen by everyone (initially not received)', function (assert) {
+    // IMPORTANT: This test make sense if this feature is on multi user channels,
+    // which was the case in case at some point. It has been disabled for
+    // performance reasons. We are unsure if this will come back in the future,
+    // so we keep to tests in 'skip' mode in the mean time.
+
     // seen icon and content should change for my messages based on other users
     // actions (received and/or seen the message)
     assert.expect(26);
@@ -826,7 +841,12 @@ QUnit.test('several members: 1 myself message seen by everyone (initially not re
     });
 });
 
-QUnit.test('several members: other message seen by everyone (initially not received)', function (assert) {
+QUnit.skip('several members: other message seen by everyone (initially not received)', function (assert) {
+    // IMPORTANT: This test make sense if this feature is on multi user channels,
+    // which was the case in case at some point. It has been disabled for
+    // performance reasons. We are unsure if this will come back in the future,
+    // so we keep to tests in 'skip' mode in the mean time.
+
     // seen icon should never be visible for message of others
     assert.expect(9);
     var done = assert.async();
@@ -989,7 +1009,11 @@ QUnit.test('several members: other message seen by everyone (initially not recei
     });
 });
 
-QUnit.test('several members: only show seen icons from last message seen by everyone', function (assert) {
+QUnit.skip('several members: only show seen icons from last message seen by everyone', function (assert) {
+    // IMPORTANT: This test make sense if this feature is on multi user channels,
+    // which was the case in case at some point. It has been disabled for
+    // performance reasons. We are unsure if this will come back in the future,
+    // so we keep to tests in 'skip' mode in the mean time.
     assert.expect(12);
     var done = assert.async();
 
diff --git a/addons/mail/static/tests/systray/systray_messaging_menu_tests.js b/addons/mail/static/tests/systray/systray_messaging_menu_tests.js
index 8f4851ada0d..18a8838be7e 100644
--- a/addons/mail/static/tests/systray/systray_messaging_menu_tests.js
+++ b/addons/mail/static/tests/systray/systray_messaging_menu_tests.js
@@ -586,10 +586,18 @@ QUnit.test('grouped preview for needaction messages linked to same document', fu
     messagingMenu.destroy();
 });
 
-QUnit.test("messaging menu widget: channel seen notification", function ( assert ) {
+QUnit.test("messaging menu widget: channel seen notification", function (assert) {
     assert.expect(4);
 
-    this.data.initMessaging.channel_slots.channel_channel[0].message_unread_counter = 1;
+    this.data.initMessaging.channel_slots = {
+        channel_direct_message: [{
+            id: 1,
+            channel_type: "chat",
+            direct_partner: [{ id: 2, name: 'Someone', im_status: '' }],
+            name: 'DM',
+            message_unread_counter: 1,
+        }],
+    };
 
     var messagingMenu = new MessagingMenu();
     testUtils.mock.addMockEnvironment(messagingMenu, {
@@ -605,7 +613,7 @@ QUnit.test("messaging menu widget: channel seen notification", function ( assert
         "should have correct messaging menu counter (1 unread message in channel)");
     assert.strictEqual(messagingMenu.$('.o_preview_counter').text().replace(/\s/g, ''),
         '(1)',
-        "should display 1 unread message on general channel preview");
+        "should display 1 unread message on DM channel preview");
 
     // Simulate received channel seen notification
     var message = {
@@ -621,7 +629,7 @@ QUnit.test("messaging menu widget: channel seen notification", function ( assert
     assert.strictEqual(messagingMenu.$('.o_notification_counter').text(), '0',
         "should no longer have a messaging menu counter (no unread message in channel)");
     assert.strictEqual(messagingMenu.$('.o_preview_counter').text().replace(/\s/g, ''), '',
-        "should no longer display unread message on general channel preview");
+        "should no longer display unread message on DM channel preview");
 
     messagingMenu.destroy();
 });