diff --git a/addons/mail/models/mail_message.py b/addons/mail/models/mail_message.py index 0c70f87735c..a1c2bf6176f 100644 --- a/addons/mail/models/mail_message.py +++ b/addons/mail/models/mail_message.py @@ -704,17 +704,17 @@ class Message(models.Model): notif_domain = [ ('res_partner_id', '=', partner_id), ('is_read', '=', False)] - if domain: - messages_ids = self.search(domain).ids # need sudo? - notif_domain = expression.AND([notif_domain, [('mail_message_id', 'in', messages_ids)]]) + messages = self.search(domain) + messages.set_message_done() + return messages.ids notifications = self.env['mail.notification'].sudo().search(notif_domain) notifications.write({'is_read': True}) ids = [n['mail_message_id'] for n in notifications.read(['mail_message_id'])] - notification = {'type': 'mark_as_read', 'message_ids': [id[0] for id in ids]} + notification = {'type': 'mark_as_read', 'message_ids': [id[0] for id in ids], 'needaction_inbox_counter': self.env.user.partner_id.get_needaction_count()} self.env['bus.bus'].sendone((self._cr.dbname, 'res.partner', partner_id), notification) return ids @@ -752,7 +752,8 @@ class Message(models.Model): notifications.write({'is_read': True}) for (msg_ids, channel_ids) in groups: - notification = {'type': 'mark_as_read', 'message_ids': msg_ids, 'channel_ids': [c.id for c in channel_ids]} + # channel_ids in result is deprecated and will be removed in a future version + notification = {'type': 'mark_as_read', 'message_ids': msg_ids, 'channel_ids': [c.id for c in channel_ids], 'needaction_inbox_counter': self.env.user.partner_id.get_needaction_count()} self.env['bus.bus'].sendone((self._cr.dbname, 'res.partner', partner_id.id), notification) @api.model 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 59a2b8cc239..dc2005e5ac7 100644 --- a/addons/mail/static/src/components/discuss/tests/discuss_tests.js +++ b/addons/mail/static/src/components/discuss/tests/discuss_tests.js @@ -2574,18 +2574,22 @@ QUnit.test('inbox: mark all messages as read', async function (assert) { body: "not empty", channel_ids: [20], // link message to channel id: 100, // random unique id, useful to link notification + model: 'mail.channel', // needaction needs to be set here for message_fetch domain, because // mocked models don't have computed fields needaction: true, + res_id: 20, }, // second expected message { body: "not empty", channel_ids: [20], // link message to channel id: 101, // random unique id, useful to link notification + model: 'mail.channel', // needaction needs to be set here for message_fetch domain, because // mocked models don't have computed fields needaction: true, + res_id: 20, } ); this.data['mail.notification'].records.push( diff --git a/addons/mail/static/src/model/model_field_command.js b/addons/mail/static/src/model/model_field_command.js index 89f42651ad9..a6dadb7b983 100644 --- a/addons/mail/static/src/model/model_field_command.js +++ b/addons/mail/static/src/model/model_field_command.js @@ -35,11 +35,37 @@ function clear() { ); } +/** + * Returns a decrement command to give to the model manager at create/update. + * + * @param {number} [amount=1] + */ +function decrement(amount = 1) { + return new FieldCommand((field, record, options) => { + const oldValue = field.get(record); + field.set(record, oldValue - amount, options); + }); +} + +/** + * Returns a increment command to give to the model manager at create/update. + * + * @param {number} [amount=1] + */ +function increment(amount = 1) { + return new FieldCommand((field, record, options) => { + const oldValue = field.get(record); + field.set(record, oldValue + amount, options); + }); +} + return { // class FieldCommand, // shortcuts clear, + decrement, + increment, }; }); 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 537007dc29c..58bf1f8f66a 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 @@ -3,6 +3,7 @@ odoo.define('mail/static/src/models/messaging_notification_handler/messaging_not const { registerNewModel } = require('mail/static/src/model/model_core.js'); const { one2one } = require('mail/static/src/model/model_field.js'); +const { decrement, increment } = require('mail/static/src/model/model_field_command.js'); const { htmlToTextContentInline } = require('mail.utils'); const PREVIEW_MSG_MAX_SIZE = 350; // optimal for native English speakers @@ -218,7 +219,7 @@ function factory(dependencies) { oldMessageWasModeratedByCurrentPartner ) { const moderation = this.env.messaging.moderation; - moderation.update({ counter: moderation.counter - 1 }); + moderation.update({ counter: decrement() }); } return; } @@ -378,15 +379,10 @@ function factory(dependencies) { const message = this.env.models['mail.message'].insert( this.env.models['mail.message'].convertData(data) ); - const inboxMailbox = this.env.messaging.inbox; - inboxMailbox.update({ counter: inboxMailbox.counter + 1 }); - for (const thread of message.threads) { - if ( - thread.channel_type === 'channel' && - message.isNeedaction - ) { - thread.update({ message_needaction_counter: thread.message_needaction_counter + 1 }); - } + this.env.messaging.inbox.update({ counter: increment() }); + const originThread = message.originThread; + if (originThread && message.isNeedaction) { + originThread.update({ message_needaction_counter: increment() }); } // manually force recompute of counter this.messaging.messagingMenu.update(); @@ -511,7 +507,7 @@ function factory(dependencies) { message.moderation_status === 'pending_moderation' && message.originThread.isModeratedByCurrentPartner ) { - moderationMailbox.update({ counter: moderationMailbox.counter - 1 }); + moderationMailbox.update({ counter: decrement() }); } message.delete(); } @@ -547,11 +543,9 @@ function factory(dependencies) { * @param {Object} param0 * @param {integer[]} [param0.channel_ids * @param {integer[]} [param0.message_ids=[]] + * @param {integer} [param0.needaction_inbox_counter] */ - _handleNotificationPartnerMarkAsRead({ channel_ids, message_ids = [] }) { - const inboxMailbox = this.env.messaging.inbox; - - // 1. move messages from inbox to history + _handleNotificationPartnerMarkAsRead({ channel_ids, message_ids = [], needaction_inbox_counter }) { for (const message_id of message_ids) { // We need to ignore all not yet known messages because we don't want them // to be shown partially as they would be linked directly to mainCache @@ -559,34 +553,33 @@ function factory(dependencies) { // but something like last read message_id or something like that. // (just imagine you mark 1000 messages as read ... ) const message = this.env.models['mail.message'].findFromIdentifyingData({ id: message_id }); - if (message) { - message.update({ - isNeedaction: false, - isHistory: true, - }); + if (!message) { + continue; } + // update thread counter + const originThread = message.originThread; + if (originThread && message.isNeedaction) { + originThread.update({ message_needaction_counter: decrement() }); + } + // move messages from Inbox to history + message.update({ + isHistory: true, + isNeedaction: false, + }); } - - // 2. remove "needaction" from channels - let channels; - if (channel_ids) { - channels = channel_ids - .map(id => this.env.models['mail.thread'].findFromIdentifyingData({ - id, - model: 'mail.channel', - })) - .filter(thread => !!thread); + const inbox = this.env.messaging.inbox; + if (needaction_inbox_counter !== undefined) { + inbox.update({ counter: needaction_inbox_counter }); } else { - // flux specific: channel_ids unset means "mark all as read" - channels = this.env.models['mail.thread'].all(thread => - thread.model === 'mail.channel' - ); + // kept for compatibility in stable + inbox.update({ counter: decrement(message_ids.length) }); } - for (const channel of channels) { - channel.update({ message_needaction_counter: 0 }); + if (inbox.counter > inbox.mainCache.fetchedMessages.length) { + // Force refresh Inbox because depending on what was marked as + // read the cache might become empty even though there are more + // messages on the server. + inbox.mainCache.update({ hasToLoadMessages: true }); } - inboxMailbox.update({ counter: inboxMailbox.counter - message_ids.length }); - // manually force recompute of counter this.messaging.messagingMenu.update(); } @@ -602,7 +595,7 @@ function factory(dependencies) { ); const moderationMailbox = this.env.messaging.moderation; if (moderationMailbox) { - moderationMailbox.update({ counter: moderationMailbox.counter + 1 }); + moderationMailbox.update({ counter: increment() }); } // manually force recompute of counter this.messaging.messagingMenu.update(); @@ -625,9 +618,7 @@ function factory(dependencies) { } message.update({ isStarred: starred }); starredMailbox.update({ - counter: starred - ? starredMailbox.counter + 1 - : starredMailbox.counter - 1, + counter: starred ? increment() : decrement(), }); } } @@ -751,7 +742,7 @@ function factory(dependencies) { title: notificationTitle, type: 'info', }); - messaging.update({ outOfFocusUnreadMessageCounter: messaging.outOfFocusUnreadMessageCounter + 1 }); + messaging.update({ outOfFocusUnreadMessageCounter: increment() }); const titlePattern = messaging.outOfFocusUnreadMessageCounter === 1 ? this.env._t("%d Message") : this.env._t("%d Messages"); diff --git a/addons/mail/static/tests/helpers/mock_server.js b/addons/mail/static/tests/helpers/mock_server.js index 11ad476b3e2..481b8d64f9c 100644 --- a/addons/mail/static/tests/helpers/mock_server.js +++ b/addons/mail/static/tests/helpers/mock_server.js @@ -223,7 +223,8 @@ MockServer.include({ return this._mockMailMessageModerate(args); } if (args.model === 'mail.message' && args.method === 'set_message_done') { - return this._mockMailMessageSetMessageDone(args); + const ids = args.args[0]; + return this._mockMailMessageSetMessageDone(ids); } if (args.model === 'mail.message' && args.method === 'toggle_message_starred') { const ids = args.args[0]; @@ -389,10 +390,7 @@ MockServer.include({ const currentPartner = this._getRecords('res.partner', [['id', '=', this.currentPartnerId]])[0]; const currentPartnerFormat = this._mockResPartnerMailPartnerFormat(currentPartner.id); - const needaction_inbox_counter = this._getRecords('mail.notification', [ - ['res_partner_id', '=', this.currentPartnerId], - ['is_read', '=', false], - ]).length; + const needaction_inbox_counter = this._mockResPartnerGetNeedactionCount(); const mailFailures = this._mockMailMessageMessageFetchFailed(); @@ -1082,9 +1080,9 @@ MockServer.include({ ]; if (domain) { const messages = this._getRecords('mail.message', domain); - notifDomain.push( - ['mail_message_id', 'in', messages.map(messages => messages.id)] - ); + const ids = messages.map(messages => messages.id); + this._mockMailMessageSetMessageDone(ids); + return ids; } const notifications = this._getRecords('mail.notification', notifDomain); this._mockWrite('mail.notification', [ @@ -1110,7 +1108,7 @@ MockServer.include({ }, ]); } - const notificationData = { type: 'mark_as_read', message_ids: messageIds }; + const notificationData = { type: 'mark_as_read', message_ids: messageIds, needaction_inbox_counter: this._mockResPartnerGetNeedactionCount() }; const notification = [[false, 'res.partner', this.currentPartnerId], notificationData]; this._widget.call('bus_service', 'trigger', 'notification', [notification]); return messageIds; @@ -1313,10 +1311,9 @@ MockServer.include({ * messages have been marked as read, so that UI is updated. * * @private - * @param {Object} args + * @param {integer[]} ids */ - _mockMailMessageSetMessageDone(args) { - const ids = args.args[0]; + _mockMailMessageSetMessageDone(ids) { const messages = this._getRecords('mail.message', [['id', 'in', ids]]); const notifications = this._getRecords('mail.notification', [ @@ -1341,7 +1338,7 @@ MockServer.include({ ]); // NOTE server is sending grouped notifications per channel_ids but // this optimization is not needed here. - const data = { type: 'mark_as_read', message_ids: [message.id], channel_ids: message.channel_ids }; + const data = { type: 'mark_as_read', message_ids: [message.id], channel_ids: message.channel_ids, needaction_inbox_counter: this._mockResPartnerGetNeedactionCount() }; const busNotifications = [[[false, 'res.partner', this.currentPartnerId], data]]; this._widget.call('bus_service', 'trigger', 'notification', busNotifications); } @@ -1735,6 +1732,17 @@ MockServer.include({ } return [mainMatchingPartners, extraMatchingPartners]; }, + /** + * Simulates `get_needaction_count` on `res.partner`. + * + * @private + */ + _mockResPartnerGetNeedactionCount() { + return this._getRecords('mail.notification', [ + ['res_partner_id', '=', this.currentPartnerId], + ['is_read', '=', false], + ]).length; + }, /** * Simulates `im_search` on `res.partner`. * diff --git a/addons/test_mail/tests/test_mail_thread_internals.py b/addons/test_mail/tests/test_mail_thread_internals.py index 307049cdaff..440fe38f7ae 100644 --- a/addons/test_mail/tests/test_mail_thread_internals.py +++ b/addons/test_mail/tests/test_mail_thread_internals.py @@ -137,7 +137,7 @@ class TestDiscuss(TestMailCommon, TestRecipients): msg1 = self.test_record.message_post(body='Test', message_type='comment', subtype_xmlid='mail.mt_comment', partner_ids=[employee_partner.id]) self._reset_bus() employee_partner.env['mail.message'].mark_all_as_read(domain=[]) - self.assertBusNotifications([(self.cr.dbname, 'res.partner', employee_partner.id)], [{ 'type': 'mark_as_read', 'message_ids': [msg1.id] }]) + self.assertBusNotifications([(self.cr.dbname, 'res.partner', employee_partner.id)], [{ 'type': 'mark_as_read', 'message_ids': [msg1.id], 'needaction_inbox_counter': 0 }]) na_count = employee_partner.get_needaction_count() self.assertEqual(na_count, 0, "mark all as read should conclude all needactions") @@ -156,7 +156,7 @@ class TestDiscuss(TestMailCommon, TestRecipients): self._reset_bus() employee_partner.env['mail.message'].mark_all_as_read(domain=[]) - self.assertBusNotifications([(self.cr.dbname, 'res.partner', employee_partner.id)], [{ 'type': 'mark_as_read', 'message_ids': [msg2.id] }]) + self.assertBusNotifications([(self.cr.dbname, 'res.partner', employee_partner.id)], [{ 'type': 'mark_as_read', 'message_ids': [msg2.id], 'needaction_inbox_counter': 0 }]) na_count = employee_partner.get_needaction_count() self.assertEqual(na_count, 0, "mark all read should conclude all needactions even inacessible ones")