[IMP] mail: split multi-emails partners for outgoing emails
PURPOSE Be defensive when dealing with email fields, notably when having multi-emails or email field containing an already-formatted email. SPECIFICATIONS When building the final 'email_to' of outgoing emails using 'formataddr' we have issues if email contains multi emails or formatted email. Main fix of this commit is to extract emails and rebuild the 'email_to' list based on found emails. E.g. partner Raoul - email: "Raoul" <raoul@raoul.fr> -> before: to: "Raoul" <"Raoul" <raoul@raoul.fr>> (double format) -> after: to: "Raoul" <raoul@raoul.fr> E.g. partner Raoul - email: raoul1@raoul.fr, raoul2@raoul.fr -> before: to: "Raoul" <raoul1@raoul.fr, raoul2@raoul.fr> single email with multiple emails, depends on server fault tolerance) -> after: to: "Raoul" <raoul1@raoul.fr>, "Raoul" <raoul2@raoul.fr> multi emails Fix that computation by using all normalized emails found in 'email' fields and rebuilding a formatted email based on name + those emails. We do not use `email_formatted` as it is not really multi-enabled. We prefer a local defensive approach to be as tolerant as possible with respect to user inputs. Task-2612945 (Mail: Defensive email formatting) X-original-commit: odoo/odoo@1c4b704149 Part-of: odoo/odoo#134934
This commit is contained in:
@@ -410,7 +410,14 @@ class MailMail(models.Model):
|
||||
# specific behavior to customize the send email for notified partners
|
||||
for partner in self.recipient_ids:
|
||||
# check partner email content
|
||||
email_to = [tools.formataddr((partner.name or '', partner.email or 'False'))]
|
||||
emails_normalized = tools.email_normalize_all(partner.email)
|
||||
if emails_normalized:
|
||||
email_to = [
|
||||
tools.formataddr((partner.name or "", email or "False"))
|
||||
for email in emails_normalized
|
||||
]
|
||||
else:
|
||||
email_to = [tools.formataddr((partner.name or "", partner.email or "False"))]
|
||||
email_list.append({
|
||||
'email_cc': [],
|
||||
'email_to': email_to,
|
||||
|
||||
@@ -20,7 +20,7 @@ from odoo.addons.mail.models.mail_message import Message
|
||||
from odoo.addons.mail.models.mail_notification import MailNotification
|
||||
from odoo.addons.mail.models.res_users import Users
|
||||
from odoo.tests import common, new_test_user
|
||||
from odoo.tools import formataddr, mute_logger, pycompat
|
||||
from odoo.tools import email_normalize, formataddr, mute_logger, pycompat
|
||||
from odoo.tools.translate import code_translations
|
||||
|
||||
mail_new_test_user = partial(new_test_user, context={'mail_create_nolog': True,
|
||||
@@ -585,7 +585,7 @@ class MockEmail(common.BaseCase, MockSmtplibCase):
|
||||
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')))
|
||||
email_to_list.append(formataddr((email_to.name, email_normalize(email_to.email, strict=False) or email_to.email)))
|
||||
else:
|
||||
email_to_list.append(email_to)
|
||||
expected['email_to'] = email_to_list
|
||||
|
||||
@@ -18,7 +18,7 @@ from odoo.fields import Datetime as FieldDatetime
|
||||
from odoo.exceptions import AccessError
|
||||
from odoo.tests import tagged
|
||||
from odoo.tests.common import users, Form
|
||||
from odoo.tools import mute_logger, formataddr
|
||||
from odoo.tools import email_normalize, mute_logger, formataddr
|
||||
|
||||
|
||||
@tagged('mail_composer')
|
||||
@@ -1814,6 +1814,10 @@ class TestComposerResultsComment(TestMailComposer, CronMixinCase):
|
||||
self.assertMailMail(
|
||||
self.partner_1 + self.partner_2 + mailed_new_partners, 'sent',
|
||||
author=self.partner_employee,
|
||||
email_to_recipients=[
|
||||
[self.partner_1.email_formatted],
|
||||
[f'"{self.partner_2.name}" <valid.other.1@agrolait.com>', f'"{self.partner_2.name}" <valid.other.cc@agrolait.com>'],
|
||||
] + [[email] for email in mailed_new_partners.mapped('email_formatted')],
|
||||
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')),
|
||||
@@ -2185,7 +2189,7 @@ class TestComposerResultsMass(TestMailComposer):
|
||||
# to ease translations checks.
|
||||
sent_mail = self._find_sent_email(
|
||||
self.partner_employee_2.email_formatted,
|
||||
[formataddr((record.customer_id.name, record.customer_id.email))]
|
||||
[formataddr((record.customer_id.name, email_normalize(record.customer_id.email, strict=False)))]
|
||||
)
|
||||
debug_info = ''
|
||||
if not sent_mail:
|
||||
@@ -2473,6 +2477,10 @@ class TestComposerResultsMass(TestMailComposer):
|
||||
self.partner_1 + self.partner_2 + mailed_new_partners,
|
||||
'sent',
|
||||
author=self.partner_employee,
|
||||
email_to_recipients=[
|
||||
[self.partner_1.email_formatted],
|
||||
[f'"{self.partner_2.name}" <valid.other.1@agrolait.com>', f'"{self.partner_2.name}" <valid.other.cc@agrolait.com>'],
|
||||
] + [[email] for email in mailed_new_partners.mapped('email_formatted')],
|
||||
email_values={
|
||||
'body_content': f'TemplateBody {record.name}',
|
||||
# currently holding multi-email 'email_from'
|
||||
|
||||
@@ -753,9 +753,9 @@ class TestMailMailServer(MailCommon):
|
||||
sorted(sorted(_mail['email_to']) for _mail in self._mails),
|
||||
sorted([sorted(['"Raoul, le Grand" <test.email.1@test.example.com>', '"Micheline, l\'immense" <test.email.2@test.example.com>']),
|
||||
[formataddr((self.user_employee.name, self.user_employee.email_normalized))],
|
||||
[formataddr(("Tony Customer", '"Formatted Emails" <tony.customer@test.example.com>'))]
|
||||
[formataddr(("Tony Customer", 'tony.customer@test.example.com'))]
|
||||
]),
|
||||
'Mail (FIXME): double encapsulation of emails ("Tony" <"Formatted" <tony@e.com>>)'
|
||||
'Mail: formatting issues should have been removed as much as possible'
|
||||
)
|
||||
# CC are added to first email
|
||||
self.assertEqual(
|
||||
@@ -784,10 +784,12 @@ class TestMailMailServer(MailCommon):
|
||||
self.assertEqual(
|
||||
sorted(sorted(_mail['email_to']) for _mail in self._mails),
|
||||
sorted([sorted(['test.email.1@test.example.com', 'test.email.2@test.example.com']),
|
||||
[formataddr((self.user_employee.name, self.user_employee.email_normalized))],
|
||||
[formataddr(("Tony Customer", 'tony.customer@test.example.com, norbert.customer@test.example.com'))]
|
||||
[formataddr((self.user_employee.name, self.user_employee.email_normalized))],
|
||||
sorted([formataddr(("Tony Customer", 'tony.customer@test.example.com')),
|
||||
formataddr(("Tony Customer", 'norbert.customer@test.example.com'))]),
|
||||
]),
|
||||
'Mail: currenty broken (multi email in a single address) but supported by some providers ("Tony" <tony@e.com, tony2@e.com>)'
|
||||
'Mail: formatting issues should have been removed as much as possible (multi emails in a single address are managed '
|
||||
'like separate emails when sending with recipient_ids'
|
||||
)
|
||||
# CC are added to first email
|
||||
self.assertEqual(
|
||||
@@ -812,10 +814,12 @@ class TestMailMailServer(MailCommon):
|
||||
self.assertEqual(
|
||||
sorted(sorted(_mail['email_to']) for _mail in self._mails),
|
||||
sorted([sorted(['test.email.1@test.example.com', 'test.email.2@test.example.com']),
|
||||
[formataddr((self.user_employee.name, self.user_employee.email_normalized))],
|
||||
[formataddr(("Tony Customer", 'tony.customer@test.example.com, "Norbert Customer" <norbert.customer@test.example.com>'))]
|
||||
[formataddr((self.user_employee.name, self.user_employee.email_normalized))],
|
||||
sorted([formataddr(("Tony Customer", 'tony.customer@test.example.com')),
|
||||
formataddr(("Tony Customer", 'norbert.customer@test.example.com'))]),
|
||||
]),
|
||||
'Mail: currently broken, double encapsulation with formatting ("Tony" <tony@e.com, "Tony2" <tony2@e.com>>)'
|
||||
'Mail: formatting issues should have been removed as much as possible (multi emails in a single address are managed '
|
||||
'like separate emails when sending with recipient_ids (and partner name is always used as name part)'
|
||||
)
|
||||
# CC are added to first email
|
||||
self.assertEqual(
|
||||
|
||||
@@ -861,10 +861,11 @@ class TestMessagePost(TestMessagePostCommon, CronMixinCase):
|
||||
False, '', ' ', # falsy
|
||||
]
|
||||
expected_tos = [
|
||||
# Currently semi-broken: sending to an incorrect email "Name" <address1, address2>
|
||||
[f'"{self.partner_1.name}" <valid.lelitre@agrolait.com, valid.lelitre.cc@agrolait.com>',],
|
||||
# Currently broken: sending to an incorrect email "Name" <"Name" <address>>
|
||||
[f'"{self.partner_1.name}" <"Valid Lelitre" <valid.lelitre@agrolait.com>>',],
|
||||
# Sends multi-emails
|
||||
[f'"{self.partner_1.name}" <valid.lelitre@agrolait.com>',
|
||||
f'"{self.partner_1.name}" <valid.lelitre.cc@agrolait.com>',],
|
||||
# Avoid double encapsulation
|
||||
[f'"{self.partner_1.name}" <valid.lelitre@agrolait.com>',],
|
||||
# sent "normally": formats email based on wrong / falsy email
|
||||
[f'"{self.partner_1.name}" <@wrong>',],
|
||||
[f'"{self.partner_1.name}" <@False>',],
|
||||
|
||||
@@ -237,8 +237,8 @@ class TestMassMailing(TestMassMailCommon):
|
||||
'partner': customer_mult,
|
||||
'trace_status': 'cancel'},
|
||||
{'email': '"Formatted Customer" <test.customer.format@example.com>',
|
||||
# double encapsulation, not good
|
||||
'email_to_recipients': [[f'"{customer_fmt.name}" <"Formatted Customer" <test.customer.format@example.com>>']],
|
||||
# mail to avoids double encapsulation
|
||||
'email_to_recipients': [[f'"{customer_fmt.name}" <test.customer.format@example.com>']],
|
||||
'failure_type': False,
|
||||
'partner': customer_fmt,
|
||||
'trace_status': 'sent'},
|
||||
@@ -249,17 +249,17 @@ class TestMassMailing(TestMassMailCommon):
|
||||
'partner': customer_unic,
|
||||
'trace_status': 'cancel'}, # email_re usage forbids mailing to unicode
|
||||
{'email': 'TEST.CUSTOMER.CASE@EXAMPLE.COM',
|
||||
'email_to_recipients': [[f'"{customer_case.name}" <TEST.CUSTOMER.CASE@EXAMPLE.COM>']],
|
||||
'email_to_recipients': [[f'"{customer_case.name}" <test.customer.case@example.com>']],
|
||||
'failure_type': False,
|
||||
'partner': customer_case,
|
||||
'trace_status': 'sent'}, # lower cased
|
||||
{'email': 'test.customer.weird@example.com Weird Format',
|
||||
'email_to_recipients': [[f'"{customer_weird.name}" <test.customer.weird@example.com Weird Format>']],
|
||||
'email_to_recipients': [[f'"{customer_weird.name}" <test.customer.weird@example.comweirdformat>']],
|
||||
'failure_type': False,
|
||||
'partner': customer_weird,
|
||||
'trace_status': 'sent'}, # concatenates everything after domain
|
||||
{'email': 'Weird Format2 test.customer.weird.2@example.com',
|
||||
'email_to_recipients': [[f'"{customer_weird_2.name}" <Weird Format2 test.customer.weird.2@example.com>']],
|
||||
'email_to_recipients': [[f'"{customer_weird_2.name}" <weird format2 test.customer.weird.2@example.com>']],
|
||||
'failure_type': False,
|
||||
'partner': customer_weird_2,
|
||||
'trace_status': 'sent'},
|
||||
|
||||
Reference in New Issue
Block a user