[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) <pydu@odoo.com> Signed-off-by: Thibault Delavallee (tde) <tde@openerp.com>
This commit is contained in:
@@ -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):
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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):
|
||||
|
||||
@@ -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)
|
||||
|
||||
Reference in New Issue
Block a user