diff --git a/addons/mail/models/mail_mail.py b/addons/mail/models/mail_mail.py index 1eb110a4abf..d92090feb75 100644 --- a/addons/mail/models/mail_mail.py +++ b/addons/mail/models/mail_mail.py @@ -182,7 +182,7 @@ class MailMail(models.Model): messages = notifications.mapped('mail_message_id').filtered(lambda m: m.is_thread_message()) # TDE TODO: could be great to notify message-based, not notifications-based, to lessen number of notifs messages._notify_message_notification_update() # notify user that we have a failure - if not failure_type or failure_type == 'RECIPIENT': # if we have another error, we want to keep the mail. + if not failure_type or failure_type in ['RECIPIENT', 'mail_email_missing']: # if we have another error, we want to keep the mail. mail_to_delete_ids = [mail.id for mail in self if mail.auto_delete] self.browse(mail_to_delete_ids).sudo().unlink() return True @@ -376,6 +376,8 @@ class MailMail(models.Model): # build an RFC2822 email.message.Message object and send it without queuing res = None + # TDE note: could be great to pre-detect missing to/cc and skip sending it + # to go directly to failed state update for email in email_list: msg = IrMailServer.build_email( email_from=mail.email_from, @@ -401,7 +403,11 @@ class MailMail(models.Model): processing_pid = None except AssertionError as error: if str(error) == IrMailServer.NO_VALID_RECIPIENT: - failure_type = "RECIPIENT" + # if we have a list of void emails for email_list -> email missing, otherwise generic email failure + if not email.get('email_to') and failure_type != "RECIPIENT": + failure_type = "mail_email_missing" + else: + failure_type = "RECIPIENT" # No valid recipient found for this particular # mail item -> ignore error to avoid blocking # delivery to next recipients, if any. If this is diff --git a/addons/mail/models/mail_message.py b/addons/mail/models/mail_message.py index 88da42103d9..f7d19c4988b 100644 --- a/addons/mail/models/mail_message.py +++ b/addons/mail/models/mail_message.py @@ -27,37 +27,37 @@ class Message(models.Model): * notification_status 'ready', 'sent', 'bounce', 'exception', 'canceled' * notification_type - 'inbox', 'email', 'sms' (SMS addon), 'snail' (snailmail addon) + 'inbox', 'email', 'sms' (SMS addon), 'snail' (snailmail addon) * failure_type - # mail - "SMTP", "RECIPIENT", "BOUNCE", "UNKNOWN" - # sms (SMS addon) - 'sms_number_missing', 'sms_number_format', 'sms_credit', - 'sms_server', 'sms_acc' - # snailmail (snailmail addon) - 'sn_credit', 'sn_trial', 'sn_price', 'sn_fields', - 'sn_format', 'sn_error' + # mail + "SMTP", "RECIPIENT", "BOUNCE", "UNKNOWN", "mail_email_missing" + # sms (SMS addon) + 'sms_number_missing', 'sms_number_format', 'sms_credit', + 'sms_server', 'sms_acc' + # snailmail (snailmail addon) + 'sn_credit', 'sn_trial', 'sn_price', 'sn_fields', + 'sn_format', 'sn_error' * mail.mail * state - 'outgoing', 'sent', 'received', 'exception', 'cancel' + 'outgoing', 'sent', 'received', 'exception', 'cancel' * failure_reason: text * sms.sms (SMS addon) * state - 'outgoing', 'sent', 'error', 'canceled' + 'outgoing', 'sent', 'error', 'canceled' * error_code - 'sms_number_missing', 'sms_number_format', 'sms_credit', - 'sms_server', 'sms_acc', - # mass mode specific codes - 'sms_blacklist', 'sms_duplicate' + 'sms_number_missing', 'sms_number_format', 'sms_credit', + 'sms_server', 'sms_acc', + # mass mode specific codes + 'sms_blacklist', 'sms_duplicate' * snailmail.letter (snailmail addon) * state - 'pending', 'sent', 'error', 'canceled' + 'pending', 'sent', 'error', 'canceled' * error_code - 'CREDIT_ERROR', 'TRIAL_ERROR', 'NO_PRICE_AVAILABLE', 'FORMAT_ERROR', - 'UNKNOWN_ERROR', + 'CREDIT_ERROR', 'TRIAL_ERROR', 'NO_PRICE_AVAILABLE', 'FORMAT_ERROR', + 'UNKNOWN_ERROR', See ``mailing.trace`` model in mass_mailing application for mailing trace information. diff --git a/addons/mail/models/mail_notification.py b/addons/mail/models/mail_notification.py index daa88004c6a..8110ff22612 100644 --- a/addons/mail/models/mail_notification.py +++ b/addons/mail/models/mail_notification.py @@ -38,6 +38,7 @@ class MailNotification(models.Model): ("RECIPIENT", "Invalid email address"), ("BOUNCE", "Email address rejected by destination"), ("UNKNOWN", "Unknown error"), + ("mail_email_missing", "Missing email address"), ], string='Failure type') failure_reason = fields.Text('Failure reason', copy=False) diff --git a/addons/mass_mailing/models/mailing_trace.py b/addons/mass_mailing/models/mailing_trace.py index f35e54f0f3a..a50aefbb59a 100644 --- a/addons/mass_mailing/models/mailing_trace.py +++ b/addons/mass_mailing/models/mailing_trace.py @@ -17,12 +17,14 @@ class MailingTrace(models.Model): 'exception', 'bounced', 'ignored' * failure_type # mass_mailing - "SMTP", "RECIPIENT", "BOUNCE", "UNKNOWN" + "SMTP", "RECIPIENT", "BOUNCE", "UNKNOWN", "mail_email_missing" + # mass mailing mass mode specific codes + "mail_bl", "mail_optout", "mail_dup" # mass_mailing_sms 'sms_number_missing', 'sms_number_format', 'sms_credit', 'sms_server', 'sms_acc' # mass_mailing_sms mass mode specific codes - 'sms_blacklist', 'sms_duplicate' + 'sms_blacklist', 'sms_duplicate', 'sms_optout', * ignored: * mail: set in get_mail_values in composer, if email is blacklisted (mail) or in opt_out / seen list (mass_mailing) or email_to is void @@ -30,13 +32,10 @@ class MailingTrace(models.Model): * sms: set in _prepare_mass_sms_trace_values in composer if sms is in cancel state; either blacklisted (sms) or in opt_out / seen list (sms); - * difference: void mail -> cancel -> ignore, void sms -> error - sms_number_missing -> exception - * difference: invalid mail -> cancel -> ignore, invalid sms -> error - sms_number_format -> sent + bounce; + * void mail / void sms number -> error (mail_missing, sms_number_missing) + * invalid mail / invalid sms number -> error (RECIPIENT, sms_number_format) * exception: set in _postprocess_sent_message (_postprocess_iap_sent_sms) - if mail (sms) not sent with failure type, reset if sent; also set for - sms in _prepare_mass_sms_trace_values if void number + if mail (sms) not sent with failure type, reset if sent; * sent: set in _postprocess_sent_message (_postprocess_iap_sent_sms) if mail (sms) sent * clicked: triggered by add_click @@ -100,6 +99,11 @@ class MailingTrace(models.Model): ("RECIPIENT", "Invalid email address"), ("BOUNCE", "Email address rejected by destination"), ("UNKNOWN", "Unknown error"), + ("mail_email_missing", "Missing email address"), + # mass mode + ("mail_bl", "Blacklisted Address"), + ("mail_optout", "Opted Out"), + ("mail_dup", "Duplicated Email"), ], string='Failure type') state_update = fields.Datetime(compute="_compute_state", string='State Update', help='Last state update of the mail', diff --git a/addons/mass_mailing/tests/common.py b/addons/mass_mailing/tests/common.py index 315aacef3be..56b7155919d 100644 --- a/addons/mass_mailing/tests/common.py +++ b/addons/mass_mailing/tests/common.py @@ -104,14 +104,15 @@ class MassMailCase(MailCase, MockLinkTracker): 'MailTrace: email %s (recipient %s, state: %s, record: %s): found %s records (1 expected)' % (email, partner, state, record, len(recipient_trace)) ) self.assertTrue(bool(recipient_trace.mail_mail_id_int)) + if 'failure_type' in recipient_info or state in ('ignored', 'exception', 'canceled', 'bounced'): + self.assertEqual(recipient_trace.failure_type, recipient_info['failure_type']) if check_mail: if author is None: author = self.env.user.partner_id + # mail.mail specific values to check fields_values = {'mailing_id': mailing} - if 'failure_type' in recipient_info: - fields_values['failure_type'] = recipient_info['failure_type'] # specific for partner: email_formatted is used if partner: diff --git a/addons/mass_mailing/tests/test_mailing_internals.py b/addons/mass_mailing/tests/test_mailing_internals.py index 6b39104e9dc..4fafbf6e3f8 100644 --- a/addons/mass_mailing/tests/test_mailing_internals.py +++ b/addons/mass_mailing/tests/test_mailing_internals.py @@ -276,7 +276,7 @@ class TestMassMailFeatures(MassMailCommon, CronMixinCase): self.assertMailTraces( [{'partner': partner_a}, - {'partner': partner_b, 'state': 'ignored'}], + {'partner': partner_b, 'state': 'ignored', 'failure_type': 'mail_bl'}], mailing, partner_a + partner_b, check_mail=True ) diff --git a/addons/mass_mailing/wizard/mail_compose_message.py b/addons/mass_mailing/wizard/mail_compose_message.py index 39043268a3f..5d769938bd5 100644 --- a/addons/mass_mailing/wizard/mail_compose_message.py +++ b/addons/mass_mailing/wizard/mail_compose_message.py @@ -57,18 +57,38 @@ class MailComposeMessage(models.TransientModel): seen_list = self._context.get('mass_mailing_seen_list') mass_mail_layout = self.env.ref('mass_mailing.mass_mailing_mail_layout', raise_if_not_found=False) for res_id in res_ids: + if mail_values.get('body_html') and mass_mail_layout: + mail_values['body_html'] = mass_mail_layout._render({'body': mail_values['body_html']}, engine='ir.qweb', minimal_qcontext=True) + mail_values = res[res_id] if mail_values.get('email_to'): - mail_to = tools.email_normalize(mail_values['email_to']) + mail_to = mail_values['email_to'] else: partner_id = (mail_values.get('recipient_ids') or [(False, '')])[0][1] - mail_to = tools.email_normalize(partners_email.get(partner_id)) - if (opt_out_list and mail_to in opt_out_list) or (seen_list and mail_to in seen_list) \ - or (not mail_to or not email_re.findall(mail_to)): - # prevent sending to blocked addresses that were included by mistake + mail_to = partners_email.get(partner_id) + mail_to_normalized = tools.email_normalize(mail_to) + + # prevent sending to blocked addresses that were included by mistake + # blacklisted or optout or duplicate -> cancel + error_code = False + if mail_values.get('state') == 'cancel': + error_code = 'mail_bl' + elif opt_out_list and mail_to in opt_out_list: mail_values['state'] = 'cancel' + error_code = 'mail_optout' + elif seen_list and mail_to in seen_list: + mail_values['state'] = 'cancel' + error_code = 'mail_dup' + # void of falsey values -> error + elif not mail_to: + mail_values['state'] = 'cancel' + error_code = 'mail_email_missing' + elif not mail_to_normalized or not email_re.findall(mail_to): + mail_values['state'] = 'cancel' + error_code = "RECIPIENT" elif seen_list is not None: seen_list.add(mail_to) + trace_vals = { 'model': self.model, 'res_id': res_id, @@ -76,11 +96,14 @@ class MailComposeMessage(models.TransientModel): # if mail_to is void, keep falsy values to allow searching / debugging traces 'email': mail_to or mail_values.get('email_to'), } - if mail_values.get('body_html') and mass_mail_layout: - mail_values['body_html'] = mass_mail_layout._render({'body': mail_values['body_html']}, engine='ir.qweb', minimal_qcontext=True) - # propagate ignored state to trace when still-born + # propagate failed states to trace when still-born if mail_values.get('state') == 'cancel': trace_vals['ignored'] = fields.Datetime.now() + elif mail_values.get('state') == 'exception': + trace_vals['exception'] = fields.Datetime.now() + if error_code: + trace_vals['failure_type'] = error_code + mail_values.update({ 'mailing_id': mass_mailing.id, 'mailing_trace_ids': [(0, 0, trace_vals)], diff --git a/addons/mass_mailing_sms/models/mailing_trace.py b/addons/mass_mailing_sms/models/mailing_trace.py index 743afe14192..99b4173643e 100644 --- a/addons/mass_mailing_sms/models/mailing_trace.py +++ b/addons/mass_mailing_sms/models/mailing_trace.py @@ -36,6 +36,7 @@ class MailingTrace(models.Model): # mass mode specific codes ('sms_blacklist', 'Blacklisted'), ('sms_duplicate', 'Duplicate'), + ('sms_optout', 'Opted Out'), ]) @api.model_create_multi diff --git a/addons/mass_mailing_sms/wizard/sms_composer.py b/addons/mass_mailing_sms/wizard/sms_composer.py index 9a4d8aae85b..961144325af 100644 --- a/addons/mass_mailing_sms/wizard/sms_composer.py +++ b/addons/mass_mailing_sms/wizard/sms_composer.py @@ -35,22 +35,19 @@ class SMSComposer(models.TransientModel): 'sms_code': trace_code, } if sms_values['state'] == 'error': - if sms_values['error_code'] == 'sms_number_format': - trace_values['sent'] = fields.Datetime.now() - trace_values['bounced'] = fields.Datetime.now() - else: - trace_values['exception'] = fields.Datetime.now() + trace_values['failure_type'] = sms_values['error_code'] + trace_values['exception'] = fields.Datetime.now() elif sms_values['state'] == 'canceled': + trace_values['failure_type'] = sms_values['error_code'] trace_values['ignored'] = fields.Datetime.now() else: if self.mass_sms_allow_unsubscribe: sms_values['body'] = '%s\n%s' % (sms_values['body'] or '', _('STOP SMS : %s', self._get_unsubscribe_url(record.id, trace_code, sms_values['number']))) return trace_values - def _get_blacklist_record_ids(self, records, recipients_info): - """ Consider opt-outed contact as being blacklisted for that specific - mailing. """ - res = super(SMSComposer, self)._get_blacklist_record_ids(records, recipients_info) + def _get_optout_record_ids(self, records, recipients_info): + """ Fetch opt-out records based on mailing. """ + res = super(SMSComposer, self)._get_optout_record_ids(records, recipients_info) if self.mailing_id: optout_res_ids = self.mailing_id._get_opt_out_list_sms() res += optout_res_ids diff --git a/addons/sms/models/sms_sms.py b/addons/sms/models/sms_sms.py index f4f59fe25cb..a6d5d1daf1f 100644 --- a/addons/sms/models/sms_sms.py +++ b/addons/sms/models/sms_sms.py @@ -42,6 +42,7 @@ class SmsSms(models.Model): # mass mode specific codes ('sms_blacklist', 'Blacklisted'), ('sms_duplicate', 'Duplicate'), + ('sms_optout', 'Opted Out'), ], copy=False) def send(self, delete_all=False, auto_commit=False, raise_exception=False): diff --git a/addons/sms/wizard/sms_composer.py b/addons/sms/wizard/sms_composer.py index 8fd47779e44..6cf06a77517 100644 --- a/addons/sms/wizard/sms_composer.py +++ b/addons/sms/wizard/sms_composer.py @@ -273,6 +273,11 @@ class SendSMS(models.TransientModel): return [r.id for r in records if recipients_info[r.id]['sanitized'] in bl_numbers] return [] + def _get_optout_record_ids(self, records, recipients_info): + """ Compute opt-outed contacts, not necessarily blacklisted. Void by default + as no opt-out mechanism exist in SMS, see SMS Marketing. """ + return [] + def _get_done_record_ids(self, records, recipients_info): """ Get a list of already-done records. Order of record set is used to spot duplicates so pay attention to it if necessary. """ @@ -300,6 +305,7 @@ class SendSMS(models.TransientModel): all_bodies = self._prepare_body_values(records) all_recipients = self._prepare_recipient_values(records) blacklist_ids = self._get_blacklist_record_ids(records, all_recipients) + optout_ids = self._get_optout_record_ids(records, all_recipients) done_ids = self._get_done_record_ids(records, all_recipients) result = {} @@ -309,11 +315,14 @@ class SendSMS(models.TransientModel): if sanitized and record.id in blacklist_ids: state = 'canceled' error_code = 'sms_blacklist' + elif sanitized and record.id in optout_ids: + state = 'canceled' + error_code = 'sms_optout' elif sanitized and record.id in done_ids: state = 'canceled' error_code = 'sms_duplicate' elif not sanitized: - state = 'error' + state = 'canceled' error_code = 'sms_number_format' if recipients['number'] else 'sms_number_missing' else: state = 'outgoing' diff --git a/addons/test_mail_full/tests/test_mass_mailing.py b/addons/test_mail_full/tests/test_mass_mailing.py index 1760749e4c2..85744f54ebe 100644 --- a/addons/test_mail_full/tests/test_mass_mailing.py +++ b/addons/test_mail_full/tests/test_mass_mailing.py @@ -11,6 +11,7 @@ from odoo.tests import tagged @tagged('mass_mailing') class TestMassMailing(TestMailFullCommon): + @users('user_marketing') @mute_logger('odoo.addons.mail.models.mail_mail') def test_mailing_w_blacklist_opt_out(self): @@ -47,18 +48,23 @@ class TestMassMailing(TestMailFullCommon): # opt-out: ignored (cancel mail) if recipient in recipients[1] | recipients[2]: recipient_info['state'] = 'ignored' + recipient_info['failure_type'] = "mail_optout" # blacklisted: ignored (cancel mail) elif recipient in recipients[3] | recipients[4]: recipient_info['state'] = 'ignored' + recipient_info['failure_type'] = "mail_bl" # duplicates: ignored (cancel mail) elif recipient == recipient_dup_1: recipient_info['state'] = 'ignored' - # void: ignored (cancel mail) + recipient_info['failure_type'] = "mail_dup" + # void: error (failed mail) elif recipient == recipient_void_1: recipient_info['state'] = 'ignored' - # falsy: ignored (cancel mail) + recipient_info['failure_type'] = "mail_email_missing" + # falsy: error (failed mail) elif recipient == recipient_falsy_1: recipient_info['state'] = 'ignored' + recipient_info['failure_type'] = "RECIPIENT" recipient_info['email'] = recipient.email_from # normalized is False but email should be falsymail else: email = self._find_sent_mail_wemail(recipient.email_normalized) @@ -111,4 +117,5 @@ class TestMassMailing(TestMailFullCommon): check_mail=True,) # sent: 13, 2 bl, 2 opt-out, 3 invalid -> 6 remaining - self.assertMailingStatistics(mailing, expected=13, delivered=6, sent=6, ignored=7) + # ignored: 2 bl + 2 optout + 2 invalid + 1 duplicate; failed: 0 + self.assertMailingStatistics(mailing, expected=13, delivered=6, sent=6, ignored=7, failed=0) diff --git a/addons/test_mail_full/tests/test_mass_sms.py b/addons/test_mail_full/tests/test_mass_sms.py index 2fbdfcd9263..1ef44ffb9af 100644 --- a/addons/test_mail_full/tests/test_mass_sms.py +++ b/addons/test_mail_full/tests/test_mass_sms.py @@ -183,14 +183,14 @@ class TestMassSMSInternals(TestMassSMSCommon): # missing number self.assertSMSTraces( [{'partner': self.env['res.partner'], 'number': False, - 'content': 'Dear %s this is a mass SMS.' % void_record.display_name, 'state': 'exception', + 'content': 'Dear %s this is a mass SMS.' % void_record.display_name, 'state': 'ignored', 'failure_type': 'sms_number_missing'}], self.mailing, void_record, ) # wrong values self.assertSMSTraces( [{'partner': self.env['res.partner'], 'number': record.phone_nbr, - 'content': 'Dear %s this is a mass SMS.' % record.display_name, 'state': 'bounced', + 'content': 'Dear %s this is a mass SMS.' % record.display_name, 'state': 'ignored', 'failure_type': 'sms_number_format'} for record in falsy_record_1 + falsy_record_2], self.mailing, falsy_record_1 + falsy_record_2, @@ -395,8 +395,8 @@ class TestMassSMS(TestMassSMSCommon): mailing.action_send_sms() self.assertSMSTraces( - [{'number': '+32456000000', 'state': 'ignored', 'failure_type': 'sms_blacklist'}, # TDE FIXME: should be opt_out - {'number': '+32456000101', 'state': 'ignored', 'failure_type': 'sms_blacklist'}, # TDE FIXME: should be opt_out + [{'number': '+32456000000', 'state': 'ignored', 'failure_type': 'sms_optout'}, + {'number': '+32456000101', 'state': 'ignored', 'failure_type': 'sms_optout'}, {'number': '+32456000202', 'state': 'sent'}, {'number': '+32456000303', 'state': 'sent'}, {'number': '+32456000404', 'state': 'ignored', 'failure_type': 'sms_blacklist'}], diff --git a/addons/test_mass_mailing/tests/test_blacklist_behavior.py b/addons/test_mass_mailing/tests/test_blacklist_behavior.py index a5a77c7d437..d28ef1e1cb7 100644 --- a/addons/test_mass_mailing/tests/test_blacklist_behavior.py +++ b/addons/test_mass_mailing/tests/test_blacklist_behavior.py @@ -78,6 +78,6 @@ class TestAutoBlacklist(common.TestMassMailCommon): with self.mock_mail_gateway(mail_unlink_sent=False): new_mailing._process_mass_mailing_queue() self.assertMailTraces( - [{'email': 'test.record.00@test.example.com', 'state': 'ignored'}], + [{'email': 'test.record.00@test.example.com', 'state': 'ignored', 'failure_type': 'mail_bl'}], new_mailing, target, check_mail=True ) diff --git a/addons/test_mass_mailing/tests/test_mailing.py b/addons/test_mass_mailing/tests/test_mailing.py index 3a9336007ea..56f0165eba6 100644 --- a/addons/test_mass_mailing/tests/test_mailing.py +++ b/addons/test_mass_mailing/tests/test_mailing.py @@ -245,8 +245,8 @@ class TestMassMailing(TestMassMailCommon): [{'email': 'test.record.00@test.example.com'}, {'email': 'test.record.01@test.example.com'}, {'email': 'test.record.02@test.example.com'}, - {'email': 'test.record.03@test.example.com', 'state': 'ignored'}, - {'email': 'test.record.04@test.example.com', 'state': 'ignored'}], + {'email': 'test.record.03@test.example.com', 'state': 'ignored', 'failure_type': 'mail_bl'}, + {'email': 'test.record.04@test.example.com', 'state': 'ignored', 'failure_type': 'mail_bl'}], mailing, recipients, check_mail=True ) self.assertEqual(mailing.ignored, 2) @@ -271,11 +271,11 @@ class TestMassMailing(TestMassMailCommon): mailing._process_mass_mailing_queue() self.assertMailTraces( - [{'email': 'test.record.00@test.example.com', 'state': 'ignored'}, - {'email': 'test.record.01@test.example.com', 'state': 'ignored'}, + [{'email': 'test.record.00@test.example.com', 'state': 'ignored', 'failure_type': 'mail_optout'}, + {'email': 'test.record.01@test.example.com', 'state': 'ignored', 'failure_type': 'mail_optout'}, {'email': 'test.record.02@test.example.com'}, {'email': 'test.record.03@test.example.com'}, - {'email': 'test.record.04@test.example.com', 'state': 'ignored'}], + {'email': 'test.record.04@test.example.com', 'state': 'ignored', 'failure_type': 'mail_bl'}], mailing, recipients, check_mail=True ) self.assertEqual(mailing.ignored, 3) @@ -332,10 +332,10 @@ class TestMassMailing(TestMassMailCommon): self.assertMailTraces( [{'email': 'test@test.example.com', 'state': 'sent'}, - {'email': 'test@test.example.com', 'state': 'ignored'}, + {'email': 'test@test.example.com', 'state': 'ignored', 'failure_type': 'mail_dup'}, {'email': 'test3@test.example.com'}, {'email': 'test4@test.example.com'}, - {'email': 'test5@test.example.com', 'state': 'ignored'}], + {'email': 'test5@test.example.com', 'state': 'ignored', 'failure_type': 'mail_optout'}], mailing, mailing_contact_1 + mailing_contact_2 + mailing_contact_3 + mailing_contact_4 + mailing_contact_5, check_mail=True