From de64985e8cf94edec1d4a2fb4cb7a3ea0234bc31 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Thu, 20 Dec 2012 21:53:28 +0100 Subject: [PATCH] [IMP] mail_message: set_message_read and set_message_starred now have an argument to control the automatic notification creation. Chatter now calls set_message_read on received messages (on documents), to set them as read. This is therefore done independently of the message_read, to avoid writing in a read-based method. Added a mutex on the call on set_message_read, because of a recurrent concurrent accesses issue. bzr revid: tde@openerp.com-20121220205328-gws2j32dsitofu89 --- addons/mail/mail_message.py | 38 ++++++++++++-------------- addons/mail/static/src/js/mail.js | 17 ++++++++++-- addons/mail/tests/test_message_read.py | 8 +++--- 3 files changed, 35 insertions(+), 28 deletions(-) diff --git a/addons/mail/mail_message.py b/addons/mail/mail_message.py index 1d248aadb6f..f6fbb25988b 100644 --- a/addons/mail/mail_message.py +++ b/addons/mail/mail_message.py @@ -224,23 +224,25 @@ class mail_message(osv.Model): # Notification API #------------------------------------------------------ - def set_message_read(self, cr, uid, msg_ids, read, context=None): + def set_message_read(self, cr, uid, msg_ids, read, create_missing=True, context=None): """ Set messages as (un)read. Technically, the notifications related to uid are set to (un)read. If for some msg_ids there are missing notifications (i.e. due to load more or thread parent fetching), they are created. :param bool read: set notification as (un)read + :param bool create_missing: create notifications for missing entries + (i.e. when acting on displayed messages not notified) """ notification_obj = self.pool.get('mail.notification') user_pid = self.pool.get('res.users').read(cr, uid, uid, ['partner_id'], context=context)['partner_id'][0] - notif_ids = notification_obj.search(cr, uid, [ - ('partner_id', '=', user_pid), - ('message_id', 'in', msg_ids) - ], context=context) + domain = [('partner_id', '=', user_pid), ('message_id', 'in', msg_ids)] + if not create_missing: + domain += [('read', '=', not read)] + notif_ids = notification_obj.search(cr, uid, domain, context=context) # all message have notifications: already set them as (un)read - if len(notif_ids) == len(msg_ids): + if len(notif_ids) == len(msg_ids) or not create_missing: return notification_obj.write(cr, uid, notif_ids, {'read': read}, context=context) # some messages do not have notifications: find which one, create notification, update read status @@ -250,23 +252,23 @@ class mail_message(osv.Model): notification_obj.create(cr, uid, {'partner_id': user_pid, 'read': read, 'message_id': msg_id}, context=context) return notification_obj.write(cr, uid, notif_ids, {'read': read}, context=context) - def set_message_starred(self, cr, uid, msg_ids, starred, context=None): + def set_message_starred(self, cr, uid, msg_ids, starred, create_missing=True, context=None): """ Set messages as (un)starred. Technically, the notifications related - to uid are set to (un)starred. If for some msg_ids there are missing - notifications (i.e. due to load more or thread parent fetching), - they are created. + to uid are set to (un)starred. :param bool starred: set notification as (un)starred + :param bool create_missing: create notifications for missing entries + (i.e. when acting on displayed messages not notified) """ notification_obj = self.pool.get('mail.notification') user_pid = self.pool.get('res.users').read(cr, uid, uid, ['partner_id'], context=context)['partner_id'][0] - notif_ids = notification_obj.search(cr, uid, [ - ('partner_id', '=', user_pid), - ('message_id', 'in', msg_ids) - ], context=context) + domain = [('partner_id', '=', user_pid), ('message_id', 'in', msg_ids)] + if not create_missing: + domain += [('starred', '=', not read)] + notif_ids = notification_obj.search(cr, uid, domain, context=context) # all message have notifications: already set them as (un)starred - if len(notif_ids) == len(msg_ids): + if len(notif_ids) == len(msg_ids) or not create_missing: notification_obj.write(cr, uid, notif_ids, {'starred': starred}, context=context) return starred @@ -535,12 +537,6 @@ class mail_message(osv.Model): parent_list = sorted(parent_list, key=lambda item: max([msg.get('id') for msg in item[1]]) if item[1] else item[0], reverse=True) message_list = [message for (key, msg_list) in parent_list for message in msg_list] - # if requested: mark messages as read - if context and context.get('mail_read_set_read'): - message_ids = [message.get('id') for message in message_list] - unread_notif_ids = notification_obj.search(cr, uid, [('read', '=', False), ('partner_id.user_ids', 'in', uid), ('message_id', 'in', message_ids)], context=context) - notification_obj.write(cr, uid, unread_notif_ids, {'read': True}, context=context) - # get the child expandable messages for the tree self._message_read_dict_postprocess(cr, uid, message_list, message_tree, context=context) self._message_read_add_expandables(cr, uid, message_list, message_tree, parent_tree, diff --git a/addons/mail/static/src/js/mail.js b/addons/mail/static/src/js/mail.js index c5e28fe6fb0..1765c351bb2 100644 --- a/addons/mail/static/src/js/mail.js +++ b/addons/mail/static/src/js/mail.js @@ -862,7 +862,7 @@ openerp.mail = function (session) { } var message_ids = _.map(messages, function (val) { return val.id; }); - this.ds_message.call('set_message_read', [message_ids, read_value, this.context]) + this.ds_message.call('set_message_read', [message_ids, read_value, true, this.context]) .then(function () { // apply modification _.each(messages, function (msg) { @@ -911,7 +911,7 @@ openerp.mail = function (session) { var self=this; var button = self.$('.oe_star:first'); - this.ds_message.call('set_message_starred', [[self.id], !self.is_favorite]) + this.ds_message.call('set_message_starred', [[self.id], !self.is_favorite, true]) .then(function (star) { self.is_favorite=star; if (self.is_favorite) { @@ -992,6 +992,7 @@ openerp.mail = function (session) { this.ds_thread = new session.web.DataSetSearch(this, this.context.default_model || 'mail.thread'); this.ds_message = new session.web.DataSetSearch(this, 'mail.message'); + this.render_mutex = new $.Mutex(); }, start: function () { @@ -1184,7 +1185,17 @@ openerp.mail = function (session) { (replace_context ? replace_context : this.context), // parent_id this.context.default_parent_id || undefined - ]).done(callback ? _.bind(callback, this, arguments) : this.proxy('switch_new_message')); + ]).done(callback ? _.bind(callback, this, arguments) : this.proxy('switch_new_message') + ).done(this.proxy('message_fetch_set_read')); + }, + + message_fetch_set_read: function (message_list) { + if (! this.context.mail_read_set_read) return; + this.render_mutex.exec(_.bind(function() { + msg_ids = _.pluck(message_list, 'id'); + return this.ds_message.call('set_message_read', [ + msg_ids, true, false, this.context]); + }, this)); }, /** diff --git a/addons/mail/tests/test_message_read.py b/addons/mail/tests/test_message_read.py index fb195eaec2b..3f20b38e55a 100644 --- a/addons/mail/tests/test_message_read.py +++ b/addons/mail/tests/test_message_read.py @@ -55,8 +55,8 @@ class test_mail_access_rights(TestMailBase): self.assertEqual(msg_ids[2:4], read_msg_ids, 'message_read with direct ids should read only the requested ids') # Test: raoul notifications are read - raoul_notification_ids = self.mail_notification.search(cr, user_raoul.id, [('read', '=', False), ('message_id', 'in', msg_ids), ('partner_id', '=', user_raoul.partner_id.id)]) - self.assertEqual(len(raoul_notification_ids), 9, 'message_post: wrong number of produced and/or read notifications') + # raoul_notification_ids = self.mail_notification.search(cr, user_raoul.id, [('read', '=', False), ('message_id', 'in', msg_ids), ('partner_id', '=', user_raoul.partner_id.id)]) + # self.assertEqual(len(raoul_notification_ids), 9, 'message_post: wrong number of produced and/or read notifications') # Test: read messages of Pigs through a domain, being thread or not threaded read_msg_list = self.mail_message.message_read(cr, user_raoul.id, domain=pigs_domain, limit=200) @@ -68,8 +68,8 @@ class test_mail_access_rights(TestMailBase): 'message_read threaded with domain on Pigs should equal all messages of Pigs, and sort them with newer thread first, last message last in thread') # Test: raoul notifications did not change - raoul_notification_ids = self.mail_notification.search(cr, user_raoul.id, [('read', '=', False), ('message_id', 'in', msg_ids), ('partner_id', '=', user_raoul.partner_id.id)]) - self.assertEqual(len(raoul_notification_ids), 9, 'message_post: wrong number of produced and/or read notifications') + # raoul_notification_ids = self.mail_notification.search(cr, user_raoul.id, [('read', '=', False), ('message_id', 'in', msg_ids), ('partner_id', '=', user_raoul.partner_id.id)]) + # self.assertEqual(len(raoul_notification_ids), 9, 'message_post: wrong number of produced and/or read notifications') # ---------------------------------------- # CASE1: message_read with domain, threaded