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

[FIX] mass_mailing: prevent duplicates and sending to opted-out

Browse Source 
The mass-mailing App suffered from severe issues due to its inability
to detect and handle duplicates "at the email level", and the absence
of any global blacklist system, leading to lack of user trust.

Mailing-lists typically include multiple records with the same email,
and it is critical to avoid sending them the same email several times.
A related problem is the unsubscription of an email that is present
in other records (duplicates). Opting out the first email should
automatically blacklist it for other records as well.

Ideally we should have a global blacklist table in order to
share the unsubscription requests globally across models (Leads,
Partners, Mailing-list contacts). It would also allow importing
it from other blacklist systems. (TODO for master)

This commit introduces a partial solution, made of several
small changes:

- In mail.mail: double-check that an outgoing email has the
correct status (`outgoing`) before sending it. This allows
adding emails in the queue and cancelling them before they
actually get sent.
- In crm.lead: force predictable recipients for mass-mailing,
by always using the email of the lead rather than the email
of the linked partner when there is one. This simplifies the
computation of the blacklist and seen list. Other areas in
the codebase already assume as much.
- In mass.mailing:

 + Before sending out a mailing-list batch, compute the
   blacklist (all opted-out emails) and the seen_list
   (emails who previously received this mail) to make
   sure we only ever target valid recipients.
 + While delivering the mass-mailing, any email targeting
   an address that is in the blacklist or "seen list" is
   canceled before being sent. The corresponding statistics
   entry is considered "not delivered".
   Also updates the "seen list" continuously.
 + When a mass-mail belongs to a campaign with the
   "unique AB/B testing" flag, the "seen list" is
   common to the whole campaign, as an extra safety.
 + Auto-delete mass-mailing test messages sent with the
   test wizard, to avoid polluting the mail_mail table

Note: this fix uses a simple regex for efficiently extracting
the blacklist in pure SQL from different models, and doing so,
assumes that each record only holds a single email
(no comma-separated adresses). This should be sufficient for
most cases. The regex:

                   ([^ ,;<@]+@[^> ,;]+)
This commit is contained in:
Olivier Dony
2017-03-27 18:47:59 +02:00
parent 6d16915d39
commit ca989b6548
5 changed files with 103 additions and 8 deletions
Download Patch File Download Diff File Expand all files Collapse all files
addons/crm/models/crm_lead.py
+9
View File
@@ -1044,6 +1044,15 @@ class Lead(models.Model):
view_id = super(Lead, self).get_formview_id()
return view_id
@api.multi
def message_get_default_recipients(self):
return {
r.id : {'partner_ids': [],
'email_to': r.email_from,
'email_cc': False}
for r in self.sudo()
}
@api.multi
def message_get_suggested_recipients(self):
recipients = super(Lead, self).message_get_suggested_recipients()
addons/mail/models/mail_mail.py
+4
View File
@@ -255,6 +255,10 @@ class MailMail(models.Model):
for mail_id in self.ids:
try:
mail = self.browse(mail_id)
if mail.state != 'outgoing':
if mail.state != 'exception' and mail.auto_delete:
mail.sudo().unlink()
continue
# TDE note: remove me when model_id field is present on mail.message - done here to avoid doing it multiple times in the sub method
if mail.model:
model = self.env['ir.model']._get(mail.model)[0]
addons/mass_mailing/models/mass_mailing.py
+68 -1
View File
@@ -4,6 +4,7 @@
import hashlib
import hmac
from datetime import datetime
import logging
import random
from odoo import api, fields, models, tools, _
@@ -11,6 +12,7 @@ from odoo.exceptions import UserError
from odoo.tools.safe_eval import safe_eval
from odoo.tools.translate import html_translate
_logger = logging.getLogger(__name__)
class MassMailingTag(models.Model):
"""Model of categories of mass mailing, i.e. marketing, newsletter, ... """
@@ -567,6 +569,69 @@ class MassMailing(models.Model):
# Email Sending
#------------------------------------------------------
def _get_blacklist(self):
"""Returns a set of emails opted-out in target model"""
# TODO: implement a global blacklist table, to easily share
# it and update it.
self.ensure_one()
blacklist = {}
target = self.env[self.mailing_model_real]
mail_field = 'email' if 'email' in target._fields else 'email_from'
if 'opt_out' in target._fields:
# avoid loading a large number of records in memory
# + use a basic heuristic for extracting emails
query = """
SELECT lower(substring(%(mail_field)s, '([^ ,;<@]+@[^> ,;]+)'))
FROM %(target)s
WHERE opt_out AND
substring(%(mail_field)s, '([^ ,;<@]+@[^> ,;]+)') IS NOT NULL;
"""
query = query % {'target': target._table, 'mail_field': mail_field}
self._cr.execute(query)
blacklist = set(m[0] for m in self._cr.fetchall())
_logger.info(
"Mass-mailing %s targets %s, blacklist: %s emails",
self, target._name, len(blacklist))
else:
_logger.info("Mass-mailing %s targets %s, no blacklist available", self, target._name)
return blacklist
def _get_seen_list(self):
"""Returns a set of emails already targeted by current mailing/campaign (no duplicates)"""
self.ensure_one()
target = self.env[self.mailing_model_real]
mail_field = 'email' if 'email' in target._fields else 'email_from'
# avoid loading a large number of records in memory
# + use a basic heuristic for extracting emails
query = """
SELECT lower(substring(%(mail_field)s, '([^ ,;<@]+@[^> ,;]+)'))
FROM mail_mail_statistics s
JOIN %(target)s t ON (s.res_id = t.id)
WHERE substring(%(mail_field)s, '([^ ,;<@]+@[^> ,;]+)') IS NOT NULL
"""
if self.mass_mailing_campaign_id.unique_ab_testing:
query +="""
AND s.mass_mailing_campaign_id = %%(mailing_campaign_id)s;
"""
else:
query +="""
AND s.mass_mailing_id = %%(mailing_id)s;
"""
query = query % {'target': target._table, 'mail_field': mail_field}
params = {'mailing_id': self.id, 'mailing_campaign_id': self.mass_mailing_campaign_id.id}
self._cr.execute(query, params)
seen_list = set(m[0] for m in self._cr.fetchall())
_logger.info(
"Mass-mailing %s has already reached %s %s emails", self, len(seen_list), target._name)
return seen_list
def _get_mass_mailing_context(self):
"""Returns extra context items with pre-filled blacklist and seen list for massmailing"""
return {
'mass_mailing_blacklist': self._get_blacklist(),
'mass_mailing_seen_list': self._get_seen_list(),
}
def get_recipients(self):
if self.mailing_domain:
domain = safe_eval(self.mailing_domain)
@@ -625,7 +690,9 @@ class MassMailing(models.Model):
composer_values['reply_to'] = mailing.reply_to
composer = self.env['mail.compose.message'].with_context(active_ids=res_ids).create(composer_values)
composer.with_context(active_ids=res_ids).send_mail(auto_commit=True)
extra_context = self._get_mass_mailing_context()
composer = composer.with_context(active_ids=res_ids, **extra_context)
composer.send_mail(auto_commit=True)
mailing.state = 'done'
return True
addons/mass_mailing/wizard/mail_compose_message.py
+21 -7
View File
@@ -1,6 +1,6 @@
# -*- coding: utf-8 -*-
# Part of Odoo. See LICENSE file for full copyright and licensing details.
from odoo import api, fields, models
from odoo import api, fields, models, tools
class MailComposeMessage(models.TransientModel):
@@ -40,14 +40,28 @@ class MailComposeMessage(models.TransientModel):
'mailing_model': self.model,
'mailing_domain': self.active_domain,
})
blacklist = self._context.get('mass_mailing_blacklist')
seen_list = self._context.get('mass_mailing_seen_list')
for res_id in res_ids:
res[res_id].update({
mail_values = res[res_id]
recips = tools.email_split(mail_values.get('email_to'))
mail_to = recips[0].lower() if recips else False
if (blacklist and mail_to in blacklist) or (seen_list and mail_to in seen_list):
# prevent sending to blocked addresses that were included by mistake
mail_values['state'] = 'cancel'
elif seen_list is not None:
seen_list.add(mail_to)
stat_vals = {
'model': self.model,
'res_id': res_id,
'mass_mailing_id': mass_mailing.id
}
# propagate exception state to stat when still-born
if mail_values.get('state') == 'cancel':
stat_vals['exception'] = fields.Datetime.now()
mail_values.update({
'mailing_id': mass_mailing.id,
'statistics_ids': [(0, 0, {
'model': self.model,
'res_id': res_id,
'mass_mailing_id': mass_mailing.id,
})],
'statistics_ids': [(0, 0, stat_vals)],
# email-mode: keep original message for routing
'notification': mass_mailing.reply_to_mode == 'thread',
'auto_delete': not mass_mailing.keep_archives,
addons/mass_mailing/wizard/test_mailing.py
+1
View File
@@ -30,6 +30,7 @@ class TestMassMailing(models.TransientModel):
'notification': True,
'mailing_id': mailing.id,
'attachment_ids': [(4, attachment.id) for attachment in mailing.attachment_ids],
'auto_delete': True,
}
mail = self.env['mail.mail'].create(mail_values)
mails |= mail