From 94dc448577f1a1cdeff50c5cbe0c38484ae2d038 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Mon, 13 Nov 2023 11:03:23 +0100 Subject: [PATCH] [IMP] mass_mailing: improve some test tools and logs Move '_create_mailing_list' as a base mass mailing tool to make it easily usable by most mail-related test addons. Improve '_filter_mail' tool by allowing to give an additional 'email_from' used to filter emails produced under a mock. When having similar records mailed several times (e.g. several mailings triggered at the same time which may happen for marketing automation tests) it eases finding the right email linked to a given mailing / recipient. Improve some logs to ease debugging, notably for marketing automation. Task-3506681 (MA: Test cleanup) Prepares Task-2981581 (MA: Fix trace duplication and various issues) X-original-commit: aad84dda189e13d76a87e349a7d56d84e584879c Part-of: odoo/odoo#143088 --- addons/mail/tests/common.py | 49 ++++++++++++++++--------- addons/mass_mailing/tests/common.py | 57 ++++++++++++++++++----------- 2 files changed, 67 insertions(+), 39 deletions(-) diff --git a/addons/mail/tests/common.py b/addons/mail/tests/common.py index 97a81e22f5f..0221b80a37d 100644 --- a/addons/mail/tests/common.py +++ b/addons/mail/tests/common.py @@ -280,7 +280,7 @@ class MockEmail(common.BaseCase, MockSmtplibCase): raise AssertionError('sent mail not found for email_to %s' % (email_to)) return sent_email - def _filter_mail(self, status=None, mail_message=None, author=None): + def _filter_mail(self, status=None, mail_message=None, author=None, email_from=None): """ Filter mail generated during mock, based on common parameters :param status: state of mail.mail. If not void use it to filter mail.mail @@ -289,6 +289,8 @@ class MockEmail(common.BaseCase, MockSmtplibCase): a ``mail.message`` record; :param author: optional check/filter on author_id field aka a ``res.partner`` record; + :param email_from: optional check/filter on email_from field (may differ from + author, used notably in case of concurrent mailings to distinguish emails); """ filtered = self._new_mails.env['mail.mail'] for mail in self._new_mails: @@ -298,31 +300,33 @@ class MockEmail(common.BaseCase, MockSmtplibCase): continue if author is not None and mail.author_id != author: continue + if email_from is not None and mail.email_from != email_from: + continue filtered += mail return filtered - def _find_mail_mail_wid(self, mail_id, status=None, mail_message=None, author=None): + def _find_mail_mail_wid(self, mail_id, status=None, mail_message=None, author=None, email_from=None): """ 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; """ - filtered = self._filter_mail(status=status, mail_message=mail_message, author=author) + filtered = self._filter_mail(status=status, mail_message=mail_message, author=author, email_from=email_from) for mail in filtered: if mail.id == mail_id: break else: debug_info = '\n'.join( f'From: {mail.author_id} ({mail.email_from}) - ID {mail.id} (State: {mail.state})' - for mail in filtered + for mail in self._new_mails ) raise AssertionError( f'mail.mail not found for ID {mail_id} / message {mail_message} / status {status} / author {author}\n{debug_info}' ) return mail - def _find_mail_mail_wpartners(self, recipients, status, mail_message=None, author=None): + def _find_mail_mail_wpartners(self, recipients, status, mail_message=None, author=None, email_from=None): """ Find a mail.mail record based on various parameters, notably a list of recipients (partners). @@ -332,14 +336,14 @@ class MockEmail(common.BaseCase, MockSmtplibCase): :return mail: a ``mail.mail`` record generated during the mock and matching given parameters and filters; """ - filtered = self._filter_mail(status=status, mail_message=mail_message, author=author) + filtered = self._filter_mail(status=status, mail_message=mail_message, author=author, email_from=email_from) for mail in filtered: if all(p in mail.recipient_ids for p in recipients): break else: debug_info = '\n'.join( f'From: {mail.author_id} ({mail.email_from}) - To: {sorted(mail.recipient_ids.ids)} (State: {mail.state})' - for mail in filtered + for mail in self._new_mails ) recipients_info = f'Missing: {[r.name for r in recipients if r.id not in filtered.recipient_ids.ids]}' raise AssertionError( @@ -347,7 +351,7 @@ class MockEmail(common.BaseCase, MockSmtplibCase): ) return mail - def _find_mail_mail_wemail(self, email_to, status, mail_message=None, author=None): + def _find_mail_mail_wemail(self, email_to, status, mail_message=None, author=None, email_from=None): """ Find a mail.mail record based on various parameters, notably a list of email to (string emails). @@ -357,33 +361,33 @@ class MockEmail(common.BaseCase, MockSmtplibCase): :return mail: a ``mail.mail`` record generated during the mock and matching given parameters and filters; """ - filtered = self._filter_mail(status=status, mail_message=mail_message, author=author) + filtered = self._filter_mail(status=status, mail_message=mail_message, author=author, email_from=email_from) for mail in filtered: if (mail.email_to == email_to and not mail.recipient_ids) or (not mail.email_to and mail.recipient_ids.email == email_to): break else: debug_info = '\n'.join( f'From: {mail.author_id} ({mail.email_from}) - To: {mail.email_to} / {sorted(mail.recipient_ids.mapped("email"))} (State: {mail.state})' - for mail in filtered + for mail in self._new_mails ) raise AssertionError( f'mail.mail not found for message {mail_message} / status {status} / email_to {email_to} / author {author}\n{debug_info}' ) return mail - def _find_mail_mail_wrecord(self, record, status=None, mail_message=None, author=None): + def _find_mail_mail_wrecord(self, record, status=None, mail_message=None, author=None, email_from=None): """ Find a mail.mail record based on model / res_id of a record. :return mail: a ``mail.mail`` record generated during the mock; """ - filtered = self._filter_mail(status=status, mail_message=mail_message, author=author) + filtered = self._filter_mail(status=status, mail_message=mail_message, author=author, email_from=email_from) for mail in filtered: if mail.model == record._name and mail.res_id == record.id: break else: debug_info = '\n'.join( f'From: {mail.author_id} ({mail.email_from}) - Model{mail.model} / ResId {mail.res_id} (State: {mail.state})' - for mail in filtered + for mail in self._new_mails ) raise AssertionError( f'mail.mail not found for message {mail_message} / status {status} / record {record.model}, {record.id} / author {author}\n{debug_info}' @@ -481,7 +485,10 @@ class MockEmail(common.BaseCase, MockSmtplibCase): See '_assertMailMail' for more details about other parameters. """ - found_mail = self._find_mail_mail_wpartners(recipients, status, mail_message=mail_message, author=author) + found_mail = self._find_mail_mail_wpartners( + recipients, status, mail_message=mail_message, + author=author, email_from=(fields_values or {}).get('email_from') + ) self.assertTrue(bool(found_mail)) self._assertMailMail( found_mail, recipients, status, @@ -506,7 +513,10 @@ class MockEmail(common.BaseCase, MockSmtplibCase): """ found_mail = False for email_to in emails: - found_mail = self._find_mail_mail_wemail(email_to, status, mail_message=mail_message, author=author) + found_mail = self._find_mail_mail_wemail( + email_to, status, mail_message=mail_message, + author=author, email_from=(fields_values or {}).get('email_from') + ) self.assertTrue(bool(found_mail)) self._assertMailMail( found_mail, [email_to], status, @@ -530,7 +540,10 @@ class MockEmail(common.BaseCase, MockSmtplibCase): See '_assertMailMail' for more details about other parameters. """ - found_mail = self._find_mail_mail_wrecord(record, mail_message=mail_message, author=author) + found_mail = self._find_mail_mail_wrecord( + record, mail_message=mail_message, + author=author, email_from=(fields_values or {}).get('email_from') + ) self.assertTrue(bool(found_mail)) self._assertMailMail( found_mail, recipients, status, @@ -543,7 +556,7 @@ class MockEmail(common.BaseCase, MockSmtplibCase): def assertMailMailWId(self, mail_id, status, email_to_recipients=None, author=None, - content=None, fields_values=None): + content=None, fields_values=None, email_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 @@ -560,7 +573,7 @@ class MockEmail(common.BaseCase, MockSmtplibCase): status, email_to_recipients=email_to_recipients, author=author, content=content, - fields_values=fields_values, + fields_values=fields_values, email_values=email_values, ) return found_mail diff --git a/addons/mass_mailing/tests/common.py b/addons/mass_mailing/tests/common.py index f67df385eac..c6523a25d37 100644 --- a/addons/mass_mailing/tests/common.py +++ b/addons/mass_mailing/tests/common.py @@ -87,7 +87,7 @@ class MassMailCase(MailCase, MockLinkTracker): ]) debug_info = '\n'.join( ( - f'Trace: to {t.email} - state {t.trace_status}' + f'Trace: to {t.email} - state {t.trace_status} - res_id {t.res_id}' for t in traces ) ) @@ -134,6 +134,8 @@ class MassMailCase(MailCase, MockLinkTracker): # mail.mail specific values to check fields_values = {'mailing_id': mailing} + if recipient_info.get('mail_values'): + fields_values.update(recipient_info['mail_values']) if 'failure_reason' in recipient_info: fields_values['failure_reason'] = recipient_info['failure_reason'] if 'email_to_mail' in recipient_info: @@ -193,7 +195,9 @@ class MassMailCase(MailCase, MockLinkTracker): :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) + 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 ? @@ -212,8 +216,17 @@ class MassMailCase(MailCase, MockLinkTracker): 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) + """ Simulate a click on a sent email. + + :param mailing: a ``mailing.mailing`` record on which we find a trace + to click; + :param record: record which should click; + :param click_label: label of link on which we should click; + """ + 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']): @@ -270,23 +283,6 @@ class MassMailCase(MailCase, MockLinkTracker): ]) return traces - -class MassMailCommon(MailCommon, MassMailCase): - - @classmethod - def setUpClass(cls): - super(MassMailCommon, cls).setUpClass() - - cls.user_marketing = mail_new_test_user( - cls.env, - groups='base.group_user,base.group_partner_manager,mass_mailing.group_mass_mailing_user', - login='user_marketing', - name='Martial Marketing', - signature='--\nMartial', - ) - - cls.email_reply_to = 'MyCompany SomehowAlias ' - @classmethod def _create_mailing_list(cls): """ Shortcut to create mailing lists. Currently hardcoded, maybe evolve @@ -333,3 +329,22 @@ class MassMailCommon(MailCommon, MassMailCase): for idx in range(contacts_nbr) ], }) + + +class MassMailCommon(MailCommon, MassMailCase): + + @classmethod + def setUpClass(cls): + super(MassMailCommon, cls).setUpClass() + + cls.user_marketing = mail_new_test_user( + cls.env, + groups='base.group_user,base.group_partner_manager,mass_mailing.group_mass_mailing_user', + login='user_marketing', + name='Martial Marketing', + signature='--\nMartial', + ) + + cls.email_reply_to = 'MyCompany SomehowAlias ' + + cls.env.flush_all()