diff --git a/addons/mail/tests/common.py b/addons/mail/tests/common.py index bf3f3707008..d8dcf213197 100644 --- a/addons/mail/tests/common.py +++ b/addons/mail/tests/common.py @@ -125,6 +125,10 @@ class MockEmail(common.BaseCase): return self.env[target_model].search([(target_field, '=', subject)]) def gateway_reply_wrecord(self, template, record, use_in_reply_to=True): + """ Deprecated, remove in 14.4 """ + return self.gateway_mail_reply_wrecord(template, record, use_in_reply_to=use_in_reply_to) + + def gateway_mail_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. @@ -176,7 +180,11 @@ class MockEmail(common.BaseCase): """ Find a sent email with a given list of recipients. Email should match exactly the recipients. - :return email: a dictionary mapping values given to ``build_email``; + :param email-to: a list of emails that will be compared to email_to + of sent emails (also a list of emails); + + :return email: an email which is a dictionary mapping values given to + ``build_email``; """ for sent_email in self._mails: if set(sent_email['email_to']) == set([email_to]): @@ -185,6 +193,20 @@ class MockEmail(common.BaseCase): raise AssertionError('sent mail not found for email_to %s' % (email_to)) return sent_email + def _find_mail_mail_wid(self, mail_id): + """ Find a ``mail.mail`` record based on a given ID (used notably when having + mail ID in mailing traces). + + :return mail: a ``mail.mail`` record generated during the mock and matching + given ID; + """ + for mail in self._new_mails: + if mail.id == mail_id: + break + else: + raise AssertionError('mail.mail not found for ID %s' % (mail_id)) + return mail + def _find_mail_mail_wpartners(self, recipients, status, mail_message=None, author=None): """ Find a mail.mail record based on various parameters, notably a list of recipients (partners). @@ -271,7 +293,7 @@ class MockEmail(common.BaseCase): :param status: mail.mail state used to filter mails. If ``sent`` this method also check that emails have been sent trough gateway; :param mail_message: see ``_find_mail_mail_wpartners``; - :param author: see ``_find_mail_mail_wpartners``;; + :param author: see ``_find_mail_mail_wpartners``; :param content: if given, check it is contained within mail html body; :param fields_values: if given, should be a dictionary of field names / values allowing to check ``mail.mail`` additional values (subject, @@ -329,7 +351,33 @@ class MockEmail(common.BaseCase): for email_to in emails: self.assertSentEmail(email_values['email_from'] if email_values and email_values.get('email_from') else author, [email_to], **(email_values or {})) + def assertMailMailWId(self, mail_id, status, + content=None, fields_values=None): + """ Assert mail.mail records are created and maybe sent as emails. Allow + asserting their content. Records to check are the one generated when + using mock (mail.mail and outgoing emails). This method takes partners + as source of record fetch and assert. + + :param mail_id: a ``mail.mail`` DB ID. See ``_find_mail_mail_wid``; + :param status: mail.mail state to check upon found mail; + :param content: if given, check it is contained within mail html body; + :param fields_values: if given, should be a dictionary of field names / + values allowing to check ``mail.mail`` additional values (subject, + reply_to, ...); + """ + found_mail = self._find_mail_mail_wid(mail_id) + self.assertTrue(bool(found_mail)) + if status: + self.assertEqual(found_mail.state, status) + if content: + self.assertIn(content, found_mail.body_html) + for fname, fvalue in (fields_values or {}).items(): + self.assertEqual( + found_mail[fname], fvalue, + 'Mail: expected %s for %s, got %s' % (fvalue, fname, found_mail[fname])) + def assertNoMail(self, recipients, mail_message=None, author=None): + """ Check no mail.mail and email was generated during gateway mock. """ try: self._find_mail_mail_wpartners(recipients, False, mail_message=mail_message, author=author) except AssertionError: @@ -340,6 +388,7 @@ class MockEmail(common.BaseCase): self.assertNotSentEmail() def assertNotSentEmail(self): + """ Check no email was generated during gateway mock. """ self.assertEqual(len(self._mails), 0) def assertSentEmail(self, author, recipients, **values): diff --git a/addons/mass_mailing/models/mailing.py b/addons/mass_mailing/models/mailing.py index ba1870566df..c4e3c1e9724 100644 --- a/addons/mass_mailing/models/mailing.py +++ b/addons/mass_mailing/models/mailing.py @@ -193,6 +193,8 @@ class MassMailing(models.Model): self[key] = False if not self.ids: return + # ensure traces are sent to db + self.flush() self.env.cr.execute(""" SELECT m.id as mailing_id, diff --git a/addons/mass_mailing/tests/common.py b/addons/mass_mailing/tests/common.py index 512e1c73a2f..315aacef3be 100644 --- a/addons/mass_mailing/tests/common.py +++ b/addons/mass_mailing/tests/common.py @@ -1,32 +1,78 @@ # -*- coding: utf-8 -*- # Part of Odoo. See LICENSE file for full copyright and licensing details. +import datetime +import random +import re +import werkzeug + 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 class MassMailCase(MailCase, MockLinkTracker): + # ------------------------------------------------------------ + # ASSERTS + # ------------------------------------------------------------ + + def assertMailingStatistics(self, mailing, **kwargs): + """ Helper to assert mailing statistics fields. As we have many of them + it helps lessening test asserts. """ + if not kwargs.get('expected'): + kwargs['expected'] = len(mailing.mailing_trace_ids) + if not kwargs.get('delivered'): + kwargs['delivered'] = len(mailing.mailing_trace_ids) + for fname in ['scheduled', 'expected', 'sent', 'delivered', + 'opened', 'replied', 'clicked', + 'ignored', '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)) + ) + def assertMailTraces(self, recipients_info, mailing, records, - check_mail=True, author=None): - """ Check content of traces. + check_mail=True, sent_unlink=False, author=None, + mail_links_info=None): + """ Check content of traces. Traces are fetched based on a given mailing + and records. Their content is compared to recipients_info structure that + holds expected information. Links content may be checked, notably to + assert shortening or unsubscribe links. Mail.mail records may optionally + be checked. :param recipients_info: list[{ + # TRACE 'partner': res.partner record (may be empty), 'email': email used when sending email (may be empty, computed based on partner), - 'state': outgoing / sent / open / reply / error / cancel (sent by default), + 'state': outgoing / sent / ignored / bounced / exception / opened (sent by default), 'record: linked record, - 'content': UDPATE ME - 'failure_type': optional: UPDATE ME + # MAIL.MAIL + 'content': optional content that should be present in mail.mail body_html; + 'failure_type': optional failure reason; }, { ... }] + + :param mailing: a mailing.mailing record from which traces have been + generated; + :param records: records given to mailing that generated traces. It is + used notably to find traces using their IDs; + :param check_mail: if True, also check mail.mail records that should be + linked to traces; + :param sent_unlink: it True, sent mail.mail are deleted and we check gateway + output result instead of actual mail.mail records; + :param mail_links_info: if given, should follow order of ``recipients_info`` + and give details about links. See ``assertLinkShortenedHtml`` helper for + more details about content to give; + :param author: author of sent mail.mail; """ # map trace state to email state state_mapping = { 'sent': 'sent', - 'replied': 'sent', # replied imply something has been sent + 'opened': 'sent', # opened implies something has been sent + 'replied': 'sent', # replied implies something has been sent 'ignored': 'cancel', 'exception': 'exception', - 'canceled': 'canceled', + 'canceled': 'cancel', + 'bounced': 'cancel', } traces = self.env['mailing.trace'].search([ @@ -39,11 +85,13 @@ class MassMailCase(MailCase, MockLinkTracker): self.assertEqual(set(s.res_id for s in traces), set(records.ids)) # check each traces - for recipient_info in recipients_info: + if not mail_links_info: + mail_links_info = [None] * len(recipients_info) + 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') - record = recipient_info.get('record') + record = record or recipient_info.get('record') content = recipient_info.get('content') if email is None and partner: email = partner.email_normalized @@ -55,6 +103,7 @@ class MassMailCase(MailCase, MockLinkTracker): 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)) ) + self.assertTrue(bool(recipient_trace.mail_mail_id_int)) if check_mail: if author is None: @@ -64,11 +113,103 @@ class MassMailCase(MailCase, MockLinkTracker): if 'failure_type' in recipient_info: fields_values['failure_type'] = recipient_info['failure_type'] + # specific for partner: email_formatted is used if partner: - self.assertMailMail(partner, state_mapping[state], author=author, content=content, fields_values=fields_values) + if state == 'sent' and sent_unlink: + self.assertSentEmail(author, [partner]) + else: + self.assertMailMail(partner, state_mapping[state], 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) else: self.assertMailMailWEmails([email], state_mapping[state], author=author, content=content, fields_values=fields_values) + if link_info: + trace_mail = self._find_mail_mail_wrecord(record) + for (anchor_id, url, is_shortened, add_link_params) in link_info: + link_params = {'utm_medium': 'Email', 'utm_source': mailing.name} + if add_link_params: + link_params.update(**add_link_params) + self.assertLinkShortenedHtml( + trace_mail.body_html, + (anchor_id, url, is_shortened), + link_params=link_params, + ) + + # ------------------------------------------------------------ + # TOOLS + # ------------------------------------------------------------ + + def gateway_mail_bounce(self, mailing, record, bounce_base_values=None): + """ Generate a bounce at mailgateway level. + + :param mailing: a ``mailing.mailing`` record on which we find a trace + to bounce; + :param record: record which should bounce; + :param bounce_base_values: optional values given to routing; + """ + trace = mailing.mailing_trace_ids.filtered(lambda t: t.model == record._name and t.res_id == record.id) + + parsed_bounce_values = { + 'email_from': 'some.email@external.example.com', # TDE check: email_from -> trace email ? + 'to': 'bounce@test.example.com', # TDE check: bounce alias ? + 'message_id': tools.generate_tracking_message_id('MailTest'), + 'bounced_partner': self.env['res.partner'].sudo(), + 'bounced_message': self.env['mail.message'].sudo() + } + if bounce_base_values: + parsed_bounce_values.update(bounce_base_values) + parsed_bounce_values.update({ + 'bounced_email': trace.email, + 'bounced_msg_id': [trace.message_id], + }) + self.env['mail.thread']._routing_handle_bounce(False, parsed_bounce_values) + + def gateway_mail_click(self, mailing, record, click_label): + """ Simulate a click on a sent email. """ + trace = mailing.mailing_trace_ids.filtered(lambda t: t.model == record._name and t.res_id == record.id) + email = self._find_sent_mail_wemail(trace.email) + self.assertTrue(bool(email)) + for (_url_href, link_url, _dummy, label) in re.findall(tools.HTML_TAG_URL_REGEX, email['body']): + if label == click_label and '/r/' in link_url: # shortened link, like 'http://localhost:8069/r/LBG/m/53' + parsed_url = werkzeug.urls.url_parse(link_url) + path_items = parsed_url.path.split('/') + code, trace_id = path_items[2], int(path_items[4]) + self.assertEqual(trace.id, trace_id) + + self.env['link.tracker.click'].sudo().add_click( + code, + ip='100.200.300.%3f' % random.random(), + country_code='BE', + mailing_trace_id=trace.id + ) + break + else: + raise AssertionError('url %s not found in mailing %s for record %s' % (click_label, mailing, record)) + + @classmethod + def _create_bounce_trace(cls, mailing, record, dt=None): + if 'email_normalized' in record: + trace_email = record.email_normalized + elif 'email_from' in record: + trace_email = record.email_from + else: + trace_email = record.email + 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, + }) + return trace + class MassMailCommon(MailCommon, MassMailCase): diff --git a/addons/mass_mailing/tests/test_mailing_internals.py b/addons/mass_mailing/tests/test_mailing_internals.py index 872161e16c3..bc5408c288b 100644 --- a/addons/mass_mailing/tests/test_mailing_internals.py +++ b/addons/mass_mailing/tests/test_mailing_internals.py @@ -242,7 +242,7 @@ class TestMassMailFeatures(MassMailCommon, CronMixinCase): self.assertMailTraces( [{'partner': partner_a}, {'partner': partner_b, 'state': 'ignored'}], - mailing, partner_a | partner_b, check_mail=True + mailing, partner_a + partner_b, check_mail=True ) @users('user_marketing') diff --git a/addons/test_mail_full/tests/test_mass_mailing.py b/addons/test_mail_full/tests/test_mass_mailing.py index 693022572fa..ff0f43b7f90 100644 --- a/addons/test_mail_full/tests/test_mass_mailing.py +++ b/addons/test_mail_full/tests/test_mass_mailing.py @@ -1,9 +1,11 @@ # -*- coding: utf-8 -*- # Part of Odoo. See LICENSE file for full copyright and licensing details. +import werkzeug + from odoo.addons.test_mail_full.tests.common import TestMailFullCommon from odoo.tests.common import users -from odoo.tools import config, mute_logger +from odoo.tools import mute_logger from odoo.tests import tagged @@ -17,49 +19,55 @@ class TestMassMailing(TestMailFullCommon): @users('user_marketing') @mute_logger('odoo.addons.mail.models.mail_mail') def test_mailing_w_blacklist_opt_out(self): - # TDE FIXME: better URLs check for unsubscribe / view (res_id + email + correct parse of url) - mailing = self.mailing_bl.with_user(self.env.user) + mailing = self.env['mailing.mailing'].browse(self.mailing_bl.ids) - jinja_html = ''' -
Hello ${object.name}
-- Here are your personal links - External link - Internal link - Unsubscribe link - - View link - -
-Hello ${object.name}
', + 'preview': 'Hi ${object.name} :)', + 'body_html': """Hello ${object.name}
, +% set url = "www.odoo.com" +% set httpurl = "https://www.odoo.eu" +Website0: https://www.odoo.tz/my/${object.name} +Website1: https://www.odoo.be +Website2: https://${url} +Website3: ${httpurl} +External1: Youpie +Internal1: Internal link +Internal2: View link +Email: test@odoo.com +Stop spam ? Ok
+Hello ${object.name_id.id}
', }) with self.mock_mail_gateway(), self.assertRaises(Exception): - mailing_test.send_mail_test() - + mailing_test.send_mail_test() + # Test if bad jinja in the preview raises an error mailing.write({ 'body_html': 'Hello ${object.name}
', 'preview': 'Preview ${object.name_id.id}', }) with self.mock_mail_gateway(), self.assertRaises(Exception): - mailing_test.send_mail_test() + mailing_test.send_mail_test() def test_mailing_trace_update(self): customers = self.env['res.partner'] @@ -85,8 +85,8 @@ class TestMailingInternals(TestMassMailCommon): 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) + self.gateway_mail_reply_wrecord(MAIL_TEMPLATE, customers[0], use_in_reply_to=True) + self.gateway_mail_reply_wrecord(MAIL_TEMPLATE, customers[1], use_in_reply_to=False) # customer2 looses headers mail_mail = self._find_mail_mail_wrecord(customers[2]) @@ -152,8 +152,8 @@ class TestMailingInternals(TestMassMailCommon): self.assertEqual(len(traces), 3) # simulate response to mailing - self.gateway_reply_wrecord(MAIL_TEMPLATE, self.mailing_list_1.contact_ids[0], use_in_reply_to=True) - self.gateway_reply_wrecord(MAIL_TEMPLATE, self.mailing_list_1.contact_ids[1], use_in_reply_to=False) + self.gateway_mail_reply_wrecord(MAIL_TEMPLATE, self.mailing_list_1.contact_ids[0], use_in_reply_to=True) + self.gateway_mail_reply_wrecord(MAIL_TEMPLATE, self.mailing_list_1.contact_ids[1], use_in_reply_to=False) mailing_test_utms = self.env['mailing.test.utm'].search([('name', '=', 'Re: %s' % subject)]) self.assertEqual(len(mailing_test_utms), 2) diff --git a/addons/test_mass_mailing/tests/test_mass_mailing.py b/addons/test_mass_mailing/tests/test_mass_mailing.py index 478f9ce061d..0a2f45f0e17 100644 --- a/addons/test_mass_mailing/tests/test_mass_mailing.py +++ b/addons/test_mass_mailing/tests/test_mass_mailing.py @@ -2,21 +2,71 @@ # Part of Odoo. See LICENSE file for full copyright and licensing details. from odoo.addons.test_mass_mailing.tests.common import TestMassMailCommon +from odoo.tests import tagged from odoo.tests.common import users from odoo.tools import mute_logger +@tagged('mass_mailing') class TestMassMailing(TestMassMailCommon): @classmethod def setUpClass(cls): super(TestMassMailing, cls).setUpClass() + @users('user_marketing') + @mute_logger('odoo.addons.mail.models.mail_mail') + def test_mailing_gateway_update(self): + mailing = self.env['mailing.mailing'].browse(self.mailing_bl.ids) + recipients = self._create_mailing_test_records(model='mailing.test.optout', count=5) + self.assertEqual(len(recipients), 5) + + mailing.write({ + 'mailing_model_id': self.env['ir.model']._get('mailing.test.optout'), + 'mailing_domain': [('id', 'in', recipients.ids)] + }) + mailing.action_put_in_queue() + with self.mock_mail_gateway(mail_unlink_sent=False): + mailing._process_mass_mailing_queue() + + self.assertMailTraces( + [{'email': record.email_normalized} + for record in recipients], + mailing, recipients, + mail_links_info=[[ + ('url0', 'https://www.odoo.tz/my/%s' % record.name, True, {}), + ('url1', 'https://www.odoo.be', True, {}), + ('url2', 'https://www.odoo.com', True, {}), + ('url3', 'https://www.odoo.eu', True, {}), + ('url4', 'https://www.example.com/foo/bar?baz=qux', True, {'baz': 'qux'}), + ('url5', '%s/event/dummy-event-0' % mailing.get_base_url(), True, {}), + # view is not shortened and parsed at sending + ('url6', '%s/view' % mailing.get_base_url(), False, {}), + ('url7', 'mailto:test@odoo.com', False, {}), + # unsubscribe is not shortened and parsed at sending + ('url8', '%s/unsubscribe_from_list' % mailing.get_base_url(), False, {}), + ] for record in recipients], + check_mail=True + ) + self.assertMailingStatistics(mailing, expected=5, delivered=5, sent=5) + + # simulate a click + self.gateway_mail_click(mailing, recipients[0], 'https://www.odoo.be') + mailing.invalidate_cache() + self.assertMailingStatistics(mailing, expected=5, delivered=5, sent=5, opened=1, clicked=1) + + # simulate a bounce + self.assertEqual(recipients[1].message_bounce, 0) + self.gateway_mail_bounce(mailing, recipients[1]) + mailing.invalidate_cache() + self.assertMailingStatistics(mailing, expected=5, delivered=4, sent=5, opened=1, clicked=1, bounced=1) + self.assertEqual(recipients[1].message_bounce, 1) + @users('user_marketing') @mute_logger('odoo.addons.mail.models.mail_mail') def test_mailing_w_blacklist(self): - mailing = self.mailing_bl.with_user(self.env.user) - recipients = self._create_test_blacklist_records(count=5) + mailing = self.env['mailing.mailing'].browse(self.mailing_bl.ids) + recipients = self._create_mailing_test_records(count=5) # blacklist records 3 and 4 self.env['mail.blacklist'].create({'email': recipients[3].email_normalized}) @@ -40,8 +90,8 @@ class TestMassMailing(TestMassMailCommon): @users('user_marketing') @mute_logger('odoo.addons.mail.models.mail_mail') def test_mailing_w_opt_out(self): - mailing = self.mailing_bl.with_user(self.env.user) - recipients = self._create_test_blacklist_records(model='mailing.test.optout', count=5) + mailing = self.env['mailing.mailing'].browse(self.mailing_bl.ids) + recipients = self._create_mailing_test_records(model='mailing.test.optout', count=5) # optout records 0 and 1 (recipients[0] | recipients[1]).write({'opt_out': True}) @@ -114,13 +164,13 @@ class TestMassMailing(TestMassMailCommon): mailing._process_mass_mailing_queue() self.assertMailTraces( - [{'email': 'test@test.example.com', 'state': 'ignored'}, - {'email': 'test@test.example.com', 'state': 'sent'}, + [{'email': 'test@test.example.com', 'state': 'sent'}, + {'email': 'test@test.example.com', 'state': 'ignored'}, {'email': 'test3@test.example.com'}, {'email': 'test4@test.example.com'}, {'email': 'test5@test.example.com', 'state': 'ignored'}], mailing, - mailing_contact_1 | mailing_contact_2 | mailing_contact_3 | mailing_contact_4 | mailing_contact_5, + mailing_contact_1 + mailing_contact_2 + mailing_contact_3 + mailing_contact_4 + mailing_contact_5, check_mail=True ) self.assertEqual(mailing.ignored, 2)