diff options
| author | Xavier Dubuc <xdu@odoo.com> | 2020-07-17 14:50:29 +0000 |
|---|---|---|
| committer | Xavier Dubuc <xdu@odoo.com> | 2020-09-17 13:17:45 +0000 |
| commit | 96e94287c6518f25dee1579259707a05481de294 (patch) | |
| tree | f33e3df40d1f058d0cc2453772afa336ca2a44a0 /addons | |
| parent | d654c05f3e06898a5de025c1b830ba29c039f781 (diff) | |
[FIX] mail: fix "New messages" separator and channel_seen behaviour
- 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#57950
X-original-commit: 04edfa50ccea305050d017c4a1996e561bc7bd57
Signed-off-by: Sébastien Theys (seb) <seb@odoo.com>
Diffstat (limited to 'addons')
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 f15c5d54155..9f090a6ccfc 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 c4c8b7fbb58..ec9710fd94b 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 caff059b408..63ee621ef7c 100644 --- a/addons/mail/models/mail_thread.py +++ b/addons/mail/models/mail_thread.py @@ -2173,7 +2173,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 @@ <t t-if="chatWindow.thread and chatWindow.thread.mass_mailing"> <i class="fa fa-envelope-o" title="Messages are sent by email" role="img"/> </t> - <t t-if="chatWindow.thread and chatWindow.thread.message_unread_counter > 0"> + <t t-if="chatWindow.thread and chatWindow.thread.localMessageUnreadCounter > 0"> <div class="o_ChatWindowHeader_counter o_ChatWindowHeader_item"> - (<t t-esc="chatWindow.thread.message_unread_counter"/>) + (<t t-esc="chatWindow.thread.localMessageUnreadCounter"/>) </div> </t> <div class="o-autogrow"/> 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" > <t t-if="thread"> @@ -25,7 +25,7 @@ </div> </t> <t t-else=""> - <div class="o_DiscussSidebarItem_item o_DiscussSidebarItem_name" t-att-class="{ 'o-item-unread': thread.message_unread_counter > 0 }"> + <div class="o_DiscussSidebarItem_item o_DiscussSidebarItem_name" t-att-class="{ 'o-item-unread': thread.localMessageUnreadCounter > 0 }"> <t t-esc="thread.displayName"/> </div> <t t-if="thread.mass_mailing"> @@ -45,7 +45,7 @@ <div class="o_DiscussSidebarItem_command o_DiscussSidebarItem_commandRename fa fa-cog" t-on-click="_onClickRename" title="Rename conversation" role="img"/> </t> <t t-if="hasUnpin()"> - <t t-if="!thread.message_unread_counter"> + <t t-if="!thread.localMessageUnreadCounter"> <div class="fa fa-times o_DiscussSidebarItem_command o_DiscussSidebarItem_commandUnpin" t-on-click="_onClickUnpin" title="Unpin conversation" role="img"/> </t> </t> diff --git a/addons/mail/static/src/components/message/message_tests.js b/addons/mail/static/src/components/message/message_tests.js index e69c40e35aa..eefd6f3294d 100644 --- a/addons/mail/static/src/components/message/message_tests.js +++ b/addons/mail/static/src/components/message/message_tests.js @@ -543,14 +543,14 @@ QUnit.test('do not show messaging seen indicator if not authored by me', async f id: 11, partnerSeenInfos: [['create', [ { - id: this.env.session.partner_id, + channelId: 11, lastFetchedMessage: [['insert', { id: 100 }]], - partner: [['insert', { id: this.env.messaging.currentPartner.id }]], + partnerId: this.env.messaging.currentPartner.id, }, { - id: 100, + channelId: 11, lastFetchedMessage: [['insert', { id: 100 }]], - partner: [['link', author]], + partnerId: author.id, }, ]]], model: 'mail.channel', @@ -614,14 +614,14 @@ QUnit.test('do not show messaging seen indicator if before last seen by all mess thread.update({ partnerSeenInfos: [['create', [ { - id: currentPartner.id, + channelId: 11, lastSeenMessage: [['link', lastSeenMessage]], - partner: [['link', currentPartner]], + partnerId: this.env.messaging.currentPartner.id, }, { - id: 100, + channelId: 11, lastSeenMessage: [['link', lastSeenMessage]], - partner: [['insert', { id: 100 }]], + partnerId: 100, }, ]]], }); @@ -658,15 +658,15 @@ QUnit.test('only show messaging seen indicator if authored by me, after last see id: 11, partnerSeenInfos: [['create', [ { - id: currentPartner.id, + channelId: 11, lastSeenMessage: [['insert', { id: 100 }]], - partner: [['link', currentPartner]], + partnerId: this.env.messaging.currentPartner.id, }, { - id: 100, - partner: [['insert', { id: 100 }]], + channelId: 11, lastFetchedMessage: [['insert', { id: 100 }]], lastSeenMessage: [['insert', { id: 99 }]], + partnerId: 100, }, ]]], messageSeenIndicators: [['insert', { @@ -957,11 +957,12 @@ QUnit.test('open chat with author on avatar click should be disabled when curren id: 10, }); const thread = await correspondent.getChat(); - const threadView = this.env.models['mail.thread_view'].create({ + const threadViewer = this.env.models['mail.thread_viewer'].create({ + hasThreadView: true, thread: [['link', thread]], }); await this.createMessageComponent(message, { - threadViewLocalId: threadView.localId, + threadViewLocalId: threadViewer.threadView.localId, }); assert.containsOnce( document.body, diff --git a/addons/mail/static/src/components/message_list/message_list.xml b/addons/mail/static/src/components/message_list/message_list.xml index 0a87558e839..abc83a41f0d 100644 --- a/addons/mail/static/src/components/message_list/message_list.xml +++ b/addons/mail/static/src/components/message_list/message_list.xml @@ -43,7 +43,7 @@ <!-- MESSAGES --> <t t-set="current_day" t-value="0"/> <t t-set="prev_message" t-value="0"/> - <t t-if="threadView.thread.model === 'mail.channel' and threadView.thread.message_unread_counter > 0 and !threadView.thread.seen_message_id"> + <t t-if="threadView.thread.model === 'mail.channel' and threadView.thread.localMessageUnreadCounter > 0 and !threadView.thread.lastSeenByCurrentPartnerMessageId"> <div class="o_MessageList_separator o_MessageList_separatorNewMessages o_MessageList_item" t-att-class="{ 'o-disable-animation': env.disableAnimation }" t-transition="fade"> <hr class="o_MessageList_separatorLine o_MessageList_separatorLineNewMessages"/><span class="o_MessageList_separatorLabel o_MessageList_separatorLabelNewMessages">New messages</span> </div> @@ -75,7 +75,7 @@ t-ref="{{ message.localId }}" /> <t t-set="prev_message" t-value="message"/> - <t t-if="threadView.thread.model === 'mail.channel' and threadView.thread.message_unread_counter > 0 and threadView.thread.seen_message_id === message.id"> + <t t-if="threadView.thread.model === 'mail.channel' and threadView.thread.localMessageUnreadCounter > 0 and threadView.thread.lastSeenByCurrentPartnerMessageId === message.id"> <div class="o_MessageList_separator o_MessageList_separatorNewMessages o_MessageList_item" t-att-class="{ 'o-disable-animation': env.disableAnimation }" t-transition="fade"> <hr class="o_MessageList_separatorLine o_MessageList_separatorLineNewMessages"/><span class="o_MessageList_separatorLabel o_MessageList_separatorLabelNewMessages">New messages</span> </div> 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 66866394a17..9eb61e6a100 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 @@ -51,13 +51,13 @@ QUnit.test('rendering when just one has received the message', async function (a model: 'mail.channel', partnerSeenInfos: [['create', [ { - id: 10, - partner: [['create', { id: 10 }]], - lastFetchedMessage: [['insert', { id: 100 }]] + channelId: 1000, + lastFetchedMessage: [['insert', { id: 100 }]], + partnerId: 10, }, { - id: 100, - partner: [['create', { id: 100 }]], + channelId: 1000, + partnerId: 100, }, ]]], messageSeenIndicators: [['insert', { @@ -98,14 +98,14 @@ QUnit.test('rendering when everyone have received the message', async function ( model: 'mail.channel', partnerSeenInfos: [['create', [ { - id: 10, - partner: [['create', { id: 10 }]], + channelId: 1000, lastFetchedMessage: [['insert', { id: 100 }]], + partnerId: 10, }, { - id: 100, - partner: [['create', { id: 100 }]], + channelId: 1000, lastFetchedMessage: [['insert', { id: 99 }]], + partnerId: 100, }, ]]], messageSeenIndicators: [['insert', { @@ -146,15 +146,15 @@ QUnit.test('rendering when just one has seen the message', async function (asser model: 'mail.channel', partnerSeenInfos: [['create', [ { - id: 10, - partner: [['create', { id: 10 }]], + channelId: 1000, lastFetchedMessage: [['insert', { id: 100 }]], lastSeenMessage: [['insert', { id: 100 }]], + partnerId: 10, }, { - id: 100, - partner: [['create', { id: 100 }]], + channelId: 1000, lastFetchedMessage: [['insert', { id: 99 }]], + partnerId: 100, }, ]]], messageSeenIndicators: [['insert', { @@ -196,14 +196,14 @@ QUnit.test('rendering when just one has seen & received the message', async func model: 'mail.channel', partnerSeenInfos: [['create', [ { - id: 10, - partner: [['create', { id: 10 }]], + channelId: 1000, lastFetchedMessage: [['insert', { id: 100 }]], lastSeenMessage: [['insert', { id: 100 }]], + partnerId: 10, }, { - id: 100, - partner: [['create', { id: 100 }]], + channelId: 1000, + partnerId: 100, }, ]]], messageSeenIndicators: [['insert', { @@ -245,16 +245,16 @@ QUnit.test('rendering when just everyone has seen the message', async function ( model: 'mail.channel', partnerSeenInfos: [['create', [ { - id: 10, - partner: [['create', { id: 10 }]], + channelId: 1000, lastFetchedMessage: [['insert', { id: 100 }]], lastSeenMessage: [['insert', { id: 100 }]], + partnerId: 10, }, { - id: 100, - partner: [['create', { id: 100 }]], + channelId: 1000, lastFetchedMessage: [['insert', { id: 100 }]], lastSeenMessage: [['insert', { id: 100 }]], + partnerId: 100, }, ]]], messageSeenIndicators: [['insert', { diff --git a/addons/mail/static/src/components/notification_list/notification_list.js b/addons/mail/static/src/components/notification_list/notification_list.js index 99b66fd5a38..a36bdaa34de 100644 --- a/addons/mail/static/src/components/notification_list/notification_list.js +++ b/addons/mail/static/src/components/notification_list/notification_list.js @@ -99,10 +99,10 @@ class NotificationList extends Component { // thread notifications const threadNotifications = threads .sort((t1, t2) => { - if (t1.message_unread_counter > 0 && t2.message_unread_counter === 0) { + if (t1.localMessageUnreadCounter > 0 && t2.localMessageUnreadCounter === 0) { return -1; } - if (t1.message_unread_counter === 0 && t2.message_unread_counter > 0) { + if (t1.localMessageUnreadCounter === 0 && t2.localMessageUnreadCounter > 0) { return 1; } if (t1.lastMessage && t2.lastMessage) { diff --git a/addons/mail/static/src/components/thread_preview/thread_preview.js b/addons/mail/static/src/components/thread_preview/thread_preview.js index 6e30005225c..0e5a226a5f2 100644 --- a/addons/mail/static/src/components/thread_preview/thread_preview.js +++ b/addons/mail/static/src/components/thread_preview/thread_preview.js @@ -108,7 +108,9 @@ class ThreadPreview extends Component { * @param {MouseEvent} ev */ _onClickMarkAsRead(ev) { - this.thread.markAsSeen(); + if (this.thread.lastMessage) { + this.thread.markAsSeen(this.thread.lastMessage.id); + } } } diff --git a/addons/mail/static/src/components/thread_preview/thread_preview.xml b/addons/mail/static/src/components/thread_preview/thread_preview.xml index da364d4f99d..8a4baf3dfeb 100644 --- a/addons/mail/static/src/components/thread_preview/thread_preview.xml +++ b/addons/mail/static/src/components/thread_preview/thread_preview.xml @@ -6,7 +6,7 @@ The preview template is used by the discuss in mobile, and by the systray menu in order to show preview of threads. --> - <div class="o_ThreadPreview" t-att-class="{ 'o-muted': thread and thread.message_unread_counter === 0 }" t-on-click="_onClick" t-att-data-thread-local-id="thread ? thread.localId : undefined"> + <div class="o_ThreadPreview" t-att-class="{ 'o-muted': thread and thread.localMessageUnreadCounter === 0 }" t-on-click="_onClick" t-att-data-thread-local-id="thread ? thread.localId : undefined"> <t t-if="thread"> <div class="o_ThreadPreview_sidebar"> <div class="o_ThreadPreview_imageContainer o_ThreadPreview_sidebarItem"> @@ -16,7 +16,7 @@ class="o_ThreadPreview_partnerImStatusIcon" t-att-class="{ 'o-mobile': env.messaging.device.isMobile, - 'o-muted': thread.message_unread_counter === 0, + 'o-muted': thread.localMessageUnreadCounter === 0, }" partnerLocalId="thread.correspondent.localId" /> @@ -25,17 +25,17 @@ </div> <div class="o_ThreadPreview_content"> <div class="o_ThreadPreview_header"> - <span class="o_ThreadPreview_name" t-att-class="{ 'o-mobile': env.messaging.device.isMobile, 'o-muted': thread.message_unread_counter === 0 }"> + <span class="o_ThreadPreview_name" t-att-class="{ 'o-mobile': env.messaging.device.isMobile, 'o-muted': thread.localMessageUnreadCounter === 0 }"> <t t-esc="thread.displayName"/> </span> - <t t-if="thread.message_unread_counter > 0"> - <span class="o_ThreadPreview_counter" t-att-class="{ 'o-muted': thread.message_unread_counter === 0 }"> - (<t t-esc="thread.message_unread_counter"/>) + <t t-if="thread.localMessageUnreadCounter > 0"> + <span class="o_ThreadPreview_counter" t-att-class="{ 'o-muted': thread.localMessageUnreadCounter === 0 }"> + (<t t-esc="thread.localMessageUnreadCounter"/>) </span> </t> <span class="o-autogrow"/> <t t-if="thread.lastMessage"> - <span class="o_ThreadPreview_date" t-att-class="{ 'o-muted': thread.message_unread_counter === 0 }"> + <span class="o_ThreadPreview_date" t-att-class="{ 'o-muted': thread.localMessageUnreadCounter === 0 }"> <t t-esc="thread.lastMessage.date.fromNow()"/> </span> </t> @@ -51,7 +51,7 @@ <t t-esc="inlineLastMessageBody"/> </span> <span class="o-autogrow"/> - <t t-if="thread.message_unread_counter > 0"> + <t t-if="thread.localMessageUnreadCounter > 0"> <span class="o_ThreadPreview_coreItem o_ThreadPreview_markAsRead fa fa-check" title="Mark as Read" t-on-click="_onClickMarkAsRead" t-ref="markAsRead"/> </t> </div> diff --git a/addons/mail/static/src/components/thread_preview/thread_preview_tests.js b/addons/mail/static/src/components/thread_preview/thread_preview_tests.js index ed6ce3b6356..981abf6bca6 100644 --- a/addons/mail/static/src/components/thread_preview/thread_preview_tests.js +++ b/addons/mail/static/src/components/thread_preview/thread_preview_tests.js @@ -41,7 +41,17 @@ QUnit.module('thread_preview_tests.js', { }); QUnit.test('mark as read', async function (assert) { - assert.expect(4); + assert.expect(8); + this.data['mail.channel'].records.push({ + id: 11, + message_unread_counter: 1, + }); + this.data['mail.message'].records.push({ + channel_ids: [11], + id: 100, + model: 'mail.channel', + res_id: 11, + }); await this.start({ hasChatWindow: true, @@ -52,16 +62,20 @@ QUnit.test('mark as read', async function (assert) { return this._super(...arguments); }, }); - const thread = this.env.models['mail.thread'].create({ + const thread = this.env.models['mail.thread'].findFromIdentifyingData({ id: 11, - message_unread_counter: 1, model: 'mail.channel', }); await this.createThreadPreviewComponent({ threadLocalId: thread.localId }); assert.containsOnce( document.body, '.o_ThreadPreview_markAsRead', - "should have 1 mark as read button" + "should have the mark as read button" + ); + assert.containsOnce( + document.body, + '.o_ThreadPreview_counter', + "should have an unread counter" ); await afterNextRender(() => @@ -71,6 +85,21 @@ QUnit.test('mark as read', async function (assert) { ['channel_seen'], "should have marked the thread as seen" ); + assert.hasClass( + document.querySelector('.o_ThreadPreview'), + 'o-muted', + "should be muted once marked as read" + ); + assert.containsNone( + document.body, + '.o_ThreadPreview_markAsRead', + "should no longer have the mark as read button" + ); + assert.containsNone( + document.body, + '.o_ThreadPreview_counter', + "should no longer have an unread counter" + ); assert.containsNone( document.body, '.o_ChatWindow', diff --git a/addons/mail/static/src/components/thread_view/thread_view_tests.js b/addons/mail/static/src/components/thread_view/thread_view_tests.js index 2d0b3207ae1..3a141e751ec 100644 --- a/addons/mail/static/src/components/thread_view/thread_view_tests.js +++ b/addons/mail/static/src/components/thread_view/thread_view_tests.js @@ -272,8 +272,6 @@ QUnit.test('mark channel as fetched when a new message is loaded and as seen whe } }); const thread = this.env.models['mail.thread'].create({ - // FIXME should not be needed, see task-2277537 - composer: [['create']], // avoid initial focus id: 100, isServerPinned: true, // just to avoid joinChannel to be called members: [['insert', [ @@ -288,8 +286,8 @@ QUnit.test('mark channel as fetched when a new message is loaded and as seen whe name: "Fred", }, ]]], - message_unread_counter: 1, // seen would not be called if not > 0 model: 'mail.channel', + serverMessageUnreadCounter: 1, // seen would not be called if not > 0 }); const threadViewer = this.env.models['mail.thread_viewer'].create({ @@ -361,8 +359,8 @@ QUnit.test('mark channel as fetched and seen when a new message is loaded if com name: "Fred", }, ]]], - message_unread_counter: 1, // seen would not be called if not > 0 model: 'mail.channel', + serverMessageUnreadCounter: 1, // seen would not be called if not > 0 }); const threadViewer = this.env.models['mail.thread_viewer'].create({ @@ -431,6 +429,143 @@ QUnit.test('show message subject if thread is mailing channel', async function ( ); }); +QUnit.test('new messages separator on posting message', async function (assert) { + assert.expect(4); + + this.data['mail.channel'].records = [{ + channel_type: 'channel', + id: 20, + is_pinned: true, + message_unread_counter: 0, + name: "General", + }]; + await this.start(); + const thread = this.env.models['mail.thread'].findFromIdentifyingData({ + id: 20, + model: 'mail.channel' + }); + const threadViewer = this.env.models['mail.thread_viewer'].create({ + hasThreadView: true, + thread: [['link', thread]], + }); + await this.createThreadViewComponent(threadViewer.threadView, { hasComposer: true }); + + assert.containsNone( + document.body, + '.o_MessageList_message', + "should have no messages" + ); + assert.containsNone( + document.body, + '.o_MessageList_separatorNewMessages', + "should not display 'new messages' separator" + ); + + document.querySelector('.o_ComposerTextInput_textarea').focus(); + await afterNextRender(() => document.execCommand('insertText', false, "hey !")); + await afterNextRender(() => + document.querySelector('.o_Composer_buttonSend').click() + ); + assert.containsOnce( + document.body, + '.o_Message', + "should have the message current partner just posted" + ); + assert.containsNone( + document.body, + '.o_MessageList_separatorNewMessages', + "still no separator shown when current partner posted a message" + ); +}); + +QUnit.test('new messages separator on receiving new message', async function (assert) { + assert.expect(6); + + 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, + }); + this.data['mail.channel'].records.push({ + channel_type: 'channel', + id: 20, + is_pinned: true, + message_unread_counter: 0, + name: "General", + seen_message_id: 1, + uuid: 'randomuuid', + }); + this.data['mail.message'].records.push({ + body: "blah", + channel_ids: [20], + id: 1, + }); + await this.start(); + const thread = this.env.models['mail.thread'].findFromIdentifyingData({ + id: 20, + model: 'mail.channel' + }); + const threadViewer = this.env.models['mail.thread_viewer'].create({ + hasThreadView: true, + thread: [['link', thread]], + }); + await this.createThreadViewComponent(threadViewer.threadView, { hasComposer: true }); + + assert.containsOnce( + document.body, + '.o_MessageList_message', + "should have an initial message" + ); + assert.containsNone( + document.body, + '.o_MessageList_separatorNewMessages', + "should not display 'new messages' separator" + ); + + document.querySelector('.o_ComposerTextInput_textarea').blur(); + // simulate receiving a message + await afterNextRender(async () => this.env.services.rpc({ + route: '/mail/chat_post', + params: { + context: { + mockedUserId: 42, + }, + uuid: thread.uuid, + message_content: "hu", + }, + })); + assert.containsN( + document.body, + '.o_Message', + 2, + "should now have 2 messages after receiving a new message" + ); + assert.containsOnce( + document.body, + '.o_MessageList_separatorNewMessages', + "'new messages' separator should be shown" + ); + + assert.containsOnce( + document.body, + `.o_MessageList_separatorNewMessages ~ .o_Message[data-message-local-id="${ + this.env.models['mail.message'].findFromIdentifyingData({id: 2}).localId + }"]`, + "'new messages' separator should be shown above new message received" + ); + + await afterNextRender(() => document.querySelector('.o_ComposerTextInput_textarea').focus()); + assert.containsNone( + document.body, + '.o_MessageList_separatorNewMessages', + "'new messages' separator should no longer be shown as last message has been seen" + ); +}); + }); }); }); diff --git a/addons/mail/static/src/models/chat_window_manager/chat_window_manager.js b/addons/mail/static/src/models/chat_window_manager/chat_window_manager.js index ed4db95f585..45561e6b039 100644 --- a/addons/mail/static/src/models/chat_window_manager/chat_window_manager.js +++ b/addons/mail/static/src/models/chat_window_manager/chat_window_manager.js @@ -286,7 +286,7 @@ function factory(dependencies) { ); let amount = 0; for (const chatWindow of allHiddenWithThread) { - if (chatWindow.thread.message_unread_counter > 0) { + if (chatWindow.thread.localMessageUnreadCounter > 0) { amount++; } } @@ -393,7 +393,7 @@ function factory(dependencies) { related: 'allOrderedHidden.thread', }), allOrderedHiddenThreadMessageUnreadCounter: attr({ - related: 'allOrderedHiddenThread.message_unread_counter', + related: 'allOrderedHiddenThread.localMessageUnreadCounter', }), allOrderedVisible: one2many('mail.chat_window', { compute: '_computeAllOrderedVisible', 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 78f7431214f..bab1ef2046e 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,6 +13,8 @@ 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 @@ -192,6 +194,12 @@ function factory(dependencies) { } const otherPartnersThatHaveFetched = this.thread.partnerSeenInfos .filter(partnerSeenInfo => + /** + * Relation may not be set yet immediately + * @see mail.thread_partner_seen_info:partnerId field + * FIXME task-2278551 + */ + partnerSeenInfo.partner && partnerSeenInfo.partner !== this.message.author && partnerSeenInfo.lastFetchedMessage && partnerSeenInfo.lastFetchedMessage.id >= this.message.id @@ -216,6 +224,12 @@ function factory(dependencies) { } const otherPartnersThatHaveSeen = this.thread.partnerSeenInfos .filter(partnerSeenInfo => + /** + * Relation may not be set yet immediately + * @see mail.thread_partner_seen_info:partnerId field + * FIXME task-2278551 + */ + partnerSeenInfo.partner && partnerSeenInfo.partner !== this.message.author && partnerSeenInfo.lastSeenMessage && partnerSeenInfo.lastSeenMessage.id >= this.message.id) diff --git a/addons/mail/static/src/models/messaging_menu/messaging_menu.js b/addons/mail/static/src/models/messaging_menu/messaging_menu.js index 9f1cd8d7218..615082ea847 100644 --- a/addons/mail/static/src/models/messaging_menu/messaging_menu.js +++ b/addons/mail/static/src/models/messaging_menu/messaging_menu.js @@ -72,7 +72,7 @@ function factory(dependencies) { } const inboxMailbox = this.env.messaging.inbox; const unreadChannels = this.env.models['mail.thread'].all(thread => - thread.message_unread_counter > 0 && + thread.localMessageUnreadCounter > 0 && thread.model === 'mail.channel' ); let counter = unreadChannels.length; 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 0ce0279e87d..12976aaadad 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 @@ -104,7 +104,6 @@ function factory(dependencies) { */ async _handleNotificationChannel(channelId, data) { const { - id, info, is_typing, last_message_id, @@ -113,13 +112,11 @@ function factory(dependencies) { switch (info) { case 'channel_fetched': return this._handleNotificationChannelFetched(channelId, { - id, last_message_id, partner_id, }); case 'channel_seen': return this._handleNotificationChannelSeen(channelId, { - id, last_message_id, partner_id, }); @@ -137,12 +134,10 @@ function factory(dependencies) { * @private * @param {integer} channelId * @param {Object} param1 - * @param {integer} param1.id * @param {integer} param1.last_message_id * @param {integer} param1.partner_id */ async _handleNotificationChannelFetched(channelId, { - id, last_message_id, partner_id, }) { @@ -159,14 +154,12 @@ function factory(dependencies) { // disabled on `channel` channels for performance reasons return; } + this.env.models['mail.thread_partner_seen_info'].insert({ + channelId: channel.id, + lastFetchedMessage: [['insert', { id: last_message_id }]], + partnerId: partner_id, + }); channel.update({ - partnerSeenInfos: [['insert', - { - lastFetchedMessage: [['insert', {id: last_message_id}]], - id, - partner: [['insert', {id: partner_id}]], - } - ]], messageSeenIndicators: [['insert', { id: this.env.models['mail.message_seen_indicator'].computeId(last_message_id, channel.id), @@ -279,26 +272,21 @@ function factory(dependencies) { if (!wasChannelExisting) { return; } - channel.update({ - message_unread_counter: channel.message_unread_counter + 1, - }); // manually force recompute of counter this.messaging.messagingMenu.update(); } /** - * Called when a channel has been seen, and the server responses with the + * Called when a channel has been seen, and the server responds with the * last message seen. Useful in order to track last message seen. * * @private * @param {integer} channelId * @param {Object} param1 - * @param {integer} param1.id * @param {integer} param1.last_message_id * @param {integer} param1.partner_id */ async _handleNotificationChannelSeen(channelId, { - id, last_message_id, partner_id, }) { @@ -311,38 +299,40 @@ function factory(dependencies) { // knowledge of the channel return; } - if (channel.channel_type === 'channel') { - // disabled on `channel` channels for performance reasons - return; - } const lastMessage = this.env.models['mail.message'].insert({id: last_message_id}); - const updateData = { - partnerSeenInfos: [['insert', - { - id, - lastFetchedMessage: [['link', lastMessage]], - lastSeenMessage: [['link', lastMessage]], - partner: [['insert', {id: partner_id}]], - }], - ], - messageSeenIndicators: [['insert', - { - id: this.env.models['mail.message_seen_indicator'].computeId(last_message_id, channel.id), - message: [['link', lastMessage]], - } - ]], - }; + // restrict computation of seen indicator for "non-channel" channels + // for performance reasons + const shouldComputeSeenIndicators = channel.channel_type !== 'channel'; + const updateData = {}; + if (shouldComputeSeenIndicators) { + this.env.models['mail.thread_partner_seen_info'].insert({ + channelId: channel.id, + lastSeenMessage: [['link', lastMessage]], + partnerId: partner_id, + }); + Object.assign(updateData, { + // 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]], + }, + ]], + }); + } if (this.env.messaging.currentPartner.id === partner_id) { Object.assign(updateData, { - message_unread_counter: 0, - seen_message_id: last_message_id, + lastSeenByCurrentPartnerMessageId: last_message_id, + pendingSeenMessageId: undefined, }); } channel.update(updateData); - // FIXME force the computing of thread values (cf task-2261221) - this.env.models['mail.thread'].computeLastCurrentPartnerMessageSeenByEveryone(channel); - // FIXME force the computing of message values (cf task-2261221) - this.env.models['mail.message_seen_indicator'].recomputeSeenValues(channel); + if (shouldComputeSeenIndicators) { + // FIXME force the computing of thread values (cf task-2261221) + this.env.models['mail.thread'].computeLastCurrentPartnerMessageSeenByEveryone(channel); + // FIXME force the computing of message values (cf task-2261221) + this.env.models['mail.message_seen_indicator'].recomputeSeenValues(channel); + } // manually force recompute of counter this.messaging.messagingMenu.update(); } @@ -420,6 +410,11 @@ function factory(dependencies) { this.env.bus.trigger('activity_updated', data); } else if (type === 'author') { return this._handleNotificationPartnerAuthor(data); + } else if (info === 'channel_seen') { + return this._handleNotificationChannelSeen(data.channel_id, { + last_message_id: data.last_message_id, + partner_id: this.env.messaging.currentPartner.id, + }); } else if (type === 'deletion') { return this._handleNotificationPartnerDeletion(data); } else if (type === 'message_notification_update') { diff --git a/addons/mail/static/src/models/thread/thread.js b/addons/mail/static/src/models/thread/thread.js index ea67741f9a5..75a53f67259 100644 --- a/addons/mail/static/src/models/thread/thread.js +++ b/addons/mail/static/src/models/thread/thread.js @@ -132,7 +132,9 @@ function factory(dependencies) { * @return {Object} */ static convertData(data) { - const data2 = {}; + const data2 = { + messagesAsServerChannel: [], + }; if ('model' in data) { data2.model = data.model; } @@ -168,6 +170,14 @@ function factory(dependencies) { data2.isServerPinned = true; } } + if ('last_message' in data && data.last_message) { + data2.messagesAsServerChannel.push(['insert', { id: data.last_message.id }]); + data2.serverLastMessageId = data.last_message.id; + } + if ('last_message_id' in data && data.last_message_id) { + data2.messagesAsServerChannel.push(['insert', { id: data.last_message_id }]); + data2.serverLastMessageId = data.last_message_id; + } if ('mass_mailing' in data) { data2.mass_mailing = data.mass_mailing; } @@ -178,7 +188,7 @@ function factory(dependencies) { data2.message_needaction_counter = data.message_needaction_counter; } if ('message_unread_counter' in data) { - data2.message_unread_counter = data.message_unread_counter; + data2.serverMessageUnreadCounter = data.message_unread_counter; } if ('name' in data) { data2.name = data.name; @@ -187,7 +197,7 @@ function factory(dependencies) { data2.public = data.public; } if ('seen_message_id' in data) { - data2.seen_message_id = data.seen_message_id; + data2.lastSeenByCurrentPartnerMessageId = data.seen_message_id; } if ('uuid' in data) { data2.uuid = data.uuid; @@ -212,18 +222,18 @@ function factory(dependencies) { data2.partnerSeenInfos = [ ['insert-and-replace', data.seen_partners_info.map( - ({ fetched_message_id, id, partner_id, seen_message_id}) => { + ({ fetched_message_id, partner_id, seen_message_id }) => { return { - id, - lastFetchedMessage: [seen_message_id ? ['insert', {id: seen_message_id}] : ['unlink-all']], - lastSeenMessage: [fetched_message_id ? ['insert', {id: fetched_message_id}] : ['unlink-all']], - partner: [['insert', {id: partner_id}]], + channelId: data2.id, + lastFetchedMessage: [fetched_message_id ? ['insert', { id: fetched_message_id }] : ['unlink-all']], + lastSeenMessage: [seen_message_id ? ['insert', { id: seen_message_id }] : ['unlink-all']], + partnerId: partner_id, }; }) ] ]; if (data.id || this.id) { - const messageIds = data.seen_partners_info.reduce((currentSet, { fetched_message_id, seen_message_id}) => { + const messageIds = data.seen_partners_info.reduce((currentSet, { fetched_message_id, seen_message_id }) => { if (fetched_message_id) { currentSet.add(fetched_message_id); } @@ -238,7 +248,7 @@ function factory(dependencies) { [...messageIds].map(messageId => { return { id: this.env.models['mail.message_seen_indicator'].computeId(messageId, data.id || this.id), - message: [['insert', {id: messageId}]], + message: [['insert', { id: messageId }]], }; }) ] @@ -297,6 +307,26 @@ function factory(dependencies) { return channels; } + + /** + * Performs the `channel_seen` RPC on `mail.channel`. + * + * @static + * @param {Object} param0 + * @param {integer[]} param0.ids list of id of channels + * @param {integer[]} param0.lastMessageId + */ + static async performRpcChannelSeen({ ids, lastMessageId }) { + return this.env.services.rpc({ + model: 'mail.channel', + method: 'channel_seen', + args: [ids], + kwargs: { + last_message_id: lastMessageId, + }, + }, { shadow: true }); + } + /** * Performs the `channel_create` RPC on `mail.channel`. * @@ -464,20 +494,24 @@ function factory(dependencies) { /** * Mark the specified conversation as read/seen. + * + * @param {integer} messageId the message to be considered as last seen */ - async markAsSeen() { - if (this.message_unread_counter === 0) { + async markAsSeen(messageId) { + if (this.model !== 'mail.channel') { return; } - if (this.model === 'mail.channel') { - const seen_message_id = await this.async(() => this.env.services.rpc({ - model: 'mail.channel', - method: 'channel_seen', - args: [[this.id]] - }, { shadow: true })); - this.update({ seen_message_id }); + if (this.pendingSeenMessageId && messageId <= this.pendingSeenMessageId) { + return; } - this.update({ message_unread_counter: 0 }); + if ( + this.lastSeenByCurrentPartnerMessageId && + messageId <= this.lastSeenByCurrentPartnerMessageId + ) { + return; + } + this.update({ pendingSeenMessageId: messageId }); + return this.env.models['mail.thread'].performRpcChannelSeen({ ids: [this.id], lastMessageId: messageId }); } /** @@ -960,6 +994,43 @@ function factory(dependencies) { /** * @private + * @returns {integer} + */ + _computeLocalMessageUnreadCounter() { + // the only situations where serverMessageUnreadCounter can/have to be + // trusted are: + // - we have no last message (and then no messages at all) + // - the message it used to compute is the last message we know + if (this.orderedMessages.length === 0) { + return this.serverMessageUnreadCounter; + } + // from here serverLastMessageId is not undefined because + // orderedMessages contain at least one message. + if (!this.lastSeenByCurrentPartnerMessageId) { + return this.serverMessageUnreadCounter; + } + const firstMessage = this.orderedMessages[0]; + // if the lastSeenByCurrentPartnerMessageId is not known (not fetched), then we + // need to rely on server value to determine the amount of unread + // messages until the last message it knew when computing the + // serverMessageUnreadCounter + if (this.lastSeenByCurrentPartnerMessageId < firstMessage.id) { + const fetchedNotSeenMessages = this.orderedMessages.filter(message => + message.id > this.serverLastMessageId + ); + return this.serverMessageUnreadCounter + fetchedNotSeenMessages.length; + } + // lastSeenByCurrentPartnerMessageId is a known message, + // then we can forget serverMessageUnreadCounter + const maxId = Math.max(this.serverLastMessageId, this.lastSeenByCurrentPartnerMessageId); + return this.orderedMessages.reduce( + (acc, message) => acc + (message.id > maxId ? 1 : 0), + 0 + ); + } + + /** + * @private * @returns {mail.messaging} */ _computeMessaging() { @@ -1355,6 +1426,32 @@ function factory(dependencies) { compute: '_computeLastNeedactionMessage', dependencies: ['needactionMessages'], }), + /** + * Last seen message id of the channel by current partner. + * + * If there is a pending seen message id change, it is immediately applied + * on the interface to avoid a feeling of unresponsiveness. Otherwise the + * last known message id of the server is used. + * + * Also, it needs to be kept as an id because it's considered like a "date" and could stay + * even if corresponding message is deleted. It is basically used to know which + * messages are before or after it. + */ + lastSeenByCurrentPartnerMessageId: attr(), + /** + * Local value of message unread counter, that means it is based on initial server value and + * updated with interface updates. + */ + localMessageUnreadCounter: attr({ + compute: '_computeLocalMessageUnreadCounter', + dependencies: [ + 'lastMessage', + 'lastSeenByCurrentPartnerMessageId', + 'orderedMessages', + 'serverLastMessageId', + 'serverMessageUnreadCounter', + ], + }), mainCache: one2one('mail.thread_cache', { compute: '_computeMainCache', }), @@ -1367,9 +1464,6 @@ function factory(dependencies) { message_needaction_counter: attr({ default: 0, }), - message_unread_counter: attr({ - default: 0, - }), /** * All messages that this thread is linked to. * Note that this field is automatically computed by inverse @@ -1457,8 +1551,13 @@ function factory(dependencies) { * interface and to notify the server of the new state. */ pendingFoldState: attr(), + /** + * Determine if there is a pending seen message change, which is a change + * of seen message requested by the client but not yet confirmed by the + * server. + */ + pendingSeenMessageId: attr(), public: attr(), - seen_message_id: attr(), /** * Determine the last fold state known by the server, which is the fold * state displayed after initialization or when the last pending @@ -1472,6 +1571,29 @@ function factory(dependencies) { default: 'closed', }), /** + * Last message id considered by the server. + * + * Useful to compute localMessageUnreadCounter field. + * + * @see localMessageUnreadCounter + */ + serverLastMessageId: attr({ + default: 0, + }), + /** + * Message unread counter coming from server. + * + * Value of this field is unreliable, due to dynamic nature of + * messaging. So likely outdated/unsync with server. Should use + * localMessageUnreadCounter instead, which smartly guess the actual + * message unread counter at all time. + * + * @see localMessageUnreadCounter + */ + serverMessageUnreadCounter: attr({ + default: 0, + }), + /** * Members that are currently typing something in the composer of this * thread, including current partner. */ diff --git a/addons/mail/static/src/models/thread/thread_tests.js b/addons/mail/static/src/models/thread/thread_tests.js index eb4ea3edc53..8f9efe0111f 100644 --- a/addons/mail/static/src/models/thread/thread_tests.js +++ b/addons/mail/static/src/models/thread/thread_tests.js @@ -65,10 +65,10 @@ QUnit.test('create (channel)', async function (assert) { name: "Fred", }]]], message_needaction_counter: 6, - message_unread_counter: 5, model: 'mail.channel', name: "General", public: 'public', + serverMessageUnreadCounter: 5, }); assert.ok(thread); assert.ok(this.env.models['mail.partner'].find(partner => partner.id === 9)); @@ -89,9 +89,9 @@ QUnit.test('create (channel)', async function (assert) { assert.ok(thread.members.includes(partner9)); assert.ok(thread.members.includes(partner10)); assert.strictEqual(thread.message_needaction_counter, 6); - assert.strictEqual(thread.message_unread_counter, 5); assert.strictEqual(thread.name, "General"); assert.strictEqual(thread.public, 'public'); + assert.strictEqual(thread.serverMessageUnreadCounter, 5); assert.strictEqual(partner9.email, "john@example.com"); assert.strictEqual(partner9.id, 9); assert.strictEqual(partner9.name, "John"); diff --git a/addons/mail/static/src/models/thread_partner_seen_info/thread_partner_seen_info.js b/addons/mail/static/src/models/thread_partner_seen_info/thread_partner_seen_info.js index 1a2d19480a3..8fd3b95af45 100644 --- a/addons/mail/static/src/models/thread_partner_seen_info/thread_partner_seen_info.js +++ b/addons/mail/static/src/models/thread_partner_seen_info/thread_partner_seen_info.js @@ -16,7 +16,27 @@ function factory(dependencies) { * @override */ static _createRecordLocalId(data) { - return `${this.modelName}_${data.id}`; + const { channelId, partnerId } = data; + return `${this.modelName}_${channelId}_${partnerId}`; + } + + /** + * @private + * @returns {mail.partner|undefined} + */ + _computePartner() { + return [['insert', { id: this.partnerId }]]; + } + + /** + * @private + * @returns {mail.thread|undefined} + */ + _computeThread() { + return [['insert', { + id: this.channelId, + model: 'mail.channel', + }]]; } } @@ -24,11 +44,59 @@ function factory(dependencies) { ThreadPartnerSeenInfo.modelName = 'mail.thread_partner_seen_info'; ThreadPartnerSeenInfo.fields = { - id: attr(), + /** + * The id of channel this seen info is related to. + * + * Should write on this field to set relation between the channel and + * this seen info, not on `thread`. + * + * Reason for not setting the relation directly is the necessity to + * uniquely identify a seen info based on channel and partner 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(), lastFetchedMessage: many2one('mail.message'), lastSeenMessage: many2one('mail.message'), - partner: many2one('mail.partner'), + /** + * Partner that this seen info is related to. + * + * Should not write on this field to update relation, and instead + * should write on @see partnerId field. + */ + partner: many2one('mail.partner', { + compute: '_computePartner', + dependencies: ['partnerId'], + }), + /** + * The id of partner this seen info is related to. + * + * Should write on this field to set relation between the partner and + * this seen info, not on `partner`. + * + * Reason for not setting the relation directly is the necessity to + * uniquely identify a seen info based on channel and partner 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. + */ + partnerId: attr(), + /** + * Thread (channel) that this seen info is related to. + * + * Should not write on this field to update relation, and instead + * should write on @see channelId field. + */ thread: many2one('mail.thread', { + compute: '_computeThread', + dependencies: ['channelId'], inverse: 'partnerSeenInfos', }), }; diff --git a/addons/mail/static/src/models/thread_view/thread_view.js b/addons/mail/static/src/models/thread_view/thread_view.js index d7ea1508067..599a99cc936 100644 --- a/addons/mail/static/src/models/thread_view/thread_view.js +++ b/addons/mail/static/src/models/thread_view/thread_view.js @@ -95,7 +95,7 @@ function factory(dependencies) { const lastMessageIsVisible = this.lastVisibleMessage && this.lastVisibleMessage === this.lastMessage; if (lastMessageIsVisible && this.hasComposerFocus && this.thread) { - this.thread.markAsSeen().catch(e => { + this.thread.markAsSeen(this.lastMessage.id).catch(e => { // prevent crash when executing compute during destroy if (!(e instanceof RecordDeletedError)) { throw e; diff --git a/addons/mail/static/tests/helpers/mock_server.js b/addons/mail/static/tests/helpers/mock_server.js index 68f22c4f7f5..5da2d6e52a3 100644 --- a/addons/mail/static/tests/helpers/mock_server.js +++ b/addons/mail/static/tests/helpers/mock_server.js @@ -171,7 +171,9 @@ MockServer.include({ return; } if (args.model === 'mail.channel' && args.method === 'channel_seen') { - return; + const channel_ids = args.args[0]; + const last_message_id = args.args[1] || args.kwargs.last_message_id; + return this._mockMailChannelChannelSeen(channel_ids, last_message_id); } if (args.model === 'mail.channel' && args.method === 'channel_set_custom_name') { const channel_id = args.args[0] || args.kwargs.channel_id; @@ -287,7 +289,7 @@ MockServer.include({ let email_from; if (user_id) { const author = this._getRecords('res.users', [['id', '=', user_id]])[0]; - author_id = author.id; + author_id = author.partner_id; email_from = `${author.display_name} <${author.email}>`; } else { author_id = false; @@ -697,10 +699,14 @@ MockServer.include({ return channels.map(channel => { const members = channel.members.map(partnerId => partnerInfos[partnerId]); const messages = this._getRecords('mail.message', [ - ['res_id', '=', channel.id], - ['model', '=', 'mail.channel'], - ['message_type', '!=', 'user_notification'], + ['channel_ids', 'in', [channel.id]], ]); + const lastMessageId = messages.reduce((lastMessageId, message) => { + if (!lastMessageId || message.id > lastMessageId) { + return message.id; + } + return lastMessageId; + }, undefined); const messageNeedactionCounter = this._getRecords('mail.notification', [ ['res_partner_id', '=', this.currentPartnerId], ['is_read', '=', false], @@ -708,6 +714,7 @@ MockServer.include({ ]).length; return Object.assign({}, channel, { info: extra_info, + last_message_id: lastMessageId, members, message_needaction_counter: messageNeedactionCounter, }); @@ -745,6 +752,53 @@ MockServer.include({ return channelInfo; }, /** + * Simulates the `channel_seen` method of `mail.channel`. + * + * @private + * @param integer[] ids + * @param {integer} last_message_id + */ + async _mockMailChannelChannelSeen(ids, last_message_id) { + // Update record + const channel_id = ids[0]; + if (!channel_id) { + throw new Error('Should only be one channel in channel_seen mock params'); + } + const messagesBeforeGivenLastMessage = this._getRecords('mail.message', [ + ['channel_ids', 'in', [channel_id]], + ['id', '<=', last_message_id] + ]); + if (!messagesBeforeGivenLastMessage || messagesBeforeGivenLastMessage.length === 0) { + return; + } + const channel = this._getRecords('mail.channel', [['id', '=', channel_id]])[0]; + if (!channel) { + return; + } + if (channel.seen_message_id && channel.seen_message_id >= last_message_id) { + return; + } + this._mockWrite('mail.channel', [[channel.id], { + fetched_message_id: last_message_id, + seen_message_id: last_message_id, + }]); + + // Send notification + const payload = { + channel_id, + info: 'channel_seen', + last_message_id, + partner_id: this.currentPartnerId, + }; + let notification; + if (channel.channel_type === 'chat') { + notification = [[false, 'mail.channel', channel_id], payload]; + } else { + notification = [[false, 'res.partner', this.currentPartnerId], payload]; + } + this._widget.call('bus_service', 'trigger', 'notification', [notification]); + }, + /** * Simulates `channel_set_custom_name` on `mail.channel`. * * @private diff --git a/addons/test_mail/tests/test_mail_channel.py b/addons/test_mail/tests/test_mail_channel.py index e08d45caa69..42fba8eeee4 100644 --- a/addons/test_mail/tests/test_mail_channel.py +++ b/addons/test_mail/tests/test_mail_channel.py @@ -257,6 +257,31 @@ class TestChannelFeatures(TestMailCommon): same_solo_channel_info = current_user.env['mail.channel'].channel_get(partners_to=current_partner.ids) self.assertEqual(same_solo_channel_info['id'], solo_channel_info['id']) + def test_channel_seen(self): + """ + In case of concurrent channel_seen RPC, ensure the oldest call has no effect. + """ + self.test_channel.write({'channel_type': 'chat'}) + self.test_channel.action_follow() + msg_1 = self._add_messages(self.test_channel, 'Body1', author=self.user_employee.partner_id, + channel_ids=[self.test_channel.id]) + msg_2 = self._add_messages(self.test_channel, 'Body2', author=self.user_employee.partner_id, + channel_ids=[self.test_channel.id]) + ChannelAsUser = self.test_channel.with_user(self.user_employee).browse(self.test_channel.id) + + self.test_channel.channel_seen(msg_2.id) + self.assertEqual( + ChannelAsUser.channel_info()[0]['seen_partners_info'][0]['seen_message_id'], + msg_2.id, + "Last message id should have been updated" + ) + + self.test_channel.channel_seen(msg_1.id) + self.assertEqual( + ChannelAsUser.channel_info()[0]['seen_partners_info'][0]['seen_message_id'], + msg_2.id, + "Last message id should stay the same after mark channel as seen with an older message" + ) @tagged('moderation') class TestChannelModeration(TestMailCommon): |
