From 689a496f1c60eb662dec7bbf7079f33b070a2cee Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= <tde@odoo.com>
Date: Fri, 27 Apr 2018 17:44:28 +0200
Subject: [PATCH] [REF] mail: propagate recipient data through notification
 process

Purpose of this commit is to avoid browsing and prefetching data about
recipients when notifying a message to partners and channels. Mail message
_notify computes all necessary data in a single query. This commit allow
to re-use this data by propagating it through the call chain.

Addons inheriting from classification methods used when sending notification
emails are updated accordingly to the API and data update.

This commit is linked to task ID 47934 and PR #24033. No functional change
should occur with this commit.
---
 addons/crm/models/crm_lead.py                 |  3 +-
 addons/hr_holidays/models/hr_leave.py         |  3 +-
 .../hr_holidays/models/hr_leave_allocation.py |  3 +-
 addons/mail/models/mail_message.py            | 26 +++++------
 addons/mail/models/mail_thread.py             | 22 +++++-----
 addons/mail/models/res_partner.py             | 43 +++++++++++--------
 addons/mail/wizard/invite.py                  |  7 ++-
 addons/mail/wizard/mail_resend_message.py     | 21 ++++++++-
 addons/portal/models/portal_mixin.py          |  2 +-
 addons/project/models/project.py              |  3 +-
 addons/test_mail/tests/test_performance.py    | 16 +++----
 addons/website_blog/models/website_blog.py    |  9 ++--
 addons/website_forum/models/forum.py          | 10 ++---
 13 files changed, 101 insertions(+), 67 deletions(-)

diff --git a/addons/crm/models/crm_lead.py b/addons/crm/models/crm_lead.py
index 35806bddb6f..929dda2f379 100644
--- a/addons/crm/models/crm_lead.py
+++ b/addons/crm/models/crm_lead.py
@@ -1116,8 +1116,9 @@ class Lead(models.Model):
         if self.team_id:
             salesman_actions.append({'url': self._notify_get_action_link('view', res_id=self.team_id.id, model=self.team_id._name), 'title': _('Sales Team Settings')})
 
+        salesman_group_id = self.env.ref('sales_team.group_sale_salesman').id
         new_group = (
-            'group_sale_salesman', lambda partner: bool(partner.user_ids) and any(user.has_group('sales_team.group_sale_salesman') for user in partner.user_ids), {
+            'group_sale_salesman', lambda pdata: pdata['type'] == 'user' and salesman_group_id in pdata['groups'], {
                 'actions': salesman_actions,
             })
 
diff --git a/addons/hr_holidays/models/hr_leave.py b/addons/hr_holidays/models/hr_leave.py
index 201c62eb8f0..8a6474ba841 100644
--- a/addons/hr_holidays/models/hr_leave.py
+++ b/addons/hr_holidays/models/hr_leave.py
@@ -664,8 +664,9 @@ class HolidaysRequest(models.Model):
             ref_action = self._notify_get_action_link('controller', controller='/leave/refuse')
             hr_actions += [{'url': ref_action, 'title': _('Refuse')}]
 
+        holiday_user_group_id = self.env.ref('hr_holidays.group_hr_holidays_user').id
         new_group = (
-            'group_hr_holidays_user', lambda partner: bool(partner.user_ids) and any(user.has_group('hr_holidays.group_hr_holidays_user') for user in partner.user_ids), {
+            'group_hr_holidays_user', lambda pdata: pdata['type'] == 'user' and holiday_user_group_id in pdata['groups'], {
                 'actions': hr_actions,
             })
 
diff --git a/addons/hr_holidays/models/hr_leave_allocation.py b/addons/hr_holidays/models/hr_leave_allocation.py
index 7e952ebe42d..1417150f150 100644
--- a/addons/hr_holidays/models/hr_leave_allocation.py
+++ b/addons/hr_holidays/models/hr_leave_allocation.py
@@ -509,8 +509,9 @@ class HolidaysAllocation(models.Model):
             ref_action = self._notify_get_action_link('controller', controller='/allocation/refuse')
             hr_actions += [{'url': ref_action, 'title': _('Refuse')}]
 
+        holiday_user_group_id = self.env.ref('hr_holidays.group_hr_holidays_user').id
         new_group = (
-            'group_hr_holidays_user', lambda partner: bool(partner.user_ids) and any(user.has_group('hr_holidays.group_hr_holidays_user') for user in partner.user_ids), {
+            'group_hr_holidays_user', lambda pdata: pdata['type'] == 'user' and holiday_user_group_id in pdata['groups'], {
                 'actions': hr_actions,
             })
 
diff --git a/addons/mail/models/mail_message.py b/addons/mail/models/mail_message.py
index 78487f35af7..07ec041050b 100644
--- a/addons/mail/models/mail_message.py
+++ b/addons/mail/models/mail_message.py
@@ -1054,7 +1054,6 @@ class Message(models.Model):
         self.ensure_one()
 
         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']
 
         message_values = {}
@@ -1062,24 +1061,27 @@ class Message(models.Model):
             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 message_values and record and hasattr(record, 'message_get_message_notify_values'):
-            message_values.update(record.message_get_message_notify_values(self, message_values))
+        if message_values and record and hasattr(record, '_notify_customize_recipients'):
+            message_values.update(record._notify_customize_recipients(self, message_values, rdata))
         if message_values:
             self.write(message_values)
 
         # 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_cids or email_pids:
-            self.env['res.partner'].sudo().search([
-                '|',
-                ('id', 'in', email_pids),
+        if email_cids:
+            new_pids = self.env['res.partner'].sudo().search([
+                ('id', 'not in', [r['id'] for r in rdata['partners']]),
                 ('channel_ids', 'in', email_cids),
-                ('email', '!=', self.author_id.email or self.email_from),
-            ])._notify(self, record, force_send=force_send, send_after_commit=send_after_commit, model_description=model_description, mail_auto_delete=mail_auto_delete)
+                ('email', 'not in', [self.author_id.email, self.email_from]),
+            ])
+            for partner in new_pids:
+                rdata['partners'].append({'id': partner.id, 'share': True, 'notif': 'email', 'type': 'customer', 'groups': []})
+
+        partner_email_rdata = [r for r in rdata['partners'] if r['notif'] == 'email']
+        if partner_email_rdata:
+            self.env['res.partner']._notify(self, partner_email_rdata, record, force_send=force_send, send_after_commit=send_after_commit, model_description=model_description, mail_auto_delete=mail_auto_delete)
 
         if inbox_pids:
-            self.env['res.partner'].sudo().browse(inbox_pids)._notify_by_chat(self)
+            self.env['res.partner'].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)
diff --git a/addons/mail/models/mail_thread.py b/addons/mail/models/mail_thread.py
index 1fe466d164a..fbdfcbd42e9 100644
--- a/addons/mail/models/mail_thread.py
+++ b/addons/mail/models/mail_thread.py
@@ -707,7 +707,7 @@ class MailThread(models.AbstractModel):
         return groups
 
     @api.multi
-    def _notify_classify_recipients(self, message, recipients):
+    def _notify_classify_recipients(self, message, recipient_data):
         """ Classify recipients to be notified of a message in groups to have
         specific rendering depending on their group. For example users could
         have access to buttons customers should not have in their emails.
@@ -716,7 +716,7 @@ class MailThread(models.AbstractModel):
         method defined here-under.
 
         :param message: mail.message record about to be notified
-        :param recipients: res.partner recordset to notify
+        :param recipients: res.partner recordset to notify UPDATE ME
         """
         result = {}
 
@@ -729,11 +729,11 @@ class MailThread(models.AbstractModel):
             view_title = _('View')
 
         default_groups = [
-            ('user', lambda partner: bool(partner.user_ids) and not any(user.share for user in partner.user_ids), {}),
-            ('portal', lambda partner: bool(partner.user_ids) and all(user.share for user in partner.user_ids), {
+            ('user', lambda pdata: pdata['type'] == 'user', {}),
+            ('portal', lambda pdata: pdata['type'] == 'portal', {
                 'has_button_access': False,
             }),
-            ('customer', lambda partner: True, {
+            ('customer', lambda pdata: True, {
                 'has_button_access': False,
             })
         ]
@@ -746,12 +746,12 @@ class MailThread(models.AbstractModel):
                 'url': access_link,
                 'title': view_title})
             group_data.setdefault('actions', list())
-            group_data.setdefault('recipients', self.env['res.partner'])
+            group_data.setdefault('recipients', list())
 
-        for recipient in recipients:
+        for recipient in recipient_data:
             for group_name, group_func, group_data in groups:
                 if group_func(recipient):
-                    group_data['recipients'] |= recipient
+                    group_data['recipients'].append(recipient['id'])
                     break
 
         for group_name, group_method, group_data in groups:
@@ -759,13 +759,13 @@ class MailThread(models.AbstractModel):
 
         return result
 
-    def _notify_classify_recipients_on_records(self, message, recipients, records=None):
+    def _notify_classify_recipients_on_records(self, message, recipient_data, records=None):
         """ Generic wrapper on ``_notify_classify_recipients`` checking mail.thread
         inheritance and allowing to call model-specific implementation in a one liner.
         This method should not be overridden. """
         if records and hasattr(records, '_notify_classify_recipients'):
-            return records._notify_classify_recipients(message, recipients)
-        return self._notify_classify_recipients(message, recipients)
+            return records._notify_classify_recipients(message, recipient_data)
+        return self._notify_classify_recipients(message, recipient_data)
 
     @api.multi
     def _notify_get_reply_to(self, default=None, records=None, company=None, doc_names=None):
diff --git a/addons/mail/models/res_partner.py b/addons/mail/models/res_partner.py
index aa52ca8d339..b0bdf1e09b2 100644
--- a/addons/mail/models/res_partner.py
+++ b/addons/mail/models/res_partner.py
@@ -79,12 +79,13 @@ class Partner(models.Model):
             'subtype': message.subtype_id,
         }
 
-    @api.multi
-    def _notify(self, message, record, force_send=False, send_after_commit=True, model_description=False, mail_auto_delete=True):
+    @api.model
+    def _notify(self, message, rdata, record, force_send=False, send_after_commit=True, model_description=False, mail_auto_delete=True):
         """ Method to send email linked to notified messages. The recipients are
         the recordset on which this method is called.
 
         :param message: mail.message record to notify;
+        :param rdata: recipient data (see mail.message _notify);
         :param record: optional record on which the message was posted;
         :param force_send: tells whether to send notification emails within the
           current transaction or to use the email queue;
@@ -94,7 +95,7 @@ class Partner(models.Model):
           notification templates);
         :param mail_auto_delete: delete notification emails once sent;
         """
-        if not self.ids:
+        if not rdata:
             return True
 
         template_xmlid = message.layout if message.layout else 'mail.message_notification_email'
@@ -117,9 +118,11 @@ class Partner(models.Model):
             base_mail_values.update(self.env['mail.thread']._notify_specific_email_values_on_records(message, records=record))
 
         # classify recipients: actions / no action
-        recipients = self.env['mail.thread']._notify_classify_recipients_on_records(message, self, records=record)
+        recipients = self.env['mail.thread']._notify_classify_recipients_on_records(message, rdata, records=record)
 
-        emails = self.env['mail.mail']
+        Mail = self.env['mail.mail'].sudo()
+        emails = self.env['mail.mail'].sudo()
+        email_pids = set()
         recipients_nbr, recipients_max = 0, 50
         for group_tpl_values in [group for group in recipients.values() if group['recipients']]:
             # generate notification email content
@@ -129,7 +132,7 @@ class Partner(models.Model):
             mail_subject = message.subject or (message.record_name and 'Re: %s' % message.record_name)
 
             # send email
-            for email_chunk in split_every(50, group_tpl_values['recipients'].ids):
+            for email_chunk in split_every(50, group_tpl_values['recipients']):
                 recipient_values = self.env['mail.thread']._notify_email_recipients_on_records(message, email_chunk, records=record)
                 create_values = {
                     'body_html': mail_body,
@@ -137,19 +140,23 @@ class Partner(models.Model):
                 }
                 create_values.update(base_mail_values)
                 create_values.update(recipient_values)
-                emails |= self.env['mail.mail'].create(create_values)
+                recipient_ids = [r[1] for r in create_values.get('recipient_ids', [])]
+                email = Mail.create(create_values)
 
-        # update notifications
-        for email in emails:
-            notifications = self.env['mail.notification'].sudo().search([
-                ('mail_message_id', '=', email.mail_message_id.id),
-                ('res_partner_id', 'in', email.recipient_ids.ids)])
-            notifications.write({
-                'mail_id': email.id,
-                'is_email': True,
-                'is_read': True,  # handle by email discards Inbox notification
-                'email_status': 'ready',
-            })
+                if email and recipient_ids:
+                    notifications = self.env['mail.notification'].sudo().search([
+                        ('mail_message_id', '=', email.mail_message_id.id),
+                        ('res_partner_id', 'in', list(recipient_ids))
+                    ])
+                    notifications.write({
+                        'is_email': True,
+                        'mail_id': email.id,
+                        'is_read': True,  # handle by email discards Inbox notification
+                        'email_status': 'ready',
+                    })
+
+                emails |= email
+                email_pids.update(recipient_ids)
 
         # NOTE:
         #   1. for more than 50 followers, use the queue system
diff --git a/addons/mail/wizard/invite.py b/addons/mail/wizard/invite.py
index 541d3c1ea4f..39d6c0bdeeb 100644
--- a/addons/mail/wizard/invite.py
+++ b/addons/mail/wizard/invite.py
@@ -60,6 +60,11 @@ class Invite(models.TransientModel):
                     'no_auto_thread': True,
                     'add_sign': True,
                 })
-                new_partners.with_context(auto_delete=True)._notify(message, document, force_send=True, send_after_commit=False)
+                self.env['res.partner'].with_context(auto_delete=True)._notify(
+                    message,
+                    [{'id': pid, 'share': True, 'notif': 'email', 'type': 'customer', 'groups': []} for pid in new_partners.ids],
+                    document,
+                    force_send=True,
+                    send_after_commit=False)
                 message.unlink()
         return {'type': 'ir.actions.act_window_close'}
diff --git a/addons/mail/wizard/mail_resend_message.py b/addons/mail/wizard/mail_resend_message.py
index bf56128ad61..77b1a2a102d 100644
--- a/addons/mail/wizard/mail_resend_message.py
+++ b/addons/mail/wizard/mail_resend_message.py
@@ -60,7 +60,26 @@ class MailResendMessage(models.TransientModel):
             if to_send:
                 message = wizard.mail_message_id
                 record = self.env[message.model].browse(message.res_id) if message.model and message.res_id else None
-                to_send.sudo()._notify(self.mail_message_id, record, force_send=True, send_after_commit=False)
+
+                rdata = []
+                for pid, cid, active, pshare, ctype, notif, groups in self.env['mail.followers']._get_recipient_data(None, False, pids=to_send.ids):
+                    if pid and notif == 'email' or not notif:
+                        pdata = {'id': pid, 'share': pshare, 'active': active, 'notif': 'email', 'groups': groups}
+                        if not pshare and notif:  # has an user and is not shared, is therefore user
+                            rdata.append(dict(pdata, type='user'))
+                        elif pshare and notif:  # has an user and is shared, is therefore portal
+                            rdata.append(dict(pdata, type='portal'))
+                        else:  # has no user, is therefore customer
+                            rdata.append(dict(pdata, type='customer'))
+
+                self.env['res.partner']._notify(
+                    message,
+                    rdata,
+                    record,
+                    force_send=True,
+                    send_after_commit=False
+                )
+
             self.mail_message_id._notify_failure_update()
         return {'type': 'ir.actions.act_window_close'}
 
diff --git a/addons/portal/models/portal_mixin.py b/addons/portal/models/portal_mixin.py
index 2a0c0d2a5a4..a03794b5dae 100644
--- a/addons/portal/models/portal_mixin.py
+++ b/addons/portal/models/portal_mixin.py
@@ -73,7 +73,7 @@ class PortalMixin(models.AbstractModel):
             access_link = self._notify_get_action_link('view', **additional_params)
 
             new_group = [
-                ('portal_customer', lambda partner: partner.id == customer.id, {
+                ('portal_customer', lambda pdata: pdata['id'] == customer.id, {
                     'has_button_access': False,
                     'button_access': {
                         'url': access_link,
diff --git a/addons/project/models/project.py b/addons/project/models/project.py
index 373e6e0eee7..c5c5370d78a 100644
--- a/addons/project/models/project.py
+++ b/addons/project/models/project.py
@@ -829,8 +829,9 @@ class Task(models.Model):
         if not self.user_id and not self.stage_id.fold:
             take_action = self._notify_get_action_link('assign')
             project_actions = [{'url': take_action, 'title': _('I take it')}]
+            project_user_group_id = self.env.ref('project.group_project_user').id
             new_group = (
-                'group_project_user', lambda partner: bool(partner.user_ids) and any(user.has_group('project.group_project_user') for user in partner.user_ids), {
+                'group_project_user', lambda pdata: pdata['type'] == 'user' and project_user_group_id in pdata['groups'], {
                     'actions': project_actions,
                 })
             groups = [new_group] + groups
diff --git a/addons/test_mail/tests/test_performance.py b/addons/test_mail/tests/test_performance.py
index 95e950fa97c..94f5fcd7817 100644
--- a/addons/test_mail/tests/test_performance.py
+++ b/addons/test_mail/tests/test_performance.py
@@ -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=59, emp=76):  # com runbot: 59 - 76 //  test_mail only: 59 - 76
+        with self.assertQueryCount(margin=1, admin=56, emp=71):  # com runbot: 56 - 71 //  test_mail only: 56 - 71
             record.write({
                 'user_id': self.user_test.id,
             })
@@ -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=50, emp=68):  # com runbot: 48 - 66 // test_mail only: 50 - 68
+        with self.assertQueryCount(margin=1, admin=47, emp=64):  # test_mail only: 47 - 64
             record.message_post(
                 body='<p>Test Post Performances with an email ping</p>',
                 partner_ids=self.customer.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=82, emp=103):  # com runbot 80 - 101 // test_mail only: 82 - 103
+        with self.assertQueryCount(admin=79, emp=99):  # com runbot 77 - 97 // test_mail only: 79 - 99
             record.message_post(
                 body='<p>Test Post Performances</p>',
                 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=101, emp=134):  # com runbot 99 - 132 // test_mail only: 101 - 134
+        with self.assertQueryCount(admin=98, emp=130):  # com runbot 96 - 128 // test_mail only: 98 - 130
             record.message_post_with_template(template_id, message_type='comment', composition_mode='comment')
 
         self.assertEqual(record.message_ids[0].body, '<p>Adding stuff on %s</p>' % 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=61, emp=78):  # com runbot: 61 - 78 // test_mail only: 61 - 78
+        with self.assertQueryCount(admin=58, emp=73):  # com runbot: 58 - 73 // test_mail only: 58 - 73
             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=160, emp=193):  # com runbot: 160 - 193 // test_mail only: 160 - 193
+        with self.assertQueryCount(margin=1, admin=155, emp=184):  # com runbot: 154 - 183 // test_mail only: 155 - 184
             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=96, emp=112):  # com runbot: 96 - 112 // test_mail only: 96 - 112
+        with self.assertQueryCount(margin=1, admin=94, emp=109):  # com runbot: 94 - 109 // test_mail only: 94 - 109
             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=103, emp=122):  # test_mail only: 103 - 122
+        with self.assertQueryCount(margin=1, admin=100, emp=119):  # test_mail only: 100 - 119
             rec.write({
                 'name': 'Test2',
                 'umbrella_id': umbrella_id,
diff --git a/addons/website_blog/models/website_blog.py b/addons/website_blog/models/website_blog.py
index 6ff422579fd..b1e0253b385 100644
--- a/addons/website_blog/models/website_blog.py
+++ b/addons/website_blog/models/website_blog.py
@@ -256,12 +256,11 @@ class BlogPost(models.Model):
         return groups
 
     @api.multi
-    def message_get_message_notify_values(self, message, message_values):
+    def _notify_customize_recipients(self, message, msg_vals, recipients_vals):
         """ Override to avoid keeping all notified recipients of a comment.
         We avoid tracking needaction on post comments. Only emails should be
         sufficient. """
-        if message.message_type == 'comment':
-            return {
-                'needaction_partner_ids': [],
-            }
+        msg_type = msg_vals.get('message_type') or message.message_type
+        if msg_type == 'comment':
+            return {'needaction_partner_ids': []}
         return {}
diff --git a/addons/website_forum/models/forum.py b/addons/website_forum/models/forum.py
index 74af33b462c..b9368c8d4d8 100644
--- a/addons/website_forum/models/forum.py
+++ b/addons/website_forum/models/forum.py
@@ -833,15 +833,13 @@ class Post(models.Model):
         return super(Post, self).message_post(message_type=message_type, **kwargs)
 
     @api.multi
-    def message_get_message_notify_values(self, message, message_values):
+    def _notify_customize_recipients(self, message, msg_vals, recipients_vals):
         """ Override to avoid keeping all notified recipients of a comment.
         We avoid tracking needaction on post comments. Only emails should be
         sufficient. """
-        if message.message_type == 'comment':
-            return {
-                'needaction_partner_ids': [],
-                'partner_ids': [],
-            }
+        msg_type = msg_vals.get('message_type') or message.message_type
+        if msg_type == 'comment':
+            return {'needaction_partner_ids': [], 'partner_ids': []}
         return {}