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

[FIX] mail: make Inbox counters more reliable in mark as read flows

Browse Source 
- Send the new Inbox counter when marking as read, to guarantee the JS has the
  correct value (instead of having to guess it which was impossible when an
  arbitrary domain was given to the method).

  NOTE: Manual increment is kept on receiving a new Inbox message because that
  one was reliable and it would have been more tricky to compute the new counter
  for each target partner (mark as read is one user doing an action for himself,
  whereas new message is from another user to potentially many different users).
  Generic counter for threads is also not sent on every change for similar
  reasons.

- Refresh Inbox automatically when some messages are marked as read and there
  are more message on the server than currently loaded.

- Remove confusing (and impossible to maintain) counter on non-origin threads
  (in particular channel followers). This behavior in JS was not consistent with
  the server which always counted only origin thread (model/res_id).

task-2446302

closes odoo/odoo#65842

X-original-commit: f9bc1f98ed8f76846697cde68a760217b3af2aaf
Signed-off-by: Alexandre Kühn (aku) <aku@odoo.com>
Signed-off-by: Sébastien Theys (seb) <seb@odoo.com>
This commit is contained in:
Sébastien Theys
2021-02-09 17:45:00 +00:00
parent 18780e9e5a
commit 3d219cf548
6 changed files with 93 additions and 63 deletions
Download Patch File Download Diff File Expand all files Collapse all files
addons/mail/models/mail_message.py
+6 -5
View File
@@ -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
addons/mail/static/src/components/discuss/tests/discuss_tests.js
+4
View File
@@ -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(
addons/mail/static/src/model/model_field_command.js
+26
View File
@@ -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,
};
});
addons/mail/static/src/models/messaging_notification_handler/messaging_notification_handler.js
+34 -43
View File
@@ -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");
addons/mail/static/tests/helpers/mock_server.js
+21 -13
View File
@@ -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`.
*
addons/test_mail/tests/test_mail_thread_internals.py
+2 -2
View File
@@ -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")