From 607ac6e9125ee0e82a3901cd01abb16ad60a8a6b Mon Sep 17 00:00:00 2001 From: Xavier Dubuc Date: Tue, 29 Sep 2020 14:39:46 +0000 Subject: [PATCH] [FIX] mail: do not show new messages separator for own messages MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Also fix compute of local counter in some specific situations. task-2341740 closes odoo/odoo#58866 X-original-commit: 0df66708defd0fcb288979b1431d8180831e43d3 Signed-off-by: Alexandre Kühn (aku) Signed-off-by: Sébastien Theys (seb) --- addons/mail/models/mail_channel.py | 42 ++++++++++------ .../components/discuss/tests/discuss_tests.js | 32 ++++++------ .../messaging_notification_handler.js | 5 +- .../mail/static/src/models/thread/thread.js | 12 +++-- .../mail/static/tests/helpers/mock_server.js | 50 +++++++++++-------- 5 files changed, 81 insertions(+), 60 deletions(-) diff --git a/addons/mail/models/mail_channel.py b/addons/mail/models/mail_channel.py index 8b150fafeb7..b6dcde47403 100644 --- a/addons/mail/models/mail_channel.py +++ b/addons/mail/models/mail_channel.py @@ -385,6 +385,13 @@ class Channel(models.Model): }) return message + def _message_post_after_hook(self, message, msg_vals): + """ + Automatically set the message posted by the current user as seen for himself. + """ + self._set_last_seen_message(message) + return super()._message_post_after_hook(message=message, msg_vals=msg_vals) + def _alias_get_error_message(self, message, message_dict, alias): if alias.alias_contact == 'followers' and self.ids: author = self.env['res.partner'].browse(message_dict.get('author_id', False)) @@ -744,21 +751,7 @@ class Channel(models.Model): 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, - }) + self._set_last_seen_message(last_message) data = { 'info': 'channel_seen', @@ -772,6 +765,25 @@ class Channel(models.Model): self.env['bus.bus'].sendone((self._cr.dbname, 'res.partner', self.env.user.partner_id.id), data) return last_message.id + def _set_last_seen_message(self, last_message): + """ + Set last seen message of `self` channels for the current user. + :param last_message: the message to set as last seen message + """ + 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) + channel_partner.write({ + 'fetched_message_id': last_message.id, + 'seen_message_id': last_message.id, + }) + def channel_fetched(self): """ Broadcast the channel_fetched notification to channel members :param channel_ids : list of channel id that has been fetched by current user 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 5308ac818ea..cbd7c8136a1 100644 --- a/addons/mail/static/src/components/discuss/tests/discuss_tests.js +++ b/addons/mail/static/src/components/discuss/tests/discuss_tests.js @@ -3211,20 +3211,20 @@ QUnit.test('mark channel as seen on last message visible [REQUIRE FOCUS]', async assert.containsOnce( document.body, `.o_DiscussSidebar_item[data-thread-local-id="${ - this.env.models['mail.thread'].find(thread => - thread.id === 10 && - thread.model === 'mail.channel' - ).localId + this.env.models['mail.thread'].findFromIdentifyingData({ + id: 10, + model: 'mail.channel', + }).localId }"]`, "should have discuss sidebar item with the channel" ); assert.hasClass( document.querySelector(` .o_DiscussSidebar_item[data-thread-local-id="${ - this.env.models['mail.thread'].find(thread => - thread.id === 10 && - thread.model === 'mail.channel' - ).localId + this.env.models['mail.thread'].findFromIdentifyingData({ + id: 10, + model: 'mail.channel', + }).localId }"] `), 'o-unread', @@ -3234,20 +3234,20 @@ QUnit.test('mark channel as seen on last message visible [REQUIRE FOCUS]', async await afterNextRender(() => document.querySelector(` .o_DiscussSidebar_item[data-thread-local-id="${ - this.env.models['mail.thread'].find(thread => - thread.id === 10 && - thread.model === 'mail.channel' - ).localId + this.env.models['mail.thread'].findFromIdentifyingData({ + id: 10, + model: 'mail.channel', + }).localId }"] `).click() ); assert.doesNotHaveClass( document.querySelector(` .o_DiscussSidebar_item[data-thread-local-id="${ - this.env.models['mail.thread'].find(thread => - thread.id === 10 && - thread.model === 'mail.channel' - ).localId + this.env.models['mail.thread'].findFromIdentifyingData({ + id: 10, + model: 'mail.channel', + }).localId }"] `), 'o-unread', 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 8123a69e7a5..ede9245602d 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 @@ -411,10 +411,7 @@ function factory(dependencies) { } 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, - }); + return this._handleNotificationChannelSeen(data.channel_id, data); } 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 4ad43577b1f..837de254d9d 100644 --- a/addons/mail/static/src/models/thread/thread.js +++ b/addons/mail/static/src/models/thread/thread.js @@ -1108,6 +1108,11 @@ function factory(dependencies) { if (!this.lastSeenByCurrentPartnerMessageId) { return this.serverMessageUnreadCounter; } + // if server knows more messages than the client knows then we just + // need to trust him + if (this.serverLastMessageId > 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 @@ -1115,15 +1120,15 @@ function factory(dependencies) { // serverMessageUnreadCounter if (this.lastSeenByCurrentPartnerMessageId < firstMessage.id) { const fetchedNotSeenMessages = this.orderedMessages.filter(message => - message.id > this.serverLastMessageId + message.id > this.serverLastMessageId && message.author !== this.env.messaging.currentPartner ); return this.serverMessageUnreadCounter + fetchedNotSeenMessages.length; } // lastSeenByCurrentPartnerMessageId is a known message, // then we can forget serverMessageUnreadCounter - const maxId = Math.max(this.serverLastMessageId, this.lastSeenByCurrentPartnerMessageId); + const maxId = this.lastSeenByCurrentPartnerMessageId; return this.orderedMessages.reduce( - (acc, message) => acc + (message.id > maxId ? 1 : 0), + (acc, message) => acc + ((message.id > maxId && message.author !== this.env.messaging.currentPartner) ? 1 : 0), 0 ); } @@ -1544,6 +1549,7 @@ function factory(dependencies) { dependencies: [ 'lastMessage', 'lastSeenByCurrentPartnerMessageId', + 'messagingCurrentPartner', 'orderedMessages', 'serverLastMessageId', 'serverMessageUnreadCounter', diff --git a/addons/mail/static/tests/helpers/mock_server.js b/addons/mail/static/tests/helpers/mock_server.js index 5aaafff59eb..2961b2ea311 100644 --- a/addons/mail/static/tests/helpers/mock_server.js +++ b/addons/mail/static/tests/helpers/mock_server.js @@ -795,24 +795,21 @@ MockServer.include({ if (!channel_id) { throw new Error('Should only be one channel in channel_seen mock params'); } + const channel = this._getRecords('mail.channel', [['id', '=', channel_id]])[0]; const messagesBeforeGivenLastMessage = this._getRecords('mail.message', [ - ['channel_ids', 'in', [channel_id]], - ['id', '<=', last_message_id] + ['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, - }]); + this._mockMailChannel_SetLastSeenMessage([channel.id], last_message_id); // Send notification const payload = { @@ -960,6 +957,9 @@ MockServer.include({ }), context, ); + if (kwargs.author_id === this.currentPartnerId) { + this._mockMailChannel_SetLastSeenMessage([channel.id], messageId); + } return messageId; }, /** @@ -1018,6 +1018,19 @@ MockServer.include({ } return partnerInfos; }, + /** + * Simulates the `_set_last_seen_message` method of `mail.channel`. + * + * @private + * @param {integer[]} ids + * @param {integer} message_id + */ + _mockMailChannel_SetLastSeenMessage(ids, message_id) { + this._mockWrite('mail.channel', [ids, { + fetched_message_id: message_id, + seen_message_id: message_id, + }]); + }, /** * Simulates `mark_all_as_read` on `mail.message`. * @@ -1210,20 +1223,13 @@ MockServer.include({ this._widget.call('bus_service', 'trigger', 'notification', [notification]); } else if (decision === 'accept') { // simulate notification back (new accepted message in channel) - const messages = _.filter(model.records, function (rec) { - return _.contains(messageIDs, rec.id); - }); - - const notifications = []; - _.each(messages, function (message) { - const dbName = undefined; // useless for tests - const messageData = message; - message.moderation_status = 'accepted'; - const metaData = [dbName, 'mail.channel', message.res_id]; - const notification = [metaData, messageData]; - notifications.push(notification); - }); - this._widget.call('bus_service', 'trigger', 'notification', notifications); + const messages = this._getRecords('mail.message', [['id', 'in', messageIDs]]); + for (const message of messages) { + this._mockWrite('mail.message', [[message.id], { + moderation_status: 'accepted', + }]); + this._mockMailThread_NotifyThread(model, message.channel_ids, message.id); + } } }, /** @@ -1513,7 +1519,7 @@ MockServer.include({ }); delete values.subtype_xmlid; const messageId = this._mockCreate('mail.message', values); - this._mockMailThread_NotifyThread(model, ids, messageId, values); + this._mockMailThread_NotifyThread(model, ids, messageId); return messageId; }, /**