[FW][MERGE][FIX] mass_mailing: correctly update mailing statistics replied status
PURPOSE Fix trace state management in mass mailing: correctly compute state and better support incoming email headers. SPECIFICATIONS: STATE Trace status is a computed field based on other fields. Those store the datetime at which a specific action has been performed: opened, replied, bounced, ... depending on those fields status of trace is computed. Replied / Bounced / Opened states should be computed prior to Sent state. Indeed Sent is the default state once the email is sent. Then replied, bounced and opened are states that happen after sending the email. It is therefore necessary to re-order the state computation to correctly compute the statistic state. SPECIFICATIONS:GATEWAY Consider an email going through the mail gateway. Its headers References and In-Reply-To are used in mail gateway to find if it is an answer to an existing discussion thread. Notably in mass mailing it is used to set mailing traces (called statistics in 11.0) as opened and replied. However currently only the References header is used. In this commit we now use References or In-Reply-To like what is done in mail gateway. LINKS Task ID 2257717 PR #51247 closes odoo/odoo#51490 Forward-port-of: odoo/odoo#51445 Forward-port-of: odoo/odoo#51247 Signed-off-by: Thibault Delavallee (tde) <tde@openerp.com>
This commit is contained in:
@@ -120,6 +120,30 @@ class MockEmail(common.BaseCase):
|
||||
self.env['mail.thread'].with_context(mail_channel_noautofollow=True).message_process(model, mail)
|
||||
return self.env[target_model].search([(target_field, '=', subject)])
|
||||
|
||||
def gateway_reply_wrecord(self, template, record, use_in_reply_to=True):
|
||||
""" Simulate a reply through the mail gateway. Usage: giving a record,
|
||||
find an email sent to him and use its message-ID to simulate a reply.
|
||||
|
||||
Some noise is added in References just to test some robustness. """
|
||||
mail_mail = self._find_mail_mail_wrecord(record)
|
||||
|
||||
if use_in_reply_to:
|
||||
extra = 'In-Reply-To:\r\n\t%s\n' % mail_mail.message_id
|
||||
else:
|
||||
disturbing_other_msg_id = '<123456.654321@another.host.com>'
|
||||
extra = 'References:\r\n\t%s\n\r%s' % (mail_mail.message_id, disturbing_other_msg_id)
|
||||
|
||||
return self.format_and_process(
|
||||
template,
|
||||
mail_mail.email_to,
|
||||
mail_mail.reply_to,
|
||||
subject='Re: %s' % mail_mail.subject,
|
||||
extra=extra,
|
||||
msg_id='<123456.%s.%d@test.example.com>' % (record._name, record.id),
|
||||
target_model=record._name,
|
||||
target_field=record._rec_name,
|
||||
)
|
||||
|
||||
def from_string(self, text):
|
||||
return email.message_from_string(pycompat.to_text(text), policy=email.policy.SMTP)
|
||||
|
||||
@@ -158,6 +182,14 @@ class MockEmail(common.BaseCase):
|
||||
raise AssertionError('mail.mail not found for message %s / recipients %s' % (mail_message, recipients.ids))
|
||||
return mail
|
||||
|
||||
def _find_mail_mail_wrecord(self, record):
|
||||
for mail in self._new_mails:
|
||||
if mail.model == record._name and mail.res_id == record.id:
|
||||
break
|
||||
else:
|
||||
raise AssertionError('mail.mail not found for record %s in %s' % (record, repr([m.email_to for m in self._new_mails])))
|
||||
return mail
|
||||
|
||||
def assertMailFailed(self, author, recipients, mail_message):
|
||||
mail = self._find_mail(author, recipients, mail_message)
|
||||
self.assertEqual(mail.state, 'exception')
|
||||
|
||||
@@ -3,7 +3,7 @@
|
||||
|
||||
import datetime
|
||||
|
||||
from odoo import api, models, fields
|
||||
from odoo import api, models, fields, tools
|
||||
|
||||
BLACKLIST_MAX_BOUNCED_LIMIT = 5
|
||||
|
||||
@@ -17,10 +17,14 @@ class MailThread(models.AbstractModel):
|
||||
""" Override to update the parent mailing traces. The parent is found
|
||||
by using the References header of the incoming message and looking for
|
||||
matching message_id in mailing.trace. """
|
||||
if message.get('References') and routes:
|
||||
message_ids = [x.strip() for x in message['References'].split()]
|
||||
self.env['mailing.trace'].set_opened(mail_message_ids=message_ids)
|
||||
self.env['mailing.trace'].set_replied(mail_message_ids=message_ids)
|
||||
if routes:
|
||||
# even if 'reply_to' in ref (cfr mail/mail_thread) that indicates a new thread redirection
|
||||
# (aka bypass alias configuration in gateway) consider it as a reply for statistics purpose
|
||||
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)
|
||||
return super(MailThread, self)._message_route_process(message, message_dict, routes)
|
||||
|
||||
def message_post_with_template(self, template_id, **kwargs):
|
||||
|
||||
@@ -80,10 +80,10 @@ class MailingTrace(models.Model):
|
||||
stat.state = 'ignored'
|
||||
elif stat.exception:
|
||||
stat.state = 'exception'
|
||||
elif stat.opened or stat.clicked:
|
||||
stat.state = 'opened'
|
||||
elif stat.replied:
|
||||
stat.state = 'replied'
|
||||
elif stat.opened or stat.clicked:
|
||||
stat.state = 'opened'
|
||||
elif stat.bounced:
|
||||
stat.state = 'bounced'
|
||||
elif stat.sent:
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
# -*- coding: utf-8 -*-
|
||||
# Part of Odoo. See LICENSE file for full copyright and licensing details.
|
||||
|
||||
from . import data
|
||||
from . import models
|
||||
|
||||
@@ -0,0 +1,4 @@
|
||||
# -*- coding: utf-8 -*-
|
||||
# Part of Odoo. See LICENSE file for full copyright and licensing details.
|
||||
|
||||
from . import mail_test_data
|
||||
@@ -0,0 +1,42 @@
|
||||
|
||||
MAIL_TEMPLATE = """Return-Path: <whatever-2a840@postmaster.twitter.com>
|
||||
To: {to}
|
||||
cc: {cc}
|
||||
Received: by mail1.openerp.com (Postfix, from userid 10002)
|
||||
id 5DF9ABFB2A; Fri, 10 Aug 2012 16:16:39 +0200 (CEST)
|
||||
From: {email_from}
|
||||
Subject: {subject}
|
||||
MIME-Version: 1.0
|
||||
Content-Type: multipart/alternative;
|
||||
boundary="----=_Part_4200734_24778174.1344608186754"
|
||||
Date: Fri, 10 Aug 2012 14:16:26 +0000
|
||||
Message-ID: {msg_id}
|
||||
{extra}
|
||||
------=_Part_4200734_24778174.1344608186754
|
||||
Content-Type: text/plain; charset=utf-8
|
||||
Content-Transfer-Encoding: quoted-printable
|
||||
|
||||
I would gladly answer to your mass mailing !
|
||||
|
||||
--
|
||||
Your Dear Customer
|
||||
------=_Part_4200734_24778174.1344608186754
|
||||
Content-Type: text/html; charset=utf-8
|
||||
Content-Transfer-Encoding: quoted-printable
|
||||
|
||||
<!DOCTYPE html PUBLIC "-//W3C//DTD HTML 4.01//EN" "http://www.w3.org/TR/html4/strict.dtd">
|
||||
<html>
|
||||
<head>=20
|
||||
<meta http-equiv=3D"Content-Type" content=3D"text/html; charset=3Dutf-8" />
|
||||
</head>=20
|
||||
<body style=3D"margin: 0; padding: 0; background: #ffffff;-webkit-text-size-adjust: 100%;">=20
|
||||
|
||||
<p>I would gladly answer to your mass mailing !</p>
|
||||
|
||||
<p>--<br/>
|
||||
Your Dear Customer
|
||||
<p>
|
||||
</body>
|
||||
</html>
|
||||
------=_Part_4200734_24778174.1344608186754--
|
||||
"""
|
||||
@@ -5,5 +5,6 @@ from . import test_blacklist
|
||||
from . import test_blacklist_behavior
|
||||
from . import test_blacklist_mixin
|
||||
from . import test_link_tracker
|
||||
from . import test_mailing_internals
|
||||
from . import test_mass_mailing
|
||||
from . import test_performance
|
||||
|
||||
@@ -0,0 +1,76 @@
|
||||
# -*- coding: utf-8 -*-
|
||||
# Part of Odoo. See LICENSE file for full copyright and licensing details.
|
||||
|
||||
from odoo.addons.test_mass_mailing.tests.common import TestMassMailCommon
|
||||
from odoo.addons.test_mass_mailing.data.mail_test_data import MAIL_TEMPLATE
|
||||
|
||||
|
||||
class TestMailingInternals(TestMassMailCommon):
|
||||
|
||||
def setUp(self):
|
||||
super(TestMailingInternals, self).setUp()
|
||||
|
||||
self.env['ir.config_parameter'].set_param('mail.bounce.alias', 'bounce.test')
|
||||
self.env['ir.config_parameter'].set_param('mail.catchall.alias', 'catchall.test')
|
||||
self.env['ir.config_parameter'].set_param('mail.catchall.domain', 'test.example.com')
|
||||
|
||||
self.test_alias = self.env['mail.alias'].create({
|
||||
'alias_name': 'test.alias',
|
||||
'alias_user_id': False,
|
||||
'alias_model_id': self.env['ir.model']._get('mailing.test.simple').id,
|
||||
'alias_contact': 'everyone'
|
||||
})
|
||||
|
||||
def test_mailing_trace_update(self):
|
||||
customers = self.env['res.partner']
|
||||
for x in range(0, 3):
|
||||
customers |= self.env['res.partner'].create({
|
||||
'name': 'Customer_%02d' % x,
|
||||
'email': '"Customer_%02d" <customer_%02d@test.example.com' % (x, x),
|
||||
})
|
||||
|
||||
mailing = self.env['mailing.mailing'].create({
|
||||
'name': 'TestName',
|
||||
'subject': 'TestSubject',
|
||||
'body_html': 'Hello ${object.name}',
|
||||
'reply_to_mode': 'email',
|
||||
'reply_to': '%s@%s' % (self.test_alias.alias_name, self.test_alias.alias_domain),
|
||||
'keep_archives': True,
|
||||
'mailing_model_id': self.env['ir.model']._get('res.partner').id,
|
||||
'mailing_domain': '%s' % [('id', 'in', customers.ids)],
|
||||
})
|
||||
mailing.action_put_in_queue()
|
||||
with self.mock_mail_gateway(mail_unlink_sent=False):
|
||||
mailing.action_send_mail()
|
||||
|
||||
self.gateway_reply_wrecord(MAIL_TEMPLATE, customers[0], use_in_reply_to=True)
|
||||
self.gateway_reply_wrecord(MAIL_TEMPLATE, customers[1], use_in_reply_to=False)
|
||||
|
||||
# customer2 looses headers
|
||||
mail_mail = self._find_mail_mail_wrecord(customers[2])
|
||||
self.format_and_process(
|
||||
MAIL_TEMPLATE,
|
||||
mail_mail.email_to,
|
||||
mail_mail.reply_to,
|
||||
subject='Re: %s' % mail_mail.subject,
|
||||
extra='',
|
||||
msg_id='<123456.%s.%d@test.example.com>' % (customers[2]._name, customers[2].id),
|
||||
target_model=customers[2]._name, target_field=customers[2]._rec_name,
|
||||
)
|
||||
mailing.flush()
|
||||
|
||||
# check traces status
|
||||
traces = self.env['mailing.trace'].search([('model', '=', customers._name), ('res_id', 'in', customers.ids)])
|
||||
self.assertEqual(len(traces), 3)
|
||||
customer0_trace = traces.filtered(lambda t: t.res_id == customers[0].id)
|
||||
self.assertEqual(customer0_trace.state, 'replied')
|
||||
customer1_trace = traces.filtered(lambda t: t.res_id == customers[1].id)
|
||||
self.assertEqual(customer1_trace.state, 'replied')
|
||||
customer2_trace = traces.filtered(lambda t: t.res_id == customers[2].id)
|
||||
self.assertEqual(customer2_trace.state, 'sent')
|
||||
|
||||
# check mailing statistics
|
||||
self.assertEqual(mailing.sent, 3)
|
||||
self.assertEqual(mailing.delivered, 3)
|
||||
self.assertEqual(mailing.opened, 2)
|
||||
self.assertEqual(mailing.replied, 2)
|
||||
Reference in New Issue
Block a user