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

[IMP] tools, base, mail: use first found email in 'email_normalized'

Browse Source 
PURPOSE

Be defensive when dealing with email fields, notably when having multi-emails
or email field containing an already-formatted email.

SPECIFICATIONS

When having multi-emails input in an email field, 'email_normalized' field is
currently 'False', as they expect the field to contain a single email. This
has several drawbacks

  * searching partners or fetching information based on emails does not work as
    most tool methods use 'email_normalized' which is False (see e.g.
    '_message_partner_info_from_emails', '_mail_find_partner_from_emails'
    or 'find_or_create');
  * blacklist is not available as it is based on 'email_normalized';
  * mass_mailing wrongly considers those emails as invalid and cancel their
    mail and related trace, as it tries to skip sending emails to invalid
    emails;

Be more defensive and use first found email in case of multi-emails field.
Other emails are ignored. It is already an improvement that does not break
flows in stable and allow more emails to be sent.

  before
  -> email: '"Raoul" <raoul1@raoul.fr>, raoul2@raoul.fr'
  -> email_normalized: False
  after
  -> email: '"Raoul" <raoul1@raoul.fr>, raoul2@raoul.fr'
  -> email_normalized: raoul1@raoul.fr

A side effect is that it helps finding back some partners, as indicated in
tests where less phantom partners are created. It also helps suggested
partners / emails flow in discuss.

Task-2612945 (Mail: Defensive email formatting)

X-original-commit: odoo/odoo@90218186c5
Part-of: odoo/odoo#134934
This commit is contained in:
Thibault Delavallée
2023-09-11 15:22:09 +00:00
parent c3d054f9c3
commit 075f073006
10 changed files with 56 additions and 47 deletions
Download Patch File Download Diff File Expand all files Collapse all files
addons/crm/tests/test_crm_lead_notification.py
+4 -1
View File
@@ -66,7 +66,10 @@ class NewLeadNotification(TestCrmCommon):
'team_id': self.sales_team_1.id,
}),
(False, '"Multi Name" <new.customer.multi.1@test.example.com,new.customer.2@test.example.com>', None, 'Customer Email',
{}), # no email_normalized -> no information
{'company_name': 'Multi Name', 'email': 'new.customer.multi.1@test.example.com',
'name': 'Multi Name', 'user_id': self.user_sales_leads.id,
'team_id': self.sales_team_1.id,
}),
(False, '"Std Name" <new.customer.simple@test.example.com>', None, 'Customer Email',
{'company_name': 'Std Name', 'email': 'new.customer.simple@test.example.com',
'name': 'Std Name', 'user_id': self.user_sales_leads.id,
addons/mail/models/mail_thread_blacklist.py
+1 -1
View File
@@ -46,7 +46,7 @@ class MailBlackListMixin(models.AbstractModel):
def _compute_email_normalized(self):
self._assert_primary_email()
for record in self:
record.email_normalized = tools.email_normalize(record[self._primary_email])
record.email_normalized = tools.email_normalize(record[self._primary_email], strict=False)
@api.model
def _search_is_blacklisted(self, operator, value):
addons/mail/tests/test_mail_tools.py
+1 -1
View File
@@ -133,7 +133,7 @@ class TestMailTools(MailCommon):
(f'{self._test_email}, {_test_email_2}', True), # multi-email, both matching, depends on comparison
(f'{self._test_email}, {_test_email_2}', False) # multi-email, both matching, depends on comparison
]
expected = [self.env['res.partner'], self.env['res.partner'],
expected = [follower_partner, test_partner,
self.env['res.partner'], self.env['res.partner'],
self.env['res.partner'], self.env['res.partner'],
self.env['res.partner'], self.env['res.partner']]
addons/mail/tests/test_res_partner.py
+15 -6
View File
@@ -182,9 +182,11 @@ class TestPartner(MailCommon):
'"FindMe Format" <find.me.format@test.example.com>',
'"FindMe Multi" <find.me.multi.1@test.example.com,find.me.multi.2@test.example.com>']
)
# when having multi emails, first found one is taken as normalized email
self.assertEqual(
partners.mapped('email_normalized'),
['classic.format@test.example.com', 'find.me.format@test.example.com', False]
['classic.format@test.example.com', 'find.me.format@test.example.com',
'find.me.multi.1@test.example.com']
)
# classic find or create: use normalized email to compare records
@@ -196,11 +198,18 @@ class TestPartner(MailCommon):
with self.subTest(email=email):
self.assertEqual(self.env['res.partner'].find_or_create(email), partners[1])
# multi-emails -> no normalized email -> fails each time, create new partner (FIXME)
for email in ('find.me.multi.1@test.example.com', 'find.me.multi.2@test.example.com'):
with self.subTest(email=email):
partner = self.env['res.partner'].find_or_create(email)
self.assertNotIn(partner, partners)
self.assertEqual(partner.email, email)
for email_input, match_partner in [
('find.me.multi.1@test.example.com', partners[2]),
('find.me.multi.2@test.example.com', self.env['res.partner']),
]:
with self.subTest(email_input=email_input):
partner = self.env['res.partner'].find_or_create(email_input)
# either matching existing, either new partner
if match_partner:
self.assertEqual(partner, match_partner)
else:
self.assertNotIn(partner, partners)
self.assertEqual(partner.email, email_input)
partner.unlink() # do not mess with subsequent tests
# now input is multi email -> '_parse_partner_name' used in 'find_or_create'
addons/test_mail/tests/test_mail_composer.py
+16 -26
View File
@@ -1744,15 +1744,14 @@ class TestComposerResultsComment(TestMailComposer, CronMixinCase):
# FIXME: currently email finding based on formatted / multi emails does
# not work
new_partners = self.env['res.partner'].search([]).search([('id', 'not in', existing_partners.ids)])
self.assertEqual(len(new_partners), 9,
'Mail (FIXME): multiple partner creation due to formatted / multi emails: 2 extra partners')
self.assertEqual(len(new_partners), 8,
'Mail (FIXME): multiple partner creation due to formatted / multi emails: 1 extra partners')
self.assertIn(partner_format_tofind, new_partners)
self.assertIn(partner_multi_tofind, new_partners)
self.assertEqual(
sorted(new_partners.mapped('email')),
sorted(['"FindMe Format" <find.me.format@test.example.com>',
'find.me.multi.1@test.example.com, "FindMe Multi" <find.me.multi.2@test.example.com>',
'find.me.multi.1@test.example.com',
'find.me.multi.2@test.example.com',
'test.cc.1@example.com', 'test.cc.2@example.com', 'test.cc.2.2@example.com',
'test.to.1@example.com', 'test.to.2@example.com']),
@@ -1762,7 +1761,6 @@ class TestComposerResultsComment(TestMailComposer, CronMixinCase):
sorted(new_partners.mapped('email_formatted')),
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>',
'"test.cc.2@example.com" <test.cc.2@example.com>',
@@ -1774,7 +1772,6 @@ class TestComposerResultsComment(TestMailComposer, CronMixinCase):
sorted(new_partners.mapped('name')),
sorted(['FindMe Format',
'FindMe Multi',
'find.me.multi.1@test.example.com',
'find.me.multi.2@test.example.com',
'test.cc.1@example.com', 'test.to.1@example.com', 'test.to.2@example.com',
'test.cc.2@example.com', 'test.cc.2.2@example.com']),
@@ -1788,14 +1785,10 @@ class TestComposerResultsComment(TestMailComposer, CronMixinCase):
# FIXME: more partners created than real emails (see above) -> due to
# transformation from email -> partner in template 'generate_recipients'
# there are more partners than email to notify;
# NOTE: 'Findme Multi' is excluded as it has the same email as 'find.me.multi.1@test.example.com'
# (created by template) and comes second in a search based on email
mailed_new_partners = new_partners.filtered(lambda p: p.name != 'FindMe Multi')
self.assertEqual(len(mailed_new_partners), 8)
self.assertEqual(len(self._new_mails), 2, 'Should have created 2 mail.mail')
self.assertEqual(
len(self._mails), len(mailed_new_partners) + 3,
f'Should have sent {len(mailed_new_partners) + 3} emails, one / recipient ({len(mailed_new_partners)} mailed partners + partner_1 + partner_2 + partner_employee)')
len(self._mails), len(new_partners) + 3,
f'Should have sent {len(new_partners) + 3} emails, one / recipient ({len(new_partners)} mailed partners + partner_1 + partner_2 + partner_employee)')
self.assertMailMail(
self.partner_employee_2, 'sent',
author=self.partner_employee,
@@ -1812,12 +1805,14 @@ class TestComposerResultsComment(TestMailComposer, CronMixinCase):
mail_message=self.test_record.message_ids[0],
)
self.assertMailMail(
self.partner_1 + self.partner_2 + mailed_new_partners, 'sent',
self.partner_1 + self.partner_2 + 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')],
] + [[new_partners[0]['email_formatted']],
['"FindMe Multi" <find.me.multi.1@test.example.com>', '"FindMe Multi" <find.me.multi.2@test.example.com>']
] + [[email] for email in new_partners[2:].mapped('email_formatted')],
email_values={
'body_content': f'TemplateBody {self.test_record.name}',
# single email event if email field is multi-email
@@ -2420,15 +2415,14 @@ class TestComposerResultsMass(TestMailComposer):
# FIXME: currently email finding based on formatted / multi emails does
# not work
new_partners = self.env['res.partner'].search([]).search([('id', 'not in', existing_partners.ids)])
self.assertEqual(len(new_partners), 9,
'Mail (FIXME): did not find existing partners for formatted / multi emails: 2 extra partners')
self.assertEqual(len(new_partners), 8,
'Mail (FIXME): did not find existing partners for formatted / multi emails: 1 extra partners')
self.assertIn(partner_format_tofind, new_partners)
self.assertIn(partner_multi_tofind, new_partners)
self.assertEqual(
sorted(new_partners.mapped('email')),
sorted(['"FindMe Format" <find.me.format@test.example.com>',
'find.me.multi.1@test.example.com, "FindMe Multi" <find.me.multi.2@test.example.com>',
'find.me.multi.1@test.example.com',
'find.me.multi.2@test.example.com',
'test.cc.1@example.com', 'test.cc.2@example.com', 'test.cc.2.2@example.com',
'test.to.1@example.com', 'test.to.2@example.com']),
@@ -2438,7 +2432,6 @@ class TestComposerResultsMass(TestMailComposer):
sorted(new_partners.mapped('email_formatted')),
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>',
'"test.cc.2@example.com" <test.cc.2@example.com>',
@@ -2450,7 +2443,6 @@ class TestComposerResultsMass(TestMailComposer):
sorted(new_partners.mapped('name')),
sorted(['FindMe Format',
'FindMe Multi',
'find.me.multi.1@test.example.com',
'find.me.multi.2@test.example.com',
'test.cc.1@example.com', 'test.to.1@example.com', 'test.to.2@example.com',
'test.cc.2@example.com', 'test.cc.2.2@example.com']),
@@ -2465,23 +2457,21 @@ class TestComposerResultsMass(TestMailComposer):
# FIXME: more partners created than real emails (see above) -> due to
# transformation from email -> partner in template 'generate_recipients'
# there are more partners than email to notify;
# NOTE: 'Findme Multi' is excluded as it has the same email as 'find.me.multi.1@test.example.com'
# (created by template) and comes second in a search based on email
mailed_new_partners = new_partners.filtered(lambda p: p.name != 'FindMe Multi')
self.assertEqual(len(mailed_new_partners), 8)
self.assertEqual(len(self._new_mails), 2, 'Should have created 2 mail.mail')
self.assertEqual(
len(self._mails), (len(mailed_new_partners) + 2) * 2,
f'Should have sent {(len(mailed_new_partners) + 2) * 2} emails, one / recipient ({len(mailed_new_partners)} mailed partners + partner_1 + partner_2) * 2 records')
len(self._mails), (len(new_partners) + 2) * 2,
f'Should have sent {(len(new_partners) + 2) * 2} emails, one / recipient ({len(new_partners)} mailed partners + partner_1 + partner_2) * 2 records')
for record in self.test_records:
self.assertMailMail(
self.partner_1 + self.partner_2 + mailed_new_partners,
self.partner_1 + self.partner_2 + 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')],
] + [[new_partners[0]['email_formatted']],
['"FindMe Multi" <find.me.multi.1@test.example.com>', '"FindMe Multi" <find.me.multi.2@test.example.com>']
] + [[email] for email in new_partners[2:].mapped('email_formatted')],
email_values={
'body_content': f'TemplateBody {record.name}',
# single email event if email field is multi-email
addons/test_mail/tests/test_mail_gateway.py
+5 -5
View File
@@ -333,8 +333,8 @@ class TestMailgateway(MailCommon):
record = self.format_and_process(
MAIL_TEMPLATE, f'"Valid Lelitre" <{test_email}>', 'groups@test.com', subject='Test3')
self.assertFalse(record.message_ids[0].author_id,
'message_process (FIXME): unrecognized email -> author_id due to multi email')
self.assertEqual(record.message_ids[0].author_id, self.partner_1,
'message_process: found author based on first found email normalized, even with multi emails')
self.assertEqual(record.message_ids[0].email_from, f'"Valid Lelitre" <{test_email}>')
self.assertNotSentEmail() # No notification / bounce should be sent
@@ -343,8 +343,8 @@ class TestMailgateway(MailCommon):
record = self.format_and_process(
MAIL_TEMPLATE, test_email, 'groups@test.com', subject='Test4')
self.assertFalse(record.message_ids[0].author_id,
'message_process (FIXME): unrecognized email -> author_id due to multi email')
self.assertEqual(record.message_ids[0].author_id, self.partner_1,
'message_process: found author based on first found email normalized, even with multi emails')
self.assertEqual(record.message_ids[0].email_from, test_email)
self.assertNotSentEmail() # No notification / bounce should be sent
@@ -692,7 +692,7 @@ class TestMailgateway(MailCommon):
for partner_email, passed in [
(formataddr((self.partner_1.name, self.partner_1.email_normalized)), True),
(f'{self.partner_1.email_normalized}, "Multi Email" <multi.email@test.example.com>', False),
(f'{self.partner_1.email_normalized}, "Multi Email" <multi.email@test.example.com>', True),
(f'"Multi Email" <multi.email@test.example.com>, {self.partner_1.email_normalized}', False),
]:
with self.subTest(partner_email=partner_email):
addons/test_mail/tests/test_mail_thread_mixins.py
+2 -2
View File
@@ -30,9 +30,9 @@ class TestMailThread(MailCommon, TestRecipients):
(' ', False)]
multi_pairs = [
(f'{base_email}, other.email@test.example.com',
False), # multi not supported currently
base_email), # multi supports first found
(f'{tools.formataddr(("Another Name", base_email))}, other.email@test.example.com',
False), # multi not supported currently
base_email), # multi supports first found
]
for email_from, exp_email_normalized in valid_pairs + void_pairs + multi_pairs:
with self.subTest(email_from=email_from, exp_email_normalized=exp_email_normalized):
addons/test_mass_mailing/tests/test_mailing.py
+6 -2
View File
@@ -213,6 +213,8 @@ class TestMassMailing(TestMassMailCommon):
# as it uses email_normalized if possible
if dst_model == 'mailing.test.customer':
formatted_mailmail_email = '"Formatted Record" <record.format@example.com>'
multi_mail_mail_email = 'record.multi.1@example.com, "Record Multi 2" <record.multi.2@example.com>'
multi_outgoing_emails = ['record.multi.1@example.com', '"Record Multi 2" <record.multi.2@example.com>']
unicode_email = '"Unicode Record" <record.😊@example.com>'
unicode_mailmail_email = '"Unicode Record" <record.😊@example.com>'
record_case_email = 'TEST.RECORD.CASE@EXAMPLE.COM'
@@ -223,6 +225,8 @@ class TestMassMailing(TestMassMailCommon):
else:
formatted_mailmail_email = 'record.format@example.com'
unicode_email = 'record.😊@example.com'
multi_mail_mail_email = 'record.multi.1@example.com'
multi_outgoing_emails = ['record.multi.1@example.com']
unicode_mailmail_email = 'record.😊@example.com'
record_case_email = 'test.record.case@example.com'
record_weird_email = 'test.record.weird@example.comweirdformat'
@@ -264,8 +268,8 @@ class TestMassMailing(TestMassMailCommon):
'partner': customer_weird_2,
'trace_status': 'sent'},
{'email': 'record.multi.1@example.com',
'email_to_mail': 'record.multi.1@example.com, "Record Multi 2" <record.multi.2@example.com>',
'email_to_recipients': [['record.multi.1@example.com', '"Record Multi 2" <record.multi.2@example.com>']],
'email_to_mail': multi_mail_mail_email,
'email_to_recipients': [multi_outgoing_emails],
'failure_type': False,
'trace_status': 'sent'},
{'email': 'record.format@example.com',
odoo/addons/base/tests/test_mail.py
+1 -1
View File
@@ -525,7 +525,7 @@ class TestEmailTools(BaseCase):
]
for source, expected in zip(sources, expected_list):
with self.subTest(source=source):
self.assertEqual(email_normalize(source), expected)
self.assertEqual(email_normalize(source, strict=True), expected)
def test_email_re(self):
""" Test 'email_re', finding emails in a given text """
odoo/tools/mail.py
+5 -2
View File
@@ -559,8 +559,11 @@ def email_normalize(text, strict=True):
- Possible Input Email : 'Name <NaMe@DoMaIn.CoM>'
- Normalized Output Email : 'name@domain.com'
:param bool strict: text should contain exactly one email (default behavior
and unique behavior before Odoo16);
:param boolean strict: if True, text should contain a single email
(default behavior in stable 14+). If more than one email is found no
normalized email is returned. If False the first found candidate is used
e.g. if email is 'tony@e.com, "Tony2" <tony2@e.com>', result is either
False (strict=True), either 'tony@e.com' (strict=False).
:return: False if no email found (or if more than 1 email found when being
in strict mode); normalized email otherwise;