diff --git a/addons/mail/models/mail_followers.py b/addons/mail/models/mail_followers.py index 55b155d85f5..ccb245e18d3 100644 --- a/addons/mail/models/mail_followers.py +++ b/addons/mail/models/mail_followers.py @@ -76,6 +76,95 @@ class Followers(models.Model): # Private tools methods to fetch followers data # -------------------------------------------------- + def _get_recipient_data(self, records, subtype_id, pids=None, cids=None): + """ Private method allowing to fetch recipients data based on a subtype. + Purpose of this method is to fetch all data necessary to notify recipients + in a single query. It fetches data from + + * followers (partners and channels) of records that follow the given + subtype if records and subtype are set; + * partners if pids is given; + * channels if cids is given; + + :param records: fetch data from followers of records that follow subtype_id; + :param subtype_id: mail.message.subtype to check against followers; + :param pids: additional set of partner IDs from which to fetch recipient data; + :param cids: additional set of channel IDs from which to fetch recipient data; + + :return: list of recipient data which is a tuple containing + partner ID (void if channel ID), + channel ID (void if partner ID), + active value (always True for channels), + share status of partner (void as irrelevant if channel ID), + notification status of partner or channel (email or inbox), + user groups of partner (void as irrelevant if channel ID), + """ + if records and subtype_id: + query = """ +WITH sub_followers AS ( + SELECT fol.id, fol.partner_id, fol.channel_id, subtype.internal + FROM mail_followers fol + RIGHT JOIN mail_followers_mail_message_subtype_rel subrel + ON subrel.mail_followers_id = fol.id + RIGHT JOIN mail_message_subtype subtype + ON subtype.id = subrel.mail_message_subtype_id + WHERE subrel.mail_message_subtype_id = %%s AND fol.res_model = %%s AND fol.res_id IN %%s +) +SELECT partner.id as pid, NULL AS cid, + partner.active as active, partner.partner_share as pshare, NULL as ctype, + users.notification_type AS notif, array_agg(groups.id) AS groups + FROM res_partner partner + LEFT JOIN res_users users ON users.partner_id = partner.id + LEFT JOIN res_groups_users_rel groups_rel ON groups_rel.uid = users.id + LEFT JOIN res_groups groups ON groups.id = groups_rel.gid + WHERE EXISTS ( + SELECT partner_id FROM sub_followers + WHERE sub_followers.channel_id IS NULL + AND sub_followers.partner_id = partner.id + AND (sub_followers.internal <> TRUE OR partner.partner_share <> TRUE) + ) %s + GROUP BY partner.id, users.notification_type +UNION +SELECT NULL AS pid, channel.id AS cid, + TRUE as active, NULL AS pshare, channel.channel_type AS ctype, + CASE WHEN channel.email_send = TRUE THEN 'email' ELSE 'inbox' END AS notif, NULL AS groups + FROM mail_channel channel + WHERE EXISTS ( + SELECT channel_id FROM sub_followers WHERE partner_id IS NULL AND sub_followers.channel_id = channel.id + ) %s +""" % ('OR partner.id IN %s' if pids else '', 'OR channel.id IN %s' if cids else '') + params = [subtype_id, records._name, tuple(records.ids)] + if pids: + params.append(tuple(pids)) + if cids: + params.append(tuple(cids)) + self.env.cr.execute(query, tuple(params)) + res = self.env.cr.fetchall() + elif pids or cids: + params, query_pid, query_cid = [], '', '' + if pids: + query_pid = """ +SELECT partner.id as pid, NULL AS cid, + partner.active as active, partner.partner_share as pshare, NULL as ctype, + users.notification_type AS notif, NULL AS groups +FROM res_partner partner +LEFT JOIN res_users users ON users.partner_id = partner.id +WHERE partner.id IN %s""" + params.append(tuple(pids)) + if cids: + query_cid = """ +SELECT NULL AS pid, channel.id AS cid, + TRUE as active, NULL AS pshare, channel.channel_type AS ctype, + CASE when channel.email_send = TRUE then 'email' else 'inbox' end AS notif, NULL AS groups +FROM mail_channel channel WHERE channel.id IN %s """ + params.append(tuple(cids)) + query = ' UNION'.join(x for x in [query_pid, query_cid] if x) + self.env.cr.execute(query, tuple(params)) + res = self.env.cr.fetchall() + else: + res = [] + return res + def _get_subscription_data(self, doc_data, pids, cids, include_pshare=False): """ Private method allowing to fetch follower data from several documents of a given model. Followers can be filtered given partner IDs and channel IDs. diff --git a/addons/mail/models/mail_message.py b/addons/mail/models/mail_message.py index 816f15a5b24..fed4ad94c88 100644 --- a/addons/mail/models/mail_message.py +++ b/addons/mail/models/mail_message.py @@ -976,46 +976,92 @@ class Message(models.Model): #------------------------------------------------------ @api.multi - def _notify(self, force_send=False, send_after_commit=True, model_description=False, mail_auto_delete=True): - """ Compute recipients to notify based on specified recipients and document - followers. Delegate notification to partners to send emails and bus notifications - and to channels to broadcast messages on channels """ - group_user = self.env.ref('base.group_user') - # have a sudoed copy to manipulate partners (public can go here with website modules like forum / blog / ... ) - self_sudo = self.sudo() + def _notify(self, record, msg_vals, force_send=False, send_after_commit=True, model_description=False, mail_auto_delete=True): + """ Main notification method. This method basically does two things + * call ``_notify_compute_recipients`` that computes recipients to + notify based on message record or message creation values if given + (to optimize performance if we already have data computed); + * call ``_notify_recipients`` that performs the notification process; + + :param record: record on which the message is posted, if any; + :param msg_vals: dictionary of values used to create the message. If given + it is used instead of accessing ``self`` to lesen query count in some + simple cases where no notification is actually required; + :param force_send: tells whether to send notification emails within the + current transaction or to use the email queue; + :param send_after_commit: if force_send, tells whether to send emails after + the transaction has been committed using a post-commit hook; + :param model_description: optional data used in notification process (see + notification templates); + :param mail_auto_delete: delete notification emails once sent; + """ + msg_vals = msg_vals if msg_vals else {} + rdata = self._notify_compute_recipients(record, msg_vals) + return self._notify_recipients( + rdata, record, msg_vals, + force_send=force_send, send_after_commit=send_after_commit, + model_description=model_description, mail_auto_delete=mail_auto_delete) + + @api.multi + def _notify_compute_recipients(self, record, msg_vals): + """ Compute recipients to notify based on subtype and followers. This + method returns data structured as expected for ``_notify_recipients``. """ + msg_sudo = self.sudo() + + pids = [x[1] for x in msg_vals.get('partner_ids')] if 'partner_ids' in msg_vals else msg_sudo.partner_ids.ids + cids = [x[1] for x in msg_vals.get('channel_ids')] if 'channel_ids' in msg_vals else msg_sudo.channel_ids.ids + subtype_id = msg_vals.get('subtype_id') if 'subtype_id' in msg_vals else msg_sudo.subtype_id.id + + recipient_data = { + 'partners': [], + 'channels': [], + } + res = self.env['mail.followers']._get_recipient_data(record, subtype_id, pids, cids) + author_id = msg_vals.get('author_id') or self.author_id.id if res else False + for pid, cid, active, pshare, ctype, notif, groups in res: + if pid and pid == author_id: # do not notify the author of its own messages + continue + if pid: + pdata = {'id': pid, 'active': active, 'share': pshare, 'groups': groups} + if notif == 'inbox': + recipient_data['partners'].append(dict(pdata, notif=notif, type='user')) + else: + if not pshare and notif: # has an user and is not shared, is therefore user + recipient_data['partners'].append(dict(pdata, notif='email', type='user')) + elif pshare and notif: # has an user but is shared, is therefore portal + recipient_data['partners'].append(dict(pdata, notif='email', type='portal')) + else: # has no user, is therefore customer + recipient_data['partners'].append(dict(pdata, notif='email', type='customer')) + elif cid: + recipient_data['channels'].append({'id': cid, 'notif': notif, 'type': ctype}) + return recipient_data + + @api.multi + def _notify_recipients(self, rdata, record, msg_vals, + force_send=False, send_after_commit=True, + model_description=False, mail_auto_delete=True): + """ Main method implementing the notification process. + + :param rdata: dict containing recipients data: { + 'partners': list of dict containing partner data: id, share status (boolean), + notification type ('email' or 'inbox'), type (main classification group used + in notification process), groups (user groups) + 'channels': list of dict containing channel data: id, notification type + ('email' for mailing list otherwise 'inbox'), type (channel_type) + } + """ self.ensure_one() - partners_sudo = self_sudo.partner_ids - channels_sudo = self_sudo.channel_ids - # all followers of the mail.message document have to be added as partners and notified - # and filter to employees only if the subtype is internal - if self_sudo.subtype_id and self.model and self.res_id: - followers = self_sudo.env['mail.followers'].search([ - ('res_model', '=', self.model), - ('res_id', '=', self.res_id), - ('subtype_ids', 'in', self_sudo.subtype_id.id), - ]) - if self_sudo.subtype_id.internal: - followers = followers.filtered(lambda fol: fol.channel_id or (fol.partner_id.user_ids and group_user in fol.partner_id.user_ids[0].mapped('groups_id'))) - channels_sudo |= followers.mapped('channel_id') - partners_sudo |= followers.mapped('partner_id') + email_cids = [r['id'] for r in rdata['channels'] if r['notif'] == 'email'] + email_pids = [r['id'] for r in rdata['partners'] if r['notif'] == 'email'] + inbox_pids = [r['id'] for r in rdata['partners'] if r['notif'] == 'inbox'] - # remove author from notified partners - if not self._context.get('mail_notify_author', False) and self_sudo.author_id: - partners_sudo = partners_sudo - self_sudo.author_id - - # list channels and partner by notification type - email_channels = channels_sudo.filtered(lambda channel: channel.email_send) - notif_partners = partners_sudo.filtered(lambda partner: 'inbox' in partner.mapped('user_ids.notification_type')) - email_partner = partners_sudo - notif_partners - - #update message, with maybe custom values message_values = {} - if channels_sudo: - message_values['channel_ids'] = [(6, 0, channels_sudo.ids)] - if partners_sudo: - message_values['needaction_partner_ids'] = [(6, 0, partners_sudo.ids)] + if rdata['channels']: + message_values['channel_ids'] = [(6, 0, [r['id'] for r in rdata['channels']])] + if rdata['partners']: + message_values['needaction_partner_ids'] = [(6, 0, [r['id'] for r in rdata['partners']])] if self.model and self.res_id and hasattr(self.env[self.model], 'message_get_message_notify_values'): message_values.update(self.env[self.model].browse(self.res_id).message_get_message_notify_values(self, message_values)) if message_values: @@ -1024,16 +1070,18 @@ class Message(models.Model): # notify partners and channels # those methods are called as SUPERUSER because portal users posting messages # have no access to partner model. Maybe propagating a real uid could be necessary. - if email_channels or email_partner: - partners_sudo.search([ + if email_cids or email_pids: + self.env['res.partner'].sudo().search([ '|', - ('id', 'in', (email_partner).ids), - ('channel_ids', 'in', email_channels.ids), - ('email', '!=', self_sudo.author_id.email or self_sudo.email_from), + ('id', 'in', email_pids), + ('channel_ids', 'in', email_cids), + ('email', '!=', self.author_id.email or self.email_from), ])._notify(self, force_send=force_send, send_after_commit=send_after_commit, model_description=model_description, mail_auto_delete=mail_auto_delete) - notif_partners._notify_by_chat(self) - channels_sudo._notify(self) + self.env['res.partner'].sudo().browse(inbox_pids)._notify_by_chat(self) + + if rdata['channels']: + self.env['mail.channel'].sudo().browse([r['id'] for r in rdata['channels']])._notify(self) return True @@ -1098,7 +1146,8 @@ class Message(models.Model): }) # proceed with notification process to send notification emails and Inbox messages for message in self: - message._notify() + record = self.env[message.model].browse(message.res_id) if message.model and message.res_id else None + message._notify(record, {}) @api.multi def _moderate_send_reject_email(self, subject, comment): diff --git a/addons/mail/models/mail_thread.py b/addons/mail/models/mail_thread.py index 55e28e5c196..1fe466d164a 100644 --- a/addons/mail/models/mail_thread.py +++ b/addons/mail/models/mail_thread.py @@ -2014,6 +2014,7 @@ class MailThread(models.AbstractModel): 'parent_id': parent_id, 'subtype_id': subtype_id, 'partner_ids': [(4, pid) for pid in partner_ids], + 'channel_ids': kwargs.get('channel_ids', []), 'add_sign': add_sign }) if notif_layout: @@ -2033,16 +2034,23 @@ class MailThread(models.AbstractModel): self._message_post_after_hook(new_message, values, model_description=model_description, mail_auto_delete=mail_auto_delete) return new_message - def _message_post_after_hook(self, message, values, model_description=False, mail_auto_delete=True): + def _message_post_after_hook(self, message, msg_vals, model_description=False, mail_auto_delete=True): """ Hook to add custom behavior after having posted the message. Both message and computed value are given, to try to lessen query count by using already-computed values instead of having to rebrowse things. """ # Notify recipients of the newly-created message (Inbox / Email + channels) - if values.get('moderation_status') != 'pending_moderation': - message._notify(force_send=self.env.context.get('mail_notify_force_send', True), model_description=model_description, mail_auto_delete=mail_auto_delete) + if msg_vals.get('moderation_status') != 'pending_moderation': + message._notify( + self, msg_vals, + force_send=self.env.context.get('mail_notify_force_send', True), + send_after_commit=True, + model_description=model_description, + mail_auto_delete=mail_auto_delete, + ) + # Post-process: subscribe author - if values['author_id'] and values['model'] and self.ids and values['message_type'] != 'notification' and not self._context.get('mail_create_nosubscribe'): - self._message_subscribe([values['author_id']]) + if msg_vals['author_id'] and msg_vals['model'] and self.ids and msg_vals['message_type'] != 'notification' and not self._context.get('mail_create_nosubscribe'): + self._message_subscribe([msg_vals['author_id']]) else: message._notify_pending_by_chat() diff --git a/addons/test_mail/tests/test_performance.py b/addons/test_mail/tests/test_performance.py index ae6c7f263a7..38fcba3be8f 100644 --- a/addons/test_mail/tests/test_performance.py +++ b/addons/test_mail/tests/test_performance.py @@ -173,7 +173,7 @@ class TestAdvMailPerformance(TransactionCase): 'activity_type_id': self.env.ref('mail.mail_activity_data_todo').id, }) - with self.assertQueryCount(margin=1, admin=37, emp=58): # test_mail only: 37 - 58 + with self.assertQueryCount(margin=1, admin=27, emp=48): # com runbot: 27 - 47 // test_mail only: 27 - 48 activity.action_feedback(feedback='Zizisse Done !') @users('admin', 'emp') @@ -187,7 +187,7 @@ class TestAdvMailPerformance(TransactionCase): record.write({'name': 'Dupe write'}) - with self.assertQueryCount(margin=1, admin=39, emp=60): # test_mail only: 39 - 60 + with self.assertQueryCount(margin=1, admin=29, emp=50): # com runbot: 27 - 49 // test_mail only: 29 - 50 record.action_close('Dupe feedback') self.assertEqual(record.activity_ids, self.env['mail.activity']) @@ -199,7 +199,7 @@ class TestAdvMailPerformance(TransactionCase): self.user_test.write({'notification_type': 'email'}) record = self.env['mail.test.track'].create({'name': 'Test'}) - with self.assertQueryCount(margin=1, admin=64, emp=82): # com runbot: 63 - 81 // test_mail only: 64 - 82 + with self.assertQueryCount(margin=1, admin=60, emp=78): # com runbot: 59 - 77 // test_mail only: 60 - 78 record.write({ 'user_id': self.user_test.id, }) @@ -209,7 +209,7 @@ class TestAdvMailPerformance(TransactionCase): def test_message_assignation_inbox(self): record = self.env['mail.test.track'].create({'name': 'Test'}) - with self.assertQueryCount(margin=1, admin=43, emp=54): # test_mail only: 43 - 54 + with self.assertQueryCount(margin=1, admin=37, emp=48): # test_mail only: 37 - 48 record.write({ 'user_id': self.user_test.id, }) @@ -229,7 +229,7 @@ class TestAdvMailPerformance(TransactionCase): def test_message_log_with_post(self): record = self.env['mail.test.simple'].create({'name': 'Test'}) - with self.assertQueryCount(admin=15, emp=22): # com runbot: 13 - 20 // test_mail only: 15 - 22 + with self.assertQueryCount(admin=11, emp=18): # com runbot: 7 - 13 // test_mail only: 11 - 18 record.message_post( body='

Test message_post as log

', subtype='mail.mt_note', @@ -240,7 +240,7 @@ class TestAdvMailPerformance(TransactionCase): def test_message_post_no_notification(self): record = self.env['mail.test.simple'].create({'name': 'Test'}) - with self.assertQueryCount(admin=15, emp=22): # com runbot: 13 - 20 // test_mail only: 15 - 22 + with self.assertQueryCount(admin=11, emp=18): # com runbot: 7 - 13 // test_mail only: 11 - 18 record.message_post( body='

Test Post Performances basic

', partner_ids=[], @@ -253,7 +253,7 @@ class TestAdvMailPerformance(TransactionCase): def test_message_post_one_email_notification(self): record = self.env['mail.test.simple'].create({'name': 'Test'}) - with self.assertQueryCount(margin=1, admin=61, emp=79): # com runbot: 54 - 72 // test_mail only: 61 - 79 + with self.assertQueryCount(margin=1, admin=55, emp=74): # com runbot: 48 - 67 // test_mail only: 55 - 74 record.message_post( body='

Test Post Performances with an email ping

', partner_ids=self.customer.ids, @@ -265,7 +265,7 @@ class TestAdvMailPerformance(TransactionCase): def test_message_post_one_inbox_notification(self): record = self.env['mail.test.simple'].create({'name': 'Test'}) - with self.assertQueryCount(margin=1, admin=39, emp=48): # com runbot 37 - 46 // test_mail only: 39 - 48 + with self.assertQueryCount(margin=1, admin=32, emp=45): # com runbot 30 - 41 // test_mail only: 32 - 45 record.message_post( body='

Test Post Performances with an inbox ping

', partner_ids=self.user_test.partner_id.ids, @@ -389,7 +389,7 @@ class TestHeavyMailPerformance(TransactionCase): self.umbrella.message_subscribe(self.user_portal.partner_id.ids) record = self.umbrella.sudo(self.env.user) - with self.assertQueryCount(admin=97, emp=120): # com runbot 90 - 113 // test_mail only: 97 - 120 + with self.assertQueryCount(admin=90, emp=112): # com runbot 83 - 105 // test_mail only: 90 - 112 record.message_post( body='

Test Post Performances

', message_type='comment', @@ -406,7 +406,7 @@ class TestHeavyMailPerformance(TransactionCase): record = self.umbrella.sudo(self.env.user) template_id = self.env.ref('test_mail.mail_test_tpl').id - with self.assertQueryCount(admin=116, emp=151): # com runbot 109 - 144 // test_mail only: 116 - 151 + with self.assertQueryCount(admin=109, emp=143): # com runbot 102 - 136 // test_mail only: 109 - 143 record.message_post_with_template(template_id, message_type='comment', composition_mode='comment') self.assertEqual(record.message_ids[0].body, '

Adding stuff on %s

' % record.name) @@ -476,7 +476,7 @@ class TestHeavyMailPerformance(TransactionCase): }) self.assertEqual(rec.message_partner_ids, self.partners | self.env.user.partner_id) - with self.assertQueryCount(admin=66, emp=84): # com runbot: 65 - 83 // test_mail only: 66 - 84 + with self.assertQueryCount(admin=62, emp=80): # com runbot: 61 - 79 // test_mail only: 62 - 80 rec.write({'user_id': self.user_portal.id}) self.assertEqual(rec.message_partner_ids, self.partners | self.env.user.partner_id | self.user_portal.partner_id) @@ -499,7 +499,7 @@ class TestHeavyMailPerformance(TransactionCase): customer_id = self.customer.id user_id = self.user_portal.id - with self.assertQueryCount(margin=1, admin=175, emp=211): # com runbot: 173 - 209 // test_mail only: 175 - 211 + with self.assertQueryCount(margin=1, admin=165, emp=200): # com runbot: 163 - 198 // test_mail only: 165 - 200 rec = self.env['mail.test.full'].create({ 'name': 'Test', 'umbrella_id': umbrella_id, @@ -528,7 +528,7 @@ class TestHeavyMailPerformance(TransactionCase): }) self.assertEqual(rec.message_partner_ids, self.user_portal.partner_id | self.env.user.partner_id) - with self.assertQueryCount(margin=1, admin=105, emp=124): # com runbot: 104 - 123 // test_mail only: 105 - 124 + with self.assertQueryCount(margin=1, admin=100, emp=118): # com runbot: 99 - 117 // test_mail only: 100 - 118 rec.write({ 'name': 'Test2', 'umbrella_id': self.umbrella.id, @@ -566,7 +566,7 @@ class TestHeavyMailPerformance(TransactionCase): }) self.assertEqual(rec.message_partner_ids, self.user_portal.partner_id | self.env.user.partner_id) - with self.assertQueryCount(margin=1, admin=111, emp=134): # test_mail only: 111 - 134 + with self.assertQueryCount(margin=1, admin=106, emp=127): # test_mail only: 106 - 127 rec.write({ 'name': 'Test2', 'umbrella_id': umbrella_id,