From a69c2bab0afbd401443685d94c12fcc39fe26b66 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Wed, 18 Nov 2015 18:11:52 +0100 Subject: [PATCH] [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 d4a1eb44354b594ee44720036a5851678b4d6b9e 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 | 23 +++++++++++++ addons/mail/models/mail_thread.py | 11 ++++++ addons/mail/models/res_partner.py | 12 ++++++- addons/mail/tests/common.py | 6 +++- addons/mail/tests/test_mail_channel.py | 47 ++++++++++++++++++++++++-- 5 files changed, 94 insertions(+), 5 deletions(-) diff --git a/addons/mail/models/mail_channel.py b/addons/mail/models/mail_channel.py index 7986a56a168..f577354a8c7 100644 --- a/addons/mail/models/mail_channel.py +++ b/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): diff --git a/addons/mail/models/mail_thread.py b/addons/mail/models/mail_thread.py index 3b2e161ffc7..99c0b90060b 100644 --- a/addons/mail/models/mail_thread.py +++ b/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 # ------------------------------------------------------ diff --git a/addons/mail/models/res_partner.py b/addons/mail/models/res_partner.py index b0ea116fef3..6b8fc7c4aed 100644 --- a/addons/mail/models/res_partner.py +++ b/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 diff --git a/addons/mail/tests/common.py b/addons/mail/tests/common.py index f70a929464c..d8865cb1bfc 100644 --- a/addons/mail/tests/common.py +++ b/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() diff --git a/addons/mail/tests/test_mail_channel.py b/addons/mail/tests/test_mail_channel.py index e2054d4704e..1c43f79edaf 100644 --- a/addons/mail/tests/test_mail_channel.py +++ b/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))])