From 0f33c2808bb28380028d61b3cd552ca229d152b4 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= <tde@odoo.com>
Date: Tue, 6 Jun 2023 16:33:45 +0200
Subject: [PATCH] [IMP] various: support multi-emails in mailings

PURPOSE

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

SPECIFICATIONS: MAIL COMPOSER IN MAILING

When using the composer with a mailing, it currently skips recipients whose
email is a multi-email due to the strict usage of 'email_normalize'.

We can improve multi-email support by effectively checking for the first
email found, using the "less strict" mode of normalize. It means more emails
are detected as valid, and therefore sent.

Due to lower support of multi-emails when sending emails, this even allows
to send multiple emails as all emails are mailed.

SPECIFICATIONS: DEFAULT RECIPIENTS

Mailings are generally done using default recipients, aka using a model method
that returns the people to mail: customers ('partner_id'), customer emails
('email_from'), specific implementation, ...

This is implementation using '_message_get_default_recipients' that returns
'partner_ids', 'email_to' and 'email_cc' that are then used in the mail
composer to generate final recipients.

In this commit we better handle the content of email fields to avoid issues
with multi-emails. For that purpose we correctly split the content of those
fields. We now have several 'email_to' for records having multi-emails instead
of a single badly-formatted 'email_to'.

Task-2612945 (Mail: Defensive email formatting)

X-original-commit: odoo/odoo@516ccbc9e716b887cbf712c72af37fd2a3c7dbe9
Part-of: odoo/odoo#134188
---
 addons/crm/models/crm_lead.py                 | 12 ++--
 addons/event/models/event_registration.py     | 14 ++--
 addons/mail/models/models.py                  | 22 ++++---
 addons/mail/wizard/mail_compose_message.py    |  4 +-
 addons/mass_mailing/models/mailing_contact.py | 13 ++--
 .../test_mass_mailing/tests/test_mailing.py   | 64 +++++++------------
 .../website_event_track/models/event_track.py |  2 +-
 7 files changed, 61 insertions(+), 70 deletions(-)

diff --git a/addons/crm/models/crm_lead.py b/addons/crm/models/crm_lead.py
index 834284f5cb08..f0664e39334a 100644
--- a/addons/crm/models/crm_lead.py
+++ b/addons/crm/models/crm_lead.py
@@ -1912,11 +1912,13 @@ class Lead(models.Model):
         return res
 
     def _message_get_default_recipients(self):
-        return {r.id: {
-            'partner_ids': [],
-            'email_to': r.email_normalized,
-            'email_cc': False}
-            for r in self}
+        return {
+            r.id: {
+                'partner_ids': [],
+                'email_to': ','.join(tools.email_normalize_all(r.email_from)) or r.email_from,
+                'email_cc': False,
+            } for r in self
+        }
 
     def _message_get_suggested_recipients(self):
         recipients = super(Lead, self)._message_get_suggested_recipients()
diff --git a/addons/event/models/event_registration.py b/addons/event/models/event_registration.py
index df85ea9bc4c9..b5015e8f5ad2 100644
--- a/addons/event/models/event_registration.py
+++ b/addons/event/models/event_registration.py
@@ -4,7 +4,7 @@
 from dateutil.relativedelta import relativedelta
 
 from odoo import _, api, fields, models, SUPERUSER_ID
-from odoo.tools import format_date, email_normalize
+from odoo.tools import format_date, email_normalize, email_normalize_all
 from odoo.exceptions import AccessError, ValidationError
 
 # phone_validation is not officially in the depends of event, but we would like
@@ -326,11 +326,13 @@ class EventRegistration(models.Model):
     def _message_get_default_recipients(self):
         # Prioritize registration email over partner_id, which may be shared when a single
         # partner booked multiple seats
-        return {r.id: {
-            'partner_ids': [],
-            'email_to': r.email,
-            'email_cc': False}
-            for r in self}
+        return {r.id:
+            {
+                'partner_ids': [],
+                'email_to': ','.join(email_normalize_all(r.email)) or r.email,
+                'email_cc': False,
+            } for r in self
+        }
 
     def _message_post_after_hook(self, message, msg_vals):
         if self.email and not self.partner_id:
diff --git a/addons/mail/models/models.py b/addons/mail/models/models.py
index 6e2e0efe44d8..301737d40f2b 100644
--- a/addons/mail/models/models.py
+++ b/addons/mail/models/models.py
@@ -74,14 +74,20 @@ class BaseModel(models.AbstractModel):
             recipient_ids, email_to, email_cc = [], False, False
             if 'partner_id' in record and record.partner_id:
                 recipient_ids.append(record.partner_id.id)
-            elif 'email_normalized' in record and record.email_normalized:
-                email_to = record.email_normalized
-            elif 'email_from' in record and record.email_from:
-                email_to = record.email_from
-            elif 'partner_email' in record and record.partner_email:
-                email_to = record.partner_email
-            elif 'email' in record and record.email:
-                email_to = record.email
+            else:
+                found_email = False
+                if 'email_from' in record and record.email_from:
+                    found_email = record.email_from
+                elif 'partner_email' in record and record.partner_email:
+                    found_email = record.partner_email
+                elif 'email' in record and record.email:
+                    found_email = record.email
+                elif 'email_normalized' in record and record.email_normalized:
+                    found_email = record.email_normalized
+                if found_email:
+                    email_to = ','.join(tools.email_normalize_all(found_email))
+                if not email_to:  # keep value to ease debug / trace update
+                    email_to = found_email
             res[record.id] = {'partner_ids': recipient_ids, 'email_to': email_to, 'email_cc': email_cc}
         return res
 
diff --git a/addons/mail/wizard/mail_compose_message.py b/addons/mail/wizard/mail_compose_message.py
index 6f163ea86e42..7b47d4d031f0 100644
--- a/addons/mail/wizard/mail_compose_message.py
+++ b/addons/mail/wizard/mail_compose_message.py
@@ -485,9 +485,9 @@ class MailComposer(models.TransientModel):
             recipients_info[record_id] = {
                 'mail_to': mail_to,
                 'mail_to_normalized': [
-                    tools.email_normalize(mail)
+                    tools.email_normalize(mail, strict=False)
                     for mail in mail_to
-                    if tools.email_normalize(mail)
+                    if tools.email_normalize(mail, strict=False)
                 ]
             }
         return recipients_info
diff --git a/addons/mass_mailing/models/mailing_contact.py b/addons/mass_mailing/models/mailing_contact.py
index 567bba5d1b34..a0fcb69859f7 100644
--- a/addons/mass_mailing/models/mailing_contact.py
+++ b/addons/mass_mailing/models/mailing_contact.py
@@ -1,7 +1,7 @@
 # -*- coding: utf-8 -*-
 # Part of Odoo. See LICENSE file for full copyright and licensing details.
 
-from odoo import _, api, fields, models
+from odoo import _, api, fields, models, tools
 from odoo.exceptions import UserError
 from odoo.osv import expression
 
@@ -178,11 +178,12 @@ class MassMailingContact(models.Model):
         return contact.name_get()[0]
 
     def _message_get_default_recipients(self):
-        return {r.id: {
-            'partner_ids': [],
-            'email_to': r.email_normalized,
-            'email_cc': False}
-            for r in self
+        return {
+            r.id: {
+                'partner_ids': [],
+                'email_to': ','.join(tools.email_normalize_all(r.email)) or r.email,
+                'email_cc': False,
+            } for r in self
         }
 
     def action_add_to_mailing_list(self):
diff --git a/addons/test_mass_mailing/tests/test_mailing.py b/addons/test_mass_mailing/tests/test_mailing.py
index 899b4eb502da..66e55fe496be 100644
--- a/addons/test_mass_mailing/tests/test_mailing.py
+++ b/addons/test_mass_mailing/tests/test_mailing.py
@@ -203,37 +203,17 @@ class TestMassMailing(TestMassMailCommon):
                 with self.mock_mail_gateway(mail_unlink_sent=False):
                     mailing.action_send_mail()
 
-                # difference in email_to management when inheriting from blacklist mixin
-                # 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'
-                    record_weird_email = 'test.record.weird@example.com'
-                    record_weird_mailmail_email = 'test.record.weird@example.com Weird Format'
-                    record_weird_email_email = 'test.record.weird@example.comWeirdFormat'
-                    record_weird_2_mailmail_email = 'Weird Format2 test.record.weird.2@example.com'
-                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'
-                    record_weird_mailmail_email = 'test.record.weird@example.comweirdformat'
-                    record_weird_email_email = 'test.record.weird@example.comweirdformat'
-                    record_weird_2_mailmail_email = 'weird format2 test.record.weird.2@example.com'
+                # Difference in email, email_to_recipients and email_to_mail
+                # -> email: trace email: normalized, to ease its management, mainly technical
+                # -> email_to_mail: mail.mail email: email_to stored in outgoing mail.mail (can be multi)
+                # -> email_to_recipients: email_to for outgoing emails, list means several recipients
                 self.assertMailTraces(
                     [
                         {'email': 'customer.multi.1@example.com, "Test Multi 2" <customer.multi.2@example.com>',
-                         'email_to_recipients': ['customer.multi.1@example.com, "Test Multi 2" <customer.multi.2@example.com>'],
-                         'failure_type': 'mail_email_invalid',
+                         'email_to_recipients': [[f'"{customer_mult.name}" <customer.multi.1@example.com>', f'"{customer_mult.name}" <customer.multi.2@example.com>']],
+                         'failure_type': False,
                          'partner': customer_mult,
-                         'trace_status': 'cancel'},
+                         'trace_status': 'sent'},
                         {'email': '"Formatted Customer" <test.customer.format@example.com>',
                          # mail to avoids double encapsulation
                          'email_to_recipients': [[f'"{customer_fmt.name}" <test.customer.format@example.com>']],
@@ -262,33 +242,33 @@ class TestMassMailing(TestMassMailCommon):
                          'partner': customer_weird_2,
                          'trace_status': 'sent'},
                         {'email': 'record.multi.1@example.com',
-                         'email_to_mail': multi_mail_mail_email,
-                         'email_to_recipients': [multi_outgoing_emails],
+                         'email_to_mail': 'record.multi.1@example.com,record.multi.2@example.com',
+                         'email_to_recipients': [['record.multi.1@example.com', 'record.multi.2@example.com']],
                          'failure_type': False,
                          'trace_status': 'sent'},
                         {'email': 'record.format@example.com',
-                         'email_to_mail': formatted_mailmail_email,
-                         'email_to_recipients': [[formatted_mailmail_email]],
+                         'email_to_mail': 'record.format@example.com',
+                         'email_to_recipients': [['record.format@example.com']],
                          'failure_type': False,
                          'trace_status': 'sent'},
-                        {'email': unicode_email,
-                         'email_to_mail': unicode_mailmail_email,
-                         'email_to_recipients': [[unicode_mailmail_email]],
+                        {'email': 'record.😊@example.com',
+                         'email_to_mail': 'record.😊@example.com',
+                         'email_to_recipients': [['record.😊@example.com']],
                          'failure_type': 'mail_email_invalid',
                          'trace_status': 'cancel'},  # email_re usage forbids mailing to unicode
-                        {'email': record_case_email,
-                         'email_to_mail': record_case_email,
-                         'email_to_recipients': [[record_case_email]],
+                        {'email': 'test.record.case@example.com',
+                         'email_to_mail': 'test.record.case@example.com',
+                         'email_to_recipients': [['test.record.case@example.com']],
                          'failure_type': False,
                          'trace_status': 'sent'},
-                        {'email': record_weird_email,
-                         'email_to_mail': record_weird_mailmail_email,
-                         'email_to_recipients': [[record_weird_email_email]],
+                        {'email': 'test.record.weird@example.comweirdformat',
+                         'email_to_mail': 'test.record.weird@example.comweirdformat',
+                         'email_to_recipients': [['test.record.weird@example.comweirdformat']],
                          'failure_type': False,
                          'trace_status': 'sent'},
                         {'email': 'test.record.weird.2@example.com',
-                         'email_to_mail': record_weird_2_mailmail_email,
-                         'email_to_recipients': [[record_weird_2_mailmail_email]],
+                         'email_to_mail': 'weird format2 test.record.weird.2@example.com',
+                         'email_to_recipients': [['weird format2 test.record.weird.2@example.com']],
                          'failure_type': False,
                          'trace_status': 'sent'},
                     ],
diff --git a/addons/website_event_track/models/event_track.py b/addons/website_event_track/models/event_track.py
index 52dbad092e3c..f796982ec781 100644
--- a/addons/website_event_track/models/event_track.py
+++ b/addons/website_event_track/models/event_track.py
@@ -450,7 +450,7 @@ class Track(models.Model):
         return {
             track.id: {
                 'partner_ids': [],
-                'email_to': track.contact_email or track.partner_email,
+                'email_to': ','.join(tools.email_normalize_all(track.contact_email or track.partner_email)) or track.contact_email or track.partner_email,
                 'email_cc': False
             } for track in self
         }
-- 
GitLab