pengyuthon/odoo_source
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

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

Browse Source 
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
This commit is contained in:
Robot Odoo
2019-02-15 09:25:06 +01:00
committed by GitHub
parent 9cd8410f69 8ee218aa5c
commit b923ef6c3f
7 changed files with 153 additions and 85 deletions
Download Patch File Download Diff File Expand all files Collapse all files
addons/mail/models/mail_channel.py
+17 -3
View File
@@ -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 = {
addons/mail/static/src/js/models/threads/channel.js
+16 -3
View File
@@ -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;
@@ -405,6 +403,21 @@ var Channel = SearchableThread.extend(ChannelSeenMixin, ThreadTypingMixin, {
kwargs: { is_typing: params.typing },
}, { 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.
*
addons/mail/static/src/js/models/threads/dm_chat.js
+3 -1
View File
@@ -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;
addons/mail/static/src/js/models/threads/mixins/channel_seen_mixin.js
-20
View File
@@ -212,26 +212,6 @@ var ChannelSeenMixin = {
this.call('mail_service', 'getMailBus').trigger('update_channel', this.getID());
},
//--------------------------------------------------------------------------
// 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
//--------------------------------------------------------------------------
addons/mail/static/src/js/services/mail_notification_manager.js
+29 -2
View File
@@ -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;
}
@@ -235,6 +239,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
*
@@ -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);
}
addons/mail/static/tests/discuss_seen_indicator_tests.js
+76 -52
View File
@@ -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();
addons/mail/static/tests/systray/systray_messaging_menu_tests.js
+12 -4
View File
@@ -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();
});