From e86ae0520845635f5be5f2730cc4dcfcc5a20877 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Wed, 7 Jun 2023 12:07:01 +0000 Subject: [PATCH] [IMP] test_mail{..mailing}: improve tooling (recipient / email check) Improve low-level checks of recipients in mail asserts. Sometimes 'email_to' cannot be easily deduced from given input (partners, records customers, ...) when some record -> email transformations are involved e.g. when dealing with multiple emails input, double encapsulation, ... Those tools are about to be used in upcoming tests and fixes. Task-3438381 (TestMail: backport tools) Prepares Task-2612945 (Mail: Defensive email formatting) X-original-commit: bdbe846329ed51e4c5e3b75740e8f594edf7b138 Part-of: odoo/odoo#129839 --- addons/mail/tests/common.py | 94 +++++++++++++++++++++-------- addons/mass_mailing/tests/common.py | 31 ++++++++-- 2 files changed, 94 insertions(+), 31 deletions(-) diff --git a/addons/mail/tests/common.py b/addons/mail/tests/common.py index 2c1c33a44c3..41a53e887db 100644 --- a/addons/mail/tests/common.py +++ b/addons/mail/tests/common.py @@ -270,7 +270,13 @@ class MockEmail(common.BaseCase, MockSmtplibCase): if mail.id == mail_id: break else: - raise AssertionError('mail.mail not found for ID %s' % (mail_id)) + debug_info = '\n'.join( + f'From: {mail.author_id} ({mail.email_from}) - ID {mail.id} (State: {mail.state})' + for mail in filtered + ) + 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): @@ -288,7 +294,14 @@ class MockEmail(common.BaseCase, MockSmtplibCase): if all(p in mail.recipient_ids for p in recipients): break else: - raise AssertionError('mail.mail not found for message %s / status %s / recipients %s / author %s' % (mail_message, status, recipients.ids, author)) + 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 + ) + recipients_info = f'Missing: {[r.name for r in recipients if r.id not in filtered.recipient_ids.ids]}' + raise AssertionError( + f'mail.mail not found for message {mail_message} / status {status} / recipients {sorted(recipients.ids)} / author {author}\n{recipients_info}\n{debug_info}' + ) return mail def _find_mail_mail_wemail(self, email_to, status, mail_message=None, author=None): @@ -306,7 +319,13 @@ class MockEmail(common.BaseCase, MockSmtplibCase): 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: - raise AssertionError('mail.mail not found for email_to %s / status %s in %s' % (email_to, status, repr([m.email_to for m in self._new_mails]))) + 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 + ) + 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): @@ -319,7 +338,13 @@ class MockEmail(common.BaseCase, MockSmtplibCase): 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]))) + 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 + ) + raise AssertionError( + f'mail.mail not found for message {mail_message} / status {status} / record {record.model}, {record.id} / author {author}\n{debug_info}' + ) return mail def _find_sent_email(self, email_from, emails_to, subject=None): @@ -343,8 +368,8 @@ class MockEmail(common.BaseCase, MockSmtplibCase): # GATEWAY ASSERTS # ------------------------------------------------------------ - def _assertMailMail(self, mail, recipients_list, - status, author=None, + def _assertMailMail(self, mail, recipients_list, status, + email_to_recipients=None, author=None, content=None, fields_values=None, email_values=None): """ Assert mail.mail record values and maybe related emails. Allow asserting their content. Records to check are the one generated when @@ -356,6 +381,9 @@ class MockEmail(common.BaseCase, MockSmtplibCase): ``_find_mail_mail_wemail``); :param status: mail.mail state used to filter mails. If ``sent`` this method also check that emails have been sent trough gateway; + :param email_to_recipients: used for assertSentEmail to find email based + on 'email_to' when doing the match directly based on recipients_list + being partners it nos easy (e.g. multi emails, ...); :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 / @@ -375,15 +403,20 @@ class MockEmail(common.BaseCase, MockSmtplibCase): 'Mail: expected %s for %s, got %s' % (fvalue, fname, mail[fname]) ) if status == 'sent': - for recipient in recipients_list: + if email_to_recipients: + recipients = email_to_recipients # already formatted + else: + recipients = [[r] for r in recipients_list] # one partner -> list of a single email + for recipient in recipients: with self.subTest(recipient=recipient): self.assertSentEmail( email_values['email_from'] if email_values and email_values.get('email_from') else author, - [recipient], + recipient, **(email_values or {}) ) def assertMailMail(self, recipients, status, + email_to_recipients=None, mail_message=None, author=None, content=None, fields_values=None, email_values=None): """ Assert mail.mail records are created and maybe sent as emails. This @@ -399,13 +432,15 @@ class MockEmail(common.BaseCase, MockSmtplibCase): found_mail = self._find_mail_mail_wpartners(recipients, status, mail_message=mail_message, author=author) self.assertTrue(bool(found_mail)) self._assertMailMail( - found_mail, recipients, - status, author=author, - content=content, fields_values=fields_values, email_values=email_values, + found_mail, recipients, status, + email_to_recipients=email_to_recipients, + author=author, content=content, + fields_values=fields_values, email_values=email_values, ) return found_mail def assertMailMailWEmails(self, emails, status, + email_to_recipients=None, mail_message=None, author=None, content=None, fields_values=None, email_values=None): """ Assert mail.mail records are created and maybe sent as emails. This @@ -422,13 +457,15 @@ class MockEmail(common.BaseCase, MockSmtplibCase): found_mail = self._find_mail_mail_wemail(email_to, status, mail_message=mail_message, author=author) self.assertTrue(bool(found_mail)) self._assertMailMail( - found_mail, [email_to], - status, author=author, - content=content, fields_values=fields_values, email_values=email_values, + found_mail, [email_to], status, + email_to_recipients=email_to_recipients, + author=author, content=content, + fields_values=fields_values, email_values=email_values, ) return found_mail def assertMailMailWRecord(self, record, recipients, status, + email_to_recipients=None, mail_message=None, author=None, content=None, fields_values=None, email_values=None): """ Assert mail.mail records are created and maybe sent as emails. This @@ -444,13 +481,15 @@ class MockEmail(common.BaseCase, MockSmtplibCase): found_mail = self._find_mail_mail_wrecord(record, mail_message=mail_message, author=author) self.assertTrue(bool(found_mail)) self._assertMailMail( - found_mail, recipients, - status, author=author, - content=content, fields_values=fields_values, email_values=email_values, + found_mail, recipients, status, + email_to_recipients=email_to_recipients, + author=author, content=content, + fields_values=fields_values, email_values=email_values, ) return found_mail def assertMailMailWId(self, mail_id, status, + email_to_recipients=None, author=None, content=None, fields_values=None): """ Assert mail.mail records are created and maybe sent as emails. Allow @@ -466,8 +505,10 @@ class MockEmail(common.BaseCase, MockSmtplibCase): self.assertTrue(bool(found_mail)) self._assertMailMail( found_mail, [], # generally used when recipients are Falsy - status, author=author, - content=content, fields_values=fields_values, + status, + email_to_recipients=email_to_recipients, + author=author, content=content, + fields_values=fields_values, ) return found_mail @@ -546,12 +587,15 @@ class MockEmail(common.BaseCase, MockSmtplibCase): else: expected['email_from'] = author - email_to_list = [] - for email_to in recipients: - if isinstance(email_to, self.env['res.partner'].__class__): - email_to_list.append(formataddr((email_to.name, email_to.email))) - else: - email_to_list.append(email_to) + if 'email_to' in values: + email_to_list = values['email_to'] + else: + email_to_list = [] + for email_to in recipients: + if isinstance(email_to, self.env['res.partner'].__class__): + email_to_list.append(formataddr((email_to.name, email_to.email or 'False'))) + else: + email_to_list.append(email_to) expected['email_to'] = email_to_list # fetch mail diff --git a/addons/mass_mailing/tests/common.py b/addons/mass_mailing/tests/common.py index c94762ec48e..d187e25fca3 100644 --- a/addons/mass_mailing/tests/common.py +++ b/addons/mass_mailing/tests/common.py @@ -36,8 +36,8 @@ class MassMailCase(MailCase, MockLinkTracker): ) def assertMailTraces(self, recipients_info, mailing, records, - check_mail=True, sent_unlink=False, author=None, - mail_links_info=None): + 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 @@ -52,6 +52,7 @@ class MassMailCase(MailCase, MockLinkTracker): 'record: linked record, # MAIL.MAIL 'content': optional content that should be present in mail.mail body_html; + 'email_to_recipients': optional, see '_assertMailMail'; 'failure_type': optional failure reason; }, { ... }] @@ -82,6 +83,12 @@ class MassMailCase(MailCase, MockLinkTracker): ('mass_mailing_id', 'in', mailing.ids), ('res_id', 'in', records.ids) ]) + debug_info = '\n'.join( + ( + f'Trace: to {t.email} - state {t.trace_status}' + for t in traces + ) + ) # ensure trace coherency self.assertTrue(all(s.model == records._name for s in traces)) @@ -93,6 +100,7 @@ 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') + email_to_recipients = recipient_info.get('email_to_recipients') status = recipient_info.get('trace_status', 'sent') record = record or recipient_info.get('record') content = recipient_info.get('content') @@ -106,7 +114,9 @@ class MassMailCase(MailCase, MockLinkTracker): ) self.assertTrue( len(recipient_trace) == 1, - 'MailTrace: email %s (recipient %s, status: %s, record: %s): found %s records (1 expected)' % (email, partner, status, record, len(recipient_trace)) + 'MailTrace: email %s (recipient %s, status: %s, record: %s): found %s records (1 expected)\n%s' % ( + email, partner, status, record, + len(recipient_trace), debug_info) ) self.assertTrue(bool(recipient_trace.mail_mail_id_int)) if 'failure_type' in recipient_info or status in ('error', 'cancel', 'bounce'): @@ -131,18 +141,27 @@ class MassMailCase(MailCase, MockLinkTracker): else: self.assertMailMail( partner, state_mapping[status], - author=author, content=content, fields_values=fields_values + author=author, + content=content, + email_to_recipients=email_to_recipients, + fields_values=fields_values, ) # specific if email is False -> could have troubles finding it if several falsy traces elif not email and status in ('cancel', 'bounce'): self.assertMailMailWId( recipient_trace.mail_mail_id_int, state_mapping[status], - author=author, content=content, fields_values=fields_values + author=author, + content=content, + email_to_recipients=email_to_recipients, + fields_values=fields_values, ) else: self.assertMailMailWEmails( [email], state_mapping[status], - author=author, content=content, fields_values=fields_values + author=author, + content=content, + email_to_recipients=email_to_recipients, + fields_values=fields_values, ) if link_info: