From 1e40fb62471e5140bcb36d0f89f2378e30adde1a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Thu, 6 Jul 2023 09:11:52 +0200 Subject: [PATCH] [IMP] mail: compute 'reply_to' using alias domain and record company PURPOSE Allow alias domains to be multiple, notably to be used in a multi company environment where each company has its own alias domain. SPECIFICATIONS When computing reply-to of message or documents, classify them per company and use alias domains when computing catchall emails. Reply-to computation based on alias is also simplified as aliases are now complete and use alias domains. They do not depend on configuration parameters anymore. LINKS Task-36879 (Mail: Support Multi Domains Aliases) Part-of: odoo/odoo#76734 --- addons/mail/models/mail_thread.py | 3 + addons/mail/models/models.py | 93 +++++++++++--------- addons/test_mail/tests/test_mail_composer.py | 7 +- addons/test_mail/tests/test_mail_message.py | 23 ++--- addons/test_mail/tests/test_message_post.py | 26 ++++-- 5 files changed, 88 insertions(+), 64 deletions(-) diff --git a/addons/mail/models/mail_thread.py b/addons/mail/models/mail_thread.py index ab5d3716bab..a73e458bf57 100644 --- a/addons/mail/models/mail_thread.py +++ b/addons/mail/models/mail_thread.py @@ -2105,6 +2105,9 @@ class MailThread(models.AbstractModel): # recipients 'partner_ids': partner_ids, }) + # add default-like values afterwards, to avoid useless queries + if 'reply_to' not in msg_values: + msg_values['reply_to'] = self._notify_get_reply_to(default=email_from)[self.id] msg_values.update( self._process_attachments_for_post(attachments, attachment_ids, msg_values) diff --git a/addons/mail/models/models.py b/addons/mail/models/models.py index 3b7d39645ad..34d03784d2c 100644 --- a/addons/mail/models/models.py +++ b/addons/mail/models/models.py @@ -1,6 +1,7 @@ # -*- coding: utf-8 -*- # Part of Odoo. See LICENSE file for full copyright and licensing details. +from collections import defaultdict from lxml.builder import E from markupsafe import Markup @@ -238,45 +239,52 @@ class BaseModel(models.AbstractModel): model = _records._name if _records and _records._name != 'mail.thread' else False res_ids = _records.ids if _records and model else [] _res_ids = res_ids or [False] # always have a default value located in False + _records_sudo = _records.sudo() + doc_names = {rec.id: rec.display_name for rec in _records_sudo} if res_ids else {} - alias_domain = self.env['ir.config_parameter'].sudo().get_param("mail.catchall.domain") - result = dict.fromkeys(_res_ids, False) - result_email = dict() - doc_names = dict() + # group ids per company + if res_ids: + company_to_res_ids = defaultdict(list) + record_ids_to_company = _records_sudo._mail_get_companies(default=self.env.company) + for record_id, company in record_ids_to_company.items(): + company_to_res_ids[company].append(record_id) + else: + company_to_res_ids = {self.env.company: _res_ids} + record_ids_to_company = {_res_id: self.env.company for _res_id in _res_ids} - if alias_domain: - if model and res_ids: - if not doc_names: - doc_names = dict((rec.id, rec.display_name) for rec in _records) + # begin with aliases (independent from company, alias_domain_id on alias wins) + reply_to_email = {} + if model and res_ids: + mail_aliases = self.env['mail.alias'].sudo().search([ + ('alias_domain_id', '!=', False), + ('alias_parent_model_id.model', '=', model), + ('alias_parent_thread_id', 'in', res_ids), + ('alias_name', '!=', False) + ]) + # take only first found alias for each thread_id, to match order (1 found -> limit=1 for each res_id) + for alias in mail_aliases: + reply_to_email.setdefault(alias.alias_parent_thread_id, alias.alias_full_name) - mail_aliases = self.env['mail.alias'].sudo().search([ - ('alias_parent_model_id.model', '=', model), - ('alias_parent_thread_id', 'in', res_ids), - ('alias_name', '!=', False)]) - # take only first found alias for each thread_id, to match order (1 found -> limit=1 for each res_id) - for alias in mail_aliases: - result_email.setdefault(alias.alias_parent_thread_id, '%s@%s' % (alias.alias_name, alias_domain)) - - # left ids: use catchall - left_ids = set(_res_ids) - set(result_email) - if left_ids: - catchall = self.env['ir.config_parameter'].sudo().get_param("mail.catchall.alias") - if catchall: - result_email.update(dict((rid, '%s@%s' % (catchall, alias_domain)) for rid in left_ids)) - - for res_id in result_email: - result[res_id] = self._notify_get_reply_to_formatted_email( - result_email[res_id], - doc_names.get(res_id) or '', - ) - - left_ids = set(_res_ids) - set(result_email) + # continue with company alias + left_ids = set(_res_ids) - set(reply_to_email) if left_ids: - result.update(dict((res_id, default) for res_id in left_ids)) + for company, record_ids in company_to_res_ids.items(): + # left ids: use catchall defined on company alias domain + if company.catchall_email: + left_ids = set(record_ids) - set(reply_to_email) + if left_ids: + reply_to_email.update({rec_id: company.catchall_email for rec_id in left_ids}) - return result + # compute name of reply-to ("Company Document" ) + reply_to_formatted = dict.fromkeys(_res_ids, default) + for res_id, record_reply_to in reply_to_email.items(): + reply_to_formatted[res_id] = self._notify_get_reply_to_formatted_email( + record_reply_to, doc_names.get(res_id) or '', company=record_ids_to_company[res_id], + ) - def _notify_get_reply_to_formatted_email(self, record_email, record_name): + return reply_to_formatted + + def _notify_get_reply_to_formatted_email(self, record_email, record_name, company=False): """ Compute formatted email for reply_to and try to avoid refold issue with python that splits the reply-to over multiple lines. It is due to a bad management of quotes (missing quotes after refold). This appears @@ -288,22 +296,27 @@ class BaseModel(models.AbstractModel): possible we return only the email and skip the formataddr which causes the issue in python. We do not use hacks like crop the name part as encoding and quoting would be error prone. + + :param company: if given, setup the company used to + complete name in formataddr. Otherwise fallback on 'company_id' + of self or environment company; """ # address itself is too long for 78 chars limit: return only email if len(record_email) >= 78: return record_email - if 'company_id' in self and len(self.company_id) == 1: - company_name = self.sudo().company_id.name - else: - company_name = self.env.company.name + if not company: + if len(self) == 1: + company = self.sudo()._mail_get_companies(default=self.env.company) + else: + company = self.env.company - # try company_name + record_name, or record_name alone (or company_name alone) - name = f"{company_name} {record_name}" if record_name else company_name + # try company.name + record_name, or record_name alone (or company.name alone) + name = f"{company.name} {record_name}" if record_name else company.name formatted_email = tools.formataddr((name, record_email)) if len(formatted_email) > 78: - formatted_email = tools.formataddr((record_name or company_name, record_email)) + formatted_email = tools.formataddr((record_name or company.name, record_email)) if len(formatted_email) > 78: formatted_email = record_email return formatted_email diff --git a/addons/test_mail/tests/test_mail_composer.py b/addons/test_mail/tests/test_mail_composer.py index 12afea1728d..88769e7f0c8 100644 --- a/addons/test_mail/tests/test_mail_composer.py +++ b/addons/test_mail/tests/test_mail_composer.py @@ -3061,9 +3061,12 @@ class TestComposerResultsMass(TestMailComposer): default_template_id=self.template.id) )) composer = composer_form.save() + + # remove alias so that _notify_get_reply_to will return the default value instead of alias + self.company_admin.write({ + 'alias_domain_id': False, + }) with self.mock_mail_gateway(mail_unlink_sent=False): - # remove alias so that _notify_get_reply_to will return the default value instead of alias - self.env['ir.config_parameter'].sudo().set_param("mail.catchall.domain", None) composer.action_send_mail() for record in self.test_records: diff --git a/addons/test_mail/tests/test_mail_message.py b/addons/test_mail/tests/test_mail_message.py index a25f35d9e2a..126bb9fd909 100644 --- a/addons/test_mail/tests/test_mail_message.py +++ b/addons/test_mail/tests/test_mail_message.py @@ -221,16 +221,8 @@ class TestMessageValues(MailCommon): self.assertEqual(msg.email_from, formataddr((self.user_employee.name, self.user_employee.email))) # no alias domain -> author - self.env['ir.config_parameter'].search([('key', '=', 'mail.catchall.domain')]).unlink() - - msg = self.Message.create({}) - self.assertIn('-private', msg.message_id.split('@')[0], 'mail_message: message_id for a void message should be a "private" one') - self.assertEqual(msg.reply_to, formataddr((self.user_employee.name, self.user_employee.email))) - self.assertEqual(msg.email_from, formataddr((self.user_employee.name, self.user_employee.email))) - - # no alias catchall, no alias -> author - self.env['ir.config_parameter'].set_param('mail.catchall.domain', self.alias_domain) - self.env['ir.config_parameter'].search([('key', '=', 'mail.catchall.alias')]).unlink() + self.env.company.alias_domain_id = False + self.assertFalse(self.env.company.catchall_email) msg = self.Message.create({}) self.assertIn('-private', msg.message_id.split('@')[0], 'mail_message: message_id for a void message should be a "private" one') @@ -249,8 +241,10 @@ class TestMessageValues(MailCommon): self.assertEqual(msg.reply_to, formataddr((reply_to_name, reply_to_email))) self.assertEqual(msg.email_from, formataddr((self.user_employee.name, self.user_employee.email))) - # no alias domain -> author - self.env['ir.config_parameter'].search([('key', '=', 'mail.catchall.domain')]).unlink() + # no alias domain, no company catchall -> author + self.alias_record.alias_domain_id = False + self.env.company.alias_domain_id = False + self.assertFalse(self.env.company.catchall_email) msg = self.Message.create({ 'model': 'mail.test.container', @@ -260,9 +254,8 @@ class TestMessageValues(MailCommon): self.assertEqual(msg.reply_to, formataddr((self.user_employee.name, self.user_employee.email))) self.assertEqual(msg.email_from, formataddr((self.user_employee.name, self.user_employee.email))) - # no catchall -> don't care, alias - self.env['ir.config_parameter'].set_param('mail.catchall.domain', self.alias_domain) - self.env['ir.config_parameter'].search([('key', '=', 'mail.catchall.alias')]).unlink() + # alias wins over company, hence no catchall is not an issue + self.alias_record.alias_domain_id = self.mail_alias_domain msg = self.Message.create({ 'model': 'mail.test.container', diff --git a/addons/test_mail/tests/test_message_post.py b/addons/test_mail/tests/test_message_post.py index c5042bf8a10..13feeb12c2b 100644 --- a/addons/test_mail/tests/test_message_post.py +++ b/addons/test_mail/tests/test_message_post.py @@ -290,7 +290,7 @@ class TestMailNotifyAPI(TestMessagePostCommon): test_record._notify_get_reply_to()[test_record.id], formataddr(( f"{self.user_employee_c2.company_id.name} {test_record.name}", - f"{self.alias_catchall}@{self.alias_domain}")) + f"{self.alias_catchall_c2}@{self.alias_domain_c2_name}")) ) test_record_c1 = test_record.with_user(self.user_employee) self.assertEqual( @@ -310,11 +310,17 @@ class TestMailNotifyAPI(TestMessagePostCommon): ]) res = test_records._notify_get_reply_to() for test_record in test_records: + company = test_record.company_id + if company == self.company_2: + alias_domain = self.alias_domain_c2_name + alias_catchall = self.alias_catchall_c2 + else: + alias_domain = self.alias_domain + alias_catchall = self.alias_catchall + self.assertEqual( res[test_record.id], - formataddr(( - f"{self.user_employee_c2.company_id.name} {test_record.name}", - f"{self.alias_catchall}@{self.alias_domain}")) + formataddr((f"{company.name} {test_record.name}", f"{alias_catchall}@{alias_domain}")) ) # Test3: get company from record (company_id field) @@ -331,7 +337,7 @@ class TestMailNotifyAPI(TestMessagePostCommon): res[test_record.id], formataddr(( f"{self.company_3.name} {test_record.name}", - f"{self.alias_catchall}@{self.alias_domain}")) + f"{self.alias_catchall_c3}@{self.alias_domain_c3_name}")) ) @@ -956,7 +962,10 @@ class TestMessagePost(TestMessagePostCommon, CronMixinCase): }, ]) expected_companies = [self.company_2, self.company_admin, self.company_2] - for record, expected_company in zip(records, expected_companies): + expected_alias_domains = [self.mail_alias_domain_c2, self.mail_alias_domain, self.mail_alias_domain_c2] + for record, expected_company, expected_alias_domain in zip( + records, expected_companies, expected_alias_domains + ): with self.subTest(record=record): with self.assertSinglePostNotifications( [{'partner': self.partner_employee_2, 'type': 'email'}], @@ -978,7 +987,10 @@ class TestMessagePost(TestMessagePostCommon, CronMixinCase): 'is_internal': False, 'notified_partner_ids': self.partner_employee_2, 'reply_to': formataddr( - (f'{expected_company.name} {record.name}', f'{self.alias_catchall}@{self.alias_domain}') + ( + f'{expected_company.name} {record.name}', + f'{expected_alias_domain.catchall_alias}@{expected_alias_domain.name}' + ) ), }, }