[IMP] base: avoid double formatting in partner 'email_formatted' field
PURPOSE
Be defensive when dealing with email fields, notably when having multi-emails
or email field containing an already-formatted email.
SPECIFICATIONS
Main fix in this commit: fix multiple nested formatting in 'email_formatted'
computation for <res.partner>. Other use cases are mainly left untouched as
we let users deal with their input. In summary :
* double format: if email already holds a formatted email, we should not use
it to compute email_formatted, like
name: Name / email: 'Format' <email@domain.com>
-> before '"Name" <"Name" <email@domain.com>>"
-> after '"Name" <email@domain.com>''
* multi emails: sometimes this field is used to hold several addresses
like email1@domain.com, email2@domain.com. We currently let this value
globally untouched by extracting emails and joining them, as we do not
expect email_formatted to be a list of emails. Extractin emails allows
to filter out extra text stored in email field, like
name: Name / email: text, email1@domain.com, email2@domain.com
-> before: "Name" <text, email1@domain.com, email2@domain.com>
-> after: "Name" <email1@domain.com,email2@domain.com>
* invalid email: if something is wrong, better keep it in email_formatted
than harcoding "False". Indeed this eases management and understanding
of failures at mail.mail, mail.notification and mailing.trace level. This
behavior does not change as it was already implemented like that even if
not sure it was intended;
Task-2612945 (Mail: Defensive email formatting)
X-original-commit: odoo/odoo@9175bbd8e2
Part-of: odoo/odoo#134934
This commit is contained in:
Thibault Delavallée
@@ -72,7 +72,7 @@ class NewLeadNotification(TestCrmCommon):
|
||||
'name': 'Std Name', 'user_id': self.user_sales_leads.id,
|
||||
'team_id': self.sales_team_1.id,
|
||||
}),
|
||||
(self.contact_1.id, '"Philip J Fry" <"Philip, J. Fry" <philip.j.fry@test.example.com>>', self.contact_1.lang, 'Customer',
|
||||
(self.contact_1.id, '"Philip J Fry" <philip.j.fry@test.example.com>', self.contact_1.lang, 'Customer',
|
||||
{}), # creation values only if not partner
|
||||
]
|
||||
):
|
||||
|
||||
@@ -179,8 +179,8 @@ class TestPartner(MailCommon):
|
||||
self.assertEqual(
|
||||
partners.mapped('email_formatted'),
|
||||
['"Classic Format" <classic.format@test.example.com>',
|
||||
'"FindMe Format" <"FindMe Format" <find.me.format@test.example.com>>',
|
||||
'"FindMe Multi" <find.me.multi.1@test.example.com, "FindMe Multi" <find.me.multi.2@test.example.com>>']
|
||||
'"FindMe Format" <find.me.format@test.example.com>',
|
||||
'"FindMe Multi" <find.me.multi.1@test.example.com,find.me.multi.2@test.example.com>']
|
||||
)
|
||||
self.assertEqual(
|
||||
partners.mapped('email_normalized'),
|
||||
|
||||
@@ -1717,15 +1717,15 @@ class TestComposerResultsComment(TestMailComposer, CronMixinCase):
|
||||
# ensure values used afterwards for testing
|
||||
self.assertEqual(
|
||||
self.partner_employee.email_formatted,
|
||||
'"Ernest Employee" <email.from.1@test.example.com, email.from.2@test.example.com>',
|
||||
'"Ernest Employee" <email.from.1@test.example.com,email.from.2@test.example.com>',
|
||||
'Formatting: wrong formatting due to multi-email')
|
||||
self.assertEqual(
|
||||
self.partner_1.email_formatted,
|
||||
'"Valid Lelitre" <"Valid Formatted" <valid.lelitre@agrolait.com>>',
|
||||
'Formatting: wrong double encapsulation')
|
||||
'"Valid Lelitre" <valid.lelitre@agrolait.com>',
|
||||
'Formatting: avoid wrong double encapsulation')
|
||||
self.assertEqual(
|
||||
self.partner_2.email_formatted,
|
||||
'"Valid Poilvache" <valid.other.1@agrolait.com, valid.other.cc@agrolait.com>',
|
||||
'"Valid Poilvache" <valid.other.1@agrolait.com,valid.other.cc@agrolait.com>',
|
||||
'Formatting: wrong formatting due to multi-email')
|
||||
|
||||
# instantiate composer, post message
|
||||
@@ -1760,8 +1760,8 @@ class TestComposerResultsComment(TestMailComposer, CronMixinCase):
|
||||
)
|
||||
self.assertEqual(
|
||||
sorted(new_partners.mapped('email_formatted')),
|
||||
sorted(['"FindMe Format" <"FindMe Format" <find.me.format@test.example.com>>',
|
||||
'"FindMe Multi" <find.me.multi.1@test.example.com, "FindMe Multi" <find.me.multi.2@test.example.com>>',
|
||||
sorted(['"FindMe Format" <find.me.format@test.example.com>',
|
||||
'"FindMe Multi" <find.me.multi.1@test.example.com,find.me.multi.2@test.example.com>',
|
||||
'"find.me.multi.1@test.example.com" <find.me.multi.1@test.example.com>',
|
||||
'"find.me.multi.2@test.example.com" <find.me.multi.2@test.example.com>',
|
||||
'"test.cc.1@example.com" <test.cc.1@example.com>',
|
||||
@@ -1802,12 +1802,12 @@ class TestComposerResultsComment(TestMailComposer, CronMixinCase):
|
||||
email_values={
|
||||
'body_content': f'TemplateBody {self.test_record.name}',
|
||||
# currently holding multi-email 'from'
|
||||
'email_from': formataddr((self.user_employee.name, 'email.from.1@test.example.com, email.from.2@test.example.com')),
|
||||
'email_from': formataddr((self.user_employee.name, 'email.from.1@test.example.com,email.from.2@test.example.com')),
|
||||
'subject': f'TemplateSubject {self.test_record.name}',
|
||||
},
|
||||
fields_values={
|
||||
# currently holding multi-email 'email_from'
|
||||
'email_from': formataddr((self.user_employee.name, 'email.from.1@test.example.com, email.from.2@test.example.com')),
|
||||
'email_from': formataddr((self.user_employee.name, 'email.from.1@test.example.com,email.from.2@test.example.com')),
|
||||
},
|
||||
mail_message=self.test_record.message_ids[0],
|
||||
)
|
||||
@@ -1816,12 +1816,12 @@ class TestComposerResultsComment(TestMailComposer, CronMixinCase):
|
||||
author=self.partner_employee,
|
||||
email_values={
|
||||
'body_content': f'TemplateBody {self.test_record.name}',
|
||||
'email_from': formataddr((self.user_employee.name, 'email.from.1@test.example.com, email.from.2@test.example.com')),
|
||||
'email_from': formataddr((self.user_employee.name, 'email.from.1@test.example.com,email.from.2@test.example.com')),
|
||||
'subject': f'TemplateSubject {self.test_record.name}',
|
||||
},
|
||||
fields_values={
|
||||
# currently holding multi-email 'email_from'
|
||||
'email_from': formataddr((self.user_employee.name, 'email.from.1@test.example.com, email.from.2@test.example.com')),
|
||||
'email_from': formataddr((self.user_employee.name, 'email.from.1@test.example.com,email.from.2@test.example.com')),
|
||||
},
|
||||
mail_message=self.test_record.message_ids[0],
|
||||
)
|
||||
@@ -2183,12 +2183,27 @@ class TestComposerResultsMass(TestMailComposer):
|
||||
# check layouting and language. Note that standard layout
|
||||
# is not tested against translations, only the custom one
|
||||
# to ease translations checks.
|
||||
email = self._find_sent_email(self.partner_employee_2.email_formatted, [record.customer_id.email_formatted])
|
||||
self.assertTrue(bool(email), 'Email not found, check recipients')
|
||||
sent_mail = self._find_sent_email(
|
||||
self.partner_employee_2.email_formatted,
|
||||
[formataddr((record.customer_id.name, record.customer_id.email))]
|
||||
)
|
||||
debug_info = ''
|
||||
if not sent_mail:
|
||||
debug_info = '-'.join('From: %s-To: %s' % (mail['email_from'], mail['email_to']) for mail in self._mails)
|
||||
self.assertTrue(
|
||||
bool(sent_mail),
|
||||
f'Expected mail from {self.partner_employee_2.email_formatted} to {formataddr((record.customer_id.name, record.customer_id.email))} not found in {debug_info}'
|
||||
)
|
||||
if record == self.test_records[0]:
|
||||
self.assertEqual(sent_mail['email_to'], ['"Partner_0" <test_partner_0@example.com>'],
|
||||
'Should take email normalized in to')
|
||||
else:
|
||||
self.assertEqual(sent_mail['email_to'], ['"Partner_1" <test_partner_1@example.com>'],
|
||||
'Should take email normalized in to')
|
||||
|
||||
if not email_layout_xmlid:
|
||||
self.assertEqual(
|
||||
email['body'],
|
||||
sent_mail['body'],
|
||||
f'<p>{exp_body}</p>'
|
||||
)
|
||||
else:
|
||||
@@ -2197,12 +2212,12 @@ class TestComposerResultsMass(TestMailComposer):
|
||||
exp_button_en = 'View Ticket-like model'
|
||||
exp_button_es = 'Spanish Layout para Spanish Model Description'
|
||||
if exp_lang == 'es_ES':
|
||||
self.assertIn(exp_layout_content_es, email['body'])
|
||||
self.assertIn(exp_button_es, email['body'])
|
||||
self.assertIn(exp_layout_content_es, sent_mail['body'])
|
||||
self.assertIn(exp_button_es, sent_mail['body'])
|
||||
else:
|
||||
self.assertIn(exp_layout_content_en, email['body'])
|
||||
# self.assertIn(exp_button_es, email['body'])
|
||||
self.assertIn(exp_button_en, email['body'])
|
||||
self.assertIn(exp_layout_content_en, sent_mail['body'])
|
||||
# self.assertIn(exp_button_es, sent_mail['body'])
|
||||
self.assertIn(exp_button_en, sent_mail['body'])
|
||||
|
||||
@users('employee')
|
||||
@mute_logger('odoo.models.unlink', 'odoo.addons.mail.models.mail_mail')
|
||||
@@ -2373,15 +2388,15 @@ class TestComposerResultsMass(TestMailComposer):
|
||||
# ensure values used afterwards for testing
|
||||
self.assertEqual(
|
||||
self.partner_employee.email_formatted,
|
||||
'"Ernest Employee" <email.from.1@test.example.com, email.from.2@test.example.com>',
|
||||
'"Ernest Employee" <email.from.1@test.example.com,email.from.2@test.example.com>',
|
||||
'Formatting: wrong formatting due to multi-email')
|
||||
self.assertEqual(
|
||||
self.partner_1.email_formatted,
|
||||
'"Valid Lelitre" <"Valid Formatted" <valid.lelitre@agrolait.com>>',
|
||||
'Formatting: wrong double encapsulation')
|
||||
'"Valid Lelitre" <valid.lelitre@agrolait.com>',
|
||||
'Formatting: avoid wrong double encapsulation')
|
||||
self.assertEqual(
|
||||
self.partner_2.email_formatted,
|
||||
'"Valid Poilvache" <valid.other.1@agrolait.com, valid.other.cc@agrolait.com>',
|
||||
'"Valid Poilvache" <valid.other.1@agrolait.com,valid.other.cc@agrolait.com>',
|
||||
'Formatting: wrong formatting due to multi-email')
|
||||
|
||||
# instantiate composer, send mailing
|
||||
@@ -2416,8 +2431,8 @@ class TestComposerResultsMass(TestMailComposer):
|
||||
)
|
||||
self.assertEqual(
|
||||
sorted(new_partners.mapped('email_formatted')),
|
||||
sorted(['"FindMe Format" <"FindMe Format" <find.me.format@test.example.com>>',
|
||||
'"FindMe Multi" <find.me.multi.1@test.example.com, "FindMe Multi" <find.me.multi.2@test.example.com>>',
|
||||
sorted(['"FindMe Format" <find.me.format@test.example.com>',
|
||||
'"FindMe Multi" <find.me.multi.1@test.example.com,find.me.multi.2@test.example.com>',
|
||||
'"find.me.multi.1@test.example.com" <find.me.multi.1@test.example.com>',
|
||||
'"find.me.multi.2@test.example.com" <find.me.multi.2@test.example.com>',
|
||||
'"test.cc.1@example.com" <test.cc.1@example.com>',
|
||||
|
||||
@@ -507,11 +507,36 @@ class Partner(models.Model):
|
||||
|
||||
@api.depends('name', 'email')
|
||||
def _compute_email_formatted(self):
|
||||
""" Compute formatted email for partner, using formataddr. Be defensive
|
||||
in computation, notably
|
||||
|
||||
* double format: if email already holds a formatted email like
|
||||
'Name' <email@domain.com> we should not use it as it to compute
|
||||
email formatted like "Name <'Name' <email@domain.com>>";
|
||||
* multi emails: sometimes this field is used to hold several addresses
|
||||
like email1@domain.com, email2@domain.com. We currently let this value
|
||||
untouched, but remove any formatting from multi emails;
|
||||
* invalid email: if something is wrong, keep it in email_formatted as
|
||||
this eases management and understanding of failures at mail.mail,
|
||||
mail.notification and mailing.trace level;
|
||||
* void email: email_formatted is False, as we cannot do anything with
|
||||
it;
|
||||
"""
|
||||
self.email_formatted = False
|
||||
for partner in self:
|
||||
if partner.email:
|
||||
partner.email_formatted = tools.formataddr((partner.name or u"False", partner.email or u"False"))
|
||||
else:
|
||||
partner.email_formatted = ''
|
||||
emails_normalized = tools.email_normalize_all(partner.email)
|
||||
if emails_normalized:
|
||||
# note: multi-email input leads to invalid email like "Name" <email1, email2>
|
||||
# but this is current behavior in Odoo 14+ and some servers allow it
|
||||
partner.email_formatted = tools.formataddr((
|
||||
partner.name or u"False",
|
||||
','.join(emails_normalized)
|
||||
))
|
||||
elif partner.email:
|
||||
partner.email_formatted = tools.formataddr((
|
||||
partner.name or u"False",
|
||||
partner.email
|
||||
))
|
||||
|
||||
@api.depends('is_company')
|
||||
def _compute_company_type(self):
|
||||
|
||||
@@ -130,25 +130,25 @@ class TestPartner(TransactionCase):
|
||||
# encapsulated email
|
||||
(
|
||||
"Vlad the Impaler <vlad.the.impaler@example.com>",
|
||||
'"Balázs" <Vlad the Impaler <vlad.the.impaler@example.com>>'
|
||||
'"Balázs" <vlad.the.impaler@example.com>'
|
||||
), (
|
||||
'"Balázs" <balazs@adam.hu>',
|
||||
'"Balázs" <"Balázs" <balazs@adam.hu>>'
|
||||
'"Balázs" <balazs@adam.hu>'
|
||||
),
|
||||
# multi email
|
||||
(
|
||||
"vlad.the.impaler@example.com, vlad.the.dragon@example.com",
|
||||
'"Balázs" <vlad.the.impaler@example.com, vlad.the.dragon@example.com>'
|
||||
'"Balázs" <vlad.the.impaler@example.com,vlad.the.dragon@example.com>'
|
||||
), (
|
||||
"vlad.the.impaler.com, vlad.the.dragon@example.com",
|
||||
'"Balázs" <vlad.the.impaler.com, vlad.the.dragon@example.com>'
|
||||
'"Balázs" <vlad.the.dragon@example.com>'
|
||||
), (
|
||||
'vlad.the.impaler.com, "Vlad the Dragon" <vlad.the.dragon@example.com>',
|
||||
'"Balázs" <vlad.the.impaler.com, "Vlad the Dragon" <vlad.the.dragon@example.com>>'
|
||||
'"Balázs" <vlad.the.dragon@example.com>'
|
||||
),
|
||||
# falsy emails
|
||||
(False, ''),
|
||||
('', ''),
|
||||
(False, False),
|
||||
('', False),
|
||||
(' ', '"Balázs" <@ >'),
|
||||
('notanemail', '"Balázs" <@notanemail>'),
|
||||
]:
|
||||
|
||||
@@ -570,6 +570,20 @@ def email_normalize(text, strict=True):
|
||||
return False
|
||||
return emails[0].lower()
|
||||
|
||||
def email_normalize_all(text):
|
||||
""" Tool method allowing to extract email addresses from a text input and returning
|
||||
normalized version of all found emails. If no email is found, a void list
|
||||
is returned.
|
||||
|
||||
e.g. if email is 'tony@e.com, "Tony2" <tony2@e.com' returned result is ['tony@e.com, tony2@e.com']
|
||||
|
||||
:return list: list of normalized emails found in text
|
||||
"""
|
||||
if not text:
|
||||
return []
|
||||
emails = email_split(text)
|
||||
return list(filter(None, [email_normalize(email) for email in emails]))
|
||||
|
||||
def email_domain_extract(email):
|
||||
""" Extract the company domain to be used by IAP services notably. Domain
|
||||
is extracted from email information e.g:
|
||||
|
||||
Reference in New Issue
Block a user