From bbf4783ac68f296dd028b46aff882b4187a0f9fb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Fri, 17 Apr 2020 09:13:32 +0000 Subject: [PATCH] [REF] mass_mailing: improve traces state management PURPOSE Improve modeling and performances of mass mailing by manually updating trace status instead of using complex computed fields and lessening fields usage. Remove some fields and keep only relevant metrics to simplify and clean trace model. SPECIFICATIONS In this commit we clean the way trace state is managed. Currently it is a computed field based on several datetime fields. However this generates a lot of noise in the table as well as unnecessary computation * there are several columns (one for each state) storing datetime at which status was reached. Generally only 2 or 3 contain relevant information; * state could be set in code directly to avoid a computed field based on many triggers; * recomputing it each time a date changes is not necessarily necessary; * state value can always be updated manually as this is main done through some automated server update (mailgateway, link clicks, ...); As trace states and its triggers should not be updated manually it is better to synchronize it in code flow. When there is an exception or update done through sending or gateway status is updated as well accordingly. Various datetime fields are also updated at the same time. In order to align with notification model mail and sms trace status are updated to a classic field. Only last status update is now kept as there is no need to store the entire history of status change. We keep only a datetime for relevant metrics: open, reply and click. Other datetime bring no real value. Knowing when a trace was in error or bounced is not necessary. Indeed exception generally indicates a server issue (at sending), cancel indicates a data issue (at sending) and bounce depends on customer email server. Status update is removed as using write_date is sufficient. Once created traces are updated only when an external event occurs (opened, replied, ...). It allows to simplify trace model. We also rename ignored field into canceled to match naming use through mail and sms. QUERY COUNTERS This change has some positive update on query counters when sending mailings as traces have less unnecessary status update compared to priori this change. LINKS Task ID-2377974 Community PR odoo/odoo#61467 Enterprise PR odoo/enterprise#14633 Upgrade PR odoo/upgrade#1907 --- addons/mass_mailing/controllers/main.py | 2 +- .../mass_mailing/data/mass_mailing_demo.xml | 45 ++++--- addons/mass_mailing/models/mail_mail.py | 4 +- addons/mass_mailing/models/mail_thread.py | 8 +- addons/mass_mailing/models/mailing.py | 52 ++++---- addons/mass_mailing/models/mailing_trace.py | 118 +++++++----------- addons/mass_mailing/models/utm.py | 22 ++-- .../report/mailing_trace_report.py | 28 +++-- addons/mass_mailing/tests/common.py | 62 +++++---- .../tests/test_mailing_internals.py | 2 +- .../views/mailing_mailing_views.xml | 16 +-- .../views/mailing_trace_views.xml | 62 ++++----- .../wizard/mail_compose_message.py | 7 +- addons/mass_mailing_sms/data/mailing_demo.xml | 45 ++++--- .../mass_mailing_sms/models/mailing_trace.py | 33 ----- addons/mass_mailing_sms/models/sms_sms.py | 2 +- addons/mass_mailing_sms/tests/common.py | 24 ++-- .../views/mailing_mailing_views.xml | 6 +- .../views/mailing_trace_views.xml | 40 ++---- .../mass_mailing_sms/wizard/sms_composer.py | 4 +- .../test_mail_full/tests/test_mass_mailing.py | 18 +-- addons/test_mail_full/tests/test_mass_sms.py | 28 ++--- .../tests/test_blacklist_behavior.py | 23 +++- .../tests/test_link_tracker.py | 13 +- .../test_mass_mailing/tests/test_mailing.py | 28 ++--- .../tests/test_performance.py | 8 +- 26 files changed, 340 insertions(+), 360 deletions(-) diff --git a/addons/mass_mailing/controllers/main.py b/addons/mass_mailing/controllers/main.py index f92a0003e00..f835071aa44 100644 --- a/addons/mass_mailing/controllers/main.py +++ b/addons/mass_mailing/controllers/main.py @@ -98,7 +98,7 @@ class MassMailController(http.Controller): if not consteq(token, tools.hmac(request.env(su=True), 'mass_mailing-mail_mail-open', mail_id)): raise BadRequest() - request.env['mailing.trace'].sudo().set_opened(mail_mail_ids=[mail_id]) + request.env['mailing.trace'].sudo().set_opened(domain=[('mail_mail_id_int', 'in', [mail_id])]) response = werkzeug.wrappers.Response() response.mimetype = 'image/gif' response.data = base64.b64decode(b'R0lGODlhAQABAIAAANvf7wAAACH5BAEAAAAALAAAAAABAAEAAAICRAEAOw==') diff --git a/addons/mass_mailing/data/mass_mailing_demo.xml b/addons/mass_mailing/data/mass_mailing_demo.xml index 15e9cc2b790..67157c1b7dd 100644 --- a/addons/mass_mailing/data/mass_mailing_demo.xml +++ b/addons/mass_mailing/data/mass_mailing_demo.xml @@ -184,9 +184,11 @@ res.partner billy.fox45@example.com - - - + reply + + + + @@ -194,9 +196,11 @@ res.partner kim.snyder96@example.com - - - + reply + + + + @@ -204,8 +208,10 @@ res.partner edith.sanchez68@example.com - - + open + + + @@ -213,8 +219,10 @@ res.partner theodore.gardner36@example.com - - + open + + + @@ -222,7 +230,9 @@ res.partner sandra.neal80@example.com - + sent + + @@ -230,8 +240,8 @@ res.partner julie.richards84@example.com - - + error + @@ -239,8 +249,9 @@ res.partner travis.mendoza24@example.com - - + bounce + + @@ -248,8 +259,8 @@ res.partner travis.mendoza24@example.com - - + bounce + diff --git a/addons/mass_mailing/models/mail_mail.py b/addons/mass_mailing/models/mail_mail.py index bc619a4067a..f9446867c70 100644 --- a/addons/mass_mailing/models/mail_mail.py +++ b/addons/mass_mailing/models/mail_mail.py @@ -81,7 +81,7 @@ class MailMail(models.Model): for mail in self: if mail.mailing_id: if mail_sent is True and mail.mailing_trace_ids: - mail.mailing_trace_ids.write({'sent': fields.Datetime.now(), 'exception': False}) + mail.mailing_trace_ids.set_sent() elif mail_sent is False and mail.mailing_trace_ids: - mail.mailing_trace_ids.write({'exception': fields.Datetime.now(), 'failure_type': failure_type}) + mail.mailing_trace_ids.set_failed(failure_type=failure_type) return super(MailMail, self)._postprocess_sent_message(success_pids, failure_reason=failure_reason, failure_type=failure_type) diff --git a/addons/mass_mailing/models/mail_thread.py b/addons/mass_mailing/models/mail_thread.py index 1f442675922..08fb3b67e86 100644 --- a/addons/mass_mailing/models/mail_thread.py +++ b/addons/mass_mailing/models/mail_thread.py @@ -23,8 +23,8 @@ class MailThread(models.AbstractModel): thread_references = message_dict['references'] or message_dict['in_reply_to'] msg_references = tools.mail_header_msgid_re.findall(thread_references) if msg_references: - self.env['mailing.trace'].set_opened(mail_message_ids=msg_references) - self.env['mailing.trace'].set_replied(mail_message_ids=msg_references) + self.env['mailing.trace'].set_opened(domain=[('message_id', 'in', msg_references)]) + self.env['mailing.trace'].set_replied(domain=[('message_id', 'in', msg_references)]) return super(MailThread, self)._message_route_process(message, message_dict, routes) def message_post_with_template(self, template_id, **kwargs): @@ -53,10 +53,10 @@ class MailThread(models.AbstractModel): bounced_partner = message_dict['bounced_partner'] if bounced_msg_id: - self.env['mailing.trace'].set_bounced(mail_message_ids=bounced_msg_id) + self.env['mailing.trace'].set_bounced(domain=[('message_id', 'in', bounced_msg_id)]) if bounced_email: three_months_ago = fields.Datetime.to_string(datetime.datetime.now() - datetime.timedelta(weeks=13)) - stats = self.env['mailing.trace'].search(['&', ('bounced', '>', three_months_ago), ('email', '=ilike', bounced_email)]).mapped('bounced') + stats = self.env['mailing.trace'].search(['&', '&', ('trace_status', '=', 'bounce'), ('write_date', '>', three_months_ago), ('email', '=ilike', bounced_email)]).mapped('write_date') if len(stats) >= BLACKLIST_MAX_BOUNCED_LIMIT and (not bounced_partner or any(p.message_bounce >= BLACKLIST_MAX_BOUNCED_LIMIT for p in bounced_partner)): if max(stats) > min(stats) + datetime.timedelta(weeks=1): blacklist_rec = self.env['mail.blacklist'].sudo()._add(bounced_email) diff --git a/addons/mass_mailing/models/mailing.py b/addons/mass_mailing/models/mailing.py index 66cfef1c12f..e956434495c 100644 --- a/addons/mass_mailing/models/mailing.py +++ b/addons/mass_mailing/models/mailing.py @@ -143,7 +143,7 @@ class MassMailing(models.Model): total = fields.Integer(compute="_compute_total") scheduled = fields.Integer(compute="_compute_statistics") expected = fields.Integer(compute="_compute_statistics") - ignored = fields.Integer(compute="_compute_statistics") + canceled = fields.Integer(compute="_compute_statistics") sent = fields.Integer(compute="_compute_statistics") delivered = fields.Integer(compute="_compute_statistics") opened = fields.Integer(compute="_compute_statistics") @@ -203,7 +203,7 @@ class MassMailing(models.Model): def _compute_statistics(self): """ Compute statistics of the mass mailing """ for key in ( - 'scheduled', 'expected', 'ignored', 'sent', 'delivered', 'opened', + 'scheduled', 'expected', 'canceled', 'sent', 'delivered', 'opened', 'clicked', 'replied', 'bounced', 'failed', 'received_ratio', 'opened_ratio', 'replied_ratio', 'bounced_ratio', ): @@ -216,15 +216,15 @@ class MassMailing(models.Model): SELECT m.id as mailing_id, COUNT(s.id) AS expected, - COUNT(CASE WHEN s.sent is not null THEN 1 ELSE null END) AS sent, - COUNT(CASE WHEN s.scheduled is not null AND s.sent is null AND s.exception is null AND s.ignored is null AND s.bounced is null THEN 1 ELSE null END) AS scheduled, - COUNT(CASE WHEN s.scheduled is not null AND s.sent is null AND s.exception is null AND s.ignored is not null THEN 1 ELSE null END) AS ignored, - COUNT(CASE WHEN s.sent is not null AND s.exception is null AND s.bounced is null THEN 1 ELSE null END) AS delivered, - COUNT(CASE WHEN s.opened is not null THEN 1 ELSE null END) AS opened, - COUNT(CASE WHEN s.clicked is not null THEN 1 ELSE null END) AS clicked, - COUNT(CASE WHEN s.replied is not null THEN 1 ELSE null END) AS replied, - COUNT(CASE WHEN s.bounced is not null THEN 1 ELSE null END) AS bounced, - COUNT(CASE WHEN s.exception is not null THEN 1 ELSE null END) AS failed + COUNT(s.sent_datetime) AS sent, + COUNT(s.trace_status) FILTER (WHERE s.trace_status = 'outgoing') AS scheduled, + COUNT(s.trace_status) FILTER (WHERE s.trace_status = 'cancel') AS canceled, + COUNT(s.trace_status) FILTER (WHERE s.trace_status in ('sent', 'open', 'reply')) AS delivered, + COUNT(s.trace_status) FILTER (WHERE s.trace_status in ('open', 'reply')) AS opened, + COUNT(s.links_click_datetime) AS clicked, + COUNT(s.trace_status) FILTER (WHERE s.trace_status = 'reply') AS replied, + COUNT(s.trace_status) FILTER (WHERE s.trace_status = 'bounce') AS bounced, + COUNT(s.trace_status) FILTER (WHERE s.trace_status = 'error') AS failed FROM mailing_trace s RIGHT JOIN @@ -236,7 +236,7 @@ class MassMailing(models.Model): m.id """, (tuple(self.ids), )) for row in self.env.cr.dictfetchall(): - total = (row['expected'] - row['ignored']) or 1 + total = (row['expected'] - row['canceled']) or 1 row['received_ratio'] = 100.0 * row['delivered'] / total row['opened_ratio'] = 100.0 * row['opened'] / total row['replied_ratio'] = 100.0 * row['replied'] / total @@ -431,8 +431,8 @@ class MassMailing(models.Model): def action_view_traces_scheduled(self): return self._action_view_traces_filtered('scheduled') - def action_view_traces_ignored(self): - return self._action_view_traces_filtered('ignored') + def action_view_traces_canceled(self): + return self._action_view_traces_filtered('canceled') def action_view_traces_failed(self): return self._action_view_traces_filtered('failed') @@ -460,25 +460,31 @@ class MassMailing(models.Model): } def action_view_opened(self): - return self._action_view_documents_filtered('opened') + return self._action_view_documents_filtered('open') def action_view_replied(self): - return self._action_view_documents_filtered('replied') + return self._action_view_documents_filtered('reply') def action_view_bounced(self): - return self._action_view_documents_filtered('bounced') + return self._action_view_documents_filtered('bounce') def action_view_delivered(self): return self._action_view_documents_filtered('delivered') def _action_view_documents_filtered(self, view_filter): - if view_filter in ('opened', 'replied', 'bounced'): - opened_stats = self.mailing_trace_ids.filtered(lambda stat: stat[view_filter]) - elif view_filter == ('delivered'): - opened_stats = self.mailing_trace_ids.filtered(lambda stat: stat.sent and not stat.bounced) + if view_filter in ('reply', 'bounce'): + found_traces = self.mailing_trace_ids.filtered(lambda trace: trace.trace_status == view_filter) + elif view_filter == 'open': + found_traces = self.mailing_trace_ids.filtered(lambda trace: trace.trace_status in ('open', 'reply')) + elif view_filter == 'click': + found_traces = self.mailing_trace_ids.filtered(lambda trace: trace.links_click_datetime) + elif view_filter == 'delivered': + found_traces = self.mailing_trace_ids.filtered(lambda trace: trace.trace_status in ('sent', 'open', 'reply')) + elif view_filter == 'sent': + found_traces = self.mailing_trace_ids.filtered(lambda trace: trace.sent_datetime) else: - opened_stats = self.env['mailing.trace'] - res_ids = opened_stats.mapped('res_id') + found_traces = self.env['mailing.trace'] + res_ids = found_traces.mapped('res_id') model_name = self.env['ir.model']._get(self.mailing_model_real).display_name return { 'name': model_name, diff --git a/addons/mass_mailing/models/mailing_trace.py b/addons/mass_mailing/models/mailing_trace.py index a50aefbb59a..cf0f3bc799f 100644 --- a/addons/mass_mailing/models/mailing_trace.py +++ b/addons/mass_mailing/models/mailing_trace.py @@ -12,9 +12,9 @@ class MailingTrace(models.Model): Note:: State management / Error codes / Failure types summary - * state + * trace_status 'outgoing', 'sent', 'opened', 'replied', - 'exception', 'bounced', 'ignored' + 'error', 'bouce', 'cancel' * failure_type # mass_mailing "SMTP", "RECIPIENT", "BOUNCE", "UNKNOWN", "mail_email_missing" @@ -25,7 +25,7 @@ class MailingTrace(models.Model): 'sms_server', 'sms_acc' # mass_mailing_sms mass mode specific codes 'sms_blacklist', 'sms_duplicate', 'sms_optout', - * ignored: + * cancel: * 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 or incorrectly formatted (mass_mailing) - based on mail cancel state @@ -47,7 +47,7 @@ class MailingTrace(models.Model): _name = 'mailing.trace' _description = 'Mailing Statistics' _rec_name = 'id' - _order = 'scheduled DESC' + _order = 'create_date DESC' trace_type = fields.Selection([('mail', 'Mail')], string='Type', default='mail', required=True) display_name = fields.Char(compute='_compute_display_name') @@ -61,39 +61,30 @@ class MailingTrace(models.Model): index=True, ) email = fields.Char(string="Email", help="Normalized email address") - message_id = fields.Char(string='Message-ID') + message_id = fields.Char(string='Message-ID', help="Technical field for the email Message-ID (RFC 2392)") medium_id = fields.Many2one(related='mass_mailing_id.medium_id') source_id = fields.Many2one(related='mass_mailing_id.source_id') # document model = fields.Char(string='Document model', required=True) res_id = fields.Many2oneReference(string='Document ID', model_field='model', required=True) - # campaign / wave data + # campaign data mass_mailing_id = fields.Many2one('mailing.mailing', string='Mailing', index=True, ondelete='cascade') campaign_id = fields.Many2one( related='mass_mailing_id.campaign_id', string='Campaign', store=True, readonly=True, index=True) - # Bounce and tracking - ignored = fields.Datetime(help='Date when the email has been invalidated. ' - 'Invalid emails are blacklisted, opted-out or invalid email format') - scheduled = fields.Datetime(help='Date when the email has been created', default=fields.Datetime.now) - sent = fields.Datetime(help='Date when the email has been sent') - exception = fields.Datetime(help='Date of technical error leading to the email not being sent') - opened = fields.Datetime(help='Date when the email has been opened the first time') - replied = fields.Datetime(help='Date when this email has been replied for the first time.') - bounced = fields.Datetime(help='Date when this email has bounced.') - # Link tracking - links_click_ids = fields.One2many('link.tracker.click', 'mailing_trace_id', string='Links click') - clicked = fields.Datetime(help='Date when customer clicked on at least one tracked link') # Status - state = fields.Selection(compute="_compute_state", - selection=[('outgoing', 'Outgoing'), - ('exception', 'Exception'), - ('sent', 'Sent'), - ('opened', 'Opened'), - ('replied', 'Replied'), - ('bounced', 'Bounced'), - ('ignored', 'Ignored')], store=True) + sent_datetime = fields.Datetime('Sent On') + open_datetime = fields.Datetime('Opened On') + reply_datetime = fields.Datetime('Replied On') + trace_status = fields.Selection(selection=[ + ('outgoing', 'Outgoing'), + ('sent', 'Sent'), + ('open', 'Opened'), + ('reply', 'Replied'), + ('bounce', 'Bounced'), + ('error', 'Exception'), + ('cancel', 'Canceled')], string='Status', default='outgoing') failure_type = fields.Selection(selection=[ ("SMTP", "Connection failed (outgoing mail server problem)"), ("RECIPIENT", "Invalid email address"), @@ -105,34 +96,15 @@ class MailingTrace(models.Model): ("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', - store=True) + # Link tracking + links_click_ids = fields.One2many('link.tracker.click', 'mailing_trace_id', string='Links click') + links_click_datetime = fields.Datetime('Clicked On', help='Stores last click datetime in case of multi clicks.') @api.depends('trace_type', 'mass_mailing_id') def _compute_display_name(self): for trace in self: trace.display_name = '%s: %s (%s)' % (trace.trace_type, trace.mass_mailing_id.name, trace.id) - @api.depends('sent', 'opened', 'clicked', 'replied', 'bounced', 'exception', 'ignored') - def _compute_state(self): - self.update({'state_update': fields.Datetime.now()}) - for stat in self: - if stat.ignored: - stat.state = 'ignored' - elif stat.exception: - stat.state = 'exception' - elif stat.replied: - stat.state = 'replied' - elif stat.opened or stat.clicked: - stat.state = 'opened' - elif stat.bounced: - stat.state = 'bounced' - elif stat.sent: - stat.state = 'sent' - else: - stat.state = 'outgoing' - @api.model_create_multi def create(self, values_list): for values in values_list: @@ -150,33 +122,37 @@ class MailingTrace(models.Model): 'res_id': self.res_id } - def _get_records(self, mail_mail_ids=None, mail_message_ids=None, domain=None): - if not self.ids and mail_mail_ids: - base_domain = [('mail_mail_id_int', 'in', mail_mail_ids)] - elif not self.ids and mail_message_ids: - base_domain = [('message_id', 'in', mail_message_ids)] - else: - base_domain = [('id', 'in', self.ids)] - if domain: - base_domain = ['&'] + domain + base_domain - return self.search(base_domain) - - def set_opened(self, mail_mail_ids=None, mail_message_ids=None): - traces = self._get_records(mail_mail_ids, mail_message_ids, [('opened', '=', False)]) - traces.write({'opened': fields.Datetime.now(), 'bounced': False}) + def set_sent(self, domain=None): + traces = self + (self.search(domain) if domain else self.env['mailing.trace']) + traces.write({'trace_status': 'sent', 'sent_datetime': fields.Datetime.now(), 'failure_type': False}) return traces - def set_clicked(self, mail_mail_ids=None, mail_message_ids=None): - traces = self._get_records(mail_mail_ids, mail_message_ids, [('clicked', '=', False)]) - traces.write({'clicked': fields.Datetime.now()}) + def set_opened(self, domain=None): + traces = self + (self.search(domain) if domain else self.env['mailing.trace']) + traces.write({'trace_status': 'open', 'open_datetime': fields.Datetime.now()}) return traces - def set_replied(self, mail_mail_ids=None, mail_message_ids=None): - traces = self._get_records(mail_mail_ids, mail_message_ids, [('replied', '=', False)]) - traces.write({'replied': fields.Datetime.now()}) + def set_clicked(self, domain=None): + traces = self + (self.search(domain) if domain else self.env['mailing.trace']) + traces.write({'links_click_datetime': fields.Datetime.now()}) return traces - def set_bounced(self, mail_mail_ids=None, mail_message_ids=None): - traces = self._get_records(mail_mail_ids, mail_message_ids, [('bounced', '=', False), ('opened', '=', False)]) - traces.write({'bounced': fields.Datetime.now()}) + def set_replied(self, domain=None): + traces = self + (self.search(domain) if domain else self.env['mailing.trace']) + traces.write({'trace_status': 'reply', 'reply_datetime': fields.Datetime.now()}) + return traces + + def set_bounced(self, domain=None): + traces = self + (self.search(domain) if domain else self.env['mailing.trace']) + traces.write({'trace_status': 'bounce'}) + return traces + + def set_failed(self, domain=None, failure_type=False): + traces = self + (self.search(domain) if domain else self.env['mailing.trace']) + traces.write({'trace_status': 'error', 'failure_type': failure_type}) + return traces + + def set_canceled(self, domain=None): + traces = self + (self.search(domain) if domain else self.env['mailing.trace']) + traces.write({'trace_status': 'cancel'}) return traces diff --git a/addons/mass_mailing/models/utm.py b/addons/mass_mailing/models/utm.py index 88c99e0f0ea..ece57e963b2 100644 --- a/addons/mass_mailing/models/utm.py +++ b/addons/mass_mailing/models/utm.py @@ -47,12 +47,12 @@ class UtmCampaign(models.Model): SELECT c.id as campaign_id, COUNT(s.id) AS expected, - COUNT(CASE WHEN s.sent is not null THEN 1 ELSE null END) AS sent, - COUNT(CASE WHEN s.scheduled is not null AND s.sent is null AND s.exception is null AND s.ignored is not null THEN 1 ELSE null END) AS ignored, - COUNT(CASE WHEN s.id is not null AND s.bounced is null THEN 1 ELSE null END) AS delivered, - COUNT(CASE WHEN s.opened is not null THEN 1 ELSE null END) AS opened, - COUNT(CASE WHEN s.replied is not null THEN 1 ELSE null END) AS replied, - COUNT(CASE WHEN s.bounced is not null THEN 1 ELSE null END) AS bounced + COUNT(s.sent_datetime) AS sent, + COUNT(s.trace_status) FILTER (WHERE s.trace_status in ('sent', 'open', 'reply')) AS delivered, + COUNT(s.trace_status) FILTER (WHERE s.trace_status in ('open', 'reply')) AS open, + COUNT(s.trace_status) FILTER (WHERE s.trace_status = 'reply') AS reply, + COUNT(s.trace_status) FILTER (WHERE s.trace_status = 'bounce') AS bounce, + COUNT(s.trace_status) FILTER (WHERE s.trace_status = 'cancel') AS cancel FROM mailing_trace s RIGHT JOIN @@ -75,13 +75,13 @@ class UtmCampaign(models.Model): if not stats: vals = default_vals else: - total = (stats['expected'] - stats['ignored']) or 1 - delivered = stats['sent'] - stats['bounced'] + total = (stats['expected'] - stats['cancel']) or 1 + delivered = stats['sent'] - stats['bounce'] vals = { 'received_ratio': 100.0 * delivered / total, - 'opened_ratio': 100.0 * stats['opened'] / total, - 'replied_ratio': 100.0 * stats['replied'] / total, - 'bounced_ratio': 100.0 * stats['bounced'] / total + 'opened_ratio': 100.0 * stats['open'] / total, + 'replied_ratio': 100.0 * stats['reply'] / total, + 'bounced_ratio': 100.0 * stats['bounce'] / total } campaign.update(vals) diff --git a/addons/mass_mailing/report/mailing_trace_report.py b/addons/mass_mailing/report/mailing_trace_report.py index 02931d629d4..e3711151077 100644 --- a/addons/mass_mailing/report/mailing_trace_report.py +++ b/addons/mass_mailing/report/mailing_trace_report.py @@ -21,10 +21,12 @@ class MailingTraceReport(models.Model): # traces sent = fields.Integer(readonly=True) delivered = fields.Integer(readonly=True) + error = fields.Integer(readonly=True) opened = fields.Integer(readonly=True) replied = fields.Integer(readonly=True) - clicked = fields.Integer(readonly=True) bounced = fields.Integer(readonly=True) + canceled = fields.Integer(readonly=True) + clicked = fields.Integer(readonly=True) def init(self): """Mass Mail Statistical Report: based on mailing.trace that models the various @@ -52,23 +54,25 @@ class MailingTraceReport(models.Model): 'utm_source.name as name', 'mailing.mailing_type', 'utm_campaign.name as campaign', - 'trace.scheduled as scheduled_date', + 'trace.create_date as scheduled_date', 'mailing.state', 'mailing.email_from', - 'count(trace.sent) as sent', - '(count(trace.sent) - count(trace.bounced)) as delivered', - 'count(trace.opened) as opened', - 'count(trace.replied) as replied', - 'count(trace.clicked) as clicked', - 'count(trace.bounced) as bounced' + 'COUNT(trace.sent_datetime) as sent', + "(COUNT(trace.sent_datetime) - COUNT(trace.trace_status) FILTER (WHERE trace.trace_status IN ('error', 'bounce', 'cancel'))) as delivered", + "COUNT(trace.trace_status) FILTER (WHERE trace.trace_status = 'error') as error", + "COUNT(trace.trace_status) FILTER (WHERE trace.trace_status = 'bounce') as bounced", + "COUNT(trace.trace_status) FILTER (WHERE trace.trace_status = 'cancel') as cancel", + "COUNT(trace.trace_status) FILTER (WHERE trace.trace_status = 'open') as opened", + "COUNT(trace.trace_status) FILTER (WHERE trace.trace_status = 'reply') as replied", + "COUNT(trace.links_click_datetime) as clicked", ] def _report_get_request_from_items(self): return [ 'mailing_trace as trace', - 'left join mailing_mailing as mailing ON (trace.mass_mailing_id=mailing.id)', - 'left join utm_campaign as utm_campaign ON (mailing.campaign_id = utm_campaign.id)', - 'left join utm_source as utm_source ON (mailing.source_id = utm_source.id)' + 'LEFT JOIN mailing_mailing as mailing ON (trace.mass_mailing_id=mailing.id)', + 'LEFT JOIN utm_campaign as utm_campaign ON (mailing.campaign_id = utm_campaign.id)', + 'LEFT JOIN utm_source as utm_source ON (mailing.source_id = utm_source.id)' ] def _report_get_request_where_items(self): @@ -76,7 +80,7 @@ class MailingTraceReport(models.Model): def _report_get_request_group_by_items(self): return [ - 'trace.scheduled', + 'trace.create_date', 'utm_source.name', 'utm_campaign.name', 'mailing.mailing_type', diff --git a/addons/mass_mailing/tests/common.py b/addons/mass_mailing/tests/common.py index 56b7155919d..96b5fcfee0c 100644 --- a/addons/mass_mailing/tests/common.py +++ b/addons/mass_mailing/tests/common.py @@ -6,9 +6,13 @@ import random import re import werkzeug +from unittest.mock import patch + +from odoo import tools from odoo.addons.link_tracker.tests.common import MockLinkTracker from odoo.addons.mail.tests.common import MailCase, MailCommon, mail_new_test_user -from odoo import tools +from odoo.sql_db import Cursor + class MassMailCase(MailCase, MockLinkTracker): @@ -25,7 +29,7 @@ class MassMailCase(MailCase, MockLinkTracker): kwargs['delivered'] = len(mailing.mailing_trace_ids) for fname in ['scheduled', 'expected', 'sent', 'delivered', 'opened', 'replied', 'clicked', - 'ignored', 'failed', 'bounced']: + 'canceled', 'failed', 'bounced']: self.assertEqual( mailing[fname], kwargs.get(fname, 0), 'Mailing %s statistics failed: got %s instead of %s' % (fname, mailing[fname], kwargs.get(fname, 0)) @@ -44,7 +48,7 @@ class MassMailCase(MailCase, MockLinkTracker): # TRACE 'partner': res.partner record (may be empty), 'email': email used when sending email (may be empty, computed based on partner), - 'state': outgoing / sent / ignored / bounced / exception / opened (sent by default), + 'trace_status': outgoing / sent / open / reply / bounce / error / cancel (sent by default), 'record: linked record, # MAIL.MAIL 'content': optional content that should be present in mail.mail body_html; @@ -67,12 +71,11 @@ class MassMailCase(MailCase, MockLinkTracker): # map trace state to email state state_mapping = { 'sent': 'sent', - 'opened': 'sent', # opened implies something has been sent - 'replied': 'sent', # replied implies something has been sent - 'ignored': 'cancel', - 'exception': 'exception', - 'canceled': 'cancel', - 'bounced': 'cancel', + 'open': 'sent', # opened implies something has been sent + 'reply': 'sent', # replied implies something has been sent + 'error': 'exception', + 'cancel': 'cancel', + 'bounce': 'cancel', } traces = self.env['mailing.trace'].search([ @@ -90,21 +93,23 @@ class MassMailCase(MailCase, MockLinkTracker): for recipient_info, link_info, record in zip(recipients_info, mail_links_info, records): partner = recipient_info.get('partner', self.env['res.partner']) email = recipient_info.get('email') - state = recipient_info.get('state', 'sent') + status = recipient_info.get('trace_status', 'sent') record = record or recipient_info.get('record') content = recipient_info.get('content') if email is None and partner: email = partner.email_normalized recipient_trace = traces.filtered( - lambda t: t.email == email and t.state == state and (t.res_id == record.id if record else True) + lambda t: (t.email == email or (not email and not t.email)) and \ + t.trace_status == status and \ + (t.res_id == record.id if record else True) ) self.assertTrue( len(recipient_trace) == 1, - 'MailTrace: email %s (recipient %s, state: %s, record: %s): found %s records (1 expected)' % (email, partner, state, record, len(recipient_trace)) + 'MailTrace: email %s (recipient %s, status: %s, record: %s): found %s records (1 expected)' % (email, partner, status, 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'): + if 'failure_type' in recipient_info or status in ('error', 'cancel', 'bounce'): self.assertEqual(recipient_trace.failure_type, recipient_info['failure_type']) if check_mail: @@ -116,15 +121,15 @@ class MassMailCase(MailCase, MockLinkTracker): # specific for partner: email_formatted is used if partner: - if state == 'sent' and sent_unlink: + if status == 'sent' and sent_unlink: self.assertSentEmail(author, [partner]) else: - self.assertMailMail(partner, state_mapping[state], author=author, content=content, fields_values=fields_values) + self.assertMailMail(partner, state_mapping[status], author=author, content=content, fields_values=fields_values) # specific if email is False -> could have troubles finding it if several falsy traces - elif not email and state in ('ignored', 'canceled', 'bounced'): - self.assertMailMailWId(recipient_trace.mail_mail_id_int, state_mapping[state], content=content, fields_values=fields_values) + elif not email and status in ('cancel', 'bounce'): + self.assertMailMailWId(recipient_trace.mail_mail_id_int, state_mapping[status], content=content, fields_values=fields_values) else: - self.assertMailMailWEmails([email], state_mapping[state], author=author, content=content, fields_values=fields_values) + self.assertMailMailWEmails([email], state_mapping[status], author=author, content=content, fields_values=fields_values) if link_info: trace_mail = self._find_mail_mail_wrecord(record) @@ -200,15 +205,18 @@ class MassMailCase(MailCase, MockLinkTracker): if dt is None: dt = datetime.datetime.now() - datetime.timedelta(days=1) randomized = random.random() - trace = cls.env['mailing.trace'].create({ - 'mass_mailing_id': mailing.id, - 'model': record._name, - 'res_id': record.id, - 'bounced': dt, - # TDE FIXME: improve this with a mail-enabled heuristics - 'email': trace_email, - 'message_id': '<%5f@gilbert.boitempomils>' % randomized, - }) + # Cursor.now() uses transaction's timestamp and not datetime lib -> freeze_time + # is not sufficient + with patch.object(Cursor, 'now', lambda *args, **kwargs: dt): + trace = cls.env['mailing.trace'].sudo().create({ + 'mass_mailing_id': mailing.id, + 'model': record._name, + 'res_id': record.id, + 'trace_status': 'bounce', + # TDE FIXME: improve this with a mail-enabled heuristics + 'email': trace_email, + 'message_id': '<%5f@gilbert.boitempomils>' % randomized, + }) return trace diff --git a/addons/mass_mailing/tests/test_mailing_internals.py b/addons/mass_mailing/tests/test_mailing_internals.py index 4fafbf6e3f8..b90210405eb 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', 'failure_type': 'mail_bl'}], + {'partner': partner_b, 'trace_status': 'cancel', 'failure_type': 'mail_bl'}], mailing, partner_a + partner_b, check_mail=True ) diff --git a/addons/mass_mailing/views/mailing_mailing_views.xml b/addons/mass_mailing/views/mailing_mailing_views.xml index 248205f8a24..01e21298c95 100644 --- a/addons/mass_mailing/views/mailing_mailing_views.xml +++ b/addons/mass_mailing/views/mailing_mailing_views.xml @@ -66,14 +66,14 @@