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

[IMP] mail: strengthen assert email values check

Browse Source 
Purpose is to ensure tools are called like intended, notably by filtering
input and raising if an unexpected value check is asked. Some code is also
made a bit more generic to have the same kind of parameters when searching
for mail.mail.

In this commit we therefore fix some bad calls to assertSentEmails which
were not checking the right stuff.

closes odoo/odoo#90082

X-original-commit: d9759b2ee86b79b4fb2739a4aebd24a8044cf947
Signed-off-by: Thibault Delavallee (tde) <tde@openerp.com>
This commit is contained in:
Thibault Delavallée
2022-04-29 12:06:03 +02:00
parent 22aa1be37a
commit 5c354ece04
2 changed files with 87 additions and 78 deletions
Download Patch File Download Diff File Expand all files Collapse all files
addons/mail/tests/common.py
+74 -69
View File
@@ -230,14 +230,36 @@ class MockEmail(common.BaseCase, MockSmtplibCase):
raise AssertionError('sent mail not found for email_to %s' % (email_to))
return sent_email
def _find_mail_mail_wid(self, mail_id):
def _filter_mail(self, status=None, mail_message=None, author=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
record;
:param mail_message: optional check/filter on mail_message_id field aka
a ``mail.message`` record;
:param author: optional check/filter on author_id field aka a ``res.partner``
record;
"""
filtered = self._new_mails.env['mail.mail']
for mail in self._new_mails:
if status is not None and mail.state != status:
continue
if mail_message is not None and mail.mail_message_id != mail_message:
continue
if author is not None and mail.author_id != author:
continue
filtered += mail
return filtered
def _find_mail_mail_wid(self, mail_id, status=None, mail_message=None, author=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;
"""
for mail in self._new_mails:
filtered = self._filter_mail(status=status, mail_message=mail_message, author=author)
for mail in filtered:
if mail.id == mail_id:
break
else:
@@ -250,23 +272,12 @@ class MockEmail(common.BaseCase, MockSmtplibCase):
:param recipients: a ``res.partner`` recordset Check all of them are in mail
recipients to find the right mail.mail record;
:param status: state of mail.mail. If not void use it to filter mail.mail
record;
:param mail_message: optional check/filter on mail_message_id field aka
a ``mail.message`` record;
:param author: optional check/filter on author_id field aka a ``res.partner``
record;
:return mail: a ``mail.mail`` record generated during the mock and matching
given parameters and filters;
"""
for mail in self._new_mails:
if author is not None and mail.author_id != author:
continue
if mail_message is not None and mail.mail_message_id != mail_message:
continue
if status and mail.state != status:
continue
filtered = self._filter_mail(status=status, mail_message=mail_message, author=author)
for mail in filtered:
if all(p in mail.recipient_ids for p in recipients):
break
else:
@@ -279,35 +290,25 @@ class MockEmail(common.BaseCase, MockSmtplibCase):
:param email_to: either matching mail.email_to value, either a mail sent
to a single recipient whose email is email_to;
:param status: state of mail.mail. If not void use it to filter mail.mail
record;
:param mail_message: optional check/filter on mail_message_id field aka
a ``mail.message`` record;
:param author: optional check/filter on author_id field aka a ``res.partner``
record;
:return mail: a ``mail.mail`` record generated during the mock and matching
given parameters and filters;
"""
for mail in self._new_mails:
if author is not None and mail.author_id != author:
continue
if mail_message is not None and mail.mail_message_id != mail_message:
continue
if status and mail.state != status:
continue
filtered = self._filter_mail(status=status, mail_message=mail_message, author=author)
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:
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])))
return mail
def _find_mail_mail_wrecord(self, record):
def _find_mail_mail_wrecord(self, record, status=None, mail_message=None, author=None):
""" Find a mail.mail record based on model / res_id of a record.
:return mail: a ``mail.mail`` record generated during the mock;
"""
for mail in self._new_mails:
filtered = self._filter_mail(status=status, mail_message=mail_message, author=author)
for mail in filtered:
if mail.model == record._name and mail.res_id == record.id:
break
else:
@@ -464,15 +465,18 @@ class MockEmail(common.BaseCase, MockSmtplibCase):
either a partner record;
:param values: dictionary of additional values to check email content;
"""
base_expected = {}
for fname in ['author_id', 'reply_to',
'subject', 'body', 'body_content', 'body_alternative_content',
'references', 'references_content',
'attachments']:
if fname in values:
base_expected[fname] = values[fname]
direct_check = ['body_alternative', 'email_from', 'references', 'reply_to', 'subject']
content_check = ['body_alternative_content', 'body_content', 'references_content']
other_check = ['attachments', 'body', 'attachments_info']
expected = {}
for fname in direct_check + content_check + other_check:
if fname in values:
expected[fname] = values[fname]
unknown = set(values.keys()) - set(direct_check + content_check + other_check)
if unknown:
raise NotImplementedError('Unsupported %s' % ', '.join(unknown))
expected = dict(base_expected)
if isinstance(author, self.env['res.partner'].__class__):
expected['email_from'] = formataddr((author.name, author.email))
else:
@@ -493,7 +497,7 @@ class MockEmail(common.BaseCase, MockSmtplibCase):
debug_info = '-'.join('From: %s-To: %s' % (mail['email_from'], mail['email_to']) for mail in self._mails) if not bool(sent_mail) else ''
self.assertTrue(bool(sent_mail), 'Expected mail from %s to %s not found in %s' % (expected['email_from'], expected['email_to'], debug_info))
for val in ['reply_to', 'subject', 'references']:
for val in direct_check:
if val in expected:
self.assertEqual(expected[val], sent_mail[val], 'Value for %s: expected %s, received %s' % (val, expected[val], sent_mail[val]))
if 'attachments' in expected:
@@ -501,22 +505,18 @@ class MockEmail(common.BaseCase, MockSmtplibCase):
sorted(expected['attachments']), sorted(sent_mail['attachments']),
'Value for %s: expected %s, received %s' % ('attachments', expected['attachments'], sent_mail['attachments'])
)
if 'attachments_info' in values:
if 'attachments_info' in expected:
attachments = sent_mail['attachments']
for attachment_info in values['attachments_info']:
for attachment_info in expected['attachments_info']:
attachment = next(attach for attach in attachments if attach[0] == attachment_info['name'])
if attachment_info.get('raw'):
self.assertEqual(attachment[1], attachment_info['raw'])
if attachment_info.get('type'):
self.assertEqual(attachment[2], attachment_info['type'])
self.assertEqual(len(values['attachments_info']), len(attachments))
for val in ['body', 'body_alternative']:
if val in expected:
self.assertHtmlEqual(
expected[val], sent_mail[val],
'Value for %s: expected %s, received %s' % (val, expected[val], sent_mail[val])
)
for val in ['body_content', 'body_alternative_content', 'references_content']:
self.assertEqual(len(expected['attachments_info']), len(attachments))
if 'body' in expected:
self.assertHtmlEqual(expected['body'], sent_mail['body'], 'Value for %s: expected %s, received %s' % ('body', expected['body'], sent_mail['body']))
for val in content_check:
if val in expected:
self.assertIn(
expected[val], sent_mail[val[:-8]],
@@ -761,7 +761,7 @@ class MailCase(MockEmail):
# check notifications and prepare assert data
email_groups = defaultdict(list)
mail_groups = {'failure': []}
mail_groups = {'failure': [], 'outgoing': []}
for recipient in message_info['notif']:
partner, ntype, ngroup, nstatus = recipient['partner'], recipient['type'], recipient.get('group'), recipient.get('status', 'sent')
nis_read, ncheck_send = recipient.get('is_read', False if recipient['type'] == 'inbox' else True), recipient.get('check_send', True)
@@ -788,6 +788,12 @@ class MailCase(MockEmail):
if nstatus == 'sent':
if ncheck_send:
email_groups[ngroup].append(partner)
# when force_send is False notably, notifications are ready and emails outgoing
elif nstatus == 'ready':
mail_groups['outgoing'].append(partner)
if ncheck_send:
email_groups[ngroup].append(partner)
# canceled: currently nothing checked
elif nstatus == 'exception':
mail_groups['failure'].append(partner)
if ncheck_send:
@@ -813,27 +819,26 @@ class MailCase(MockEmail):
for recipients in email_groups.values():
partners = self.env['res.partner'].sudo().concat(*recipients)
if all(p in mail_groups['failure'] for p in partners):
if not self.mail_unlink_sent:
self.assertMailMail(partners, 'exception',
author=message.author_id,
mail_message=message)
else:
for recipient in partners:
self.assertSentEmail(message.author_id, [recipient])
mail_status = 'exception'
elif all(p in mail_groups['outgoing'] for p in partners):
mail_status = 'outgoing'
else:
if not self.mail_unlink_sent:
self.assertMailMail(
partners, 'sent',
author=message.author_id if message.author_id else message.email_from,
mail_message=message,
email_values=email_values,
mail_status = 'sent'
if not self.mail_unlink_sent:
self.assertMailMail(
partners, mail_status,
author=message.author_id if message.author_id else message.email_from,
mail_message=message,
email_values=email_values,
)
else:
for recipient in partners:
self.assertSentEmail(
message.author_id if message.author_id else message.email_from,
[recipient],
**email_values
)
else:
for recipient in partners:
self.assertSentEmail(message.author_id if message.author_id else message.email_from,
[recipient],
**email_values
)
if not any(p for recipients in email_groups.values() for p in recipients):
self.assertNoMail(partners, mail_message=message, author=message.author_id)
addons/test_mail/tests/test_mail_composer.py
+13 -9
View File
@@ -867,7 +867,6 @@ class TestComposerResultsComment(TestMailComposer):
email_values={
'body_content': 'TemplateBody %s' % self.test_record.name,
'email_from': self.test_record.user_id.email_formatted, # set by template
'mail_server_id': self.mail_server_domain,
'subject': 'TemplateSubject %s' % self.test_record.name,
'attachments_info': [
{'name': 'AttFileName_00.txt', 'raw': b'AttContent_00', 'type': 'text/plain'},
@@ -875,7 +874,9 @@ class TestComposerResultsComment(TestMailComposer):
{'name': 'TestReport for %s.html' % self.test_record.name, 'type': 'text/plain'},
]
},
fields_values={},
fields_values={
'mail_server_id': self.mail_server_domain,
},
)
self.assertMailMail(self.test_record.customer_id + new_partners, 'sent',
mail_message=message,
@@ -883,7 +884,6 @@ class TestComposerResultsComment(TestMailComposer):
email_values={
'body_content': 'TemplateBody %s' % self.test_record.name,
'email_from': self.test_record.user_id.email_formatted, # set by template
'mail_server_id': self.mail_server_domain,
'subject': 'TemplateSubject %s' % self.test_record.name,
'attachments_info': [
{'name': 'AttFileName_00.txt', 'raw': b'AttContent_00', 'type': 'text/plain'},
@@ -891,7 +891,9 @@ class TestComposerResultsComment(TestMailComposer):
{'name': 'TestReport for %s.html' % self.test_record.name, 'type': 'text/plain'},
]
},
fields_values={},
fields_values={
'mail_server_id': self.mail_server_domain,
},
)
# message is posted and notified admin
@@ -1081,14 +1083,16 @@ class TestComposerResultsMass(TestMailComposer):
email_values={
'body_content': 'TemplateBody %s' % record.name,
'email_from': self.partner_employee_2.email_formatted,
'mail_server_id': self.mail_server_global.id,
'subject': 'TemplateSubject %s' % record.name,
'attachments_info': [
{'name': 'AttFileName_00.txt', 'raw': b'AttContent_00', 'type': 'text/plain'},
{'name': 'AttFileName_01.txt', 'raw': b'AttContent_01', 'type': 'text/plain'},
{'name': 'TestReport for %s.html' % record.name, 'type': 'text/plain'},
]
}
},
fields_values={
'mail_server_id': self.mail_server_domain,
},
)
@users('employee')
@@ -1113,9 +1117,9 @@ class TestComposerResultsMass(TestMailComposer):
self.assertEqual(len(self._mails), 2, 'Should have sent 1 email per record')
for record in self.test_records:
# template is sent directly using customer field
self.assertSentEmail(self.partner_employee_2.email_formatted, record.customer_id,
author_id=self.partner_employee)
# template is sent directly using customer field, even if author is partner_employee
self.assertSentEmail(self.partner_employee_2.email_formatted,
record.customer_id)
# 2: active_domain not taken into account if use_active_domain is False
composer_form = Form(self.env['mail.compose.message'].with_context(