pengyuthon/odoo_source
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

[REF] mass_mailing(_sms): improve mail/sms and traces failed/ignored state management

Browse Source 
RATIONALE

Currently there are differences between mail and sms error management
especially when sending them in batch (mass mode). Moreover cancel
(ignored) and error (exception) states meaning is not clear. Finally
some failure types management between mail and sms can be cleaned.

SPECIFICATIONS

Meaning of ignored / error we want to enforce now is

  * error: there was something wrong at sending and user has an action to
    perform, i.e. server failed -> check its logs;
  * canceled: invalid recipients due to contact information or mailing
    configuration (blacklist, opt out, void or invalid email or phone number).
    In indicates issues linked to records themselves;

In this task we also add failure information granularity on mailing traces
linked to email like what is done currently on SMS. This can be related to
mailing (blacklist, optout, duplicates) or related to recipient (no recipient,
incorrectly formatted).

We also correctly distinguish optout from blacklist when sending SMS.

SPECIFIC USE CASES

  * recipient without email / number: mail / sms is set as canceled, trace is
    ignored;
  * recipient with invalid email (no @) / number (formatting impossible):
    mail / sms is set as canceled, trace is ignored;

    -> we now distinguish when possible a void email from a wrong email using
       a newly-added selection key (mail_email_missing);

  * recipient with email / number blacklisted: mail / sms is canceled, trace
    is ignored;
  * recipient with email / number that optouted from mailing: mail / sms is
    canceled, trace is ignored;
  * recipient with email that bounces: mail is sent and will be set as bounce
    when receiving bounce in gateway; trace follow same path;
  * recipient with number that bounces: not supported as currently no support
    of bounce through IAP;
  * mail server error, IAP error: mail / sms is set as exception / error,
    trace is set in exception;

This means we introduce new failure types on mailing.trace model to reflect
those failure types

  * ``mail_missing``: missing email (different from wrong value);
  * ``mail_bl``: blacklisted;
  * ``mail_optout``: optouted;
  * ``mail_dup``: duplicated email skipped during mass email send;

We introduce ``sms_optout`` on SMS and trace models as it was merged with
blacklist previously. Now both errors are distinguished.

LINKS

Task ID-2377974
Community PR odoo/odoo#61467
Enterprise PR odoo/enterprise#14633
Upgrade PR odoo/upgrade#1907
This commit is contained in:
Thibault Delavallée
2021-08-18 13:37:33 +00:00
parent e9af609616
commit d950b97dbf
15 changed files with 114 additions and 64 deletions
Download Patch File Download Diff File Expand all files Collapse all files
addons/mail/models/mail_mail.py
+8 -2
View File
@@ -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
addons/mail/models/mail_message.py
+18 -18
View File
@@ -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.
addons/mail/models/mail_notification.py
+1
View File
@@ -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)
addons/mass_mailing/models/mailing_trace.py
+12 -8
View File
@@ -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',
addons/mass_mailing/tests/common.py
+3 -2
View File
@@ -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:
addons/mass_mailing/tests/test_mailing_internals.py
+1 -1
View File
@@ -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
)
addons/mass_mailing/wizard/mail_compose_message.py
+31 -8
View File
@@ -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)],
addons/mass_mailing_sms/models/mailing_trace.py
+1
View File
@@ -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
addons/mass_mailing_sms/wizard/sms_composer.py
+6 -9
View File
@@ -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
addons/sms/models/sms_sms.py
+1
View File
@@ -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):
addons/sms/wizard/sms_composer.py
+10 -1
View File
@@ -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'
addons/test_mail_full/tests/test_mass_mailing.py
+10 -3
View File
@@ -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)
addons/test_mail_full/tests/test_mass_sms.py
+4 -4
View File
@@ -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'}],
addons/test_mass_mailing/tests/test_blacklist_behavior.py
+1 -1
View File
@@ -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
)
addons/test_mass_mailing/tests/test_mailing.py
+7 -7
View File
@@ -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