From ed8a101a0872410492c717e6dea78db9262ba4d0 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Dufays Date: Tue, 19 Jul 2022 14:00:28 +0000 Subject: [PATCH] [FIX] test_mass_mailing, {test_}mail: always apply blacklist for mass mailing Always apply the blacklist in mass_mail composition mode regardless of the recipient model implementing mail.thread.blacklist or not. This solves the problem of mail sent to black listed address for model not inheriting from mail.thread.blacklist. Technical notes: - it has been done in mail.compose.message _get_blacklist_record_ids ignoring the mixin mail.thread.blacklist to avoid model change in stable. - some tests have one added query because the blacklist is now queried for each batch mail sends even if the model of the recipient doesn't implement mail.thread.blacklist. Task-2834862 closes odoo/odoo#118497 X-original-commit: 263e86114c60650f421354e165364afcd4122461 Signed-off-by: Dufays Pierre-Yves (pydu) Signed-off-by: Thibault Delavallee (tde) --- addons/mail/wizard/mail_compose_message.py | 15 ++++++++---- addons/test_mail/tests/test_performance.py | 4 ++-- .../test_mass_mailing/tests/test_mailing.py | 24 ++++++++++++++++++- .../tests/test_performance.py | 2 +- 4 files changed, 37 insertions(+), 8 deletions(-) diff --git a/addons/mail/wizard/mail_compose_message.py b/addons/mail/wizard/mail_compose_message.py index c9213eb02e5..abdb04c9db5 100644 --- a/addons/mail/wizard/mail_compose_message.py +++ b/addons/mail/wizard/mail_compose_message.py @@ -1089,7 +1089,7 @@ class MailComposer(models.TransientModel): :return: updated mail_values_dict """ recipients_info = self._get_recipients_data(mail_values_dict) - blacklist_ids = self._get_blacklist_record_ids(mail_values_dict) + blacklist_ids = self._get_blacklist_record_ids(mail_values_dict, recipients_info) optout_emails = self._get_optout_emails(mail_values_dict) done_emails = self._get_done_emails(mail_values_dict) # in case of an invoice e.g. @@ -1183,20 +1183,27 @@ class MailComposer(models.TransientModel): # EMAIL MANAGEMENT # --------------------------------------------------------------------- - def _get_blacklist_record_ids(self, mail_values_dict): + def _get_blacklist_record_ids(self, mail_values_dict, recipients_info=None): blacklisted_rec_ids = set() if not self.use_exclusion_list: return blacklisted_rec_ids - if self.composition_mode == 'mass_mail' and issubclass(type(self.env[self.model]), self.pool['mail.thread.blacklist']): + if self.composition_mode == 'mass_mail': self.env['mail.blacklist'].flush_model(['email', 'active']) self._cr.execute("SELECT email FROM mail_blacklist WHERE active=true") blacklist = {x[0] for x in self._cr.fetchall()} - if blacklist: + if not blacklist: + return blacklisted_rec_ids + if issubclass(type(self.env[self.model]), self.pool['mail.thread.blacklist']): targets = self.env[self.model].browse(mail_values_dict.keys()) targets.fetch(['email_normalized']) # First extract email from recipient before comparing with blacklist blacklisted_rec_ids.update(target.id for target in targets if target.email_normalized in blacklist) + elif recipients_info: + # Note that we exclude the record if at least one recipient is blacklisted (-> even if not all) + # But as commented above: Mass mailing should always have a single recipient per record. + blacklisted_rec_ids.update(res_id for res_id, recipient_info in recipients_info.items() + if blacklist & set(recipient_info['mail_to_normalized'])) return blacklisted_rec_ids def _get_done_emails(self, mail_values_dict): diff --git a/addons/test_mail/tests/test_performance.py b/addons/test_mail/tests/test_performance.py index 9f8508f6d57..f8e77061eac 100644 --- a/addons/test_mail/tests/test_performance.py +++ b/addons/test_mail/tests/test_performance.py @@ -415,7 +415,7 @@ class TestMailAPIPerformance(BaseMailPerformance): 'default_template_id': test_template.id, }).create({}) - with self.assertQueryCount(admin=118, employee=121), self.mock_mail_gateway(): + with self.assertQueryCount(admin=119, employee=122), self.mock_mail_gateway(): composer._action_send_mail() self.assertEqual(len(self._new_mails), 10) @@ -1079,7 +1079,7 @@ class TestMailComplexPerformance(BaseMailPerformance): rec1 = rec.with_context(active_test=False) # to see inactive records self.assertEqual(rec1.message_partner_ids, self.partners | self.env.user.partner_id | self.user_portal.partner_id) - with self.assertQueryCount(admin=32, employee=32): + with self.assertQueryCount(admin=33, employee=33): rec.write({ 'name': 'Test2', 'customer_id': customer_id, diff --git a/addons/test_mass_mailing/tests/test_mailing.py b/addons/test_mass_mailing/tests/test_mailing.py index 9f7cec41480..ed5d9863b40 100644 --- a/addons/test_mass_mailing/tests/test_mailing.py +++ b/addons/test_mass_mailing/tests/test_mailing.py @@ -5,7 +5,7 @@ from odoo.addons.test_mass_mailing.data.mail_test_data import MAIL_TEMPLATE from odoo.addons.test_mass_mailing.tests.common import TestMassMailCommon from odoo.tests import tagged from odoo.tests.common import users -from odoo.tools import mute_logger +from odoo.tools import mute_logger, email_normalize @tagged('mass_mailing') @@ -254,6 +254,28 @@ class TestMassMailing(TestMassMailCommon): ) self.assertEqual(mailing.canceled, 2) + @users('user_marketing') + @mute_logger('odoo.addons.mail.models.mail_mail') + def test_mailing_w_blacklist_nomixin(self): + """Test that blacklist is applied even if the target model doesn't inherit + from mail.thread.blacklist.""" + test_records = self._create_mailing_test_records(model='mailing.test.simple', count=2) + self.mailing_bl.write({ + 'mailing_domain': [('id', 'in', test_records.ids)], + 'mailing_model_id': self.env['ir.model']._get('mailing.test.simple').id, + }) + self.env['mail.blacklist'].create([{ + 'email': test_records[0].email_from, + 'active': True, + }]) + + with self.mock_mail_gateway(mail_unlink_sent=False): + self.mailing_bl.action_send_mail() + self.assertMailTraces([ + {'email': email_normalize(test_records[0].email_from), 'trace_status': 'cancel', 'failure_type': 'mail_bl'}, + {'email': email_normalize(test_records[1].email_from), 'trace_status': 'sent'}, + ], self.mailing_bl, test_records, check_mail=False) + @users('user_marketing') @mute_logger('odoo.addons.mail.models.mail_mail') def test_mailing_w_opt_out(self): diff --git a/addons/test_mass_mailing/tests/test_performance.py b/addons/test_mass_mailing/tests/test_performance.py index c1e9d4942f4..31824514d7e 100644 --- a/addons/test_mass_mailing/tests/test_performance.py +++ b/addons/test_mass_mailing/tests/test_performance.py @@ -47,7 +47,7 @@ class TestMassMailPerformance(TestMassMailPerformanceBase): }) # runbot needs +51 compared to local - with self.assertQueryCount(__system__=1523, marketing=1524): + with self.assertQueryCount(__system__=1524, marketing=1525): mailing.action_send_mail() self.assertEqual(mailing.sent, 50)