From b2d9170aef57325f0df54180804de86ff6db2c49 Mon Sep 17 00:00:00 2001 From: Xavier Dubuc Date: Fri, 17 Jul 2020 14:50:29 +0000 Subject: [PATCH] [FIX] mail: fix "New messages" separator and channel_seen behaviour MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - channel_seen is now called even for own message and only when needed - channel_seen is now called with a message_id which transfer the decision of what is seen to the client which makes more sense task-2282235 task-2282248 closes odoo/odoo#57956 X-original-commit: 04edfa50ccea305050d017c4a1996e561bc7bd57 Signed-off-by: Sébastien Theys (seb) --- addons/im_livechat/models/mail_channel.py | 3 - .../static/src/legacy/public_livechat.js | 15 +- .../static/src/models/thread/thread.js | 6 - .../static/tests/helpers/mock_server.js | 5 - addons/mail/models/mail_channel.py | 95 ++++++---- addons/mail/models/mail_thread.py | 2 +- .../chat_window_header/chat_window_header.xml | 4 +- .../components/discuss/tests/discuss_tests.js | 48 +++-- .../discuss_sidebar_item.js | 2 +- .../discuss_sidebar_item.xml | 6 +- .../src/components/message/message_tests.js | 29 +-- .../components/message_list/message_list.xml | 4 +- .../message_seen_indicator_tests.js | 42 ++--- .../notification_list/notification_list.js | 4 +- .../thread_preview/thread_preview.js | 4 +- .../thread_preview/thread_preview.xml | 16 +- .../thread_preview/thread_preview_tests.js | 37 +++- .../thread_view/thread_view_tests.js | 143 ++++++++++++++- .../chat_window_manager.js | 4 +- .../message_seen_indicator.js | 14 ++ .../models/messaging_menu/messaging_menu.js | 2 +- .../messaging_notification_handler.js | 83 ++++----- .../mail/static/src/models/thread/thread.js | 170 +++++++++++++++--- .../static/src/models/thread/thread_tests.js | 4 +- .../thread_partner_seen_info.js | 74 +++++++- .../src/models/thread_view/thread_view.js | 2 +- .../mail/static/tests/helpers/mock_server.js | 64 ++++++- addons/test_mail/tests/test_mail_channel.py | 25 +++ 28 files changed, 691 insertions(+), 216 deletions(-) diff --git a/addons/im_livechat/models/mail_channel.py b/addons/im_livechat/models/mail_channel.py index f5df253beb1..664cd8d2627 100644 --- a/addons/im_livechat/models/mail_channel.py +++ b/addons/im_livechat/models/mail_channel.py @@ -87,9 +87,6 @@ class MailChannel(models.Model): channel_infos_dict[channel.id]['operator_pid'] = (res[0], res[1].replace(',', '')) # add the anonymous or partner name channel_infos_dict[channel.id]['livechat_visitor'] = channel._channel_get_livechat_visitor_info() - last_msg = self.env['mail.message'].search([("channel_ids", "in", [channel.id])], limit=1) - if last_msg: - channel_infos_dict[channel.id]['last_message_id'] = last_msg.id return list(channel_infos_dict.values()) @api.model diff --git a/addons/im_livechat/static/src/legacy/public_livechat.js b/addons/im_livechat/static/src/legacy/public_livechat.js index 35512fca51d..321c46184d3 100644 --- a/addons/im_livechat/static/src/legacy/public_livechat.js +++ b/addons/im_livechat/static/src/legacy/public_livechat.js @@ -177,21 +177,22 @@ var LivechatButton = Widget.extend({ * @param {Array} notification */ _handleNotification: function (notification) { - if (this._livechat && (notification[0] === this._livechat.getUUID())) { - if (notification[1]._type === 'history_command') { // history request - var cookie = utils.get_cookie(LIVECHAT_COOKIE_HISTORY); - var history = cookie ? JSON.parse(cookie) : []; + const [livechatUUID, notificationData] = notification; + if (this._livechat && (livechatUUID === this._livechat.getUUID())) { + if (notificationData._type === 'history_command') { // history request + const cookie = utils.get_cookie(LIVECHAT_COOKIE_HISTORY); + const history = cookie ? JSON.parse(cookie) : []; session.rpc('/im_livechat/history', { pid: this._livechat.getOperatorPID()[0], channel_uuid: this._livechat.getUUID(), page_history: history, }); - } else { // normal message + } else if ('body' in notificationData) { // normal message // If message from notif is already in chatter messages, stop handling - if (this._messages.some(message => message.getID() === notification[1].id)) { + if (this._messages.some(message => message.getID() === notificationData.id)) { return; } - this._addMessage(notification[1]); + this._addMessage(notificationData); this._renderMessages(); if (this._chatWindow.isFolded() || !this._chatWindow.isAtBottom()) { this._livechat.incrementUnreadCounter(); diff --git a/addons/im_livechat/static/src/models/thread/thread.js b/addons/im_livechat/static/src/models/thread/thread.js index 1ec06246ec7..9f728f62d40 100644 --- a/addons/im_livechat/static/src/models/thread/thread.js +++ b/addons/im_livechat/static/src/models/thread/thread.js @@ -17,12 +17,6 @@ registerClassPatchModel('mail.thread', 'im_livechat/static/src/models/thread/thr */ convertData(data) { const data2 = this._super(data); - if ('last_message_id' in data) { - if (!data2.messagesAsServerChannel) { - data2.messagesAsServerChannel = []; - } - data2.messagesAsServerChannel.push(['insert', { id: data.last_message_id }]); - } if ('livechat_visitor' in data && data.livechat_visitor) { if (!data2.members) { data2.members = []; diff --git a/addons/im_livechat/static/tests/helpers/mock_server.js b/addons/im_livechat/static/tests/helpers/mock_server.js index b8f597dd418..e940cd5ed7d 100644 --- a/addons/im_livechat/static/tests/helpers/mock_server.js +++ b/addons/im_livechat/static/tests/helpers/mock_server.js @@ -155,11 +155,6 @@ MockServer.include({ } // add the anonymous or partner name channelInfo.livechat_visitor = this._mockMailChannel_ChannelGetLivechatVisitorInfo([channel.id]); - const messages = this._getRecords('mail.message', [['channel_ids', 'in', channel.id]]); - const last_msg = messages.sort((m1, m2) => m1.id > m2.id ? -1 : 1)[0]; - if (last_msg) { - channelInfo.last_message_id = last_msg.id; - } } } return channelInfos; diff --git a/addons/mail/models/mail_channel.py b/addons/mail/models/mail_channel.py index a27cec795c0..aab6f581ab9 100644 --- a/addons/mail/models/mail_channel.py +++ b/addons/mail/models/mail_channel.py @@ -4,7 +4,6 @@ import base64 import logging import re - from uuid import uuid4 from odoo import _, api, fields, models, modules, tools @@ -547,6 +546,8 @@ class Channel(models.Model): addPreview = self._context.get('isMobile', False) if addPreview: channel_previews = {channel_preview['id']: channel_preview for channel_preview in self.channel_fetch_preview()} + else: + channel_last_message_ids = dict((r['id'], r['message_id']) for r in self._channel_last_message_ids()) for channel in self: info = { @@ -570,7 +571,11 @@ class Channel(models.Model): if addPreview: if channel in channel_previews: info['last_message'] = channel_previews[channel] - + info['last_message_id'] = channel_previews[channel]['last_message']['id'] + else: + info['last_message_id'] = False + else: + info['last_message_id'] = channel_last_message_ids.get(channel.id, False) # listeners of the channel channel_partners = all_partner_channel.filtered(lambda pc: channel.id == pc.channel_id.id) @@ -724,30 +729,48 @@ class Channel(models.Model): if channel_partners: channel_partners.write({'is_pinned': pinned}) - def channel_seen(self): + def channel_seen(self, last_message_id=None): + """ + Mark channel as seen by updating seen message id of the current logged partner + :param last_message_id: the id of the message to be marked as seen, last message of the + thread by default. This param SHOULD be required, the default behaviour is DEPRECATED and + kept only for compatibility reasons. + """ 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 - channel_partner = self.env['mail.channel.partner'].search([('channel_id', 'in', self.ids), ('partner_id', '=', self.env.user.partner_id.id)], limit=1) - 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, - }) - data = { - 'id': channel_partner.id, - 'info': 'channel_seen', - 'last_message_id': last_message_id, - 'partner_id': self.env.user.partner_id.id, - } - 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 + domain = [('channel_ids', 'in', self.ids)] + if last_message_id: + domain = expression.AND([domain, [('id', '<=', last_message_id)]]) + last_message = self.env['mail.message'].search(domain, order="id DESC", limit=1) + if not last_message: + return + + channel_partner_domain = expression.AND([ + [('channel_id', 'in', self.ids)], + [('partner_id', '=', self.env.user.partner_id.id)], + expression.OR([ + [('seen_message_id', '=', False)], + [('seen_message_id', '<', last_message.id)] + ]) + ]) + channel_partner = self.env['mail.channel.partner'].search(channel_partner_domain, limit=1) + if not channel_partner: + return + channel_partner.write({ + 'fetched_message_id': last_message.id, + 'seen_message_id': last_message.id, + }) + + data = { + 'info': 'channel_seen', + 'last_message_id': last_message.id, + 'partner_id': self.env.user.partner_id.id, + } + 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 def channel_fetched(self): """ Broadcast the channel_fetched notification to channel members @@ -929,13 +952,8 @@ class Channel(models.Model): """ Return the last message of the given channels """ if not self: return [] - self._cr.execute(""" - SELECT mail_channel_id AS id, MAX(mail_message_id) AS message_id - FROM mail_message_mail_channel_rel - WHERE mail_channel_id IN %s - GROUP BY mail_channel_id - """, (tuple(self.ids),)) - channels_preview = dict((r['message_id'], r) for r in self._cr.dictfetchall()) + channels_last_message_ids = self._channel_last_message_ids() + channels_preview = dict((r['message_id'], r) for r in channels_last_message_ids) last_messages = self.env['mail.message'].browse(channels_preview).message_format() for message in last_messages: channel = channels_preview[message['id']] @@ -943,6 +961,19 @@ class Channel(models.Model): channel['last_message'] = message return list(channels_preview.values()) + def _channel_last_message_ids(self): + """ Return the last message of the given channels.""" + if not self: + return [] + self.flush() + self.env.cr.execute(""" + SELECT mail_channel_id AS id, MAX(mail_message_id) AS message_id + FROM mail_message_mail_channel_rel + WHERE mail_channel_id IN %s + GROUP BY mail_channel_id + """, (tuple(self.ids),)) + return self.env.cr.dictfetchall() + #------------------------------------------------------ # Commands #------------------------------------------------------ diff --git a/addons/mail/models/mail_thread.py b/addons/mail/models/mail_thread.py index 8cb3695d1db..a013c640779 100644 --- a/addons/mail/models/mail_thread.py +++ b/addons/mail/models/mail_thread.py @@ -2147,7 +2147,7 @@ class MailThread(models.AbstractModel): for channel in channels.filtered(lambda c: c.email_send): users = channel.channel_partner_ids.mapped('user_ids') for user in users.filtered(lambda u: u.notification_type == 'email'): - channel.with_user(user).channel_seen() + channel.with_user(user).channel_seen(message.id) if bus_notifications: self.env['bus.bus'].sudo().sendmany(bus_notifications) diff --git a/addons/mail/static/src/components/chat_window_header/chat_window_header.xml b/addons/mail/static/src/components/chat_window_header/chat_window_header.xml index 63e4fb2cd77..99f982408b7 100644 --- a/addons/mail/static/src/components/chat_window_header/chat_window_header.xml +++ b/addons/mail/static/src/components/chat_window_header/chat_window_header.xml @@ -21,9 +21,9 @@ - +
- () + ()
diff --git a/addons/mail/static/src/components/discuss/tests/discuss_tests.js b/addons/mail/static/src/components/discuss/tests/discuss_tests.js index c63ec07520b..e5885d6f7d4 100644 --- a/addons/mail/static/src/components/discuss/tests/discuss_tests.js +++ b/addons/mail/static/src/components/discuss/tests/discuss_tests.js @@ -513,9 +513,8 @@ QUnit.test('sidebar: channel rendering with needaction counter', async function this.data['mail.channel'].records.push({ id: 20 }); // expected needaction message this.data['mail.message'].records.push({ + channel_ids: [20], // link message to channel id: 100, // random unique id, useful to link notification - model: 'mail.channel', // value to link message to channel - res_id: 20, // id of related channel }); // expected needaction notification this.data['mail.notification'].records.push({ @@ -1744,9 +1743,23 @@ QUnit.test('new messages separator [REQUIRE FOCUS]', async function (assert) { // AKU TODO: thread specific test assert.expect(6); + // Needed partner & user to allow simulation of message reception + this.data['res.partner'].records.push({ + id: 11, + name: "Foreigner partner", + }); + this.data['res.users'].records.push({ + id: 42, + name: "Foreigner user", + partner_id: 11, + }); // channel expected to be rendered, with a random unique id that will be // referenced in the test and the seen_message_id value set to last message - this.data['mail.channel'].records.push({ id: 20, seen_message_id: 125 }); + this.data['mail.channel'].records.push({ + id: 20, + seen_message_id: 125, + uuid: 'randomuuid', + }); for (let i = 1; i <= 25; i++) { this.data['mail.message'].records.push({ channel_ids: [20], @@ -1777,17 +1790,18 @@ QUnit.test('new messages separator [REQUIRE FOCUS]', async function (assert) { document.querySelector(`.o_Discuss_thread .o_ThreadView_messageList`).scrollTop = 0; // composer is focused by default, we remove that focus document.querySelector('.o_ComposerTextInput_textarea').blur(); - // simulate receiving a new message - const data = { - channel_ids: [20], - id: 126, - model: 'mail.channel', - res_id: 20, - }; - await afterNextRender(() => { - const notifications = [[['my-db', 'mail.channel', 20], data]]; - this.widget.call('bus_service', 'trigger', 'notification', notifications); - }); + // simulate receiving a message + await afterNextRender(async () => this.env.services.rpc({ + route: '/mail/chat_post', + params: { + context: { + mockedUserId: 42, + }, + uuid: 'randomuuid', + message_content: "hu", + }, + })); + assert.containsN( document.body, '.o_MessageList_message', @@ -2413,21 +2427,19 @@ QUnit.test('inbox: mark all messages as read', async function (assert) { this.data['mail.message'].records.push( // first expected message { + channel_ids: [20], // link message to channel id: 100, // random unique id, useful to link notification - model: 'mail.channel', // value to link message to channel // needaction needs to be set here for message_fetch domain, because // mocked models don't have computed fields needaction: true, - res_id: 20, // id of related channel }, // second expected message { + channel_ids: [20], // link message to channel id: 101, // random unique id, useful to link notification - model: 'mail.channel', // value to link message to channel // needaction needs to be set here for message_fetch domain, because // mocked models don't have computed fields needaction: true, - res_id: 20, // id of related channel } ); this.data['mail.notification'].records.push( diff --git a/addons/mail/static/src/components/discuss_sidebar_item/discuss_sidebar_item.js b/addons/mail/static/src/components/discuss_sidebar_item/discuss_sidebar_item.js index 2b84762c9dd..26499d0f179 100644 --- a/addons/mail/static/src/components/discuss_sidebar_item/discuss_sidebar_item.js +++ b/addons/mail/static/src/components/discuss_sidebar_item/discuss_sidebar_item.js @@ -44,7 +44,7 @@ class DiscussSidebarItem extends Component { } else if (this.thread.channel_type === 'channel') { return this.thread.message_needaction_counter; } else if (this.thread.channel_type === 'chat') { - return this.thread.message_unread_counter; + return this.thread.localMessageUnreadCounter; } return 0; } diff --git a/addons/mail/static/src/components/discuss_sidebar_item/discuss_sidebar_item.xml b/addons/mail/static/src/components/discuss_sidebar_item/discuss_sidebar_item.xml index 0a56d5c43f5..9b52651f0c6 100644 --- a/addons/mail/static/src/components/discuss_sidebar_item/discuss_sidebar_item.xml +++ b/addons/mail/static/src/components/discuss_sidebar_item/discuss_sidebar_item.xml @@ -6,7 +6,7 @@ t-att-class="{ 'o-active': thread and discuss.thread === thread, 'o-starred-box': thread and thread === env.messaging.starred, - 'o-unread': thread and thread.message_unread_counter > 0, + 'o-unread': thread and thread.localMessageUnreadCounter > 0, }" t-on-click="_onClick" t-att-data-thread-local-id="thread ? thread.localId : undefined" t-att-data-thread-name="thread ? thread.displayName : undefined" > @@ -25,7 +25,7 @@
-
+
@@ -45,7 +45,7 @@