[FIX] mail, mail_channel: recipients of channels

Mailing lists (mail.channel) should not send specific notification emails.
Indeed there can be a lot of recipients and customizing each email can
take time to compute. This leads to posting a message on a mail.channel
being very slow.

It is now possible for a model to customize the notification email
recipients computation. The first use is to ensure that mail.channel
encodes recipients using email_to instead of recipients_ids. This way
less processing is performed on notification emails.

This is a manual cherry-pick of commit d4a1eb4435
that was not forard ported. Mail has indeed evolved since 8.0 and its
forward port was not an easy task.

This commit also includes the fix of mail.channel recipients. All are now
considered as equal and receive a simple notification email. No distinction
is done between partners and users. The notification emails for channels
should be as simple as possible.

addons/mail/models/mail_channel.py

@@ -1,5 +1,7 @@
 # -*- coding: utf-8 -*-
 
+from email.utils import formataddr
+
 import datetime
 import uuid
 
@@ -179,6 +181,17 @@ class Channel(models.Model):
             self.sudo().message_post(body=notification, message_type="notification", subtype="mail.mt_comment", author_id=partner_id)
         return result
 
+    @api.multi
+    def _notification_group_recipients(self, message, recipients, done_ids, group_data):
+        """ All recipients of a message on a channel are considered as partners.
+        This means they will receive a minimal email, without a link to access
+        in the backend. Mailing lists should indeed send minimal emails to avoid
+        the noise. """
+        for recipient in recipients:
+            group_data['partner'] |= recipient
+            done_ids.add(recipient.id)
+        return super(Channel, self)._notification_group_recipients(message, recipients, done_ids, group_data)
+
     @api.multi
     def message_get_email_values(self, notif_mail=None):
         self.ensure_one()
@@ -203,6 +216,16 @@ class Channel(models.Model):
         res['headers'] = repr(headers)
         return res
 
+    @api.multi
+    def message_get_recipient_values(self, notif_message=None, recipient_ids=None):
+        # real mailing list: multiple recipients (hidden by X-Forge-To)
+        if self.alias_domain and self.alias_name:
+            return {
+                'email_to': ','.join(formataddr((partner.name, partner.email)) for partner in self.env['res.partner'].sudo().browse(recipient_ids)),
+                'recipient_ids': [],
+            }
+        return super(Channel, self).message_get_recipient_values(notif_message=notif_message, recipient_ids=recipient_ids)
+
     @api.multi
     @api.returns('self', lambda value: value.id)
     def message_post(self, body='', subject=None, message_type='notification', subtype=None, parent_id=False, attachments=None, content_subtype='html', **kwargs):

addons/mail/models/mail_thread.py

@@ -731,6 +731,17 @@ class MailThread(models.AbstractModel):
         res = dict()
         return res
 
+    @api.multi
+    def message_get_recipient_values(self, notif_message=None, recipient_ids=None):
+        """ Get specific notification recipient values to store on the notification
+        mail_mail. Basic method just set the recipient partners as mail_mail
+        recipients. Inherit this method to add custom behavior like using
+        recipient email_to to bypass the recipint_ids heuristics in the
+        mail sending mechanism. """
+        return {
+            'recipient_ids': [(4, pid) for pid in recipient_ids]
+        }
+
     # ------------------------------------------------------
     # Mail gateway
     # ------------------------------------------------------

addons/mail/models/res_partner.py

@@ -105,12 +105,22 @@ class Partner(models.Model):
         recipients_nbr, recipients_max = len(recipients), 50
         email_chunks = [recipients[x:x + recipients_max] for x in xrange(0, len(recipients), recipients_max)]
         for email_chunk in email_chunks:
+            # TDE FIXME: missing message parameter. So we will find mail_message_id
+            # in the mail_values and browse it. It should already be in the
+            # cache so should not impact performances.
+            mail_message_id = mail_values.get('mail_message_id')
+            message = self.env['mail.message'].browse(mail_message_id) if mail_message_id else None
+            if message and message.model and message.res_id and message.model in self.env and hasattr(self.env[message.model], 'message_get_recipient_values'):
+                tig = self.env[message.model].browse(message.res_id)
+                recipient_values = tig.message_get_recipient_values(notif_message=message, recipient_ids=email_chunk.ids)
+            else:
+                recipient_values = self.env['mail.thread'].message_get_recipient_values(notif_message=None, recipient_ids=email_chunk.ids)
             create_values = {
                 'body_html': body,
                 'subject': subject,
-                'recipient_ids': [(4, recipient.id) for recipient in email_chunk],
             }
             create_values.update(mail_values)
+            create_values.update(recipient_values)
             emails |= self.env['mail.mail'].create(create_values)
         return emails, recipients_nbr
 

addons/mail/tests/common.py

@@ -36,9 +36,12 @@ class TestMail(common.SavepointCase):
         def send_email(self, cr, uid, message, *args, **kwargs):
             return message['Message-Id']
 
+        def mail_group_message_get_recipient_values(self, cr, uid, ids, notif_message=None, recipient_ids=None, context=None):
+            return self.pool['mail.thread'].message_get_recipient_values(cr, uid, ids, notif_message=notif_message, recipient_ids=recipient_ids, context=context)
+
         cls.env['ir.mail_server']._patch_method('build_email', build_email)
         cls.env['ir.mail_server']._patch_method('send_email', send_email)
-
+        cls.env['mail.channel']._patch_method('message_get_recipient_values', mail_group_message_get_recipient_values)
 
         # User groups
         user_group_employee = cls.env.ref('base.group_user')
@@ -119,4 +122,5 @@ class TestMail(common.SavepointCase):
         # Remove mocks
         cls.env['ir.mail_server']._revert_method('build_email')
         cls.env['ir.mail_server']._revert_method('send_email')
+        cls.env['mail.channel']._revert_method('message_get_recipient_values')
         super(TestMail, cls).tearDownClass()

addons/mail/tests/test_mail_channel.py

@@ -1,5 +1,7 @@
 # -*- coding: utf-8 -*-
 
+from email.utils import formataddr
+
 from .common import TestMail
 from openerp.exceptions import AccessError
 from openerp.exceptions import except_orm
@@ -8,10 +10,14 @@ from openerp.tools import mute_logger
 
 class TestMailGroup(TestMail):
 
-    def setUp(self):
-        super(TestMailGroup, self).setUp()
+    @classmethod
+    def setUpClass(cls):
+        super(TestMailGroup, cls).setUpClass()
+        # for specific tests of mail channel, get back to its expected behavior
+        cls.registry('mail.channel')._revert_method('message_get_recipient_values')
+
         # Private: private group
-        self.group_private = self.env['mail.channel'].with_context({
+        cls.group_private = cls.env['mail.channel'].with_context({
             'mail_create_nolog': True,
             'mail_create_nosubscribe': True
         }).create({
@@ -19,6 +25,14 @@ class TestMailGroup(TestMail):
             'public': 'private'}
         ).with_context({'mail_create_nosubscribe': False})
 
+    @classmethod
+    def tearDownClass(cls):
+        # set master class behavior back
+        def mail_group_message_get_recipient_values(self, cr, uid, ids, notif_message=None, recipient_ids=None, context=None):
+            return self.pool['mail.thread'].message_get_recipient_values(cr, uid, ids, notif_message=notif_message, recipient_ids=recipient_ids, context=context)
+        cls.env['mail.channel']._patch_method('message_get_recipient_values', mail_group_message_get_recipient_values)
+        super(TestMail, cls).tearDownClass()
+
     @mute_logger('openerp.addons.base.ir.ir_model', 'openerp.models')
     def test_access_rights_public(self):
         # Read public group -> ok
@@ -88,3 +102,30 @@ class TestMailGroup(TestMail):
             # TODO Change the except_orm to Warning
             with self.assertRaises(except_orm):
                 trigger_read = partner.name
+
+    def test_mail_group_notification_recipients_grouped(self):
+        # Data: set alias_domain to see emails with alias
+        self.registry('ir.config_parameter').set_param(self.cr, self.uid, 'mail.catchall.domain', 'schlouby.fr')
+        self.group_private.write({'alias_name': 'Test'})
+        self.group_private.message_subscribe_users([self.user_employee.id, self.user_portal.id])
+
+        self.group_private.message_post(body="Test", message_type='comment', subtype='mt_comment')
+        sent_emails = self._mails
+        self.assertEqual(len(sent_emails), 1)
+        for email in sent_emails:
+            self.assertEqual(
+                set(email['email_to']),
+                set([formataddr((self.user_employee.name, self.user_employee.email)), formataddr((self.user_portal.name, self.user_portal.email))]))
+
+    def test_mail_group_notification_recipients_separated(self):
+        # Remove alias, should trigger classic behavior of mail group
+        self.group_private.write({'alias_name': False})
+        self.group_private.message_subscribe_users([self.user_employee.id, self.user_portal.id])
+
+        self.group_private.message_post(body="Test", message_type='comment', subtype='mt_comment')
+        sent_emails = self._mails
+        self.assertEqual(len(sent_emails), 2)
+        for email in sent_emails:
+            self.assertIn(
+                email['email_to'][0],
+                [formataddr((self.user_employee.name, self.user_employee.email)), formataddr((self.user_portal.name, self.user_portal.email))])