From 93f378ea4f4be0c998fe92357f972e299a9b55ab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Tue, 24 Oct 2023 10:50:56 +0200 Subject: [PATCH 01/18] [FIX] mail: add default recipients from composer only in email mode When composer runs in 'rendering' mode, it is often based on a mail.template record that gives information about recipients. When having no template default recipients are added to be sure to contact 'intended people'. When rewriting code in 16.2, support of batch-comment was added in addition to mass mailing. Result is that now default recipients are also computed when posting in batch. However when posting we consider recipients are already set on records using followers. Adding default recipients to avoid dummy emails is present mainly for the mass mailing mode. Followup of odoo/odoo#107356 Prepares Task-36879 (Mail: Support Multi Domains Aliases) Part-of: odoo/odoo#76734 --- addons/mail/wizard/mail_compose_message.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/addons/mail/wizard/mail_compose_message.py b/addons/mail/wizard/mail_compose_message.py index c7a342b9bf3..89fc4d9441a 100644 --- a/addons/mail/wizard/mail_compose_message.py +++ b/addons/mail/wizard/mail_compose_message.py @@ -952,7 +952,7 @@ class MailComposer(models.TransientModel): # Handle recipients. Without template, if no partner_ids is given, update # recipients using default recipients to be sure to notify someone - if not self.template_id and not self.partner_ids: + if not self.template_id and not self.partner_ids and email_mode: default_recipients = RecordsModel.browse(res_ids)._message_get_default_recipients() for res_id in res_ids: mail_values_all[res_id].update( From 503508fb8e0480cbfdf5a6213ff19ffe281d06ee Mon Sep 17 00:00:00 2001 From: Stephane Debauche Date: Tue, 24 Oct 2023 14:13:08 +0200 Subject: [PATCH 02/18] [FIX] base: fix mock of IrMailServer methods Due to current implementation of mocks context is lost when accessing methods 'connect' and '_find_mail_server' of IrMailServer. Indeed self is replaced by an instance of IrMailServer and code relying on context could not be called as planned. This commit aims at doing a custom mock so that we can correctly rely on self and context in those methods. This is necessary for incoming changes in mail notably to allow testing SMTP feature in multi domains environment. Logged information when having failed SMTP check is improved: we now also log found From in order to ease debugging failing tests due to invalid from. Prepares Task-36879 (Mail: Support Multi Domains Aliases) Part-of: odoo/odoo#76734 --- odoo/addons/base/tests/common.py | 39 +++++++++++++++++++++++--------- 1 file changed, 28 insertions(+), 11 deletions(-) diff --git a/odoo/addons/base/tests/common.py b/odoo/addons/base/tests/common.py index 014a6365bcb..83e972544ee 100644 --- a/odoo/addons/base/tests/common.py +++ b/odoo/addons/base/tests/common.py @@ -4,7 +4,7 @@ import threading from contextlib import contextmanager -from unittest.mock import patch +from unittest.mock import patch, Mock from odoo.tests.common import TransactionCase, HttpCase from odoo import Command @@ -326,16 +326,27 @@ class MockSmtplibCase: self.testing_smtp_session = TestingSMTPSession() IrMailServer = self.env['ir.mail_server'] - connect_origin = IrMailServer.connect - find_mail_server_origin = IrMailServer._find_mail_server + connect_origin = type(IrMailServer).connect + find_mail_server_origin = type(IrMailServer)._find_mail_server + + # custom mock to avoid losing context + def mock_function(func): + mock = Mock() + + def _call(*args, **kwargs): + mock(*args[1:], **kwargs) + return func(*args, **kwargs) + + _call.mock = mock + return _call with patch('smtplib.SMTP_SSL', side_effect=lambda *args, **kwargs: self.testing_smtp_session), \ patch('smtplib.SMTP', side_effect=lambda *args, **kwargs: self.testing_smtp_session), \ patch.object(type(IrMailServer), '_is_test_mode', lambda self: False), \ - patch.object(type(IrMailServer), 'connect', wraps=IrMailServer, side_effect=connect_origin) as connect_mocked, \ - patch.object(type(IrMailServer), '_find_mail_server', side_effect=find_mail_server_origin) as find_mail_server_mocked: - self.connect_mocked = connect_mocked - self.find_mail_server_mocked = find_mail_server_mocked + patch.object(type(IrMailServer), 'connect', mock_function(connect_origin)) as connect_mocked, \ + patch.object(type(IrMailServer), '_find_mail_server', mock_function(find_mail_server_origin)) as find_mail_server_mocked: + self.connect_mocked = connect_mocked.mock + self.find_mail_server_mocked = find_mail_server_mocked.mock yield def _build_email(self, mail_from, return_path=None, **kwargs): @@ -385,15 +396,21 @@ class MockSmtplibCase: debug_info = '' matching_emails_count = len(list(matching_emails)) if matching_emails_count != emails_count: + emails_from = [] + for email in self.emails: + from_found = next(( + line.split('From:')[1].strip() for line in email['message'].splitlines() + if line.startswith('From:')), '') + emails_from.append(from_found) debug_info = '\n'.join( - f"SMTP-From: {email['smtp_from']}, SMTP-To: ({email['smtp_to_list']}) - From_filter {email['from_filter']})" - for email in self.emails + f"SMTP-From: {email['smtp_from']}, SMTP-To: {email['smtp_to_list']}, Msg-From: {email_msg_from}, From_filter: {email['from_filter']})" + for email, email_msg_from in zip(self.emails, emails_from) ) self.assertEqual( matching_emails_count, emails_count, msg=f'Incorrect emails sent: {matching_emails_count} found, {emails_count} expected' - f'\nConditions: SMTP-From {smtp_from}, SMTP-To {smtp_to_list}, Msg-From {message_from}, From_filter {from_filter}' - f'\n{debug_info}' + f'\nConditions\nSMTP-From: {smtp_from}, SMTP-To: {smtp_to_list}, Msg-From: {message_from}, From_filter: {from_filter}' + f'\nNot found in\n{debug_info}' ) @classmethod From b99d16670c05da22164d9543ea0ee7e3811e4d9c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Mon, 23 Oct 2023 14:07:58 +0200 Subject: [PATCH 03/18] [FIX] base: make ir_mail_server ordering deterministic This may have an impact when trying to find an outgoing mail server based on from and filter, as first match wins when checking matching filter on a bunch of mail servers. Prepares Task-36879 (Mail: Support Multi Domains Aliases) Part-of: odoo/odoo#76734 --- addons/mail/models/mail_mail.py | 2 +- odoo/addons/base/models/ir_mail_server.py | 2 +- odoo/addons/base/tests/common.py | 4 ++++ 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/addons/mail/models/mail_mail.py b/addons/mail/models/mail_mail.py index 0c31e94428e..7673cfcae9b 100644 --- a/addons/mail/models/mail_mail.py +++ b/addons/mail/models/mail_mail.py @@ -494,7 +494,7 @@ class MailMail(models.Model): # Then find the mail server for each email_from and group the # per mail_server_id and smtp_from - mail_servers = self.env['ir.mail_server'].sudo().search([], order='sequence') + mail_servers = self.env['ir.mail_server'].sudo().search([], order='sequence, id') group_per_smtp_from = defaultdict(list) for (mail_server_id, email_from), mail_ids in group_per_email_from.items(): if not mail_server_id: diff --git a/odoo/addons/base/models/ir_mail_server.py b/odoo/addons/base/models/ir_mail_server.py index 72389b038f4..03529e9e2f9 100644 --- a/odoo/addons/base/models/ir_mail_server.py +++ b/odoo/addons/base/models/ir_mail_server.py @@ -87,7 +87,7 @@ class IrMailServer(models.Model): """Represents an SMTP server, able to send outgoing emails, with SSL and TLS capabilities.""" _name = "ir.mail_server" _description = 'Mail Server' - _order = 'sequence' + _order = 'sequence, id' NO_VALID_RECIPIENT = ("At least one valid recipient address should be " "specified for outgoing emails (To/Cc/Bcc)") diff --git a/odoo/addons/base/tests/common.py b/odoo/addons/base/tests/common.py index 83e972544ee..937479ad452 100644 --- a/odoo/addons/base/tests/common.py +++ b/odoo/addons/base/tests/common.py @@ -430,18 +430,22 @@ class MockSmtplibCase: { 'name': 'Domain based server', 'from_filter': 'test.mycompany.com', + 'sequence': 0, ** ir_mail_server_values, }, { 'name': 'User specific server', 'from_filter': 'specific_user@test.mycompany.com', + 'sequence': 1, ** ir_mail_server_values, }, { 'name': 'Server Notifications', 'from_filter': 'notifications.test@test.mycompany.com', + 'sequence': 2, ** ir_mail_server_values, }, { 'name': 'Server No From Filter', 'from_filter': False, + 'sequence': 3, ** ir_mail_server_values, }, ]) From c53aae845d91b03ac30c55e14f2e4b4b54d0a39f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Fri, 20 Oct 2023 09:21:05 +0200 Subject: [PATCH 04/18] [REF] mail, various: lint / reorder code bits, improve tests Perform some code cleanup not really related to other commits notably in mail gateway where alias domains will have some impact. Extract some processing in sub-methods, allowing to better distinguish code purpose. This implies notably some checks in mail gateway (write to bounce or catchall detection). In mail.message, reorder some fields according to their usage, just to keep definitions / section. Also improve docstrings and/or fix some of them. This is mainly a "reduce diff in other commits" commit. No change should occur with this commit. Task-36879 (Mail: Support Multi Domains Aliases) Part-of: odoo/odoo#76734 --- .../models/hr_recruitment_source.py | 10 +- addons/mail/models/mail_alias.py | 7 +- addons/mail/models/mail_message.py | 10 +- addons/mail/models/mail_thread.py | 142 +++++++++++------- addons/mail/wizard/mail_compose_message.py | 3 +- 5 files changed, 102 insertions(+), 70 deletions(-) diff --git a/addons/hr_recruitment/models/hr_recruitment_source.py b/addons/hr_recruitment/models/hr_recruitment_source.py index ca65cec23b2..2b8bd6ea3c2 100644 --- a/addons/hr_recruitment/models/hr_recruitment_source.py +++ b/addons/hr_recruitment/models/hr_recruitment_source.py @@ -22,18 +22,18 @@ class RecruitmentSource(models.Model): def create_alias(self): campaign = self.env.ref('hr_recruitment.utm_campaign_job') medium = self.env.ref('utm.utm_medium_email') - for source in self: + for source in self.filtered(lambda s: not s.alias_id): vals = { - 'alias_parent_thread_id': source.job_id.id, - 'alias_model_id': self.env['ir.model']._get('hr.applicant').id, - 'alias_parent_model_id': self.env['ir.model']._get('hr.job').id, - 'alias_name': "%s+%s" % (source.job_id.alias_name or source.job_id.name, source.name), 'alias_defaults': { 'job_id': source.job_id.id, 'campaign_id': campaign.id, 'medium_id': medium.id, 'source_id': source.source_id.id, }, + 'alias_model_id': self.env['ir.model']._get_id('hr.applicant'), + 'alias_name': f"{source.job_id.alias_name or source.job_id.name}+{source.name}", + 'alias_parent_thread_id': source.job_id.id, + 'alias_parent_model_id': self.env['ir.model']._get_id('hr.job'), } source.alias_id = self.env['mail.alias'].create(vals) diff --git a/addons/mail/models/mail_alias.py b/addons/mail/models/mail_alias.py index bfed641636e..c06f45919aa 100644 --- a/addons/mail/models/mail_alias.py +++ b/addons/mail/models/mail_alias.py @@ -117,10 +117,9 @@ class Alias(models.Model): @api.depends('alias_domain', 'alias_name') def _compute_display_name(self): - """Return the mail alias display alias_name, including the implicit - mail catchall domain if exists from config otherwise "New Alias". - e.g. `jobs@mail.odoo.com` or `jobs` or 'New Alias' - """ + """ Return the mail alias display alias_name, including the catchall + domain if found otherwise "Inactive Alias". e.g.`jobs@mail.odoo.com` + or `jobs` or 'Inactive Alias' """ for record in self: if record.alias_name and record.alias_domain: record.display_name = f"{record.alias_name}@{record.alias_domain}" diff --git a/addons/mail/models/mail_message.py b/addons/mail/models/mail_message.py index 94ff8661ce5..bd99cfaa5e6 100644 --- a/addons/mail/models/mail_message.py +++ b/addons/mail/models/mail_message.py @@ -93,6 +93,12 @@ class Message(models.Model): preview = fields.Char( 'Preview', compute='_compute_preview', help='The text-only beginning of the body used as email preview.') + link_preview_ids = fields.One2many( + 'mail.link.preview', 'message_id', string='Link Previews', + groups="base.group_erp_manager") + reaction_ids = fields.One2many( + 'mail.message.reaction', 'message_id', string="Reactions", + groups="base.group_system") # Attachments are linked to a document through model / res_id and to the message through this field. attachment_ids = fields.Many2many( 'ir.attachment', 'message_attachment_rel', @@ -105,7 +111,6 @@ class Message(models.Model): model = fields.Char('Related Document Model') res_id = fields.Many2oneReference('Related Document ID', model_field='model') record_name = fields.Char('Message Record Name') # display_name of the related document - link_preview_ids = fields.One2many('mail.link.preview', 'message_id', string='Link Previews', groups="base.group_erp_manager") # characteristics message_type = fields.Selection([ ('email', 'Email'), @@ -167,7 +172,7 @@ class Message(models.Model): message_id = fields.Char('Message-Id', help='Message unique identifier', index='btree', readonly=True, copy=False) reply_to = fields.Char('Reply-To', help='Reply email address. Setting the reply_to bypasses the automatic thread creation.') mail_server_id = fields.Many2one('ir.mail_server', 'Outgoing mail server') - # keep notification layout informations to be able to generate mail again + # send notification information (for resend / reschedule) email_layout_xmlid = fields.Char('Layout', copy=False) # xml id of layout email_add_signature = fields.Boolean(default=True) # `test_adv_activity`, `test_adv_activity_full`, `test_message_assignation_inbox`,... @@ -178,7 +183,6 @@ class Message(models.Model): # as the cache value for this inverse one2many is up-to-date. # Besides for new messages, and messages never sending emails, there was no mail, and it was searching for nothing. mail_ids = fields.One2many('mail.mail', 'mail_message_id', string='Mails', groups="base.group_system") - reaction_ids = fields.One2many('mail.message.reaction', 'message_id', string="Reactions", groups="base.group_system") @api.depends('body', 'subject') def _compute_description(self): diff --git a/addons/mail/models/mail_thread.py b/addons/mail/models/mail_thread.py index 5f7a1bd2242..ab5d3716bab 100644 --- a/addons/mail/models/mail_thread.py +++ b/addons/mail/models/mail_thread.py @@ -874,6 +874,40 @@ class MailThread(models.AbstractModel): for model in [bl_model for bl_model in bl_models if bl_model.model in self.env]: # transient test mode self.env[model.model].sudo().search([('message_bounce', '>', 0), ('email_normalized', '=', valid_email)])._message_reset_bounce(valid_email) + @api.model + def _detect_is_bounce(self, message, message_dict): + """Return True if the given email is a bounce email. + + Bounce alias: if any To contains bounce_alias@domain + Bounce message (not alias) + See http://datatracker.ietf.org/doc/rfc3462/?include_text=1 + As all MTA does not respect this RFC (googlemail is one of them), + we also need to verify if the message come from "mailer-daemon" + """ + # detection based on email_to + bounce_alias = self.env['ir.config_parameter'].sudo().get_param("mail.bounce.alias") + email_to = message_dict['to'] + email_to_localparts = [ + e.split('@', 1)[0].lower() + for e in (tools.email_split(email_to) or ['']) + ] + if bounce_alias and bounce_alias in email_to_localparts: + return True + + email_from = message_dict['email_from'] + email_from_localpart = (tools.email_split(email_from) or [''])[0].split('@', 1)[0].lower() + + # detection based on email_from + if email_from_localpart == 'mailer-daemon': + return True + + # detection based on content type + content_type = message.get_content_type() + if content_type == 'multipart/report' or 'report-type=delivery-status' in content_type: + return True + + return False + @api.model def _detect_loop_sender_domain(self, email_from_normalized): """Return the domain to be used to detect duplicated records created by alias. @@ -900,7 +934,8 @@ class MailThread(models.AbstractModel): email_from_normalized = tools.email_normalize(email_from) if self.env['mail.gateway.allowed'].sudo().search_count( - [('email_normalized', '=', email_from_normalized)]): + [('email_normalized', '=', email_from_normalized)] + ): return False # Detect the email address sent to many emails @@ -963,6 +998,21 @@ class MailThread(models.AbstractModel): return False + @api.model + def _detect_write_to_catchall(self, msg_dict): + """Return True if directly contacts catchall.""" + catchall_alias = self.env['ir.config_parameter'].sudo().get_param("mail.catchall.alias") + email_to = msg_dict['to'] + email_to_localparts = [ + e.split('@', 1)[0].lower() + for e in (tools.email_split(email_to) or ['']) + ] + # check it does not directly contact catchall + return ( + catchall_alias and email_to_localparts and + all(email_localpart == catchall_alias for email_localpart in email_to_localparts) + ) + @api.model def message_route(self, message, message_dict, model=None, thread_id=None, custom_values=None): """ Attempt to figure out the correct target model, thread_id, @@ -1000,13 +1050,19 @@ class MailThread(models.AbstractModel): """ if not isinstance(message, EmailMessage): raise TypeError('message must be an email.message.EmailMessage at this point') - catchall_alias = self.env['ir.config_parameter'].sudo().get_param("mail.catchall.alias") catchall_domain_lowered = self.env["ir.config_parameter"].sudo().get_param("mail.catchall.domain", "").strip().lower() catchall_domains_allowed = self.env["ir.config_parameter"].sudo().get_param("mail.catchall.domain.allowed") if catchall_domain_lowered and catchall_domains_allowed: catchall_domains_allowed = catchall_domains_allowed.split(',') + [catchall_domain_lowered] fallback_model = model + # handle bounce: verify whether this is a bounced email and use it to + # collect bounce data and update notifications for customers + if message_dict.get('is_bounce'): + self._routing_handle_bounce(message, message_dict) + return [] + self._routing_reset_bounce(message, message_dict) + # get email.message.Message variables for future processing message_id = message_dict['message_id'] @@ -1017,9 +1073,10 @@ class MailThread(models.AbstractModel): for ref in tools.mail_header_msgid_re.findall(thread_references) if 'reply_to' not in ref ] - mail_messages = self.env['mail.message'].sudo().search([('message_id', 'in', msg_references)], limit=1, order='id desc, message_id') - is_a_reply = bool(mail_messages) - reply_model, reply_thread_id = mail_messages.model, mail_messages.res_id + replying_to_msg = self.env['mail.message'].sudo().search( + [('message_id', 'in', msg_references)], limit=1, order='id desc, message_id' + ) if msg_references else self.env['mail.message'] + is_a_reply, reply_model, reply_thread_id = bool(replying_to_msg), replying_to_msg.model, replying_to_msg.res_id # author and recipients email_from = message_dict['email_from'] @@ -1037,12 +1094,6 @@ class MailThread(models.AbstractModel): rcpt_tos_localparts.append(to_local.lower()) rcpt_tos_valid_localparts = [to for to in rcpt_tos_localparts] - # Handle bounce: verify whether this is a bounced email and use it to collect bounce data and update notifications for customers - if message_dict.get('is_bounce'): - self._routing_handle_bounce(message, message_dict) - return [] - self._routing_reset_bounce(message, message_dict) - # 1. Handle reply # if destination = alias with different model -> consider it is a forward and not a reply # if destination = alias with same model -> check contact settings as they still apply @@ -1055,7 +1106,7 @@ class MailThread(models.AbstractModel): ('alias_model_id', '!=', reply_model_id), ]) if other_model_aliases: - is_a_reply = False + is_a_reply, reply_model, reply_thread_id = False, False, False rcpt_tos_valid_localparts = [to for to in rcpt_tos_valid_localparts if to in other_model_aliases.mapped('alias_name')] if is_a_reply and reply_model: @@ -1075,7 +1126,7 @@ class MailThread(models.AbstractModel): 'Routing mail from %s to %s with Message-Id %s: direct reply to msg: model: %s, thread_id: %s, custom_values: %s, uid: %s', email_from, email_to, message_id, reply_model, reply_thread_id, custom_values, self._uid) return [route] - elif route is False: + if route is False: return [] # 2. Handle new incoming email by checking aliases and applying their settings @@ -1084,8 +1135,9 @@ class MailThread(models.AbstractModel): message_dict.pop('parent_id', None) # check it does not directly contact catchall - if catchall_alias and email_to_localparts and all(email_localpart == catchall_alias for email_localpart in email_to_localparts): - _logger.info('Routing mail from %s to %s with Message-Id %s: direct write to catchall, bounce', email_from, email_to, message_id) + if self._detect_write_to_catchall(message_dict): + _logger.info('Routing mail from %s to %s with Message-Id %s: direct write to catchall, bounce', + email_from, email_to, message_id) body = self.env['ir.qweb']._render('mail.mail_bounce_catchall', { 'message': message, }) @@ -1344,7 +1396,10 @@ class MailThread(models.AbstractModel): extracted from the email. Note that this processing is specific to the mail module, and should not contain security or generic html cleaning. Indeed those aspects should be covered by the html_sanitize method - located in tools. """ + located in tools. + + :param string message: an email.message instance + """ body, attachments = payload_dict['body'], payload_dict['attachments'] if not body.strip(): return {'body': body, 'attachments': attachments} @@ -1376,7 +1431,10 @@ class MailThread(models.AbstractModel): return {'body': body, 'attachments': attachments} def _message_parse_extract_payload(self, message, message_dict, save_original=False): - """Extract body as HTML and attachments from the mail message""" + """Extract body as HTML and attachments from the mail message + + :param string message: an email.message instance + """ attachments = [] body = '' if save_original: @@ -1467,6 +1525,7 @@ class MailThread(models.AbstractModel): :return dict: bounce-related values will be added, containing + * is_bounce: whether the email is recognized as a bounce email; * bounced_email: email that bounced (normalized); * bounce_partner: res.partner recordset whose email_normalized = bounced_email; @@ -1477,6 +1536,10 @@ class MailThread(models.AbstractModel): if not isinstance(email_message, EmailMessage): raise TypeError('message must be an email.message.EmailMessage at this point') + is_bounce = self._detect_is_bounce(email_message, message_dict) + if not is_bounce: + return {'is_bounce': False} + email_part = next((part for part in email_message.walk() if part.get_content_type() in {'message/rfc822', 'text/rfc822-headers'}), None) if not email_part: # In the case of a bounce message (e.g. bounce message of GMX), the "rfc822" @@ -1520,41 +1583,9 @@ class MailThread(models.AbstractModel): 'bounced_partner': bounced_partner, 'bounced_msg_ids': bounced_msg_ids, 'bounced_message': bounced_message, + 'is_bounce': True, } - def _message_parse_is_bounce(self, message, message_dict): - """Return True if the given email is a bounce email. - - Bounce alias: if any To contains bounce_alias@domain - Bounce message (not alias) - See http://datatracker.ietf.org/doc/rfc3462/?include_text=1 - As all MTA does not respect this RFC (googlemail is one of them), - we also need to verify if the message come from "mailer-daemon" - """ - # detection based on email_to - bounce_alias = self.env['ir.config_parameter'].sudo().get_param("mail.bounce.alias") - email_to = message_dict['to'] - email_to_localparts = [ - e.split('@', 1)[0].lower() - for e in (tools.email_split(email_to) or ['']) - ] - if bounce_alias and any(email == bounce_alias for email in email_to_localparts): - return True - - email_from = message_dict['email_from'] - email_from_localpart = (tools.email_split(email_from) or [''])[0].split('@', 1)[0].lower() - - # detection based on email_from - if email_from_localpart == 'mailer-daemon': - return True - - # detection based on content type - content_type = message.get_content_type() - if content_type == 'multipart/report' or 'report-type=delivery-status' in content_type: - return True - - return False - @api.model def message_parse(self, message, save_original=False): """ Parses an email.message.Message representing an RFC-2822 email @@ -1651,10 +1682,8 @@ class MailThread(models.AbstractModel): msg_dict['date'] = stored_date.strftime(tools.DEFAULT_SERVER_DATETIME_FORMAT) msg_dict.update(self._message_parse_extract_from_parent(self._get_parent_message(msg_dict))) - msg_dict['is_bounce'] = self._message_parse_is_bounce(message, msg_dict) + msg_dict.update(self._message_parse_extract_bounce(message, msg_dict)) msg_dict.update(self._message_parse_extract_payload(message, msg_dict, save_original=save_original)) - if msg_dict['is_bounce']: - msg_dict.update(self._message_parse_extract_bounce(message, msg_dict)) return msg_dict def _message_parse_extract_from_parent(self, parent_message): @@ -1789,7 +1818,7 @@ class MailThread(models.AbstractModel): domain = expression.AND([domain, extra_domain]) return self.env['res.partner'].search(domain) - def _mail_find_user_for_gateway(self, email, alias=None): + def _mail_find_user_for_gateway(self, email_value, alias=None): """ Utility method to find user from email address that can create documents in the target model. Purpose is to link document creation to users whenever possible, for example when creating document through mailgateway. @@ -1802,15 +1831,14 @@ class MailThread(models.AbstractModel): Note that standard search order is applied. - :param str email: will be sanitized and parsed to find email; + :param str email_value: will be sanitized and parsed to find email; :param mail.alias alias: optional alias. Used to fetch owner followers or fallback user (alias owner); - :param fallback_model: if not alias, related model to check access rights; :return res.user user: user matching email or void recordset if none found """ # find normalized emails and exclude aliases (to avoid subscribing alias emails to records) - normalized_email = tools.email_normalize(email) + normalized_email = tools.email_normalize(email_value) if not normalized_email: return self.env['res.users'] diff --git a/addons/mail/wizard/mail_compose_message.py b/addons/mail/wizard/mail_compose_message.py index 89fc4d9441a..95798f552c1 100644 --- a/addons/mail/wizard/mail_compose_message.py +++ b/addons/mail/wizard/mail_compose_message.py @@ -916,13 +916,14 @@ class MailComposer(models.TransientModel): 'email_from': emails_from[res_id], 'scheduled_date': False, 'subject': subjects[res_id], - # some fields are specific to mail or message + # some fields are specific to mail **( { 'body_html': bodies[res_id], 'res_id': res_id, } if email_mode else {} ), + # some fields are specific to message **( { # notify parameter to force layout lang From f153e52e309e8a84f948aabbd42dda3ce35095ea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Fri, 20 Oct 2023 09:22:08 +0200 Subject: [PATCH 05/18] [IMP] test_mail: improve multi-company / server config tests Composer tests now use the multi-company enabled model by default, allowing to test company-dependent behavior. This has no impact on current tests, as there is no company-dependent fields on composer model, and all tests are anyway run into the main company (except multi-company specific tests, suffixed by '_mc' generally). We therefore also add some multi-company oriented tests to check notably return-path or environment companies in various scenarios. Go until the SMTP generation to test mail server choice, smtp_from and filtering, notifications email. Followup of odoo/odoo#136318 and odoo/odoo@3ffa1a061162b32a395fc2d3f9754da8295e01e7 notably. Task-36879 (Mail: Support Multi Domains Aliases) Part-of: odoo/odoo#76734 --- addons/mail/tests/common.py | 2 +- addons/mail/tests/test_ir_mail_server.py | 18 +- addons/test_mail/tests/test_mail_composer.py | 301 +++++++++++++++++- addons/test_mail/tests/test_mail_gateway.py | 4 +- .../tests/test_mailing_server.py | 10 +- 5 files changed, 308 insertions(+), 27 deletions(-) diff --git a/addons/mail/tests/common.py b/addons/mail/tests/common.py index 9d6ccbabe07..59c0df79f3f 100644 --- a/addons/mail/tests/common.py +++ b/addons/mail/tests/common.py @@ -104,7 +104,7 @@ class MockEmail(common.BaseCase, MockSmtplibCase): cls.env['ir.config_parameter'].set_param('mail.catchall.domain', cls.alias_domain) cls.env['ir.config_parameter'].set_param('mail.catchall.alias', cls.alias_catchall) cls.env['ir.config_parameter'].set_param('mail.default.from', cls.default_from) - cls.env['ir.config_parameter'].sudo().set_param('mail.default.from_filter', cls.default_from_filter) + cls.env['ir.config_parameter'].set_param('mail.default.from_filter', cls.default_from_filter) # mailer daemon email preformatting cls.mailer_daemon_email = formataddr(('MAILER-DAEMON', f'{cls.alias_bounce}@{cls.alias_domain}')) diff --git a/addons/mail/tests/test_ir_mail_server.py b/addons/mail/tests/test_ir_mail_server.py index ffc1324eda6..9fbd71f6d27 100644 --- a/addons/mail/tests/test_ir_mail_server.py +++ b/addons/mail/tests/test_ir_mail_server.py @@ -91,8 +91,8 @@ class TestIrMailServer(MailCommon): # outside "from_filter" domain: we will use notifications emails in the # headers, and bounce address in the envelope because the "from_filter" # allows to use the entire domain - (self.default_bounce_address, '"test" '), - (self.default_bounce_address, '"Formatted Name" '), + (self.default_bounce_address, f'"test" <{self.default_from}@{self.alias_domain}>'), + (self.default_bounce_address, f'"Formatted Name" <{self.default_from}@{self.alias_domain}>'), ] ): for provide_smtp in [False, True]: # providing smtp session should ont impact test @@ -146,7 +146,8 @@ class TestIrMailServer(MailCommon): ('notifications', 'example_2.com, example_3.com'), ('notifications@example.com', 'dummy.com, full_email@example_2.com, dummy2.com'), ('notifications', 'dummy.com, full_email@example_2.com, dummy2.com'), - ('notifications@example.com', 'example.com'), + (f'notifications@{self.alias_domain}', f'{self.alias_domain}'), + (f'notifications@{self.alias_domain}', f'{self.alias_domain}, example_2.com'), # default relies on "odoo" (False, 'example.com'), # fallback on user email if no from_filter @@ -159,7 +160,8 @@ class TestIrMailServer(MailCommon): 'notifications@example_2.com', 'full_email@example_2.com', 'full_email@example_2.com', - 'notifications@example.com', + f'notifications@{self.alias_domain}', + f'notifications@{self.alias_domain}', 'odoo@example.com', self.env.user.email, self.env.user.email, @@ -219,7 +221,7 @@ class TestIrMailServer(MailCommon): (self.mail_server_domain, 'unknown_email@test.mycompany.com'), (self.mail_server_domain, 'unknown_email@TEST.MYCOMPANY.COM'), (self.mail_server_domain, '"Unknown" '), - (self.mail_server_notification, 'notifications.test@test.mycompany.com'), + (self.mail_server_notification, f'{self.default_from}@test.mycompany.com'), # mail_server_user multiple from_filter check (self.mail_server_user, '"Example" '), (self.mail_server_user, '"Example" '), @@ -247,9 +249,9 @@ class TestIrMailServer(MailCommon): ('specific_user@test.mycompany.com', 'specific_user@test.mycompany.com', self.mail_server_user), # No mail server are configured for the email address, so it will use the # notifications email instead and encapsulate the old email - ('notifications.test@test.mycompany.com', '"Name" ', self.mail_server_notification), + (f'{self.default_from}@{self.alias_domain}', f'"Name" <{self.default_from}@{self.alias_domain}>', self.mail_server_notification), # same situation, but the original email has no name part - ('notifications.test@test.mycompany.com', '"test" ', self.mail_server_notification), + (f'{self.default_from}@{self.alias_domain}', f'"test" <{self.default_from}@{self.alias_domain}>', self.mail_server_notification), # A mail server is configured for the entire domain name, so we can use the bounce # email address because the mail server supports it (self.default_bounce_address, '"Name" ', self.mail_server_domain), @@ -294,7 +296,7 @@ class TestIrMailServer(MailCommon): self.assertEqual(len(self.emails), 1) self.assertSMTPEmailsSent( smtp_from=self.default_bounce_address, - message_from='"Name" ', + message_from=f'"Name" <{self.default_from}@{self.alias_domain}>', mail_server=self.mail_server_domain, ) diff --git a/addons/test_mail/tests/test_mail_composer.py b/addons/test_mail/tests/test_mail_composer.py index d81162fdab4..12afea1728d 100644 --- a/addons/test_mail/tests/test_mail_composer.py +++ b/addons/test_mail/tests/test_mail_composer.py @@ -56,32 +56,32 @@ class TestMailComposer(MailCommon, TestRecipients): ) cls.env.ref('mail.group_mail_template_editor').users -= cls.user_rendering_restricted - cls.test_record = cls.env['mail.test.ticket'].with_context(cls._test_context).create({ + cls.test_record = cls.env['mail.test.ticket.mc'].with_context(cls._test_context).create({ 'name': 'TestRecord', 'customer_id': cls.partner_1.id, 'user_id': cls.user_employee_2.id, }) cls.test_records, cls.test_partners = cls._create_records_for_batch( - 'mail.test.ticket', 2, + 'mail.test.ticket.mc', 2, additional_values={'user_id': cls.user_employee_2.id}, ) cls.test_report, cls.test_report_2, cls.test_report_3 = cls.env['ir.actions.report'].create([ { 'name': 'Test Report on Mail Test Ticket', - 'model': 'mail.test.ticket', + 'model': 'mail.test.ticket.mc', 'print_report_name': "'TestReport for %s' % object.name", 'report_type': 'qweb-pdf', 'report_name': 'test_mail.mail_test_ticket_test_template', }, { 'name': 'Test Report 2 on Mail Test Ticket', - 'model': 'mail.test.ticket', + 'model': 'mail.test.ticket.mc', 'print_report_name': "'TestReport2 for %s' % object.name", 'report_type': 'qweb-pdf', 'report_name': 'test_mail.mail_test_ticket_test_template_2', }, { 'name': 'Test Report 3 with variable data on Mail Test Ticket', - 'model': 'mail.test.ticket', + 'model': 'mail.test.ticket.mc', 'print_report_name': "'TestReport3 for %s' % object.name", 'report_type': 'qweb-pdf', 'report_name': 'test_mail.mail_test_ticket_test_variable_template', @@ -100,7 +100,7 @@ class TestMailComposer(MailCommon, TestRecipients): 'email_from': '{{ (object.user_id.email_formatted or user.email_formatted) }}', 'lang': '{{ object.customer_id.lang }}', 'mail_server_id': cls.mail_server_domain.id, - 'model_id': cls.env['ir.model']._get('mail.test.ticket').id, + 'model_id': cls.env['ir.model']._get('mail.test.ticket.mc').id, 'reply_to': '{{ ctx.get("custom_reply_to") or "info@test.example.com" }}', 'scheduled_date': '{{ (object.create_date or datetime.datetime(2022, 12, 26, 18, 0, 0)) + datetime.timedelta(days=2) }}', }) @@ -332,7 +332,7 @@ class TestComposerForm(TestMailComposer): when nothing is given as context. """ composer_form = Form(self.env['mail.compose.message'].with_context( default_composition_mode='comment', - default_model='mail.test.ticket', + default_model='mail.test.ticket.mc', default_template_id=self.template.id, )) self.assertTrue(composer_form.auto_delete, 'Should take composer value') @@ -454,7 +454,7 @@ class TestComposerForm(TestMailComposer): when nothing is given as context. """ composer_form = Form(self.env['mail.compose.message'].with_context( default_composition_mode='mass_mail', - default_model='mail.test.ticket', + default_model='mail.test.ticket.mc', default_template_id=self.template.id, )) self.assertTrue(composer_form.auto_delete, 'Should take composer value') @@ -1267,6 +1267,7 @@ class TestComposerResultsComment(TestMailComposer, CronMixinCase): self.assertEqual(self.test_records.message_partner_ids, self.partner_employee_2) self.assertEqual(self.test_records[0].customer_id.lang, 'en_US') self.assertEqual(self.test_records[1].customer_id.lang, 'en_US') + self.assertEqual(self.test_records.company_id, self.company_admin) self.assertEqual(len(self.test_partners), 2) @@ -1426,6 +1427,88 @@ class TestComposerResultsComment(TestMailComposer, CronMixinCase): self.assertEqual(message.subtype_id, self.env.ref('mail.mt_comment')) self.assertEqual(message.partner_ids, self.partner_1 | self.partner_2) + @users('employee') + @mute_logger('odoo.addons.mail.models.mail_mail') + def test_mail_composer_server_config(self): + """ Test various configuration to check behavior of outgoing mail + servers, notifications, .... """ + # add access to second company to avoid MC rules on ticket model + self.env.user.company_ids = [(4, self.company_2.id)] + + # initial data + self.assertEqual(self.env.company, self.company_admin) + self.assertEqual(self.user_admin.company_id, self.company_admin) + + # update test configuration + test_records = self.test_records + test_companies = self.company_admin + self.company_2 + for company, record in zip(test_companies, test_records): + record.company_id = company.id + + # various from / servers configuration + self.template.write({'mail_server_id': False}) # allow server archive + server_other = self.env['ir.mail_server'].sudo().create({ + 'name': 'Server Other', + 'from_filter': 'test.othercompany.com', + 'sequence': 4, + 'smtp_encryption': 'none', + 'smtp_host': 'smtp_host', + }) + servers_all = self.mail_servers + server_other + for (emails_from, servers_active, mail_config), (exp_smtp_from_lst, exp_msg_from_lst, exp_from_filter) in zip( + [ + ( + [f'user.from@{self.alias_domain}', 'user@other.domain.com'], + self.env['ir.mail_server'], + {'default_from': f'notifications@{self.alias_domain}', 'from_filter': self.alias_domain} + ), # odoo-style configuration + ], [ + ( + [f'{self.alias_bounce}@{self.alias_domain}', f'{self.alias_bounce}@{self.alias_domain}'], + [f'"{self.env.user.name}" ', f'"{self.env.user.name}" '], + self.alias_domain + ), # no spoof + ], + ): + with self.subTest(emails_from=emails_from, + servers_active=servers_active): + # update servers + servers_all.active = False + if servers_active: + servers_active.active = True + # update mail config + default_from = mail_config.get('default_from', self.default_from) + from_filter = mail_config.get('from_filter', self.default_from_filter) + self.env['ir.config_parameter'].sudo().set_param('mail.default.from', default_from) + self.env['ir.config_parameter'].sudo().set_param('mail.default.from_filter', from_filter) + + for email_from, exp_smtp_from, exp_msg_from in zip(emails_from, exp_smtp_from_lst, exp_msg_from_lst): + self.env.user.email = email_from + + # open a composer and run it in comment mode + composer = Form(self.env['mail.compose.message'].with_context( + default_composition_mode='comment', + default_force_send=True, # force sending emails directly to check SMTP + default_model=test_records._name, + default_res_ids=test_records.ids, + # avoid successive tests issues with followers + mail_create_nosubscribe=True, + )) + composer.body = 'Hello {{ object.name }}' + composer.subject = 'My Subject' + composer = composer.save() + with self.mock_mail_gateway(mail_unlink_sent=False), \ + self.mock_mail_app(): + composer._action_send_mail() + + self.assertSMTPEmailsSent( + smtp_from=exp_smtp_from, + smtp_to_list=[self.partner_employee_2.email_normalized], + emails_count=2, # same on both records + message_from=exp_msg_from, + from_filter=exp_from_filter, + ) + @users('employee') @mute_logger('odoo.models.unlink', 'odoo.addons.mail.models.mail_mail', 'odoo.addons.mail.models.mail_message_schedule') def test_mail_composer_wtpl_complete(self): @@ -1761,6 +1844,105 @@ class TestComposerResultsComment(TestMailComposer, CronMixinCase): self.assertEqual(set(message.attachment_ids.mapped('res_id')), set(test_record.ids)) self.assertTrue(all(attach not in message.attachment_ids for attach in attachs), 'Should have copied attachments') + + @users('employee') + @mute_logger('odoo.addons.mail.models.mail_mail') + def test_mail_composer_wtpl_mc(self): + """ Test specific to multi-company environment, notably company propagation + or aliases. """ + # add access to second company to avoid MC rules on ticket model + self.env.user.company_ids = [(4, self.company_2.id)] + + # initial data + self.assertEqual(self.env.company, self.company_admin) + self.assertEqual(self.user_admin.company_id, self.company_admin) + + attachment_data = self._generate_attachments_data(2, self.template._name, self.template.id) + email_to_1 = 'test.to.1@test.example.com' + self.template.write({ + 'auto_delete': False, # keep sent emails to check content + 'attachment_ids': [(0, 0, a) for a in attachment_data], + 'email_from': False, # use current user as author + 'email_layout_xmlid': 'mail.test_layout', + 'email_to': email_to_1, + 'mail_server_id': False, # let it find a server + 'partner_to': '%s, {{ object.customer_id.id if object.customer_id else "" }}' % self.partner_admin.id, + }) + attachs = self.env['ir.attachment'].search([('name', 'in', [a['name'] for a in attachment_data])]) + self.assertEqual(len(attachs), 2) + + for batch, companies, expected_companies in [ + ( + False, self.company_admin, self.company_admin + ), ( + True, self.company_admin + self.company_2, self.company_admin + self.company_2, + ), + ]: + with self.subTest(batch=batch, + companies=companies): + # update test configuration + test_records = self.test_records if batch else self.test_record + for company, record in zip(companies, test_records): + record.company_id = company.id + + # open a composer and run it in comment mode + composer = Form(self.env['mail.compose.message'].with_context( + default_composition_mode='comment', + default_force_send=True, # force sending emails directly to check SMTP + default_model=test_records._name, + default_res_ids=test_records.ids, + default_template_id=self.template.id, + # avoid successive tests issues with followers + mail_create_nosubscribe=True, + )).save() + with self.mock_mail_gateway(mail_unlink_sent=False), \ + self.mock_mail_app(): + composer._action_send_mail() + + new_partner = self.env['res.partner'].search([('email_normalized', '=', 'test.to.1@test.example.com')]) + self.assertEqual(len(new_partner), 1) + # check output, company-specific values mainly for this test + for record, exp_company in zip( + test_records, expected_companies + ): + message = record.message_ids[0] + for recipient in [self.partner_employee_2, new_partner, record.customer_id]: + self.assertMailMail( + recipient, + 'sent', + author=self.partner_employee, + mail_message=message, + email_values={ + 'headers': { + 'Return-Path': f'{self.alias_bounce}@{self.alias_domain}', + 'X-Odoo-Objects': f'{record._name}-{record.id}', + }, + 'subject': f'TemplateSubject {record.name}', + }, + fields_values={ + 'headers': { + 'X-Odoo-Objects': f'{record._name}-{record.id}', + }, + 'mail_server_id': self.env['ir.mail_server'], + 'subject': f'TemplateSubject {record.name}', + }, + ) + # to check behavior of extract_rfc2822_addresses + if recipient == new_partner: + smtp_to_list = ['"test.to.1@test.example.com"', 'test.to.1@test.example.com'] + else: + smtp_to_list = [recipient.email_normalized] + if recipient != record.customer_id: + emails_count = len(test_records) # not distinguishable using this assert + else: + emails_count = 1 + self.assertSMTPEmailsSent( + smtp_from=f'{self.default_from}@{self.alias_domain}', + smtp_to_list=smtp_to_list, + mail_server=self.mail_server_notification, + emails_count=emails_count, + ) + @users('employee') @mute_logger('odoo.addons.mail.models.mail_mail') def test_mail_composer_wtpl_recipients_email_fields(self): @@ -2462,6 +2644,105 @@ class TestComposerResultsMass(TestMailComposer): # self.assertIn(exp_button_es, sent_mail['body']) self.assertIn(exp_button_en, sent_mail['body']) + + @users('employee') + @mute_logger('odoo.addons.mail.models.mail_mail') + def test_mail_composer_wtpl_mc(self): + """ Test specific to multi-company environment, notably company propagation + or aliases. """ + # add access to second company to avoid MC rules on ticket model + self.env.user.company_ids = [(4, self.company_2.id)] + + # initial data + self.assertEqual(self.env.company, self.company_admin) + self.assertEqual(self.user_admin.company_id, self.company_admin) + + attachment_data = self._generate_attachments_data(2, self.template._name, self.template.id) + email_to_1 = 'test.to.1@test.example.com' + self.template.write({ + 'auto_delete': False, # keep sent emails to check content + 'attachment_ids': [(0, 0, a) for a in attachment_data], + 'email_from': False, # use current user as author + 'email_layout_xmlid': 'mail.test_layout', + 'email_to': email_to_1, + 'mail_server_id': False, # let it find a server + 'partner_to': '%s, {{ object.customer_id.id if object.customer_id else "" }}' % self.partner_admin.id, + }) + attachs = self.env['ir.attachment'].search([('name', 'in', [a['name'] for a in attachment_data])]) + self.assertEqual(len(attachs), 2) + + for companies, expected_companies in [ + ( + self.company_admin + self.company_2, + self.company_admin + self.company_2, + ), + ]: + with self.subTest(companies=companies): + # update test configuration + test_records = self.test_records + for company, record in zip(companies, test_records): + record.company_id = company.id + + # open a composer and run it in comment mode + composer = Form(self.env['mail.compose.message'].with_context( + default_composition_mode='mass_mail', + default_force_send=True, # force sending emails directly to check SMTP + default_model=test_records._name, + default_res_ids=test_records.ids, + default_template_id=self.template.id, + # avoid successive tests issues with followers + mail_create_nosubscribe=True, + )).save() + with self.mock_mail_gateway(mail_unlink_sent=False), \ + self.mock_mail_app(): + composer._action_send_mail() + + new_partner = self.env['res.partner'].search([('email_normalized', '=', 'test.to.1@test.example.com')]) + self.assertEqual(len(new_partner), 1) + # check output, company-specific values mainly for this test + for record, exp_company in zip( + test_records, expected_companies + ): + # message copy is kept + message = record.message_ids[0] + recipients = record.customer_id + new_partner + self.partner_admin + self.assertMailMail( + record.customer_id + new_partner + self.partner_admin, + 'sent', + author=self.partner_employee, + mail_message=message, + email_values={ + 'headers': { + 'Return-Path': f'{self.alias_bounce}@{self.alias_domain}', + 'X-Odoo-Objects': f'{record._name}-{record.id}', + }, + 'subject': f'TemplateSubject {record.name}', + }, + fields_values={ + 'headers': { + 'X-Odoo-Objects': f'{record._name}-{record.id}', + }, + 'mail_server_id': self.env['ir.mail_server'], + 'subject': f'TemplateSubject {record.name}', + }, + ) + for recipient in recipients: + # to check behavior of extract_rfc2822_addresses + if recipient == new_partner: + smtp_to_list = ['"test.to.1@test.example.com"', 'test.to.1@test.example.com'] + else: + smtp_to_list = [recipient.email_normalized] + if recipient != record.customer_id: + emails_count = len(test_records) # not distinguishable using this assert + else: + emails_count = 1 + self.assertSMTPEmailsSent( + smtp_from=f'{self.default_from}@{self.alias_domain}', + smtp_to_list=smtp_to_list, + mail_server=self.mail_server_notification, + emails_count=emails_count, + ) + @users('employee') @mute_logger('odoo.models.unlink', 'odoo.addons.mail.models.mail_mail') def test_mail_composer_wtpl_recipients(self): @@ -2581,7 +2862,7 @@ class TestComposerResultsMass(TestMailComposer): # 6: void is void: raise in comment mode, just don't send anything in mass mail mode composer_form = Form(self.env['mail.compose.message'].with_context( - default_model='mail.test.ticket', + default_model='mail.test.ticket.mc', default_template_id=self.template.id )) composer = composer_form.save() @@ -2592,7 +2873,7 @@ class TestComposerResultsMass(TestMailComposer): composer_form = Form(self.env['mail.compose.message'].with_context( default_composition_mode='mass_mail', - default_model='mail.test.ticket', + default_model='mail.test.ticket.mc', default_template_id=self.template.id )) composer = composer_form.save() diff --git a/addons/test_mail/tests/test_mail_gateway.py b/addons/test_mail/tests/test_mail_gateway.py index 63c8a3444d7..50baa6f4485 100644 --- a/addons/test_mail/tests/test_mail_gateway.py +++ b/addons/test_mail/tests/test_mail_gateway.py @@ -128,9 +128,7 @@ class TestEmailParsing(MailCommon): email_to='bounce@xxx.odoo.com', delivered_to='bounce@xxx.odoo.com' ) - - msg_dict = {} - msg = self.env['mail.thread']._message_parse_extract_bounce(self.from_string(incoming_bounce), msg_dict) + msg = self.env['mail.thread'].message_parse(self.from_string(incoming_bounce)) self.assertEqual(msg['bounced_email'], partner.email, "The sender email should be correctly parsed") self.assertEqual(msg['bounced_partner'], partner, "A partner with this email should exist") self.assertEqual(msg['bounced_msg_ids'][0], message.message_id, "The sender message-id should correctly parsed") diff --git a/addons/test_mass_mailing/tests/test_mailing_server.py b/addons/test_mass_mailing/tests/test_mailing_server.py index fda438bf015..874aed17e90 100644 --- a/addons/test_mass_mailing/tests/test_mailing_server.py +++ b/addons/test_mass_mailing/tests/test_mailing_server.py @@ -112,8 +112,8 @@ class TestMassMailingServer(TestMassMailCommon): ('unknow_email@unknow_domain.com', 'unknow_email@unknow_domain.com', self.mail_server_notification), ]: self.assertSMTPEmailsSent( - smtp_from=expected_smtp_from, - message_from=expected_msg_from, - mail_server=expected_mail_server, - emails_count=8, - ) + smtp_from=expected_smtp_from, + message_from=expected_msg_from, + mail_server=expected_mail_server, + emails_count=8, + ) From 1b16757d285577b22a2c6968eceefc5b5a5052e7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Fri, 17 Sep 2021 15:54:30 +0200 Subject: [PATCH 06/18] [IMP] mail: add 'mail.alias.domain' model to store alias domains 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 Add a new 'mail.alias.domain' model storing information previously stored as a unique value in 'mail.catchall.domain' configuration parameter. Domains are not completely linked to companies, allowing to have a mono-domain MC setup, or mono-company multi-domain setup. A main email domain is present on companies for all default domain computation when being in that company while keeping flexibility of having other domains defined. Now that a new 'mail.alias.domain' model exists we move bounce and catchall aliases definition directly on this model. Update related computation on company model. Default_from is also moved on this model, allowing an higher level default from computation for mail servers when having knowledge of alias domain environment. Filtering configuration based on default_from_filter is kept as an ICP as it is mainly used for odoo-bin with smtp-host. Usage of 'mail.catchall.domain' will soon be completely removed to be replaced by proposed company-based email alias domain usage. Constraints are added so that each bounce and catchall defined on domains do not clash with existing aliases. Sanitize of bounce and catchall is also performed to ensure they make valid emails. This matches previous behavior of ICP parameters. Domain name is also sanitized like alias names. Currently only base modeling and computation is done. Their usage is about to be gradually added in mail stack. LINKS Task-36879 (Mail: Support Multi Domains Aliases) Part-of: odoo/odoo#76734 --- addons/mail/__manifest__.py | 2 + addons/mail/models/__init__.py | 1 + addons/mail/models/ir_config_parameter.py | 5 +- addons/mail/models/mail_alias_domain.py | 145 +++++++++++ addons/mail/models/res_company.py | 42 +++- addons/mail/security/ir.model.access.csv | 2 + addons/mail/tests/common.py | 64 ++++- addons/mail/views/mail_alias_domain_views.xml | 63 +++++ addons/mail/views/mail_menus.xml | 15 +- addons/mail/views/res_company_views.xml | 16 ++ addons/test_mail/tests/test_mail_alias.py | 234 +++++++++++++++++- 11 files changed, 550 insertions(+), 39 deletions(-) create mode 100644 addons/mail/models/mail_alias_domain.py create mode 100644 addons/mail/views/mail_alias_domain_views.xml create mode 100644 addons/mail/views/res_company_views.xml diff --git a/addons/mail/__manifest__.py b/addons/mail/__manifest__.py index 288c21018cc..f87807e5ddd 100644 --- a/addons/mail/__manifest__.py +++ b/addons/mail/__manifest__.py @@ -103,6 +103,7 @@ For more specific needs, you may also assign custom-defined actions 'security/mail_security.xml', 'security/ir.model.access.csv', 'views/discuss_public_templates.xml', + 'views/mail_alias_domain_views.xml', 'views/mail_alias_views.xml', 'views/mail_gateway_allowed_views.xml', 'views/mail_guest_views.xml', @@ -116,6 +117,7 @@ For more specific needs, you may also assign custom-defined actions 'views/res_partner_views.xml', 'views/mail_blacklist_views.xml', 'views/mail_menus.xml', + 'views/res_company_views.xml', ], 'demo': [ 'data/discuss_channel_demo.xml', diff --git a/addons/mail/models/__init__.py b/addons/mail/models/__init__.py index 8b1713567c9..e5ec79f2273 100644 --- a/addons/mail/models/__init__.py +++ b/addons/mail/models/__init__.py @@ -3,6 +3,7 @@ # core models (required for mixins) from . import mail_alias +from . import mail_alias_domain from . import models # mixin diff --git a/addons/mail/models/ir_config_parameter.py b/addons/mail/models/ir_config_parameter.py index 44fde74d8b4..5bf36e62605 100644 --- a/addons/mail/models/ir_config_parameter.py +++ b/addons/mail/models/ir_config_parameter.py @@ -20,12 +20,9 @@ class IrConfigParameter(models.Model): # remove existing users, including inactive template user # admin will regain the right via implied_ids on group_system group_user._remove_group(group_mail_template_editor) - # sanitize, normalize and check uniqueness of catchall / bounce aliases - # note that False = unlink ICP + # sanitize, normalize; note that False = unlink ICP elif key in {'mail.bounce.alias', 'mail.catchall.alias'} and value: value = self.env['mail.alias']._sanitize_alias_name(value) - if value: - self.env['mail.alias']._check_unique([value], skip_icp_keys={key}) # sanitize and normalize allowed catchall domains elif key == 'mail.catchall.domain.allowed' and value: value = self.env['mail.alias']._clean_and_check_mail_catchall_allowed_domains(value) diff --git a/addons/mail/models/mail_alias_domain.py b/addons/mail/models/mail_alias_domain.py new file mode 100644 index 00000000000..b5ab29bff91 --- /dev/null +++ b/addons/mail/models/mail_alias_domain.py @@ -0,0 +1,145 @@ +# -*- coding: utf-8 -*- +# Part of Odoo. See LICENSE file for full copyright and licensing details. + +from odoo import api, exceptions, fields, models, _ +from odoo.addons.mail.models.mail_alias import dot_atom_text + +class AliasDomain(models.Model): + """ Model alias domains, now company-specific. Alias domains are email + domains used to receive emails through catchall and bounce aliases, as + well as using mail.alias records to redirect email replies. + + This replaces ``mail.alias.domain`` configuration parameter use until v16. + """ + _name = 'mail.alias.domain' + _description = "Email Domain" + _order = 'sequence ASC, id ASC' + + name = fields.Char('Name', required=True) + company_ids = fields.One2many( + 'res.company', 'alias_domain_id', string='Companies') + sequence = fields.Integer(default=10) + bounce_alias = fields.Char('Bounce Alias', default='bounce', required=True) + bounce_email = fields.Char('Bounce Email', compute='_compute_bounce_email') + catchall_alias = fields.Char('Catchall Alias', default='catchall', required=True) + catchall_email = fields.Char('Catchall Email', compute='_compute_catchall_email') + default_from = fields.Char('Default From Alias', default='notifications') + default_from_email = fields.Char('Default From', compute='_compute_default_from_email') + + _sql_constraints = [ + ( + 'bounce_email_uniques', + 'UNIQUE(bounce_alias, name)', + 'Bounce emails should be unique' + ), + ( + 'catchall_email_uniques', + 'UNIQUE(catchall_alias, name)', + 'Catchall emails should be unique' + ), + ] + + @api.depends('bounce_alias', 'name') + def _compute_bounce_email(self): + self.bounce_email = '' + for domain in self.filtered('bounce_alias'): + domain.bounce_email = f'{domain.bounce_alias}@{domain.name}' + + @api.depends('catchall_alias', 'name') + def _compute_catchall_email(self): + self.catchall_email = '' + for domain in self.filtered('catchall_alias'): + domain.catchall_email = f'{domain.catchall_alias}@{domain.name}' + + @api.depends('default_from', 'name') + def _compute_default_from_email(self): + self.default_from_email = '' + for domain in self.filtered('default_from'): + domain.default_from_email = f'{domain.default_from}@{domain.name}' + + @api.constrains('bounce_alias', 'catchall_alias') + def _check_bounce_catchall_uniqueness(self): + names = self.filtered('bounce_alias').mapped('bounce_alias') + self.filtered('catchall_alias').mapped('catchall_alias') + if not names: + return + + similar_domains = self.env['mail.alias.domain'].search([('name', 'in', self.mapped('name'))]) + for tocheck in self: + if any(similar.bounce_alias == tocheck.bounce_alias + for similar in similar_domains if similar != tocheck and similar.name == tocheck.name): + raise exceptions.ValidationError( + _('Bounce alias %(bounce)s is already used for another domain with same name. ' + 'Use another bounce or simply use the other alias domain.', + bounce=tocheck.bounce_email) + ) + if any(similar.catchall_alias == tocheck.catchall_alias + for similar in similar_domains if similar != tocheck and similar.name == tocheck.name): + raise exceptions.ValidationError( + _('Catchall alias %(catchall)s is already used for another domain with same name. ' + 'Use another catchall or simply use the other alias domain.', + catchall=tocheck.catchall_email) + ) + + existing = self.env['mail.alias'].search( + [('alias_name', 'in', list(set(names)))], + limit=1, + ) + if existing: + document_name = False + # If owner or target: display document name also in the warning + if existing.alias_parent_model_id and existing.alias_parent_thread_id: + document_name = self.env[existing.alias_parent_model_id.model].sudo().browse(existing.alias_parent_thread_id).display_name + elif existing.alias_model_id and existing.alias_force_thread_id: + document_name = self.env[existing.alias_model_id.model].sudo().browse(existing.alias_force_thread_id).display_name + if document_name: + raise exceptions.ValidationError( + _("Bounce/Catchall '%(matching_alias_name)s' is already used by %(document_name)s. Choose another alias or change it on the other document.", + matching_alias_name=existing.display_name, + document_name=document_name) + ) + raise exceptions.ValidationError( + _("Bounce/Catchall '%(matching_alias_name)s' is already used. Choose another alias or change it on the linked model.", + matching_alias_name=existing.display_name) + ) + + @api.constrains('name') + def _check_name(self): + """ Should match a sanitized version of itself, otherwise raise to warn + user (do not dynamically change it, would be confusing). """ + for domain in self: + if not dot_atom_text.match(domain.name): + raise exceptions.ValidationError( + _("You cannot use anything else than unaccented latin characters in the domain name %(domain_name)s.", + domain_name=domain.name) + ) + + @api.model_create_multi + def create(self, vals_list): + """ Sanitize bounce_alias / catchall_alias """ + for vals in vals_list: + if vals.get('bounce_alias'): + vals['bounce_alias'] = self.env['mail.alias']._sanitize_alias_name(vals['bounce_alias']) + if vals.get('catchall_alias'): + vals['catchall_alias'] = self.env['mail.alias']._sanitize_alias_name(vals['catchall_alias']) + if vals.get('default_from'): + vals['default_from'] = self.env['mail.alias']._sanitize_alias_name(vals['default_from']) + + alias_domains = super().create(vals_list) + + # alias domain init: populate companies at first creation + if alias_domains and self.search_count([]) == len(alias_domains): + self.env['res.company'].search( + [('alias_domain_id', '=', False)] + ).alias_domain_id = alias_domains[0].id + + return alias_domains + + def write(self, vals): + """ Sanitize bounce_alias / catchall_alias """ + if vals.get('bounce_alias'): + vals['bounce_alias'] = self.env['mail.alias']._sanitize_alias_name(vals['bounce_alias']) + if vals.get('catchall_alias'): + vals['catchall_alias'] = self.env['mail.alias']._sanitize_alias_name(vals['catchall_alias']) + if vals.get('default_from'): + vals['default_from'] = self.env['mail.alias']._sanitize_alias_name(vals['default_from']) + return super().write(vals) diff --git a/addons/mail/models/res_company.py b/addons/mail/models/res_company.py index 1b450e75f16..75a8a9f749b 100644 --- a/addons/mail/models/res_company.py +++ b/addons/mail/models/res_company.py @@ -8,8 +8,20 @@ class Company(models.Model): _name = 'res.company' _inherit = 'res.company' + def _default_alias_domain_id(self): + return self.env['mail.alias.domain'].search([], limit=1) + + alias_domain_id = fields.Many2one( + 'mail.alias.domain', string='Email Domain', + default=lambda self: self._default_alias_domain_id()) + alias_domain_name = fields.Char('Alias Domain Name', related='alias_domain_id.name', readonly=True, store=True) + bounce_email = fields.Char(string="Bounce Email", compute="_compute_bounce") + bounce_formatted = fields.Char(string="Bounce", compute="_compute_bounce") catchall_email = fields.Char(string="Catchall Email", compute="_compute_catchall") catchall_formatted = fields.Char(string="Catchall", compute="_compute_catchall") + default_from_email = fields.Char( + string="Default From", related="alias_domain_id.default_from_email", + readonly=True) # the compute method is sudo'ed because it needs to access res.partner records # portal users cannot access those (but they should be able to read the company email address) email_formatted = fields.Char( @@ -22,19 +34,25 @@ class Company(models.Model): "Email Button Color", compute="_compute_email_secondary_color", readonly=False, store=True) - @api.depends('name') + @api.depends('alias_domain_id', 'name') + def _compute_bounce(self): + self.bounce_email = '' + self.bounce_formatted = '' + + for company in self.filtered('alias_domain_id'): + bounce_email = company.alias_domain_id.bounce_email + company.bounce_email = bounce_email + company.bounce_formatted = tools.formataddr((company.name, bounce_email)) + + @api.depends('alias_domain_id', 'name') def _compute_catchall(self): - ConfigParameter = self.env['ir.config_parameter'].sudo() - alias = ConfigParameter.get_param('mail.catchall.alias') - domain = ConfigParameter.get_param('mail.catchall.domain') - if alias and domain: - for company in self: - company.catchall_email = '%s@%s' % (alias, domain) - company.catchall_formatted = tools.formataddr((company.name, company.catchall_email)) - else: - for company in self: - company.catchall_email = '' - company.catchall_formatted = '' + self.catchall_email = '' + self.catchall_formatted = '' + + for company in self.filtered('alias_domain_id'): + catchall_email = company.alias_domain_id.catchall_email + company.catchall_email = catchall_email + company.catchall_formatted = tools.formataddr((company.name, catchall_email)) @api.depends('partner_id', 'catchall_formatted') def _compute_email_formatted(self): diff --git a/addons/mail/security/ir.model.access.csv b/addons/mail/security/ir.model.access.csv index 953c7d590f3..2c3d3d31e51 100644 --- a/addons/mail/security/ir.model.access.csv +++ b/addons/mail/security/ir.model.access.csv @@ -20,6 +20,8 @@ access_discuss_channel_member_user,discuss.channel.member.user,model_discuss_cha access_discuss_channel_rtc_session_system,discuss.channel.rtc.session.system,model_discuss_channel_rtc_session,base.group_system,1,1,1,1 access_mail_alias_user,mail.alias.user,model_mail_alias,base.group_user,1,0,0,0 access_mail_alias_system,mail.alias.system,model_mail_alias,base.group_system,1,1,1,1 +access_mail_alias_domain_user,mail.alias.domain.user,model_mail_alias_domain,base.group_user,1,0,0,0 +access_mail_alias_domain_system,mail.alias.domain.system,model_mail_alias_domain,base.group_erp_manager,1,1,1,1 access_mail_gateway_allowed_system,mail.gateway.allowed.system,model_mail_gateway_allowed,base.group_system,1,1,1,1 access_mail_message_reaction_system,mail.message.reaction.system,model_mail_message_reaction,base.group_system,1,1,1,1 access_mail_message_subtype_public,mail.message.subtype.all,model_mail_message_subtype,base.group_public,1,0,0,0 diff --git a/addons/mail/tests/common.py b/addons/mail/tests/common.py index 59c0df79f3f..0149d0cfec2 100644 --- a/addons/mail/tests/common.py +++ b/addons/mail/tests/common.py @@ -95,6 +95,7 @@ class MockEmail(common.BaseCase, MockSmtplibCase): @classmethod def _init_mail_gateway(cls): super()._init_mail_gateway() + # main company alias parameters cls.alias_domain = 'test.mycompany.com' cls.alias_catchall = 'catchall.test' cls.alias_bounce = 'bounce.test' @@ -106,9 +107,58 @@ class MockEmail(common.BaseCase, MockSmtplibCase): cls.env['ir.config_parameter'].set_param('mail.default.from', cls.default_from) cls.env['ir.config_parameter'].set_param('mail.default.from_filter', cls.default_from_filter) + # ensure global alias domain for tests: to ease tests, search or create + # the default test domains + cls.env['mail.alias.domain'].search([]).write({'sequence': 9999}) + cls.mail_alias_domain = cls._init_alias_domain(cls.alias_domain, { + 'bounce_alias': cls.alias_bounce, + 'catchall_alias': cls.alias_catchall, + 'company_ids': [(4, cls.env.ref('base.user_admin').company_id.id)], + 'default_from': cls.default_from, + 'name': cls.alias_domain, + 'sequence': 1, + }) + if cls._mc_enabled: + # alias domain specific to new company + cls.alias_bounce_c2 = 'bounce.c2' + cls.alias_catchall_c2 = 'catchall.c2' + cls.alias_default_from_c2 = 'notifications.c2' + cls.alias_domain_c2_name = 'test.mycompany2.com' + cls.mail_alias_domain_c2 = cls._init_alias_domain(cls.alias_domain_c2_name, { + 'bounce_alias': cls.alias_bounce_c2, + 'catchall_alias': cls.alias_catchall_c2, + 'company_ids': [(4, cls.company_2.id)], + 'default_from': cls.alias_default_from_c2, + 'name': cls.alias_domain_c2_name, + 'sequence': 2, + }) + + # alias domain specific to third company + cls.alias_bounce_c3 = 'bounce.c3' + cls.alias_catchall_c3 = 'catchall.c3' + cls.alias_default_from_c3 = 'notifications.c3' + cls.alias_domain_c3_name = 'test.mycompany3.com' + cls.mail_alias_domain_c3 = cls._init_alias_domain(cls.alias_domain_c3_name, { + 'bounce_alias': cls.alias_bounce_c3, + 'catchall_alias': cls.alias_catchall_c3, + 'company_ids': [(4, cls.company_3.id)], + 'default_from': cls.alias_default_from_c3, + 'name': cls.alias_domain_c3_name, + 'sequence': 3, + }) + # mailer daemon email preformatting cls.mailer_daemon_email = formataddr(('MAILER-DAEMON', f'{cls.alias_bounce}@{cls.alias_domain}')) + @classmethod + def _init_alias_domain(cls, name, values): + alias_domain = cls.env['mail.alias.domain'].search([('name', '=', name)]) + if alias_domain: + alias_domain.write(values) + else: + alias_domain = cls.env['mail.alias.domain'].create(values) + return alias_domain + # ------------------------------------------------------------ # GATEWAY TOOLS # ------------------------------------------------------------ @@ -1269,20 +1319,24 @@ class MailCommon(common.TransactionCase, MailCase): different companies. """ cls._mc_enabled = True - # new company + # new companies cls.company_2 = cls.env['res.company'].create({ 'currency_id': cls.env.ref('base.CAD').id, 'email': 'company_2@test.example.com', 'name': 'Company 2', }) cls.company_3 = cls.env['res.company'].create({ - 'name': 'ELIT', - }) - cls.user_admin.write({'company_ids': [(4, cls.company_2.id)]}) - cls.company_3 = cls.env['res.company'].create({ + 'country_id': cls.env.ref('base.be').id, + 'currency_id': cls.env.ref('base.EUR').id, 'email': 'company_3@test.example.com', 'name': 'Company 3', }) + cls.user_admin.write({ + 'company_ids': [ + (4, cls.company_2.id), + (4, cls.company_3.id), + ], + }) # employee specific to second company cls.user_employee_c2 = mail_new_test_user( diff --git a/addons/mail/views/mail_alias_domain_views.xml b/addons/mail/views/mail_alias_domain_views.xml new file mode 100644 index 00000000000..b88196c7cc8 --- /dev/null +++ b/addons/mail/views/mail_alias_domain_views.xml @@ -0,0 +1,63 @@ + + + + + mail.alias.domain.view.form + mail.alias.domain + +
+ + + + + + + + + +
+ + + + + mail.alias.domain.view.tree + mail.alias.domain + + + + + + + + + + + + + + mail.alias.domain.view.search + mail.alias.domain + + + + + + + + + + + + + + + Alias Domains + mail.alias.domain + tree,form + + + diff --git a/addons/mail/views/mail_menus.xml b/addons/mail/views/mail_menus.xml index 197952af055..291bdb8fa86 100644 --- a/addons/mail/views/mail_menus.xml +++ b/addons/mail/views/mail_menus.xml @@ -18,6 +18,16 @@ parent="base.menu_email" action="action_view_mail_mail" sequence="1"/> + + - + + + res.company.view.form.inherit.mail + res.company + + + + + + + + + + + diff --git a/addons/test_mail/tests/test_mail_alias.py b/addons/test_mail/tests/test_mail_alias.py index 5f4e2002827..194d76cfb34 100644 --- a/addons/test_mail/tests/test_mail_alias.py +++ b/addons/test_mail/tests/test_mail_alias.py @@ -133,12 +133,6 @@ class TestMailAlias(TestMailAliasCommon): with self.assertRaises(exceptions.UserError), self.cr.savepoint(): other_alias.write({'alias_name': 'unused.test.alias'}) - # cannot set catchall / bounce to used alias - with self.assertRaises(exceptions.UserError), self.cr.savepoint(): - self.env['ir.config_parameter'].sudo().set_param('mail.catchall.alias', new_mail_alias.alias_name) - with self.assertRaises(exceptions.UserError), self.cr.savepoint(): - self.env['ir.config_parameter'].sudo().set_param('mail.bounce.alias', new_mail_alias.alias_name) - @users('admin') def test_alias_name_unique_copy(self): """ Check uniqueness constraint check when copying aliases """ @@ -250,26 +244,91 @@ class TestMailAlias(TestMailAliasCommon): class TestAliasCompany(TestMailAliasCommon): """ Test company / alias domain and configuration synchronization """ + @mute_logger('odoo.models.unlink') + @users('erp_manager') + def test_alias_domain_setup(self): + """ Test synchronization of alias domain with companies when adding / + updating / removing alias domains """ + self.assertEqual(self.company_admin.alias_domain_id, self.mail_alias_domain) + self.assertEqual(self.company_2.alias_domain_id, self.mail_alias_domain_c2) + + # remove alias domain of first company, should not impact second company + self.mail_alias_domain.unlink() + self.assertFalse(self.company_admin.alias_domain_id) + self.assertEqual(self.company_2.alias_domain_id, self.mail_alias_domain_c2) + + # remove all alias domains + self.env['mail.alias.domain'].search([]).unlink() + self.assertFalse(self.company_2.alias_domain_id) + self.assertEqual(self.company_2.bounce_email, '') + self.assertEqual(self.company_2.bounce_formatted, '') + self.assertEqual(self.company_2.catchall_email, '') + self.assertEqual(self.company_2.catchall_formatted, '') + self.assertFalse(self.company_2.default_from_email, '') + self.assertFalse(self.company_3.alias_domain_id) + + # create a new alias domain -> consider as re-init, populate all companies + alias_domain_new = self.env['mail.alias.domain'].create({ + 'bounce_alias': 'bounce.new', + 'catchall_alias': 'catchall.new', + 'name': 'test.global.bitnurk.com', + }) + self.assertEqual(self.company_admin.alias_domain_id, alias_domain_new, + 'MC Alias: from no domain to new domain') + self.assertEqual(self.company_2.alias_domain_id, alias_domain_new, + 'MC Alias: should take alias domain with lower sequence') + self.assertEqual(self.company_3.alias_domain_id, alias_domain_new, + 'MC Alias: should take alias domain with lower sequence') + + # manual update + self.company_2.alias_domain_id = alias_domain_new.id + self.assertEqual(self.company_2.alias_domain_id, alias_domain_new) + self.assertEqual(self.company_2.bounce_email, 'bounce.new@test.global.bitnurk.com') + self.assertEqual(self.company_2.catchall_email, 'catchall.new@test.global.bitnurk.com') + def test_assert_initial_values(self): """ Test initial setup values: currently all companies share the same alias configuration as it is unique. """ + self.assertEqual(self.test_alias_mc.alias_domain, self.mail_alias_domain.name) + + self.assertEqual(self.company_admin.alias_domain_id, self.mail_alias_domain) + self.assertEqual(self.company_admin.bounce_email, f'{self.alias_bounce}@{self.alias_domain}') + self.assertEqual( + self.company_admin.bounce_formatted, + formataddr((self.company_admin.name, f'{self.alias_bounce}@{self.alias_domain}')) + ) self.assertEqual(self.company_admin.catchall_email, f'{self.alias_catchall}@{self.alias_domain}') self.assertEqual( self.company_admin.catchall_formatted, formataddr((self.company_admin.name, f'{self.alias_catchall}@{self.alias_domain}')) ) + self.assertEqual(self.company_admin.default_from_email, f'{self.default_from}@{self.alias_domain}') - self.assertEqual(self.company_2.catchall_email, f'{self.alias_catchall}@{self.alias_domain}') + self.assertEqual(self.company_2.alias_domain_id, self.mail_alias_domain_c2) + self.assertEqual(self.company_2.bounce_email, f'{self.alias_bounce_c2}@{self.alias_domain_c2_name}') + self.assertEqual( + self.company_2.bounce_formatted, + formataddr((self.company_2.name, f'{self.alias_bounce_c2}@{self.alias_domain_c2_name}')) + ) + self.assertEqual(self.company_2.catchall_email, f'{self.alias_catchall_c2}@{self.alias_domain_c2_name}') self.assertEqual( self.company_2.catchall_formatted, - formataddr((self.company_2.name, f'{self.alias_catchall}@{self.alias_domain}')) + formataddr((self.company_2.name, f'{self.alias_catchall_c2}@{self.alias_domain_c2_name}')) ) + self.assertEqual(self.company_2.default_from_email, f'{self.alias_default_from_c2}@{self.alias_domain_c2_name}') - self.assertEqual(self.company_3.catchall_email, f'{self.alias_catchall}@{self.alias_domain}') + self.assertEqual(self.company_3.alias_domain_id, self.mail_alias_domain_c3) + self.assertEqual(self.company_3.bounce_email, f'{self.alias_bounce_c3}@{self.alias_domain_c3_name}') + self.assertEqual( + self.company_3.bounce_formatted, + formataddr((self.company_3.name, f'{self.alias_bounce_c3}@{self.alias_domain_c3_name}')) + ) + self.assertEqual(self.company_3.catchall_email, f'{self.alias_catchall_c3}@{self.alias_domain_c3_name}') self.assertEqual( self.company_3.catchall_formatted, - formataddr((self.company_3.name, f'{self.alias_catchall}@{self.alias_domain}')) + formataddr((self.company_3.name, f'{self.alias_catchall_c3}@{self.alias_domain_c3_name}')) ) + self.assertEqual(self.company_3.default_from_email, f'{self.alias_default_from_c3}@{self.alias_domain_c3_name}') @users('erp_manager') def test_res_company_creation_alias_domain(self): @@ -280,9 +339,158 @@ class TestAliasCompany(TestMailAliasCommon): }) company.flush_recordset() self.assertEqual( - company.catchall_formatted, - formataddr((company.name, f'{self.alias_catchall}@{self.alias_domain}')) - ) + company.alias_domain_id, self.mail_alias_domain, + 'Default alias domain: sequence based') + + # respect forced value + company = self.env['res.company'].create({ + 'alias_domain_id': self.mail_alias_domain_c2.id, + 'email': '"Yet Another Company" ', + 'name': 'Yet Another Company', + }) + self.assertEqual(company.alias_domain_id, self.mail_alias_domain_c2) + + +@tagged('mail_gateway', 'mail_alias', 'multi_company') +class TestMailAliasDomain(TestMailAliasCommon): + + @users('admin') + def test_alias_domain_config_alias_clash(self): + """ Domain names are not unique e.g. owning multiple gmail.com accounts. + However bounce / catchall should not clash with aliases. """ + alias_domain = self.mail_alias_domain.with_env(self.env) + + for domain_config in {'bounce_alias', 'catchall_alias'}: + with self.subTest(domain_config=domain_config): + with self.assertRaises(exceptions.ValidationError): + self.env['mail.alias.domain'].create({ + domain_config: self.test_alias_mc.alias_name, + 'name': 'another.domain.name.com', + }) + + # should not clash with existing aliases, to avoid valid aliases be + # considered as bounce / catchall + with self.assertRaises(exceptions.UserError), self.cr.savepoint(): + alias_domain.write({'bounce_alias': self.test_alias_mc.alias_name}) + with self.assertRaises(exceptions.UserError), self.cr.savepoint(): + alias_domain.write({'catchall_alias': self.test_alias_mc.alias_name}) + + @users('admin') + def test_alias_domain_config_unique(self): + """ Domain names are not unique e.g. owning multiple gmail.com accounts. + However bounce / catchall should be unique. """ + alias_domain = self.mail_alias_domain.with_env(self.env) + + # copying directly would duplicate bounce / catchall emails + with mute_logger('odoo.sql_db'), self.assertRaises(psycopg2.errors.UniqueViolation), self.cr.savepoint(): + new_alias_domain = alias_domain.copy() + + # same domain name is authorized if bounce and catchall are different + new_alias_domain = alias_domain.copy({ + 'bounce_alias': 'new.bounce', + 'catchall_alias': 'new.catchall', + }) + self.assertEqual(new_alias_domain.bounce_email, f'new.bounce@{alias_domain.name}') + self.assertEqual(new_alias_domain.catchall_email, f'new.catchall@{alias_domain.name}') + self.assertEqual(new_alias_domain.name, alias_domain.name) + + # check bounce / catchall are unique at create + self.env['mail.alias.domain'].create({ + 'bounce_alias': 'unique.bounce', + 'catchall_alias': 'unique.catchall', + 'name': alias_domain.name, + }) + # any not unique should raise UniqueViolation (SQL constraint fired after check) + with mute_logger('odoo.sql_db'), self.assertRaises(psycopg2.errors.UniqueViolation), self.cr.savepoint(): + self.env['mail.alias.domain'].create({ + 'bounce_alias': alias_domain.bounce_alias, + 'name': alias_domain.name, + }) + with mute_logger('odoo.sql_db'), self.assertRaises(psycopg2.errors.UniqueViolation), self.cr.savepoint(): + self.env['mail.alias.domain'].create({ + 'catchall_alias': alias_domain.catchall_alias, + 'name': alias_domain.name, + }) + + # also check write operation + with self.assertRaises(exceptions.ValidationError): + new_alias_domain.write({'bounce_alias': alias_domain.bounce_alias}) + with self.assertRaises(exceptions.ValidationError): + new_alias_domain.write({'catchall_alias': alias_domain.catchall_alias}) + + @users('admin') + def test_alias_domain_parameters_validation(self): + """ Test validation of bounce and catchall fields of alias domain as + well as sanitization. """ + alias_domain = self.mail_alias_domain.with_env(self.env) + + # sanitization of name (both create and write) + for failing_name in [ + 'outlook.fr, gmail.com', + # accents + 'provaïder', + 'provaïder.cöm', + # fail + '', ' ', + ]: + with self.subTest(failing_name=failing_name): + with self.assertRaises(exceptions.ValidationError): + new_domain = self.env['mail.alias.domain'].create({'name': failing_name}) + + with self.assertRaises(exceptions.ValidationError): + alias_domain.write({'name': failing_name}) + + # sanitization of bounce / catchall + for (bounce_alias, catchall_alias, default_from), (exp_bounce, exp_catchall, exp_default_from) in zip( + [ + ( + 'bounce+b4r=*R3wl_#_-$€{}[]()~|\\/!?&%^\'"`~', + 'catchall+b4r=*R3wl_#_-$€{}[]()~|\\/!?&%^\'"`~', + 'notifications+b4r=*R3wl_#_-$€{}[]()~|\\/!?&%^\'"`~', + ), + ('bounce+😊', 'catchall+😊', 'notifications+😊'), + ('Bouncâïde 😊', 'Catchôïee 😊', 'Notificâtïons 😊'), + ('ぁ', 'ぁぁ', 'ぁぁぁ'), + ], + [ + ( + 'bounce+b4r=*r3wl_#_-$-{}-~|-/!?&%^\'-`~', + 'catchall+b4r=*r3wl_#_-$-{}-~|-/!?&%^\'-`~', + 'notifications+b4r=*r3wl_#_-$-{}-~|-/!?&%^\'-`~', + ), + ('bounce+-', 'catchall+-', 'notifications+-'), + ('bouncaide-', 'catchoiee-', 'notifications-'), + ('?', '??', '???'), + ] + ): + with self.subTest(bounce_alias=bounce_alias): + alias_domain.write({'bounce_alias': bounce_alias}) + self.assertEqual(alias_domain.bounce_alias, exp_bounce) + with self.subTest(catchall_alias=catchall_alias): + alias_domain.write({'catchall_alias': catchall_alias}) + self.assertEqual(alias_domain.catchall_alias, exp_catchall) + with self.subTest(default_from=default_from): + alias_domain.write({'default_from': default_from}) + self.assertEqual(alias_domain.default_from, exp_default_from) + + # falsy values + for config_value in [False, None, '', ' ']: + with self.subTest(config_value=config_value): + alias_domain.write({'bounce_alias': config_value}) + self.assertFalse(alias_domain.bounce_alias) + alias_domain.write({'catchall_alias': config_value}) + self.assertFalse(alias_domain.catchall_alias) + alias_domain.write({'default_from': config_value}) + self.assertFalse(alias_domain.default_from) + + # check successive param set, should not raise for unicity against itself + for _ in range(2): + alias_domain.write({ + 'bounce_alias': 'bounce+double.test', + 'catchall_alias': 'catchall+double.test', + }) + self.assertEqual(alias_domain.bounce_alias, 'bounce+double.test') + self.assertEqual(alias_domain.catchall_alias, 'catchall+double.test') @tagged('mail_gateway', 'mail_alias', 'multi_company') From e8a41762ad599e08535cdfef9bcb433343b7d285 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Fri, 22 Oct 2021 16:47:46 +0200 Subject: [PATCH 07/18] [IMP] mail: link 'mail.alias_domain' to aliases 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 Add 'mail.alias.domain' information on alias model. Aliases do not use global configuration parameters anymore. Instead they are linked to an alias domain e.g. 'sales' linked to 'mycompany.com' alias domain: 'sales@mycompany.com'. It is now considered as a different alias compared to 'sales@mycompany.in' which has the same alias_name but a different alias domain. Constraints and checks are added, as * uniqueness of aliases is now checked inside a given domain; * an alias name must not clash with its domain bounce or catchall; * a combination of (alias_name, alias_domain_id) must be unique; Crm multi-company environment is also updated to match the new alias domain behavior. Task-36879 (Mail: Support Multi Domains Aliases) Part-of: odoo/odoo#76734 --- addons/crm/tests/common.py | 12 ++ .../crm/tests/test_crm_lead_notification.py | 26 ++- .../hr_expense/models/res_config_settings.py | 1 + .../models/hr_recruitment_source.py | 1 + addons/mail/models/mail_alias.py | 191 +++++++++-------- addons/mail/models/mail_alias_domain.py | 17 +- .../mail/models/mail_alias_mixin_optional.py | 68 +++++- addons/mail/models/models.py | 49 ++++- addons/mail/views/mail_alias_views.xml | 10 +- addons/test_mail/tests/test_mail_alias.py | 199 ++++++++++++++---- 10 files changed, 426 insertions(+), 148 deletions(-) diff --git a/addons/crm/tests/common.py b/addons/crm/tests/common.py index 468f8cd46cc..053c8aaca71 100644 --- a/addons/crm/tests/common.py +++ b/addons/crm/tests/common.py @@ -265,6 +265,18 @@ class TestCrmCommon(TestSalesCommon, MailCase): 'email': 'company.2@test.example.com', 'name': 'New Test Company', }) + cls.alias_bounce_c2 = 'bounce.c2' + cls.alias_catchall_c2 = 'catchall.c2' + cls.alias_default_from_c2 = 'notifications.c2' + cls.alias_domain_c2_name = 'test.mycompany2.com' + cls.mail_alias_domain_c2 = cls.env['mail.alias.domain'].create({ + 'bounce_alias': cls.alias_bounce_c2, + 'catchall_alias': cls.alias_catchall_c2, + 'company_ids': [(4, cls.company_2.id)], + 'default_from': cls.alias_default_from_c2, + 'name': cls.alias_domain_c2_name, + 'sequence': 2, + }) cls.user_sales_manager_mc = mail_new_test_user( cls.env, diff --git a/addons/crm/tests/test_crm_lead_notification.py b/addons/crm/tests/test_crm_lead_notification.py index 046271094c3..1ecc805ad86 100644 --- a/addons/crm/tests/test_crm_lead_notification.py +++ b/addons/crm/tests/test_crm_lead_notification.py @@ -14,6 +14,8 @@ class NewLeadNotification(TestCrmCommon): def setUpClass(cls): """ Activate some langs to test lang propagation in various mail flows """ super(NewLeadNotification, cls).setUpClass() + cls._activate_multi_company() + cls.test_email = '"Test Email" ' model_lang = cls.env['res.lang'].sudo().with_context(active_test=False) @@ -221,14 +223,14 @@ class NewLeadNotification(TestCrmCommon): @mute_logger('odoo.addons.mail.models.mail_thread') def test_new_lead_from_email_multicompany(self): company0 = self.env.company - company1 = self.env['res.company'].create({'name': 'new_company'}) + company1 = self.company_2 self.env.user.write({ 'company_ids': [(4, company0.id, False), (4, company1.id, False)], }) - crm_team_model = self.env['ir.model'].search([('model', '=', 'crm.team')]) - crm_lead_model = self.env['ir.model'].search([('model', '=', 'crm.lead')]) + crm_team_model_id = self.env['ir.model']._get_id('crm.team') + crm_lead_model_id = self.env['ir.model']._get_id('crm.lead') crm_team0 = self.env['crm.team'].create({ 'name': 'crm team 0', @@ -240,16 +242,18 @@ class NewLeadNotification(TestCrmCommon): }) mail_alias0 = self.env['mail.alias'].create({ + 'alias_domain_id': company0.alias_domain_id.id, 'alias_name': 'sale_team_0', - 'alias_model_id': crm_lead_model.id, - 'alias_parent_model_id': crm_team_model.id, + 'alias_model_id': crm_lead_model_id, + 'alias_parent_model_id': crm_team_model_id, 'alias_parent_thread_id': crm_team0.id, 'alias_defaults': "{'type': 'opportunity', 'team_id': %s}" % crm_team0.id, }) mail_alias1 = self.env['mail.alias'].create({ + 'alias_domain_id': company1.alias_domain_id.id, 'alias_name': 'sale_team_1', - 'alias_model_id': crm_lead_model.id, - 'alias_parent_model_id': crm_team_model.id, + 'alias_model_id': crm_lead_model_id, + 'alias_parent_model_id': crm_team_model_id, 'alias_parent_thread_id': crm_team1.id, 'alias_defaults': "{'type': 'opportunity', 'team_id': %s}" % crm_team1.id, }) @@ -257,12 +261,12 @@ class NewLeadNotification(TestCrmCommon): crm_team0.write({'alias_id': mail_alias0.id}) crm_team1.write({'alias_id': mail_alias1.id}) - new_message0 = """MIME-Version: 1.0 + new_message0 = f"""MIME-Version: 1.0 Date: Thu, 27 Dec 2018 16:27:45 +0100 Message-ID: Subject: sale team 0 in company 0 From: A client -To: sale_team_0@test.mycompany.com +To: {mail_alias0.display_name} Content-Type: multipart/alternative; boundary="000000000000a47519057e029630" --000000000000a47519057e029630 @@ -278,12 +282,12 @@ Content-Transfer-Encoding: quoted-printable --000000000000a47519057e029630-- """ - new_message1 = """MIME-Version: 1.0 + new_message1 = f"""MIME-Version: 1.0 Date: Thu, 27 Dec 2018 16:27:45 +0100 Message-ID: Subject: sale team 1 in company 1 From: B client -To: sale_team_1@test.mycompany.com +To: {mail_alias1.display_name} Content-Type: multipart/alternative; boundary="000000000000a47519057e029630" --000000000000a47519057e029630 diff --git a/addons/hr_expense/models/res_config_settings.py b/addons/hr_expense/models/res_config_settings.py index c82e228920f..852e0d81601 100644 --- a/addons/hr_expense/models/res_config_settings.py +++ b/addons/hr_expense/models/res_config_settings.py @@ -40,6 +40,7 @@ class ResConfigSettings(models.TransientModel): # create data again alias = self.env['mail.alias'].sudo().create({ 'alias_contact': 'employees', + 'alias_domain_id': self.env.company.alias_domain_id.id, 'alias_model_id': self.env['ir.model']._get_id('hr.expense'), 'alias_name': self.hr_expense_alias_prefix, }) diff --git a/addons/hr_recruitment/models/hr_recruitment_source.py b/addons/hr_recruitment/models/hr_recruitment_source.py index 2b8bd6ea3c2..8a3e765422e 100644 --- a/addons/hr_recruitment/models/hr_recruitment_source.py +++ b/addons/hr_recruitment/models/hr_recruitment_source.py @@ -30,6 +30,7 @@ class RecruitmentSource(models.Model): 'medium_id': medium.id, 'source_id': source.source_id.id, }, + 'alias_domain_id': source.job_id.company_id.alias_domain_id.id or self.env.company.alias_domain_id.id, 'alias_model_id': self.env['ir.model']._get_id('hr.applicant'), 'alias_name': f"{source.job_id.alias_name or source.job_id.name}+{source.name}", 'alias_parent_thread_id': source.job_id.id, diff --git a/addons/mail/models/mail_alias.py b/addons/mail/models/mail_alias.py index c06f45919aa..40f39d05240 100644 --- a/addons/mail/models/mail_alias.py +++ b/addons/mail/models/mail_alias.py @@ -3,11 +3,12 @@ import ast import re - +from collections import defaultdict from markupsafe import Markup from odoo import _, api, fields, models from odoo.exceptions import ValidationError, UserError +from odoo.osv import expression from odoo.tools import is_html_empty, remove_accents # see rfc5322 section 3.2.3 @@ -37,7 +38,10 @@ class Alias(models.Model): alias_name = fields.Char( 'Alias Name', copy=False, help="The name of the email alias, e.g. 'jobs' if you want to catch emails for ") - alias_domain = fields.Char('Alias domain', compute='_compute_alias_domain') + alias_domain_id = fields.Many2one( + 'mail.alias.domain', string='Alias Domain', ondelete='restrict', + default=lambda self: self.env.company.alias_domain_id) + alias_domain = fields.Char('Alias domain name', related='alias_domain_id.name') # target: create / update alias_model_id = fields.Many2one('ir.model', 'Aliased Model', required=True, ondelete="cascade", help="The model (Odoo Document Kind) to which this alias " @@ -86,9 +90,14 @@ class Alias(models.Model): ], compute='_compute_alias_status', store=True, help='Alias status assessed on the last message received.') - _sql_constraints = [ - ('alias_unique', 'UNIQUE(alias_name)', 'Unfortunately this email alias is already used, please choose a unique one') - ] + def init(self): + """Make sure there aren't multiple records for the same name and alias + domain. Not in _sql_constraint because COALESCE is not supported for + PostgreSQL constraint. """ + self.env.cr.execute(""" + CREATE UNIQUE INDEX IF NOT EXISTS mail_alias_name_domain_unique + ON mail_alias (alias_name, COALESCE(alias_domain_id, 0)) + """) @api.constrains('alias_name') def _check_alias_is_ascii(self): @@ -115,6 +124,19 @@ class Alias(models.Model): _('Invalid expression, it must be a literal python dictionary definition e.g. "{\'field\': \'value\'}"') ) from e + @api.constrains('alias_name', 'alias_domain_id') + def _check_alias_domain_clash(self): + """ Within a given alias domain, aliases should not conflict with bounce + or catchall email addresses, as emails should be unique for the gateway. """ + failing = self.filtered(lambda alias: alias.alias_name and alias.alias_name in [ + alias.alias_domain_id.bounce_alias, alias.alias_domain_id.catchall_alias + ]) + if failing: + raise ValidationError( + _('Aliases %(alias_names)s is already used as bounce or catchall address. Please choose another alias.', + alias_names=', '.join(failing.mapped('display_name'))) + ) + @api.depends('alias_domain', 'alias_name') def _compute_display_name(self): """ Return the mail alias display alias_name, including the catchall @@ -128,10 +150,6 @@ class Alias(models.Model): else: record.display_name = _("Inactive Alias") - @api.depends('alias_name') - def _compute_alias_domain(self): - self.alias_domain = self.env["ir.config_parameter"].sudo().get_param("mail.catchall.domain") - @api.depends('alias_contact', 'alias_defaults', 'alias_model_id') def _compute_alias_status(self): """Reset alias_status to "not_tested" when fields, that can be the source of an error, are modified.""" @@ -139,33 +157,48 @@ class Alias(models.Model): @api.model_create_multi def create(self, vals_list): - """ Creates email.alias records according to the values provided in - ``vals`` with 1 alteration: + """ Creates mail.alias records according to the values provided in + ``vals`` but sanitize 'alias_name' by replacing certain unsafe + characters; set default alias domain if not given. - * ``alias_name`` value may be cleaned by replacing certain unsafe - characters; - - :raise UserError: if given alias_name is already assigned or there are - duplicates in given vals_list; + :raise UserError: if given (alias_name, alias_domain_id) already exists + or if there are duplicates in given vals_list; """ - alias_names = [self._sanitize_alias_name(vals.get('alias_name')) for vals in vals_list] - self._check_unique(alias_names) - for vals, alias_name in zip(vals_list, alias_names): - vals['alias_name'] = alias_name + alias_names, alias_domains = [], [] + for vals in vals_list: + vals['alias_name'] = self._sanitize_alias_name(vals.get('alias_name')) + alias_names.append(vals['alias_name']) + vals['alias_domain_id'] = vals.get('alias_domain_id', self.env.company.alias_domain_id.id) + alias_domains.append(self.env['mail.alias.domain'].browse(vals['alias_domain_id'])) + + self._check_unique(alias_names, alias_domains) return super().create(vals_list) def write(self, vals): - """ Raise UserError with a meaningfull message instead of letting the - unicity constraint give its error. """ + """ Raise UserError with a meaningful message instead of letting the + uniqueness constraint raise an SQL error. To check uniqueness we have + to rebuild pairs of names / domains to validate, taking into account + that a void alias_domain_id is acceptable (but also raises for + uniqueness). + """ + alias_names, alias_domains = [], [] if 'alias_name' in vals: vals['alias_name'] = self._sanitize_alias_name(vals['alias_name']) if vals.get('alias_name') and self.ids: - self._check_unique([vals['alias_name']]) - if len(self) > 1: - raise UserError( - _('Email alias %(alias_name)s cannot be used on %(count)d records at the same time. Please update records one by one.', - alias_name=vals['alias_name'], count=len(self)) - ) + alias_names = [vals['alias_name']] * len(self) + elif 'alias_name' not in vals and 'alias_domain_id' in vals: + # avoid checking when writing the same value + if [vals['alias_domain_id']] != self.alias_domain_id.ids: + alias_names = self.filtered('alias_name').mapped('alias_name') + + if alias_names: + tocheck_records = self if vals.get('alias_name') else self.filtered('alias_name') + if 'alias_domain_id' in vals: + alias_domains = [self.env['mail.alias.domain'].browse(vals['alias_domain_id'])] * len(tocheck_records) + else: + alias_domains = [record.alias_domain_id for record in tocheck_records] + self._check_unique(alias_names, alias_domains) + return super().write(vals) def _clean_and_check_mail_catchall_allowed_domains(self, value): @@ -180,70 +213,64 @@ class Alias(models.Model): ) return ",".join(value) - def _check_unique(self, sanitized_names, skip_icp_keys=None): + def _check_unique(self, alias_names, alias_domains): """ Check unicity constraint won't be raised, otherwise raise a UserError with a complete error message. Also check unicity against alias config parameters. - :param list sanitized_names: a list of names (considered as sanitized + :param list alias_names: a list of names (considered as sanitized and ready to be sent to DB); + :param list alias_domains: list of alias_domain records under which + the check is performed, as uniqueness is performed for given pair + (name, alias_domain); """ - valid_names = list(filter(None, sanitized_names)) + if len(alias_names) != len(alias_domains): + msg = (f"Invalid call to '_check_unique': names and domains should make coherent lists, " + f"received {', '.join(alias_names)} and {', '.join(alias_domains.mapped('name'))}") + raise ValueError(msg) - # list itself should be unique obviously - seen = set() - dupes = [name for name in valid_names if name in seen or seen.add(name)] - if dupes: - raise UserError( - _('Email aliases %(alias_name)s cannot be used on several records at the same time. Please update records one by one.', - alias_name=', '.join(dupes)) - ) - - alias_domain = self.env["ir.config_parameter"].sudo().get_param("mail.catchall.domain") - icp_to_check = dict( - (icp_key, self.env["ir.config_parameter"].sudo().get_param(icp_key)) - for icp_key in {'mail.bounce.alias', 'mail.catchall.alias'} - if icp_key not in (skip_icp_keys or ()) - ) - icp_label_by_key = { - 'mail.bounce.alias': _('bounce'), - 'mail.catchall.alias': _('catchall'), - } - - # matches catchall or bounce alias - for sanitized_name in valid_names: - for icp_key, icp_value in icp_to_check.items(): - if icp_value and sanitized_name == icp_value: - matching_alias_name = f'{sanitized_name}@{alias_domain}' if alias_domain else sanitized_name - raise UserError( - _('The e-mail alias %(matching_alias_name)s is already used as %(alias_duplicate)s alias. Please choose another alias.', - matching_alias_name=matching_alias_name, - alias_duplicate=icp_label_by_key[icp_key]) - ) + # reorder per alias domain, keep only not void alias names (void domain also checks uniqueness) + domain_to_names = defaultdict(list) + for alias_name, alias_domain in zip(alias_names, alias_domains): + if alias_name and alias_name in domain_to_names[alias_domain]: + raise UserError( + _('Email aliases %(alias_name)s cannot be used on several records at the same time. Please update records one by one.', + alias_name=alias_name) + ) + if alias_name: + domain_to_names[alias_domain].append(alias_name) # matches existing alias - matching_alias = self.env['mail.alias'] - if valid_names: - domain = [('alias_name', 'in', valid_names)] - if self: - domain += [('id', 'not in', self.ids)] - matching_alias = self.search(domain, limit=1) - if not matching_alias: + domain = expression.OR([ + ['&', ('alias_name', 'in', alias_names), ('alias_domain_id', '=', alias_domain.id)] + for alias_domain, alias_names in domain_to_names.items() + ]) + if domain and self: + domain = expression.AND([domain, [('id', 'not in', self.ids)]]) + existing = self.search(domain, limit=1) if domain else self.env['mail.alias'] + if not existing: return - if matching_alias.alias_parent_model_id and matching_alias.alias_parent_thread_id: - # If parent model and parent thread ID both are set, display document name also in the warning - document_name = self.env[matching_alias.alias_parent_model_id.model].sudo().browse(matching_alias.alias_parent_thread_id).display_name - raise UserError( - _('The e-mail alias %(matching_alias_name)s is already used by the %(document_name)s %(model_name)s. Choose another alias or change it on the other document.', - matching_alias_name=matching_alias.display_name, - document_name=document_name, - model_name=matching_alias.alias_parent_model_id.name) - ) - raise UserError( - _('The e-mail alias %(matching_alias_name)s is already linked with %(alias_model_name)s. Choose another alias or change it on the linked model.', - matching_alias_name=matching_alias.display_name, - alias_model_name=matching_alias.alias_model_id.name) - ) + if existing.alias_parent_model_id and existing.alias_parent_thread_id: + parent_name = self.env[existing.alias_parent_model_id.model].sudo().browse(existing.alias_parent_thread_id).display_name + msg_begin = _( + 'Alias %(matching_name)s (%(current_id)s) is already linked with %(alias_model_name)s (%(matching_id)s) and used by the %(parent_name)s %(parent_model_name)s.', + alias_model_name=existing.alias_model_id.name, + current_id=self.ids if self else _('your alias'), + matching_id=existing.id, + matching_name=existing.display_name, + parent_name=parent_name, + parent_model_name=existing.alias_parent_model_id.name + ) + else: + msg_begin = _( + 'Alias %(matching_name)s (%(current_id)s) is already linked with %(alias_model_name)s (%(matching_id)s).', + alias_model_name=existing.alias_model_id.name, + current_id=self.ids if self else _('new'), + matching_id=existing.id, + matching_name=existing.display_name, + ) + msg_end = _('Choose another value or change it on the other document.') + raise UserError(f'{msg_begin} {msg_end}') @api.model def _sanitize_alias_name(self, name): diff --git a/addons/mail/models/mail_alias_domain.py b/addons/mail/models/mail_alias_domain.py index b5ab29bff91..995976bd926 100644 --- a/addons/mail/models/mail_alias_domain.py +++ b/addons/mail/models/mail_alias_domain.py @@ -80,9 +80,15 @@ class AliasDomain(models.Model): catchall=tocheck.catchall_email) ) - existing = self.env['mail.alias'].search( - [('alias_name', 'in', list(set(names)))], - limit=1, + # search on left-part only to speedup, then filter on right part + potential_aliases = self.env['mail.alias'].search([ + ('alias_name', 'in', list(set(names))), + ('alias_domain_id', '!=', False) + ]) + existing = next( + (alias for alias in potential_aliases + if alias.display_name in (self.mapped('bounce_email') + self.mapped('catchall_email'))), + self.env['mail.alias'] ) if existing: document_name = False @@ -126,11 +132,14 @@ class AliasDomain(models.Model): alias_domains = super().create(vals_list) - # alias domain init: populate companies at first creation + # alias domain init: populate companies and aliases at first creation if alias_domains and self.search_count([]) == len(alias_domains): self.env['res.company'].search( [('alias_domain_id', '=', False)] ).alias_domain_id = alias_domains[0].id + self.env['mail.alias'].sudo().search( + [('alias_domain_id', '=', False)] + ).alias_domain_id = alias_domains[0].id return alias_domains diff --git a/addons/mail/models/mail_alias_mixin_optional.py b/addons/mail/models/mail_alias_mixin_optional.py index 95b9edf5e19..271e4eb3291 100644 --- a/addons/mail/models/mail_alias_mixin_optional.py +++ b/addons/mail/models/mail_alias_mixin_optional.py @@ -20,14 +20,13 @@ class AliasMixinOptional(models.AbstractModel): alias_id = fields.Many2one('mail.alias', string='Alias', ondelete="restrict", required=False) alias_name = fields.Char(related='alias_id.alias_name', readonly=False) - alias_domain = fields.Char('Alias domain', compute='_compute_alias_domain') + alias_domain_id = fields.Many2one( + 'mail.alias.domain', string='Alias Domain', + related='alias_id.alias_domain_id', readonly=False) + alias_domain = fields.Char('Alias Domain Name', related='alias_id.alias_domain') alias_defaults = fields.Text(related='alias_id.alias_defaults') alias_email = fields.Char('Email Alias', compute='_compute_alias_email') - @api.depends('alias_name') - def _compute_alias_domain(self): - self.alias_domain = self.env["ir.config_parameter"].sudo().get_param("mail.catchall.domain") - @api.depends('alias_domain', 'alias_name') def _compute_alias_email(self): """ Alias email can be used in views, as it is Falsy when having no domain @@ -45,15 +44,29 @@ class AliasMixinOptional(models.AbstractModel): def create(self, vals_list): """ Create aliases using sudo if an alias is required, notably if its name is given. """ + # prefetch company information, used for alias domain + company_fname = self._mail_get_company_field() + if company_fname: + company_id_default = self.default_get([company_fname]).get(company_fname) or self.env.company.id + company_prefetch_ids = {vals[company_fname] for vals in vals_list if vals.get(company_fname)} + company_prefetch_ids.add(company_id_default) + else: + company_id_default = self.env.company.id + company_prefetch_ids = {company_id_default} + # prepare all alias values alias_vals_list, record_vals_list = [], [] for vals in vals_list: if vals.get('alias_name'): vals['alias_name'] = self.env['mail.alias']._sanitize_alias_name(vals['alias_name']) if self._require_new_alias(vals): + company_id = vals.get(company_fname) or company_id_default + company = self.env['res.company'].with_prefetch(company_prefetch_ids).browse(company_id) alias_vals, record_vals = self._alias_filter_fields(vals) # generate record-agnostic base alias values - alias_vals.update(self.env[self._name]._alias_get_creation_values()) + alias_vals.update(self.env[self._name].with_context( + default_alias_domain_id=company.alias_domain_id.id, + )._alias_get_creation_values()) alias_vals_list.append(alias_vals) record_vals_list.append(record_vals) @@ -75,15 +88,20 @@ class AliasMixinOptional(models.AbstractModel): records = super().create(valid_vals_list) - for record in records.filtered('alias_id'): - record.alias_id.sudo().write(record._alias_get_creation_values()) + # update alias values with values coming from record, post-create to have + # access to all its values (notably its ID) + records_walias = records.filtered('alias_id') + for record in records_walias: + alias_values = record._alias_get_creation_values() + record.alias_id.sudo().write(alias_values) return records def write(self, vals): """ Split writable fields of mail.alias and other fields alias fields will - write with sudo and the other normally. If alias does not exist and we - try to set a name, create the alias automatically. """ + write with sudo and the other normally. Also handle alias_domain_id + update. If alias does not exist and we try to set a name, create the + alias automatically. """ # create missing aliases if vals.get('alias_name'): alias_create_values = [ @@ -101,6 +119,14 @@ class AliasMixinOptional(models.AbstractModel): alias_vals, record_vals = self._alias_filter_fields(vals, filters=self.ALIAS_WRITEABLE_FIELDS) if record_vals: super().write(record_vals) + + # synchronize alias domain if company environment changed + company_fname = self._mail_get_company_field() + if company_fname in vals: + alias_domain_values = self.filtered('alias_id')._alias_get_alias_domain_id() + for record, alias_domain_id in alias_domain_values.items(): + record.sudo().alias_domain_id = alias_domain_id.id + if alias_vals and (record_vals or self.check_access_rights('write', raise_exception=False)): self.mapped('alias_id').sudo().write(alias_vals) @@ -131,14 +157,34 @@ class AliasMixinOptional(models.AbstractModel): # MIXIN TOOL OVERRIDE METHODS # -------------------------------------------------- + def _alias_get_alias_domain_id(self): + """ Return alias domain value to synchronize with owner's company. + Implementing it with a compute is complicated, as its 'alias_domain_id' + is a field on 'mail.alias' model, coming from 'alias_id' field and due + to current implementation of the mixin, notably the create / write + overrides, compute is not called in all cases. We therefore use a tool + method to call in the mixin. """ + alias_domain_values = {} + record_companies = self._mail_get_companies() + for record in self: + record_company = record_companies[record.id] + alias_domain_values[record] = ( + record_company.alias_domain_id + or record.alias_domain_id or self.env.company.alias_domain_id + ) + return alias_domain_values + def _alias_get_creation_values(self): """ Return values to create an alias, or to write on the alias after its creation. """ - return { + values = { 'alias_parent_thread_id': self.id if self.id else False, 'alias_parent_model_id': self.env['ir.model']._get_id(self._name), } + if self.env.context.get('default_alias_domain_id'): + values['alias_domain_id'] = self.env.context['default_alias_domain_id'] + return values def _alias_filter_fields(self, values, filters=False): """ Split the vals dict into two dictionnary of vals, one for alias diff --git a/addons/mail/models/models.py b/addons/mail/models/models.py index fe6864fa3a6..3b7d39645ad 100644 --- a/addons/mail/models/models.py +++ b/addons/mail/models/models.py @@ -22,6 +22,51 @@ class BaseModel(models.AbstractModel): # FIELDS HELPERS # ------------------------------------------------------------ + def _mail_get_alias_domains(self, default_company=False): + """ Return alias domain linked to each record in self. It is based + on the company (record's company, environment company) and fallback + on the first found alias domain if configuration is not correct. + + :param default_company: default company in case records + have no company (or no company field); defaults to env.company; + + :return: for each record ID in self, found + """ + record_companies = self._mail_get_companies(default=(default_company or self.env.company)) + + # prepare default alias domain, fetch only if necessary + default_domain = (default_company or self.env.company).alias_domain_id + all_companies = self.env['res.company'].browse({comp.id for comp in record_companies.values()}) + # early optimization: search only if necessary + if not default_domain and any(not comp.alias_domain_id for comp in all_companies): + default_domain = self.env['mail.alias.domain'].search([], limit=1) + + return { + record.id: ( + record_companies[record.id].alias_domain_id or default_domain + ) + for record in self + } + + @api.model + def _mail_get_company_field(self): + return 'company_id' if 'company_id' in self else False + + def _mail_get_companies(self, default=False): + """ Return company linked to each record in self. + + :param default: default value if no company field is found + or if it holds a void value. Defaults to a void recordset; + + :return: for each record ID in self, found + """ + default_company = default or self.env['res.company'] + company_fname = self._mail_get_company_field() + return { + record.id: (record[company_fname] or default_company) if company_fname else default_company + for record in self + } + @api.model def _mail_get_partner_fields(self, introspect_fields=False): """ This method returns the fields to use to find the contact to link @@ -33,7 +78,7 @@ class BaseModel(models.AbstractModel): res.partner model. This is used notably when partners are mandatory like in voip; - :return list: list of valid field names that can be used to retrieve + :return: list of valid field names that can be used to retrieve a partner (customer) on the record; """ partner_fnames = [fname for fname in ('partner_id', 'partner_ids') if fname in self] @@ -49,7 +94,7 @@ class BaseModel(models.AbstractModel): :param bool introspect_fields: see '_mail_get_partner_fields'; - :return dict: for each record ID, a res.partner recordsets being default + :return: for each record ID, a res.partner recordsets being default customers to contact; """ partner_fields = self._mail_get_partner_fields(introspect_fields=introspect_fields) diff --git a/addons/mail/views/mail_alias_views.xml b/addons/mail/views/mail_alias_views.xml index d7572778253..e3cb6ab6deb 100644 --- a/addons/mail/views/mail_alias_views.xml +++ b/addons/mail/views/mail_alias_views.xml @@ -27,7 +27,11 @@
-

@

+

+ @ + +

+ @@ -82,6 +87,7 @@ + @@ -92,6 +98,8 @@ + Date: Tue, 26 Sep 2023 09:46:47 +0200 Subject: [PATCH 08/18] [IMP] mail: allow to change 'alias_domain_id' in alias mixin 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 As 'alias_domain' is now dynamic and not based on a configuration parameter it makes sense to be able to change it when having several alias domain. We now allow writing on 'alias_domain_id' in 'mail.alias.mixin(.optional)'. Users may now change the alias domain of the alias coming with the mixin when they have write access on the record. Task-36879 (Mail: Support Multi Domains Aliases) Part-of: odoo/odoo#76734 --- addons/mail/models/mail_alias_mixin_optional.py | 2 +- addons/test_mail/tests/test_mail_alias.py | 7 ++----- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/addons/mail/models/mail_alias_mixin_optional.py b/addons/mail/models/mail_alias_mixin_optional.py index 271e4eb3291..9d18e05c613 100644 --- a/addons/mail/models/mail_alias_mixin_optional.py +++ b/addons/mail/models/mail_alias_mixin_optional.py @@ -16,7 +16,7 @@ class AliasMixinOptional(models.AbstractModel): """ _name = 'mail.alias.mixin.optional' _description = 'Email Aliases Mixin (light)' - ALIAS_WRITEABLE_FIELDS = ['alias_name', 'alias_contact', 'alias_defaults', 'alias_bounced_content'] + ALIAS_WRITEABLE_FIELDS = ['alias_domain_id', 'alias_name', 'alias_contact', 'alias_defaults', 'alias_bounced_content'] alias_id = fields.Many2one('mail.alias', string='Alias', ondelete="restrict", required=False) alias_name = fields.Char(related='alias_id.alias_name', readonly=False) diff --git a/addons/test_mail/tests/test_mail_alias.py b/addons/test_mail/tests/test_mail_alias.py index ea386329109..dc12cd8b447 100644 --- a/addons/test_mail/tests/test_mail_alias.py +++ b/addons/test_mail/tests/test_mail_alias.py @@ -600,17 +600,14 @@ class TestMailAliasMixin(TestMailAliasCommon): self.assertEqual(record.alias_id.alias_contact, 'followers') record.write({ + 'alias_domain_id': self.mail_alias_domain_c2.id, 'alias_name': 'better.alias.test', 'alias_defaults': "{'default_name': 'defaults'}" }) + self.assertEqual(record.alias_domain, self.mail_alias_domain_c2.name) self.assertEqual(record.alias_id.alias_name, 'better.alias.test') self.assertEqual(record.alias_id.alias_defaults, "{'default_name': 'defaults'}") - with self.assertRaises(exceptions.AccessError): - record.write({ - 'alias_domain_id': self.mail_alias_domain_c2.id, - }) - with self.assertRaises(exceptions.AccessError): record.write({ 'alias_force_thread_id': 0, From 019c6e3b5cbaf3c2d417eab665395818a5e6e35d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Tue, 12 Sep 2023 10:26:42 +0200 Subject: [PATCH 09/18] [IMP] mail: check alias_domain_id / company usage 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 Now that alias domains are multiple and can be linked to companies it is easy to end up with configuration where an alias is using a domain of CompanyA while the owner and/or target record belongs to CompanyB. We want to avoid that situation and strengthen company separation. When changing alias domain of an alias check that it is not used in another company than the one define on * the owner record (using owner fields like the 'project.project' for 'project.task' creating aliases); * the target record (using update fields like the 'mail.group' for group aliases that routes emails to a specific group); If the new alias domain is linked to a company that is different from the company of any related record, raise an error as it could lead to invalid multi-company setup and record creation. If the new alias domain is different from the company domain but is not used in any company it is a valid configuration. It is just flexibility offered by multi-domains aliases. Task-36879 (Mail: Support Multi Domains Aliases) Part-of: odoo/odoo#76734 --- addons/mail/models/mail_alias.py | 73 +++++++++++++++++++++++ addons/test_mail/tests/test_mail_alias.py | 58 ++++++++++++++++++ 2 files changed, 131 insertions(+) diff --git a/addons/mail/models/mail_alias.py b/addons/mail/models/mail_alias.py index 40f39d05240..91288442b06 100644 --- a/addons/mail/models/mail_alias.py +++ b/addons/mail/models/mail_alias.py @@ -99,6 +99,79 @@ class Alias(models.Model): ON mail_alias (alias_name, COALESCE(alias_domain_id, 0)) """) + @api.constrains('alias_domain_id', 'alias_force_thread_id', 'alias_parent_model_id', + 'alias_parent_thread_id', 'alias_model_id') + def _check_alias_domain_id_mc(self): + """ Check for invalid alias domains based on company configuration. + When having a parent record and/or updating an existing record alias + domain should match the one used on the related record. """ + tocheck = self.filtered(lambda domain: domain.alias_domain_id.company_ids) + if not tocheck: + return + + # helpers to find owner / target models + def _owner_model(alias): + return alias.alias_parent_model_id.model + def _owner_env(alias): + return self.env[_owner_model(alias)] + def _target_model(alias): + return alias.alias_model_id.model + def _target_env(alias): + return self.env[_target_model(alias)] + + # fetch impacted records, classify by model + recs_by_model = defaultdict(list) + for alias in tocheck: + # owner record (like 'project.project' for aliases creating new 'project.task') + if alias.alias_parent_model_id and alias.alias_parent_thread_id: + if _owner_env(alias)._mail_get_company_field(): + recs_by_model[_owner_model(alias)].append(alias.alias_parent_thread_id) + # target record (like 'mail.group' updating a given group) + if alias.alias_model_id and alias.alias_force_thread_id: + if _target_env(alias)._mail_get_company_field(): + recs_by_model[_target_model(alias)].append(alias.alias_force_thread_id) + + # helpers to fetch owner / target with prefetching + def _fetch_owner(alias): + if alias.alias_parent_thread_id in recs_by_model[alias.alias_parent_model_id.model]: + return _owner_env(alias).with_prefetch( + recs_by_model[_owner_model(alias)] + ).browse(alias.alias_parent_thread_id) + return None + def _fetch_target(alias): + if alias.alias_force_thread_id in recs_by_model[alias.alias_model_id.model]: + return _target_env(alias).with_prefetch( + recs_by_model[_target_model(alias)] + ).browse(alias.alias_force_thread_id) + return None + + # check company domains are compatible + for alias in tocheck: + if owner := _fetch_owner(alias): + company = owner[owner._mail_get_company_field()] + if company and company.alias_domain_id != alias.alias_domain_id and alias.alias_domain_id.company_ids: + raise ValidationError(_( + "We could not create alias %(alias_name)s because domain " + + "%(alias_domain_name)s belongs to company %(alias_company_names)s " + + "while the owner document belongs to company %(company_name)s.", + alias_company_names=','.join(alias.alias_domain_id.company_ids.mapped('name')), + alias_domain_name=alias.alias_domain_id.name, + alias_name=alias.display_name, + company_name=company.name, + )) + if target := _fetch_target(alias): + company = target[target._mail_get_company_field()] + if company and company.alias_domain_id != alias.alias_domain_id and alias.alias_domain_id.company_ids: + raise ValidationError(_( + "We could not create alias %(alias_name)s because domain " + + "%(alias_domain_name)s belongs to company %(alias_company_names)s " + + "while the target document belongs to company %(company_name)s.", + alias_company_names=','.join(alias.alias_domain_id.company_ids.mapped('name')), + alias_domain_name=alias.alias_domain_id.name, + alias_name=alias.display_name, + company_name=company.name, + )) + @api.constrains('alias_name') def _check_alias_is_ascii(self): """ The local-part ("display-name" ) of an diff --git a/addons/test_mail/tests/test_mail_alias.py b/addons/test_mail/tests/test_mail_alias.py index dc12cd8b447..7a43359aca7 100644 --- a/addons/test_mail/tests/test_mail_alias.py +++ b/addons/test_mail/tests/test_mail_alias.py @@ -90,6 +90,64 @@ class TestMailAlias(TestMailAliasCommon): self.assertEqual(ICP.get_param('mail.bounce.alias'), 'bounce+double.test') self.assertEqual(ICP.get_param('mail.catchall.alias'), 'catchall+double.test') + @users('erp_manager') + def test_alias_domain_company_check(self): + """ Check constraint trying to avoid ill-defined company setup aka + having an alias domain on parent record / record to update that does + not match the alias domain. """ + misc_alias_domain = self.env['mail.alias.domain'].create({'name': 'misc.com'}) + record_mc_c1, record_mc_c2 = self.env['mail.test.container.mc'].create([ + { + 'alias_name': 'Test1', + 'company_id': self.company_admin.id, + }, { + 'alias_name': 'Test2', + 'company_id': self.company_2.id, + } + ]) + alias_mc_c1, alias_mc_c2 = record_mc_c1.alias_id, record_mc_c2.alias_id + self.assertEqual( + (alias_mc_c1 + alias_mc_c2).alias_parent_model_id, + self.env['ir.model']._get('mail.test.container.mc')) + self.assertEqual( + (alias_mc_c1 + alias_mc_c2).mapped('alias_parent_thread_id'), + (record_mc_c1 + record_mc_c2).ids) + self.assertEqual(alias_mc_c1.alias_domain_id, self.mail_alias_domain) + self.assertEqual(alias_mc_c2.alias_domain_id, self.mail_alias_domain_c2) + + # mail_alias_domain_c2 is linked to a conflicting company + with self.assertRaises(exceptions.ValidationError): + record_mc_c1.alias_domain_id = self.mail_alias_domain_c2 + with self.assertRaises(exceptions.ValidationError): + alias_mc_c1.sudo().alias_domain_id = self.mail_alias_domain_c2 + # misc_alias_domain is not linked to any company, therefore ok + record_mc_c1.alias_domain_id = misc_alias_domain + + # alias updating records + record_upd_c1, record_upd_c2 = self.env['mail.test.alias.optional'].sudo().create([ + { + 'alias_name': 'Update C1', + 'company_id': self.company_admin.id, + }, { + 'alias_name': 'Update C2', + 'company_id': self.company_2.id, + } + ]) + alias_update_c1, alias_update_c2 = record_upd_c1.alias_id, record_upd_c2.alias_id + self.assertEqual( + (alias_update_c1 + alias_update_c2).mapped('alias_force_thread_id'), + (record_upd_c1 + record_upd_c2).ids) + self.assertEqual(alias_update_c1.alias_domain_id, self.mail_alias_domain) + self.assertEqual(alias_update_c2.alias_domain_id, self.mail_alias_domain_c2) + + # mail_alias_domain_c2 is linked to a conflicting company + with self.assertRaises(exceptions.ValidationError): + record_upd_c1.alias_domain_id = self.mail_alias_domain_c2 + with self.assertRaises(exceptions.ValidationError): + alias_update_c1.sudo().alias_domain_id = self.mail_alias_domain_c2 + # misc_alias_domain is not linked to any company, therefore ok + record_upd_c1.alias_domain_id = misc_alias_domain + @users('admin') def test_alias_name_unique(self): """ Check uniqueness constraint on alias names, at create and update. From bd2bdb5a1a9f969b1709c596c8577b51b1bae7ac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Thu, 27 Jul 2023 12:07:39 +0200 Subject: [PATCH 10/18] [IMP] mail: allow to search on alias complete email 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 Currently alias search is done only on 'alias_name' field. Now that alias domains can be multiple we have to be able to search on complete alias email definition e.g. when checking destination aliases of incoming emails in mail gateway. We add a new 'alias_full_name' field that is computed based on alias_name and alias_domain_id.name. It is stored so that search is possible on it. Allow to search on 'alias_full_name' from the 'mail.alias.mixin(.optional)' by making 'alias_email' field searchable, based on 'alias_full_name'. Task-36879 (Mail: Support Multi Domains Aliases) Part-of: odoo/odoo#76734 --- addons/mail/models/mail_alias.py | 14 +++ .../mail/models/mail_alias_mixin_optional.py | 5 +- addons/test_mail/tests/test_mail_alias.py | 89 +++++++++++++++++++ 3 files changed, 107 insertions(+), 1 deletion(-) diff --git a/addons/mail/models/mail_alias.py b/addons/mail/models/mail_alias.py index 91288442b06..590b86a7dd0 100644 --- a/addons/mail/models/mail_alias.py +++ b/addons/mail/models/mail_alias.py @@ -38,6 +38,8 @@ class Alias(models.Model): alias_name = fields.Char( 'Alias Name', copy=False, help="The name of the email alias, e.g. 'jobs' if you want to catch emails for ") + alias_full_name = fields.Char('Alias Email', compute='_compute_alias_full_name', store=True) + display_name = fields.Char(string='Display Name', compute='_compute_display_name') alias_domain_id = fields.Many2one( 'mail.alias.domain', string='Alias Domain', ondelete='restrict', default=lambda self: self.env.company.alias_domain_id) @@ -210,6 +212,18 @@ class Alias(models.Model): alias_names=', '.join(failing.mapped('display_name'))) ) + @api.depends('alias_domain_id.name', 'alias_name') + def _compute_alias_full_name(self): + """ A bit like display_name, but without the 'inactive alias' UI display. + Moreover it is stored, allowing to search on it. """ + for record in self: + if record.alias_domain_id and record.alias_name: + record.alias_full_name = f"{record.alias_name}@{record.alias_domain_id.name}" + elif record.alias_name: + record.alias_full_name = record.alias_name + else: + record.alias_full_name = False + @api.depends('alias_domain', 'alias_name') def _compute_display_name(self): """ Return the mail alias display alias_name, including the catchall diff --git a/addons/mail/models/mail_alias_mixin_optional.py b/addons/mail/models/mail_alias_mixin_optional.py index 9d18e05c613..fd8e383dd0e 100644 --- a/addons/mail/models/mail_alias_mixin_optional.py +++ b/addons/mail/models/mail_alias_mixin_optional.py @@ -25,7 +25,7 @@ class AliasMixinOptional(models.AbstractModel): related='alias_id.alias_domain_id', readonly=False) alias_domain = fields.Char('Alias Domain Name', related='alias_id.alias_domain') alias_defaults = fields.Text(related='alias_id.alias_defaults') - alias_email = fields.Char('Email Alias', compute='_compute_alias_email') + alias_email = fields.Char('Email Alias', compute='_compute_alias_email', search='_search_alias_email') @api.depends('alias_domain', 'alias_name') def _compute_alias_email(self): @@ -36,6 +36,9 @@ class AliasMixinOptional(models.AbstractModel): for record in self.filtered(lambda rec: rec.alias_name and rec.alias_domain): record.alias_email = f"{record.alias_name}@{record.alias_domain}" + def _search_alias_email(self, operator, operand): + return [('alias_id.alias_full_name', operator, operand)] + # -------------------------------------------------- # CRUD # -------------------------------------------------- diff --git a/addons/test_mail/tests/test_mail_alias.py b/addons/test_mail/tests/test_mail_alias.py index 7a43359aca7..13116ff19fb 100644 --- a/addons/test_mail/tests/test_mail_alias.py +++ b/addons/test_mail/tests/test_mail_alias.py @@ -348,6 +348,69 @@ class TestMailAlias(TestMailAliasCommon): for alias in aliases: self.assertEqual(alias.alias_name, False) + @users('admin') + def test_search(self): + """ Test search on aliases, notably searching on display_name which should + be split on searching on alias_name and alias_domain_id. """ + # ensure existing aliases to ease future asserts + existing = self.env['mail.alias'].search([('alias_domain_id', '!=', False)]) + self.assertEqual(existing.alias_domain_id, self.mail_alias_domain) + existing.write({'alias_name': False}) # don't be annoyed by existing aliases + + mail_alias_domain = self.mail_alias_domain.with_env(self.env) + mail_alias_domain_c2 = self.mail_alias_domain_c2.with_env(self.env) + self.assertEqual(mail_alias_domain.name, 'test.mycompany.com') + self.assertEqual(mail_alias_domain_c2.name, 'test.mycompany2.com') + + aliases = self.env['mail.alias'].create([ + { + 'alias_model_id': self.env['ir.model']._get('mail.test.container.mc').id, + 'alias_name': f'test.search.{idx}', + 'alias_domain_id': domain.id, + } + for idx in range(5) + for domain in (mail_alias_domain + mail_alias_domain_c2) + ]) + aliases_d1 = aliases.filtered(lambda a: a.alias_domain_id == mail_alias_domain) + aliases_d2 = aliases.filtered(lambda a: a.alias_domain_id == mail_alias_domain_c2) + + # search on alias_name: classic search + self.assertEqual( + self.env['mail.alias'].search([('alias_name', 'ilike', 'test.search')]), + aliases + ) + + # search on alias_fullname: search on aggregated of {name}@{domain} + for search_term, expected, msg in [ + ('mycompany', aliases, + 'Match all aliases on both domains as "mycompany" is contained in those two'), + (mail_alias_domain.name, aliases_d1, + 'Exact match on domain 1: should find all aliases in that domain'), + (mail_alias_domain_c2.name, aliases_d2, + 'Exact match on domain 2: should find all aliases in that domain'), + ('search.0@test.mycompany', aliases.filtered(lambda a: a.alias_name == 'test.search.0'), + 'Match in both domains'), + ('search.0@test.mycompany.com', aliases.filtered(lambda a: a.alias_name == 'test.search.0' and a.alias_domain_id == mail_alias_domain), + 'Match only in domain 1'), + ('search@test.mycompany.com', self.env['mail.alias'], + 'Does not match even as ilike'), + ]: + with self.subTest(search_term=search_term): + self.assertEqual( + self.env['mail.alias'].search([('alias_full_name', 'ilike', search_term)]), + expected, msg + ) + + # search using IN operator + for search_term, expected, msg in [ + (['mycompany'], self.env['mail.alias'], 'mycompany is too vague: does not match a left- and right- part (!= ilike)'), + ([mail_alias_domain.name], self.env['mail.alias'], 'Match only right-part of aliases emails'), + ]: + with self.subTest(search_term=search_term): + self.assertEqual(self.env['mail.alias'].search([('alias_full_name', 'in', search_term)]), + expected, msg + ) + @users('admin') def test_alias_setup(self): """ Test various constraints / configuration of alias model""" @@ -679,6 +742,32 @@ class TestMailAliasMixin(TestMailAliasCommon): with self.assertRaises(exceptions.ValidationError): record.write({'alias_defaults': "{'custom_field': brokendict"}) + @users('erp_manager') + def test_alias_mixin_alias_email(self): + """ Test 'alias_email' mixin field computation and search capability """ + Model = self.env['mail.test.container.mc'] + records = Model.create([ + { + 'alias_name': f'alias.email.{idx}', # will be present in all companies + 'company_id': company.id, + 'name': f'Test {company.id} {idx}', + } + for company in (self.company_admin, self.company_2) + for idx in range(5) + ]) + self.assertEqual( + Model.search([('alias_email', 'ilike', 'alias.email')]), records, + 'Search: partial search: any domain, matching all left parts') + self.assertEqual( + Model.search([('alias_email', 'ilike', 'alias.email.0')]), records[0] + records[5], + 'Search: partial search: any domain, matching some left parts') + self.assertEqual( + Model.search([('alias_email', '=', self.mail_alias_domain.name)]), Model, + 'Search: partial search: does not match any complete email') + self.assertEqual( + Model.search([('alias_email', '=', f'alias.email.1@{self.mail_alias_domain.name}')]), records[1], + 'Search: both part search: search on name + domain') + @users('employee') @mute_logger('odoo.addons.base.models.ir_model') def test_alias_mixin_alias_id_management(self): 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 11/18] [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}' + ) ), }, } From 3a0da2278f16d9915dd9001abbb1b3acdf9cb7e2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Wed, 27 Oct 2021 16:04:12 +0200 Subject: [PATCH 12/18] [IMP] mail: respect alias domains in mail gateway 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 Make mail gateway support alias domains instead of relying on configuration parameters. This implies the following changes * destination alias check is now based on full email by default. Previously only left-part of aliases were checked. Optionally an allowed list of domains could be additionally checked. Default from now on is to check the complete email e.g. 'sales@mydomain.com' != 'sales@mydomain.in'; * detection of direct write to catchall implies checking all domains catchall emails; * detection of write to bounce implies checking all domains bounce emails; * when having to send bounce emails using the bounce alias as mailer-daemon, find the bounce email from the relevant company; However we have to ease transition from the old ICP-based model used since ages to the new domain-based model. Notably a common usage of mail gateways is to do mail forwarding e.g. forward mail from domainA to domainB without rewriting destination. It means that e.g. sales@mail.domainA should be considered as a valid alias equivalent to sales@mail.domainB. This was working due to left-part only check of destination aliases. In order to keep this setup working after migration a flag is added on aliases allowing to keep the detection of those aliases based only on local parts. In summary: When searching for aliases, mailgateway now either checks for exact email, either for matching local parts when the flag is active. This is not the default behavior, as we want a stricter comparison of emails by default but it will be the default behavior at **migration time**. The 'mail.catchall.domain.allowed' configuration parameter is kept. It is used only for left-part check aliases, allowing to limit the scope of the match. Task-36879 (Mail: Support Multi Domains Aliases) Part-of: odoo/odoo#76734 --- .../test_account_incoming_supplier_invoice.py | 2 - .../tests/test_sync_google2odoo.py | 1 - addons/mail/models/ir_config_parameter.py | 2 +- addons/mail/models/mail_alias.py | 27 +-- addons/mail/models/mail_thread.py | 155 ++++++++++-------- addons/mail/views/mail_alias_views.xml | 2 + addons/project/tests/test_project_flow.py | 8 +- addons/test_mail/models/test_mail_models.py | 10 ++ addons/test_mail/security/ir.model.access.csv | 1 + addons/test_mail/tests/test_mail_alias.py | 3 +- addons/test_mail/tests/test_mail_gateway.py | 69 +++++++- odoo/addons/base/tests/test_ir_mail_server.py | 1 - 12 files changed, 181 insertions(+), 100 deletions(-) diff --git a/addons/account/tests/test_account_incoming_supplier_invoice.py b/addons/account/tests/test_account_incoming_supplier_invoice.py index 40fac722805..ac88ef5145b 100644 --- a/addons/account/tests/test_account_incoming_supplier_invoice.py +++ b/addons/account/tests/test_account_incoming_supplier_invoice.py @@ -12,8 +12,6 @@ class TestAccountIncomingSupplierInvoice(AccountTestInvoicingCommon): def setUpClass(cls, chart_template_ref=None): super().setUpClass(chart_template_ref=chart_template_ref) - cls.env['ir.config_parameter'].sudo().set_param('mail.catchall.domain', 'test.mycompany.com') - cls.internal_user = cls.env['res.users'].create({ 'name': 'Internal User', 'login': 'internal.user@test.odoo.com', diff --git a/addons/google_calendar/tests/test_sync_google2odoo.py b/addons/google_calendar/tests/test_sync_google2odoo.py index 4560ac9e692..4459d3763a2 100644 --- a/addons/google_calendar/tests/test_sync_google2odoo.py +++ b/addons/google_calendar/tests/test_sync_google2odoo.py @@ -1345,7 +1345,6 @@ class TestSyncGoogle2Odoo(TestSyncGoogle): @patch_api def test_alias_email_sync_recurrence(self): - self.env['ir.config_parameter'].sudo().set_param('mail.catchall.domain', 'test.mycompany.com') alias_model = self.env['ir.model'].search([('model', '=', 'calendar.event')]) mail_alias = self.env['mail.alias'].create({'alias_name': 'sale', 'alias_model_id': alias_model.id}) diff --git a/addons/mail/models/ir_config_parameter.py b/addons/mail/models/ir_config_parameter.py index 5bf36e62605..9112fd6ed27 100644 --- a/addons/mail/models/ir_config_parameter.py +++ b/addons/mail/models/ir_config_parameter.py @@ -25,6 +25,6 @@ class IrConfigParameter(models.Model): value = self.env['mail.alias']._sanitize_alias_name(value) # sanitize and normalize allowed catchall domains elif key == 'mail.catchall.domain.allowed' and value: - value = self.env['mail.alias']._clean_and_check_mail_catchall_allowed_domains(value) + value = self.env['mail.alias']._sanitize_allowed_domains(value) return super().set_param(key, value) diff --git a/addons/mail/models/mail_alias.py b/addons/mail/models/mail_alias.py index 590b86a7dd0..7e62142b6a3 100644 --- a/addons/mail/models/mail_alias.py +++ b/addons/mail/models/mail_alias.py @@ -81,6 +81,7 @@ class Alias(models.Model): "- everyone: everyone can post\n" "- partners: only authenticated partners\n" "- followers: only followers of the related document or members of following channels\n") + alias_incoming_local = fields.Boolean('Local-part based incoming detection', default=False) alias_bounced_content = fields.Html( "Custom Bounced Message", translate=True, help="If set, this content will automatically be sent out to unauthorized users instead of the default message.") @@ -288,18 +289,6 @@ class Alias(models.Model): return super().write(vals) - def _clean_and_check_mail_catchall_allowed_domains(self, value): - """ The purpose of this system parameter is to avoid the creation - of records from incoming emails with a domain != alias_domain - but that have a pattern matching an internal mail.alias . """ - value = [domain.strip().lower() for domain in value.split(',') if domain.strip()] - if not value: - raise ValidationError( - _("Value for `mail.catchall.domain.allowed` cannot be validated.\n" - "It should be a comma separated list of domains e.g. example.com,example.org.") - ) - return ",".join(value) - def _check_unique(self, alias_names, alias_domains): """ Check unicity constraint won't be raised, otherwise raise a UserError with a complete error message. Also check unicity against alias config @@ -359,6 +348,20 @@ class Alias(models.Model): msg_end = _('Choose another value or change it on the other document.') raise UserError(f'{msg_begin} {msg_end}') + @api.model + def _sanitize_allowed_domains(self, allowed_domains): + """ When having aliases checked on email left-part only we may define + an allowed list for right-part filtering, allowing more fine-grain than + either alias domain, either everything. This method sanitized its value. """ + value = [domain.strip().lower() for domain in allowed_domains.split(',') if domain.strip()] + if not value: + raise ValidationError(_( + "Value %(allowed_domains)s for `mail.catchall.domain.allowed` cannot be validated.\n" + "It should be a comma separated list of domains e.g. example.com,example.org.", + allowed_domains=allowed_domains + )) + return ",".join(value) + @api.model def _sanitize_alias_name(self, name): """ Cleans and sanitizes the alias name """ diff --git a/addons/mail/models/mail_thread.py b/addons/mail/models/mail_thread.py index a73e458bf57..e56c23f4578 100644 --- a/addons/mail/models/mail_thread.py +++ b/addons/mail/models/mail_thread.py @@ -345,22 +345,23 @@ class MailThread(models.AbstractModel): that adds alias information. """ model = self._context.get('empty_list_help_model') res_id = self._context.get('empty_list_help_id') - catchall_domain = self.env['ir.config_parameter'].sudo().get_param("mail.catchall.domain") document_name = self._context.get('empty_list_help_document_name', _('document')) nothing_here = is_html_empty(help_message) alias = None - if catchall_domain and model and res_id: # specific res_id -> find its alias (i.e. section_id specified) + # specific res_id -> find its alias (i.e. section_id specified) + if model and res_id: record = self.env[model].sudo().browse(res_id) # check that the alias effectively creates new records - if record.alias_id and record.alias_id.alias_name and \ - record.alias_id.alias_model_id and \ - record.alias_id.alias_model_id.model == self._name and \ - record.alias_id.alias_force_thread_id == 0: + if ('alias_id' in record and record.alias_id and + record.alias_id.alias_name and record.alias_id.alias_domain and + record.alias_id.alias_model_id.model == self._name and + record.alias_id.alias_force_thread_id == 0): alias = record.alias_id - if not alias and catchall_domain and model: # no res_id or res_id not linked to an alias -> generic help message, take a generic alias of the model - Alias = self.env['mail.alias'] - aliases = Alias.search([ + # no res_id or res_id not linked to an alias -> generic help message, take a generic alias of the model + if not alias and model and self.env.company.alias_domain_id: + aliases = self.env['mail.alias'].search([ + ("alias_domain_id", "=", self.env.company.alias_domain_id.id), ("alias_parent_model_id.model", "=", model), ("alias_name", "!=", False), ('alias_force_thread_id', '=', False), @@ -695,13 +696,19 @@ class MailThread(models.AbstractModel): 'email_to': bounce_to, 'auto_delete': True, } - bounce_from = tools.email_normalize(self.env['ir.mail_server']._get_default_bounce_address() or '') - if bounce_from: - bounce_mail_values['email_from'] = tools.formataddr(('MAILER-DAEMON', bounce_from)) - elif self.env['ir.config_parameter'].sudo().get_param("mail.catchall.alias") not in message['To']: - bounce_mail_values['email_from'] = tools.decode_message_header(message, 'To') - else: - bounce_mail_values['email_from'] = tools.formataddr(('MAILER-DAEMON', self.env.user.email_normalized)) + + # find an email_from for the bounce email + email_from = False + if bounce_from := self.env.company.bounce_email: + email_from = tools.formataddr(('MAILER-DAEMON', bounce_from)) + if not email_from: + catchall_aliases = self.env['mail.alias.domain'].search([]).mapped('catchall_email') + if not any(catchall_email in message['To'] for catchall_email in catchall_aliases): + email_from = tools.decode_message_header(message, 'To') + if not email_from: + email_from = tools.formataddr(('MAILER-DAEMON', self.env.user.email_normalized)) + + bounce_mail_values['email_from'] = email_from bounce_mail_values.update(mail_values) self.env['mail.mail'].sudo().create(bounce_mail_values).send() @@ -885,13 +892,12 @@ class MailThread(models.AbstractModel): we also need to verify if the message come from "mailer-daemon" """ # detection based on email_to - bounce_alias = self.env['ir.config_parameter'].sudo().get_param("mail.bounce.alias") - email_to = message_dict['to'] - email_to_localparts = [ - e.split('@', 1)[0].lower() - for e in (tools.email_split(email_to) or ['']) + bounce_aliases = self.env['mail.alias.domain'].search([]).mapped('bounce_email') + email_to_list = [ + tools.email_normalize(e) or e + for e in (tools.email_split(message_dict['to']) or ['']) ] - if bounce_alias and bounce_alias in email_to_localparts: + if bounce_aliases and any(email in bounce_aliases for email in email_to_list): return True email_from = message_dict['email_from'] @@ -1001,16 +1007,15 @@ class MailThread(models.AbstractModel): @api.model def _detect_write_to_catchall(self, msg_dict): """Return True if directly contacts catchall.""" - catchall_alias = self.env['ir.config_parameter'].sudo().get_param("mail.catchall.alias") - email_to = msg_dict['to'] - email_to_localparts = [ - e.split('@', 1)[0].lower() - for e in (tools.email_split(email_to) or ['']) + catchall_aliases = self.env['mail.alias.domain'].search([]).mapped('catchall_email') + email_to_list = [ + tools.email_normalize(e) or e + for e in (tools.email_split(msg_dict['to']) or ['']) ] # check it does not directly contact catchall return ( - catchall_alias and email_to_localparts and - all(email_localpart == catchall_alias for email_localpart in email_to_localparts) + catchall_aliases and email_to_list and + all(email_to in catchall_aliases for email_to in email_to_list) ) @api.model @@ -1050,10 +1055,19 @@ class MailThread(models.AbstractModel): """ if not isinstance(message, EmailMessage): raise TypeError('message must be an email.message.EmailMessage at this point') - catchall_domain_lowered = self.env["ir.config_parameter"].sudo().get_param("mail.catchall.domain", "").strip().lower() - catchall_domains_allowed = self.env["ir.config_parameter"].sudo().get_param("mail.catchall.domain.allowed") - if catchall_domain_lowered and catchall_domains_allowed: - catchall_domains_allowed = catchall_domains_allowed.split(',') + [catchall_domain_lowered] + catchall_domains_allowed = list(filter(None, (self.env["ir.config_parameter"].sudo().get_param( + "mail.catchall.domain.allowed") or '').split(','))) + if catchall_domains_allowed: + catchall_domains_allowed += self.env['mail.alias.domain'].search([]).mapped('name') + + def _filter_excluded_local_part(email): + left, _at, domain = email.partition('@') + if not domain: + return False + if catchall_domains_allowed and domain not in catchall_domains_allowed: + return False + return left + fallback_model = model # handle bounce: verify whether this is a bounced email and use it to @@ -1080,19 +1094,13 @@ class MailThread(models.AbstractModel): # author and recipients email_from = message_dict['email_from'] - email_to = message_dict['to'] - email_to_localparts = [ - e.split('@', 1)[0].lower() - for e in (tools.email_split(email_to) or ['']) - ] + email_to_list = [e.lower() for e in (tools.email_split(message_dict['to']) or [''])] + email_to_localparts = list(filter(None, (_filter_excluded_local_part(email_to) for email_to in email_to_list))) # Delivered-To is a safe bet in most modern MTAs, but we have to fallback on To + Cc values # for all the odd MTAs out there, as there is no standard header for the envelope's `rcpt_to` value. - rcpt_tos_localparts = [] - for recipient in tools.email_split(message_dict['recipients']): - to_local, to_domain = recipient.split('@', maxsplit=1) - if not catchall_domains_allowed or to_domain.lower() in catchall_domains_allowed: - rcpt_tos_localparts.append(to_local.lower()) - rcpt_tos_valid_localparts = [to for to in rcpt_tos_localparts] + rcpt_tos_list = [e.lower() for e in (tools.email_split(message_dict['recipients']) or [''])] + rcpt_tos_localparts = list(filter(None, (_filter_excluded_local_part(email_to) for email_to in rcpt_tos_list))) + rcpt_tos_valid_list = list(rcpt_tos_list) # 1. Handle reply # if destination = alias with different model -> consider it is a forward and not a reply @@ -1100,20 +1108,32 @@ class MailThread(models.AbstractModel): if reply_model and reply_thread_id: reply_model_id = self.env['ir.model']._get_id(reply_model) other_model_aliases = self.env['mail.alias'].search([ - '&', '&', - ('alias_name', '!=', False), - ('alias_name', 'in', email_to_localparts), + '&', ('alias_model_id', '!=', reply_model_id), + '|', + ('alias_full_name', 'in', email_to_list), + '&', ('alias_name', 'in', email_to_localparts), ('alias_incoming_local', '=', True), ]) if other_model_aliases: is_a_reply, reply_model, reply_thread_id = False, False, False - rcpt_tos_valid_localparts = [to for to in rcpt_tos_valid_localparts if to in other_model_aliases.mapped('alias_name')] + rcpt_tos_valid_list = [ + to + for to in rcpt_tos_valid_list + if ( + to in other_model_aliases.mapped('alias_full_name') + or to.split('@', 1)[0] in other_model_aliases.filtered('alias_incoming_local').mapped('alias_name') + ) + ] + rcpt_tos_valid_localparts = list(filter(None, (_filter_excluded_local_part(email_to) for email_to in rcpt_tos_valid_list))) if is_a_reply and reply_model: reply_model_id = self.env['ir.model']._get_id(reply_model) dest_aliases = self.env['mail.alias'].search([ - ('alias_name', 'in', rcpt_tos_localparts), - ('alias_model_id', '=', reply_model_id) + '&', + ('alias_model_id', '=', reply_model_id), + '|', + ('alias_full_name', 'in', rcpt_tos_list), + '&', ('alias_name', 'in', rcpt_tos_localparts), ('alias_incoming_local', '=', True), ], limit=1) user_id = self._mail_find_user_for_gateway(email_from, alias=dest_aliases).id or self._uid @@ -1124,27 +1144,31 @@ class MailThread(models.AbstractModel): if route: _logger.info( 'Routing mail from %s to %s with Message-Id %s: direct reply to msg: model: %s, thread_id: %s, custom_values: %s, uid: %s', - email_from, email_to, message_id, reply_model, reply_thread_id, custom_values, self._uid) + email_from, message_dict['to'], message_id, reply_model, reply_thread_id, custom_values, self._uid) return [route] if route is False: return [] # 2. Handle new incoming email by checking aliases and applying their settings - if rcpt_tos_localparts: + if rcpt_tos_list: # no route found for a matching reference (or reply), so parent is invalid message_dict.pop('parent_id', None) # check it does not directly contact catchall if self._detect_write_to_catchall(message_dict): _logger.info('Routing mail from %s to %s with Message-Id %s: direct write to catchall, bounce', - email_from, email_to, message_id) + email_from, message_dict['to'], message_id) body = self.env['ir.qweb']._render('mail.mail_bounce_catchall', { 'message': message, }) self._routing_create_bounce_email(email_from, body, message, references=message_id, reply_to=self.env.company.email) return [] - dest_aliases = self.env['mail.alias'].search([('alias_name', 'in', rcpt_tos_valid_localparts)]) + dest_aliases = self.env['mail.alias'].search([ + '|', + ('alias_full_name', 'in', rcpt_tos_valid_list), + '&', ('alias_name', 'in', rcpt_tos_valid_localparts), ('alias_incoming_local', '=', True), + ]) if dest_aliases: routes = [] for alias in dest_aliases: @@ -1154,7 +1178,7 @@ class MailThread(models.AbstractModel): if route: _logger.info( 'Routing mail from %s to %s with Message-Id %s: direct alias match: %r', - email_from, email_to, message_id, route) + email_from, message_dict['to'], message_id, route) routes.append(route) return routes @@ -1170,14 +1194,14 @@ class MailThread(models.AbstractModel): if route: _logger.info( 'Routing mail from %s to %s with Message-Id %s: fallback to model:%s, thread_id:%s, custom_values:%s, uid:%s', - email_from, email_to, message_id, fallback_model, thread_id, custom_values, user_id) + email_from, message_dict['to'], message_id, fallback_model, thread_id, custom_values, user_id) return [route] # ValueError if no routes found and if no bounce occurred raise ValueError( 'No possible route found for incoming message from %s to %s (Message-Id %s:). ' 'Create an appropriate mail.alias or force the destination model.' % - (email_from, email_to, message_id) + (email_from, message_dict['to'], message_id) ) @api.model @@ -1842,12 +1866,8 @@ class MailThread(models.AbstractModel): if not normalized_email: return self.env['res.users'] - catchall_domain = self.env['ir.config_parameter'].sudo().get_param("mail.catchall.domain") - if catchall_domain: - left_part = normalized_email.split('@')[0] if normalized_email.split('@')[1] == catchall_domain.lower() else False - if left_part: - if self.env['mail.alias'].sudo().search_count([('alias_name', '=', left_part)]): - return self.env['res.users'] + if self.env['mail.alias'].sudo().search_count([('alias_full_name', '=', email_value)]): + return self.env['res.users'] if alias and alias.alias_parent_model_id and alias.alias_parent_thread_id: followers = self.env['mail.followers'].search([ @@ -1895,7 +1915,6 @@ class MailThread(models.AbstractModel): followers = records.mapped('message_partner_ids') else: followers = self.env['res.partner'] - catchall_domain = self.env['ir.config_parameter'].sudo().get_param("mail.catchall.domain") # first, build a normalized email list and remove those linked to aliases # to avoid adding aliases as partners. In case of multi-email input, use @@ -1904,11 +1923,9 @@ class MailThread(models.AbstractModel): for email_normalized in (tools.email_normalize(contact, strict=False) for contact in emails) if email_normalized ] - if catchall_domain: - domain_left_parts = [email.split('@')[0] for email in normalized_emails if email and email.split('@')[1] == catchall_domain.lower()] - if domain_left_parts: - found_alias_names = self.env['mail.alias'].sudo().search([('alias_name', 'in', domain_left_parts)]).mapped('alias_name') - normalized_emails = [email for email in normalized_emails if email.split('@')[0] not in found_alias_names] + matching_aliases = self.env['mail.alias'].sudo().search([('alias_full_name', 'in', normalized_emails)]) + if matching_aliases: + normalized_emails = [email for email in normalized_emails if email not in matching_aliases.mapped('alias_full_name')] done_partners = [follower for follower in followers if follower.email_normalized in normalized_emails] remaining = [email for email in normalized_emails if email not in [partner.email_normalized for partner in done_partners]] diff --git a/addons/mail/views/mail_alias_views.xml b/addons/mail/views/mail_alias_views.xml index e3cb6ab6deb..294a648a17b 100644 --- a/addons/mail/views/mail_alias_views.xml +++ b/addons/mail/views/mail_alias_views.xml @@ -42,6 +42,7 @@ + @@ -66,6 +67,7 @@ + consider as a forward """ @@ -806,6 +843,7 @@ class TestMailgateway(MailCommon): # test@.. will cause the creation of new mail.test new_alias_2 = self.env['mail.alias'].create({ + 'alias_domain_id': self.mail_alias_domain.id, 'alias_name': 'test', 'alias_model_id': self.env['ir.model']._get('mail.test.container').id, 'alias_contact': 'everyone', @@ -832,6 +870,7 @@ class TestMailgateway(MailCommon): # test@.. will cause the creation of new mail.test new_alias_2 = self.env['mail.alias'].create({ + 'alias_domain_id': self.mail_alias_domain.id, 'alias_name': 'test', 'alias_model_id': self.env['ir.model']._get('mail.test.container').id, 'alias_contact': 'everyone', @@ -861,6 +900,7 @@ class TestMailgateway(MailCommon): # test@.. will cause the creation of new mail.test new_alias_2 = self.env['mail.alias'].create({ + 'alias_domain_id': self.mail_alias_domain.id, 'alias_name': 'test', 'alias_model_id': self.env['ir.model']._get('mail.test.container').id, 'alias_contact': 'everyone', @@ -886,6 +926,7 @@ class TestMailgateway(MailCommon): """ Incoming email: write to two aliases creating records: both should be activated """ # test@.. will cause the creation of new mail.test new_alias_2 = self.env['mail.alias'].create({ + 'alias_domain_id': self.mail_alias_domain.id, 'alias_name': 'test', 'alias_model_id': self.env['ir.model']._get('mail.test.container').id, 'alias_contact': 'everyone', @@ -907,9 +948,11 @@ class TestMailgateway(MailCommon): """ Incoming email: check that if domains are set in the optional system parameter `mail.catchall.domain.allowed` only incoming emails from these domains will generate records.""" - # test@.. will cause the creation of new mail.test + # test@.. will cause the creation of new mail.test.container new_alias_2 = self.env['mail.alias'].create({ 'alias_contact': 'everyone', + 'alias_domain_id': self.mail_alias_domain_c2.id, + 'alias_incoming_local': True, 'alias_model_id': self.env['ir.model']._get_id('mail.test.container.mc'), 'alias_name': 'test', }) @@ -917,6 +960,8 @@ class TestMailgateway(MailCommon): test_domain = 'hello.com' for (alias_right_part, allowed_domain), container_created in zip( [ + # Test a valid alias domain, standard case + (self.mail_alias_domain_c2.name, ""), # Test with 'mail.catchall.domain.allowed' not set in system parameters # and with a domain not allowed ('bonjour.com', ""), @@ -926,7 +971,7 @@ class TestMailgateway(MailCommon): # Test with 'mail.catchall.domain.allowed' set in system parameters # and with a domain allowed (test_domain, test_domain), - ], [True, False, True]): + ], [True, True, False, True]): with self.subTest(alias_right_part=alias_right_part, allowed_domain=allowed_domain): self.env['ir.config_parameter'].set_param('mail.catchall.domain.allowed', allowed_domain) @@ -1437,6 +1482,7 @@ class TestMailgateway(MailCommon): def test_message_process_references_forward(self): """ Incoming email using references but with alias forward should not go into references destination """ self.env['mail.alias'].create({ + 'alias_domain_id': self.mail_alias_domain.id, 'alias_name': 'test.alias', 'alias_model_id': self.env['ir.model']._get('mail.test.container').id, 'alias_contact': 'everyone', @@ -1457,6 +1503,7 @@ class TestMailgateway(MailCommon): def test_message_process_references_forward_same_model(self): """ Incoming email using references but with alias forward on same model should be considered as a reply """ self.env['mail.alias'].create({ + 'alias_domain_id': self.mail_alias_domain.id, 'alias_name': 'test.alias', 'alias_model_id': self.env['ir.model']._get('mail.test.gateway').id, 'alias_contact': 'everyone', @@ -1476,6 +1523,7 @@ class TestMailgateway(MailCommon): def test_message_process_references_forward_cc(self): """ Incoming email using references but with alias forward in CC should be considered as a repy (To > Cc) """ self.env['mail.alias'].create({ + 'alias_domain_id': self.mail_alias_domain.id, 'alias_name': 'test.alias', 'alias_model_id': self.env['ir.model']._get('mail.test.container').id, 'alias_contact': 'everyone', @@ -1549,6 +1597,7 @@ class TestMailgateway(MailCommon): """New record with mail that contains base64 inline image.""" target_model = "mail.test.field.type" alias = self.env["mail.alias"].create({ + 'alias_domain_id': self.mail_alias_domain.id, "alias_name": "base64-lover", "alias_model_id": self.env["ir.model"]._get(target_model).id, "alias_defaults": "{}", @@ -1572,6 +1621,7 @@ class TestMailgateway(MailCommon): coming from alias.""" target_model = "mail.test.field.type" alias = self.env["mail.alias"].create({ + 'alias_domain_id': self.mail_alias_domain.id, "alias_name": "base64-lover", "alias_model_id": self.env["ir.model"]._get(target_model).id, "alias_defaults": "{'type': 'second'}", @@ -1698,6 +1748,7 @@ class TestMailgateway(MailCommon): ]) alias = self.env['mail.alias'].create({ + 'alias_domain_id': self.mail_alias_domain.id, 'alias_name': 'test', 'alias_model_id': self.env['ir.model']._get('mail.test.container').id, 'alias_contact': 'everyone', @@ -1828,6 +1879,7 @@ class TestMailgateway(MailCommon): }) self.env['mail.alias'].create({ + 'alias_domain_id': self.mail_alias_domain.id, 'alias_name': 'test', 'alias_model_id': self.env['ir.model']._get('mail.test.gateway').id, }) @@ -1852,6 +1904,7 @@ class TestMailThreadCC(MailCommon): cls.email_from = 'Sylvie Lelitre ' cls.alias = cls.env['mail.alias'].create({ 'alias_contact': 'everyone', + 'alias_domain_id': cls.mail_alias_domain.id, 'alias_model_id': cls.env['ir.model']._get('mail.test.cc').id, 'alias_name': 'cc_record', }) diff --git a/odoo/addons/base/tests/test_ir_mail_server.py b/odoo/addons/base/tests/test_ir_mail_server.py index 7557421649a..b9c328f5f91 100644 --- a/odoo/addons/base/tests/test_ir_mail_server.py +++ b/odoo/addons/base/tests/test_ir_mail_server.py @@ -294,7 +294,6 @@ class TestIrMailServer(TransactionCase, MockSmtplibCase): @mute_logger('odoo.models.unlink') def test_mail_server_send_email_IDNA(self): """ Test that the mail from / recipient envelop are encoded using IDNA """ - self.env['ir.config_parameter'].sudo().set_param('mail.catchall.domain', 'ééééééé.com') with self.mock_smtplib_connection(): message = self._build_email(mail_from='test@ééééééé.com') self.env['ir.mail_server'].send_email(message) From 7239115b4b8be76f5fe4b6f8d2734e9f5af37b3e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Mon, 16 Oct 2023 13:55:10 +0200 Subject: [PATCH 13/18] [IMP] mail: store company/alias environment on mail.message 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 Store 'company_id' and 'alias_domain_id' on 'mail.message' model. It helps knowing the environment that produced the message, notably * for layouting: it will be used to improve company (and soon alias domain) given for the email notification layout; * for sending: it will be used to better compute mail-related values like default from, return path, ... When logging, those fields are kept false as anyway no notification is sent. No need to fetch extra information. Some sudo() are included in post_* methods, as portal may go through the posting method, see 'test_portal_acls' in test_message_post.py file. Task-36879 (Mail: Support Multi Domains Aliases) Part-of: odoo/odoo#76734 --- addons/mail/models/mail_message.py | 2 + addons/mail/models/mail_thread.py | 13 ++++ addons/mail/wizard/mail_compose_message.py | 45 +++++++++++++ .../wizard/mail_compose_message_views.xml | 2 + addons/test_mail/tests/test_mail_composer.py | 63 +++++++++++++++++-- 5 files changed, 119 insertions(+), 6 deletions(-) diff --git a/addons/mail/models/mail_message.py b/addons/mail/models/mail_message.py index bd99cfaa5e6..33bf3c25af9 100644 --- a/addons/mail/models/mail_message.py +++ b/addons/mail/models/mail_message.py @@ -111,6 +111,8 @@ class Message(models.Model): model = fields.Char('Related Document Model') res_id = fields.Many2oneReference('Related Document ID', model_field='model') record_name = fields.Char('Message Record Name') # display_name of the related document + record_alias_domain_id = fields.Many2one('mail.alias.domain', 'Alias Domain', ondelete='set null') + record_company_id = fields.Many2one('res.company', 'Company', ondelete='set null') # characteristics message_type = fields.Selection([ ('email', 'Email'), diff --git a/addons/mail/models/mail_thread.py b/addons/mail/models/mail_thread.py index e56c23f4578..2df2cc36a0e 100644 --- a/addons/mail/models/mail_thread.py +++ b/addons/mail/models/mail_thread.py @@ -2123,6 +2123,10 @@ class MailThread(models.AbstractModel): 'partner_ids': partner_ids, }) # add default-like values afterwards, to avoid useless queries + if 'record_alias_domain_id' not in msg_values: + msg_values['record_alias_domain_id'] = self.sudo()._mail_get_alias_domains(default_company=self.env.company)[self.id].id + if 'record_company_id' not in msg_values: + msg_values['record_company_id'] = self._mail_get_companies(default=self.env.company)[self.id].id if 'reply_to' not in msg_values: msg_values['reply_to'] = self._notify_get_reply_to(default=email_from)[self.id] @@ -2597,6 +2601,11 @@ class MailThread(models.AbstractModel): } msg_values.update(msg_kwargs) # add default-like values afterwards, to avoid useless queries + if self: + if 'record_alias_domain_id' not in msg_values: + msg_values['record_alias_domain_id'] = self._mail_get_alias_domains(default_company=self.env.company)[self.id].id + if 'record_company_id' not in msg_values: + msg_values['record_company_id'] = self._mail_get_companies(default=self.env.company)[self.id].id if 'reply_to' not in msg_values: msg_values['reply_to'] = self._notify_get_reply_to(default=email_from)[self.id if self else False] @@ -2689,6 +2698,8 @@ class MailThread(models.AbstractModel): 'email_from': email_from, # document 'model': self._name, + 'record_alias_domain_id': False, + 'record_company_id': False, 'record_name': False, # content 'attachment_ids': attachment_ids, @@ -2826,6 +2837,8 @@ class MailThread(models.AbstractModel): 'model', 'parent_id', 'partner_ids', + 'record_alias_domain_id', + 'record_company_id', 'record_name', 'reply_to', 'reply_to_force_new', diff --git a/addons/mail/wizard/mail_compose_message.py b/addons/mail/wizard/mail_compose_message.py index 95798f552c1..4d5061c7551 100644 --- a/addons/mail/wizard/mail_compose_message.py +++ b/addons/mail/wizard/mail_compose_message.py @@ -121,6 +121,12 @@ class MailComposer(models.TransientModel): res_domain_user_id = fields.Many2one( 'res.users', string='Responsible', help='Used as context used to evaluate composer domain') + record_alias_domain_id = fields.Many2one( + 'mail.alias.domain', 'Alias Domain', + compute='_compute_record_environment', readonly=False, store=True) # useful only in monorecord comment mode + record_company_id = fields.Many2one( + 'res.company', 'Company', + compute='_compute_record_environment', readonly=False, store=True) # useful only in monorecord comment mode record_name = fields.Char( 'Record Name', compute='_compute_record_name', readonly=False, store=True) # useful only in monorecord comment mode @@ -387,6 +393,34 @@ class MailComposer(models.TransientModel): elif not active_res_ids and self.env.context.get('active_id'): composer.res_ids = f"{[self.env.context['active_id']]}" + @api.depends('composition_mode', 'model', 'res_domain', 'res_ids') + def _compute_record_environment(self): + """ In monorecord mode, fetch record company and the linked alias domain, + easing future processing notably at post and notification sending time. + + In batch mode it makes no sense to compute a single company, it will be + dynamically generated. """ + toreset = self.filtered( + lambda comp: (comp.record_company_id or comp.record_alias_domain_id) and comp.composition_batch + ) + if toreset: + toreset.record_alias_domain_id = False + toreset.record_company_id = False + + toupdate = self.filtered( + lambda comp: not comp.composition_batch + ) + for composer in toupdate: + res_ids = composer._evaluate_res_ids() + if composer.model and len(res_ids) == 1: + record = self.env[composer.model].browse(res_ids) + composer.record_company_id = record._mail_get_companies( + default=self.env.company + )[record.id] + composer.record_alias_domain_id = record._mail_get_alias_domains( + default_company=self.env.company + )[record.id] + @api.depends('composition_mode', 'model', 'parent_id', 'res_domain', 'res_ids') def _compute_record_name(self): """ Computation is coming either from parent message, either from the @@ -781,6 +815,8 @@ class MailComposer(models.TransientModel): STA - 'email_add_signature', STA - 'email_layout_xmlid', DYN - 'force_email_lang', # notify parameter + STA - 'record_alias_domain_id', # monorecord only + STA - 'record_company_id', # monorecord only BOTH DYN - 'attachment_ids', @@ -879,6 +915,8 @@ class MailComposer(models.TransientModel): force_send=self.force_send, mail_auto_delete=self.auto_delete, model_description=model_description, + record_alias_domain_id=self.record_alias_domain_id.id, + record_company_id=self.record_company_id.id, ) return values @@ -899,6 +937,10 @@ class MailComposer(models.TransientModel): RecordsModel = self.env[self.model].with_prefetch(res_ids) email_mode = self.composition_mode == 'mass_mail' + # records values + companies = RecordsModel.browse(res_ids)._mail_get_companies(default=self.env.company) + alias_domains = RecordsModel.browse(res_ids)._mail_get_alias_domains(default_company=self.env.company) + # langs, used currently only to propagate in comment mode for notification # layout translation langs = self._render_field('lang', res_ids) @@ -916,6 +958,9 @@ class MailComposer(models.TransientModel): 'email_from': emails_from[res_id], 'scheduled_date': False, 'subject': subjects[res_id], + # record-specific environment values (company, alias_domain) + 'record_alias_domain_id': alias_domains[res_id].id, + 'record_company_id': companies[res_id].id, # some fields are specific to mail **( { diff --git a/addons/mail/wizard/mail_compose_message_views.xml b/addons/mail/wizard/mail_compose_message_views.xml index 7d59093065f..3370d02acf7 100644 --- a/addons/mail/wizard/mail_compose_message_views.xml +++ b/addons/mail/wizard/mail_compose_message_views.xml @@ -21,6 +21,8 @@ + + diff --git a/addons/test_mail/tests/test_mail_composer.py b/addons/test_mail/tests/test_mail_composer.py index 88769e7f0c8..3becd823fad 100644 --- a/addons/test_mail/tests/test_mail_composer.py +++ b/addons/test_mail/tests/test_mail_composer.py @@ -171,6 +171,8 @@ class TestComposerForm(TestMailComposer): self.assertFalse(composer_form.mail_server_id) self.assertEqual(composer_form.model, self.test_record._name) self.assertFalse(composer_form.partner_ids) + self.assertEqual(composer_form.record_alias_domain_id, self.mail_alias_domain) + self.assertEqual(composer_form.record_company_id, self.env.company) self.assertEqual(composer_form.record_name, self.test_record.name, 'MailComposer: comment mode should compute record name') self.assertFalse(composer_form.reply_to) self.assertFalse(composer_form.reply_to_force_new) @@ -182,6 +184,32 @@ class TestComposerForm(TestMailComposer): self.assertEqual(composer_form.subtype_id, self.env.ref('mail.mt_comment')) self.assertFalse(composer_form.subtype_is_log) + @users('employee') + def test_mail_composer_comment_mc(self): + """ Test values specific to multi-company setup, in comment mode, using + the Form tool. """ + # add access to second company to avoid MC rules on ticket model + self.env.user.company_ids = [(4, self.company_2.id)] + test_record = self.test_record.with_env(self.env) + source_company = [ + self.company_admin, + self.company_2, + self.env['res.company'], + ] + expected = [ + (self.company_admin, self.mail_alias_domain), + (self.company_2, self.mail_alias_domain_c2), + (self.company_admin, self.mail_alias_domain), # env company + ] + for company, (exp_company, exp_alias_domain) in zip(source_company, expected): + with self.subTest(cmopany=company): + test_record.write({'company_id': company.id}) + composer_form = Form(self.env['mail.compose.message'].with_context( + self._get_web_context(self.test_record, add_web=True) + )) + self.assertEqual(composer_form.record_alias_domain_id, exp_alias_domain) + self.assertEqual(composer_form.record_company_id, exp_company) + @users('employee') def test_mail_composer_comment_attachments(self): """Tests that all attachments are added to the composer, static attachments @@ -251,6 +279,8 @@ class TestComposerForm(TestMailComposer): self.assertEqual(composer_form.mail_server_id, self.mail_server_domain) self.assertEqual(composer_form.model, self.test_record._name) self.assertEqual(composer_form.partner_ids[:], self.partner_1) + self.assertEqual(composer_form.record_alias_domain_id, self.mail_alias_domain) + self.assertEqual(composer_form.record_company_id, self.env.company) self.assertEqual(composer_form.record_name, self.test_record.name, 'MailComposer: comment mode should compute record name') self.assertEqual(composer_form.reply_to, 'info@test.example.com') self.assertFalse(composer_form.reply_to_force_new) @@ -283,6 +313,8 @@ class TestComposerForm(TestMailComposer): self.assertFalse(composer_form.force_send, 'MailComposer: batch record post use email queue for notifications') self.assertEqual(composer_form.mail_server_id, self.mail_server_domain) self.assertEqual(composer_form.model, self.test_record._name) + self.assertFalse(composer_form.record_alias_domain_id, 'MailComposer: comment in batch mode should have void alias domain') + self.assertFalse(composer_form.record_company_id, 'MailComposer: comment in batch mode should have void company') self.assertFalse(composer_form.record_name, 'MailComposer: comment in batch mode should have void record name') self.assertEqual(composer_form.reply_to, self.template.reply_to) self.assertFalse(composer_form.reply_to_force_new) @@ -315,6 +347,8 @@ class TestComposerForm(TestMailComposer): self.assertFalse(composer_form.force_send, 'MailComposer: batch record post use email queue for notifications') self.assertEqual(composer_form.mail_server_id, self.mail_server_domain) self.assertEqual(composer_form.model, self.test_record._name) + self.assertFalse(composer_form.record_alias_domain_id, 'MailComposer: comment in batch mode should have void alias domain') + self.assertFalse(composer_form.record_company_id, 'MailComposer: comment in batch mode should have void company') self.assertFalse(composer_form.record_name, 'MailComposer: comment in batch mode should have void record name') self.assertEqual(composer_form.reply_to, self.template.reply_to) self.assertFalse(composer_form.reply_to_force_new) @@ -347,6 +381,8 @@ class TestComposerForm(TestMailComposer): self.assertEqual(composer_form.mail_server_id, self.mail_server_domain) self.assertEqual(composer_form.model, self.test_record._name) self.assertFalse(composer_form.partner_ids[:]) + self.assertFalse(composer_form.record_alias_domain_id) + self.assertFalse(composer_form.record_company_id) self.assertFalse(composer_form.record_name) self.assertEqual(composer_form.reply_to, 'info@test.example.com') self.assertFalse(composer_form.reply_to_force_new) @@ -375,6 +411,8 @@ class TestComposerForm(TestMailComposer): self.assertTrue(composer_form.force_send, 'MailComposer: mass mode sends emails right away') self.assertFalse(composer_form.mail_server_id) self.assertEqual(composer_form.model, self.test_records._name) + self.assertFalse(composer_form.record_alias_domain_id, 'MailComposer: mass mode should have void alias domain') + self.assertFalse(composer_form.record_company_id, 'MailComposer: mass mode should have void company') self.assertFalse(composer_form.record_name, 'MailComposer: mass mode should have void record name') self.assertFalse(composer_form.reply_to) self.assertFalse(composer_form.reply_to_force_new) @@ -404,6 +442,8 @@ class TestComposerForm(TestMailComposer): self.assertTrue(composer_form.force_send, 'MailComposer: mass mode sends emails right away') self.assertEqual(composer_form.mail_server_id, self.mail_server_domain) self.assertEqual(composer_form.model, self.test_records._name) + self.assertFalse(composer_form.record_alias_domain_id, 'MailComposer: mass mode should have void alias domain') + self.assertFalse(composer_form.record_company_id, 'MailComposer: mass mode should have void company') self.assertFalse(composer_form.record_name, 'MailComposer: mass mode should have void record name') self.assertEqual(composer_form.reply_to, self.template.reply_to) self.assertFalse(composer_form.reply_to_force_new) @@ -437,6 +477,8 @@ class TestComposerForm(TestMailComposer): self.assertTrue(composer_form.force_send, 'MailComposer: mass mode sends emails right away') self.assertEqual(composer_form.mail_server_id, self.mail_server_domain) self.assertEqual(composer_form.model, self.test_records._name) + self.assertFalse(composer_form.record_alias_domain_id, 'MailComposer: mass mode should have void alias domain') + self.assertFalse(composer_form.record_company_id, 'MailComposer: mass mode should have void company') self.assertFalse(composer_form.record_name, 'MailComposer: mass mode should have void record name') self.assertEqual(composer_form.reply_to, self.template.reply_to) self.assertFalse(composer_form.reply_to_force_new) @@ -470,6 +512,8 @@ class TestComposerForm(TestMailComposer): self.assertTrue(composer_form.force_send, 'MailComposer: mass mode sends emails right away') self.assertEqual(composer_form.mail_server_id, self.mail_server_domain) self.assertEqual(composer_form.model, self.test_records._name) + self.assertFalse(composer_form.record_alias_domain_id, 'MailComposer: mass mode should have void alias domain') + self.assertFalse(composer_form.record_company_id, 'MailComposer: mass mode should have void company') self.assertFalse(composer_form.record_name, 'MailComposer: mass mode should have void record name') self.assertEqual(composer_form.reply_to, self.template.reply_to) self.assertFalse(composer_form.reply_to_force_new) @@ -1262,6 +1306,12 @@ class TestComposerResultsComment(TestMailComposer, CronMixinCase): """ Ensure class initial data to ease understanding """ self.assertTrue(self.template.auto_delete) + self.assertEqual(len(self.test_record), 1) + self.assertEqual(self.test_record.user_id, self.user_employee_2) + self.assertEqual(self.test_record.message_partner_ids, self.partner_employee_2) + self.assertEqual(self.test_record[0].customer_id.lang, 'en_US') + self.assertEqual(self.test_record.company_id, self.company_admin) + self.assertEqual(len(self.test_records), 2) self.assertEqual(self.test_records.user_id, self.user_employee_2) self.assertEqual(self.test_records.message_partner_ids, self.partner_employee_2) @@ -1844,7 +1894,6 @@ class TestComposerResultsComment(TestMailComposer, CronMixinCase): self.assertEqual(set(message.attachment_ids.mapped('res_id')), set(test_record.ids)) self.assertTrue(all(attach not in message.attachment_ids for attach in attachs), 'Should have copied attachments') - @users('employee') @mute_logger('odoo.addons.mail.models.mail_mail') def test_mail_composer_wtpl_mc(self): @@ -1871,11 +1920,11 @@ class TestComposerResultsComment(TestMailComposer, CronMixinCase): attachs = self.env['ir.attachment'].search([('name', 'in', [a['name'] for a in attachment_data])]) self.assertEqual(len(attachs), 2) - for batch, companies, expected_companies in [ + for batch, companies, expected_companies, expected_alias_domains in [ + (False, self.company_admin, self.company_admin, self.mail_alias_domain), ( - False, self.company_admin, self.company_admin - ), ( True, self.company_admin + self.company_2, self.company_admin + self.company_2, + self.mail_alias_domain + self.mail_alias_domain_c2, ), ]: with self.subTest(batch=batch, @@ -1902,8 +1951,8 @@ class TestComposerResultsComment(TestMailComposer, CronMixinCase): new_partner = self.env['res.partner'].search([('email_normalized', '=', 'test.to.1@test.example.com')]) self.assertEqual(len(new_partner), 1) # check output, company-specific values mainly for this test - for record, exp_company in zip( - test_records, expected_companies + for record, exp_company, exp_alias_domain in zip( + test_records, expected_companies, expected_alias_domains ): message = record.message_ids[0] for recipient in [self.partner_employee_2, new_partner, record.customer_id]: @@ -1924,6 +1973,8 @@ class TestComposerResultsComment(TestMailComposer, CronMixinCase): 'X-Odoo-Objects': f'{record._name}-{record.id}', }, 'mail_server_id': self.env['ir.mail_server'], + 'record_alias_domain_id': exp_alias_domain, + 'record_company_id': exp_company, 'subject': f'TemplateSubject {record.name}', }, ) From 934506049886d4c61ad6651e114df1bae0ea5707 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Wed, 23 Aug 2023 18:32:42 +0200 Subject: [PATCH 14/18] [IMP] mail: respect alias domains in 'mail.mail' and outgoing emails 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: MAIL.MAIL Update MailMail to use alias domains. Notably "Return-Path" headers are now computed based on alias domain when possible, using the recently added fields on 'mail.message' model for that purpose. Fallback is to use current company's bounce email when mail_mail creation is done outside of classic mail flows or without that information. SPECIFICATIONS: IR.MAIL.SERVER Update IrMailServer and low-level stack to use alias domains. This has an impact notably on default values computation for from and bounce emails * '_get_default_bounce_address' is called when there is no 'Return-Path' given. Most classic mail flows will set it according to current record company / alias domain. Fallback when not set is to fallback on current company's bounce email, computed based on its alias domain; * '_get_default_from_address' is used in two use cases * computing a default 'email_from' for outgoing emails when it is not set. In most classic mail flows it is set based on current user's email. If not set fallback on current company's notification emails is considered as a safe bet, replacing the global configuration parameter; * overriding the 'email_from' of emails that are considered spoofing the mail server, allowing to wrap the sending into a 'notifications@domain' generic sender. For those we should try to keep record's information as it may be called in classic mail flows; * '_get_default_from_filter' is added in base and overridden in mail to either use 'mail.default.from_filter' ICP, or use the one defined on the alias domain. Supporting both is still an option, as its behavior is implemented for basic email sending, without mail being available. Those methods are updated to try to support multi domains / multi company setup. However as those defaults are located ar ir.mail_server level it is not always easy to have complete environment information, hence fallbacking on current company's parameters when no better information is provided. A test about 'mail.default.from' is removed, as it was testing a default_from outside of catchall domain. It is not possible anymore as default_from is now part of domain definition. As multi domains is supported, no need to support exotic configuration like that. SPECIFICATIONS: FROM MAIL.MAIL TO OUTGOING EMAILS When sending emails based on MailMail, we now prepares sending groups based on MailServer, email_from, but also alias domain to which the mail belongs to. Information about alias domain (e.g. notifications email based on default_from and bounce email) is propagated to low-level email preparation methods. It uses the context as it is the easiest way to propagate information to that level without hacking too much models or calls. Task-36879 (Mail: Support Multi Domains Aliases) Part-of: odoo/odoo#76734 --- addons/mail/models/ir_mail_server.py | 39 ++++---- addons/mail/models/mail_mail.py | 59 ++++++++----- addons/mail/models/mail_thread.py | 6 +- addons/mail/models/models.py | 17 ++-- addons/mail/tests/test_ir_mail_server.py | 88 +++++-------------- .../mass_mailing/tests/test_mailing_retry.py | 2 +- addons/project/models/project_task.py | 4 +- addons/test_mail/models/test_mail_models.py | 4 +- addons/test_mail/tests/test_mail_composer.py | 63 +++++++------ addons/test_mail/tests/test_mail_mail.py | 21 +---- addons/test_mail/tests/test_message_post.py | 5 +- odoo/addons/base/models/ir_mail_server.py | 35 +++++--- odoo/addons/base/tests/test_ir_mail_server.py | 30 +++++++ 13 files changed, 196 insertions(+), 177 deletions(-) diff --git a/addons/mail/models/ir_mail_server.py b/addons/mail/models/ir_mail_server.py index d67de643b5d..56773f26aea 100644 --- a/addons/mail/models/ir_mail_server.py +++ b/addons/mail/models/ir_mail_server.py @@ -26,25 +26,16 @@ class IrMailServer(models.Model): def _get_default_bounce_address(self): """ Compute the default bounce address. Try to use mail-defined config parameter bounce alias if set. """ - ICP = self.env['ir.config_parameter'].sudo() - bounce_alias = ICP.get_param('mail.bounce.alias') - domain = ICP.get_param('mail.catchall.domain') - if bounce_alias and domain: - return f'{bounce_alias}@{domain}' + if self.env.company.bounce_email: + return self.env.company.bounce_email return super()._get_default_bounce_address() @api.model def _get_default_from_address(self): - """ Default from: try to use ``mail.default.from`` ICP either as a - full email address, either completed with ``mail.catchall.domain`` - to form a alias-based default from. """ - get_param = self.env['ir.config_parameter'].sudo().get_param - email_from = get_param("mail.default.from") - if email_from and "@" in email_from: - return email_from - if email_from: - if domain := get_param("mail.catchall.domain"): - return f"{email_from}@{domain}" + """ Default from: try to use default_from defined on company's alias + domain. """ + if default_from := self.env.company.default_from_email: + return default_from return super()._get_default_from_address() def _get_test_email_from(self): @@ -53,13 +44,17 @@ class IrMailServer(models.Model): # find first found complete email in filter parts if mail_from := next((email for email in from_filter_parts if "@" in email), None): return mail_from - # try to complete with default.from in same domain of first found filter parts - default_from = self.env["ir.config_parameter"].sudo().get_param("mail.default.from", "odoo") - if "@" not in default_from: - return f"{default_from}@{from_filter_parts[0]}" # the mail server is configured for a domain that matches the default email address - if self._match_from_filter(default_from, self.from_filter): - return default_from + alias_domains = self.env['mail.alias.domain'].sudo().search([]) + matching = next( + (alias_domain for alias_domain in alias_domains + if self._match_from_filter(alias_domain.default_from_email, self.from_filter) + ), False + ) + if matching: + return matching.default_from_email + # fake default_from "odoo@domain" + return f"odoo@{from_filter_parts[0]}" # no from_filter or from_filter is configured for a domain different that - # the one of the full email configured in mail.default.from -> fallback + # the default_from of company's alias_domain -> fallback return super()._get_test_email_from() diff --git a/addons/mail/models/mail_mail.py b/addons/mail/models/mail_mail.py index 7673cfcae9b..5aa0dff88bc 100644 --- a/addons/mail/models/mail_mail.py +++ b/addons/mail/models/mail_mail.py @@ -362,14 +362,9 @@ class MailMail(models.Model): # headers headers = {} - ICP = self.env['ir.config_parameter'].sudo() - bounce_alias = ICP.get_param("mail.bounce.alias") - catchall_domain = ICP.get_param("mail.catchall.domain") - if bounce_alias and catchall_domain: - headers['Return-Path'] = f'{bounce_alias}@{catchall_domain}' if self.headers: try: - headers.update(ast.literal_eval(self.headers)) + headers = ast.literal_eval(self.headers) except (ValueError, TypeError) as e: _logger.warning( 'Evaluation error when evaluating mail headers (received %r): %s', @@ -382,6 +377,7 @@ class MailMail(models.Model): 'Unknown error when evaluating mail headers (received %r): %s', self.headers, e, ) + headers.setdefault('Return-Path', self.record_alias_domain_id.bounce_email or self.env.company.bounce_email) # prepare recipients: use email_to if defined then check recipient_ids # that receive a specific email, notably due to link shortening / redirect @@ -471,10 +467,11 @@ class MailMail(models.Model): return results def _split_by_mail_configuration(self): - """Group the based on their "email_from" and their "mail_server_id". + """Group the based on their "email_from", their "alias domain" + and their "mail_server_id". The will have the "same sending configuration" if they have the same - mail server or the same mail from. For performance purpose, we can use an SMTP + mail server, alias domain and mail from. For performance purpose, we can use an SMTP session in batch and therefore we need to group them by the parameter that will influence the mail server used. @@ -484,33 +481,42 @@ class MailMail(models.Model): Return iterators over mail_server_id, email_from, Records.ids """ - mail_values = self.read(['id', 'email_from', 'mail_server_id']) + mail_values = self.read(['id', 'email_from', 'mail_server_id', 'record_alias_domain_id']) - # First group the per mail_server_id and per email_from + # First group the per mail_server_id, per alias_domain (if no server) and per email_from group_per_email_from = defaultdict(list) for values in mail_values: mail_server_id = values['mail_server_id'][0] if values['mail_server_id'] else False - group_per_email_from[(mail_server_id, values['email_from'])].append(values['id']) + alias_domain_id = values['record_alias_domain_id'][0] if values['record_alias_domain_id'] else False + key = (mail_server_id, alias_domain_id, values['email_from']) + group_per_email_from[key].append(values['id']) # Then find the mail server for each email_from and group the # per mail_server_id and smtp_from mail_servers = self.env['ir.mail_server'].sudo().search([], order='sequence, id') group_per_smtp_from = defaultdict(list) - for (mail_server_id, email_from), mail_ids in group_per_email_from.items(): + for (mail_server_id, alias_domain_id, email_from), mail_ids in group_per_email_from.items(): if not mail_server_id: - mail_server, smtp_from = self.env['ir.mail_server']._find_mail_server(email_from, mail_servers) + mail_server = self.env['ir.mail_server'] + if alias_domain_id: + alias_domain = self.env['mail.alias.domain'].sudo().browse(alias_domain_id) + mail_server = mail_server.with_context( + domain_notifications_email=alias_domain.default_from_email, + domain_bounce_address=alias_domain.bounce_email, + ) + mail_server, smtp_from = mail_server._find_mail_server(email_from, mail_servers) mail_server_id = mail_server.id if mail_server else False else: smtp_from = email_from - group_per_smtp_from[(mail_server_id, smtp_from)].extend(mail_ids) + group_per_smtp_from[(mail_server_id, alias_domain_id, smtp_from)].extend(mail_ids) sys_params = self.env['ir.config_parameter'].sudo() batch_size = int(sys_params.get_param('mail.session.batch.size', 1000)) - for (mail_server_id, smtp_from), record_ids in group_per_smtp_from.items(): + for (mail_server_id, alias_domain_id, smtp_from), record_ids in group_per_smtp_from.items(): for batch_ids in tools.split_every(batch_size, record_ids): - yield mail_server_id, smtp_from, batch_ids + yield mail_server_id, alias_domain_id, smtp_from, batch_ids def send(self, auto_commit=False, raise_exception=False): """ Sends the selected emails immediately, ignoring their current @@ -527,7 +533,7 @@ class MailMail(models.Model): email sending process has failed :return: True """ - for mail_server_id, smtp_from, batch_ids in self._split_by_mail_configuration(): + for mail_server_id, alias_domain_id, smtp_from, batch_ids in self._split_by_mail_configuration(): smtp_session = None try: smtp_session = self.env['ir.mail_server'].connect(mail_server_id=mail_server_id, smtp_from=smtp_from) @@ -544,7 +550,9 @@ class MailMail(models.Model): self.browse(batch_ids)._send( auto_commit=auto_commit, raise_exception=raise_exception, - smtp_session=smtp_session) + smtp_session=smtp_session, + alias_domain_id=alias_domain_id, + ) _logger.info( 'Sent batch %s emails via mail server ID #%s', len(batch_ids), mail_server_id) @@ -552,7 +560,7 @@ class MailMail(models.Model): if smtp_session: smtp_session.quit() - def _send(self, auto_commit=False, raise_exception=False, smtp_session=None): + def _send(self, auto_commit=False, raise_exception=False, smtp_session=None, alias_domain_id=False): IrMailServer = self.env['ir.mail_server'] # Only retrieve recipient followers of the mails if needed mails_with_unfollow_link = self.filtered(lambda m: m.body_html and '/mail/unfollow' in m.body_html) @@ -610,7 +618,16 @@ class MailMail(models.Model): # send each sub-email for email in email_list: - msg = IrMailServer.build_email( + # if given, contextualize sending using alias domains + if alias_domain_id: + alias_domain = self.env['mail.alias.domain'].sudo().browse(alias_domain_id) + SendIrMailServer = IrMailServer.with_context( + domain_notifications_email=alias_domain.default_from_email, + domain_bounce_address=email['headers'].get('Return-Path') or alias_domain.bounce_email, + ) + else: + SendIrMailServer = IrMailServer + msg = SendIrMailServer.build_email( email_from=email_from, email_to=email['email_to'], subject=email['subject'], @@ -628,7 +645,7 @@ class MailMail(models.Model): ) processing_pid = email.pop("partner_id", None) try: - res = IrMailServer.send_email( + res = SendIrMailServer.send_email( msg, mail_server_id=mail.mail_server_id.id, smtp_session=smtp_session) if processing_pid: success_pids.append(processing_pid) diff --git a/addons/mail/models/mail_thread.py b/addons/mail/models/mail_thread.py index 2df2cc36a0e..5b7116607ea 100644 --- a/addons/mail/models/mail_thread.py +++ b/addons/mail/models/mail_thread.py @@ -3505,7 +3505,11 @@ class MailThread(models.AbstractModel): if additional_values: base_mail_values.update(additional_values) - headers = self._notify_by_email_get_headers() + # prepare headers (as sudo as accessing mail.alias.domain, restricted) + headers = {} + if message_sudo.record_alias_domain_id.bounce_email: + headers['Return-Path'] = message_sudo.record_alias_domain_id.bounce_email + headers = self._notify_by_email_get_headers(headers=headers) if headers: base_mail_values['headers'] = repr(headers) return base_mail_values diff --git a/addons/mail/models/models.py b/addons/mail/models/models.py index 34d03784d2c..8fd779de946 100644 --- a/addons/mail/models/models.py +++ b/addons/mail/models/models.py @@ -374,14 +374,19 @@ class BaseModel(models.AbstractModel): # GATEWAY: NOTIFICATION # ------------------------------------------------------------ - def _notify_by_email_get_headers(self): - """ Generate the email headers based on record """ + def _notify_by_email_get_headers(self, headers=None): + """ Generate the email headers based on record. Each header not already + present in 'headers' will be added in it. """ + headers = headers or {} if not self: - return {} + return headers self.ensure_one() - return { - 'X-Odoo-Objects': "%s-%s" % (self._name, self.id), - } + headers['X-Odoo-Objects'] = f"{self._name}-{self.id}" + if 'Return-Path' not in headers: + company = self._mail_get_companies(default=self.env.company)[self.id] + if company.bounce_email: + headers['Return-Path'] = company.bounce_email + return headers # ------------------------------------------------------------ # TOOLS diff --git a/addons/mail/tests/test_ir_mail_server.py b/addons/mail/tests/test_ir_mail_server.py index 9fbd71f6d27..7137951bd9c 100644 --- a/addons/mail/tests/test_ir_mail_server.py +++ b/addons/mail/tests/test_ir_mail_server.py @@ -27,26 +27,25 @@ class TestIrMailServer(MailCommon): @patch.dict(config.options, {"email_from": "settings@example.com"}) def test_default_email_from(self, *args): - """ Check that 'mail.default.from' setting is respected. """ - ICP = self.env["ir.config_parameter"].sudo() - for (icp_from, icp_domain), expected_from in zip( + """ Check that default_from parameter of alias domain respected. """ + for (default_from, domain_name), expected_from in zip( [ ('icp', 'test.mycompany.com'), - ('icp', False), - ('icp@another.company.com', 'test.mycompany.com'), - ('icp@another.company.com', False), (False, 'test.mycompany.com'), + (False, False), ], [ "icp@test.mycompany.com", "settings@example.com", - "icp@another.company.com", - "icp@another.company.com", "settings@example.com", ], ): - with self.subTest(icp_from=icp_from, icp_domain=icp_domain): - ICP.set_param("mail.default.from", icp_from) - ICP.set_param("mail.catchall.domain", icp_domain) + with self.subTest(default_from=default_from, domain_name=domain_name): + if domain_name: + self.mail_alias_domain.name = domain_name + self.mail_alias_domain.default_from = default_from + self.env.company.alias_domain_id = self.mail_alias_domain + else: + self.env.company.alias_domain_id = False message = self.env["ir.mail_server"].build_email( False, "recipient@example.com", "Subject", "The body of an email", @@ -131,7 +130,7 @@ class TestIrMailServer(MailCommon): @users('admin') def test_mail_server_get_test_email_from(self): """ Test the email used to test the mail server connection. Check - from_filter parsing / mail.default.from support. """ + from_filter parsing / alias_domain.default_from support. """ test_server = self.env['ir.mail_server'].create({ 'from_filter': 'example_2.com, example_3.com', 'name': 'Test Server', @@ -142,27 +141,21 @@ class TestIrMailServer(MailCommon): # check default.from / filter matching for (default_from, from_filter), expected_test_email in zip( [ - ('notifications@example.com', 'example_2.com, example_3.com'), - ('notifications', 'example_2.com, example_3.com'), - ('notifications@example.com', 'dummy.com, full_email@example_2.com, dummy2.com'), ('notifications', 'dummy.com, full_email@example_2.com, dummy2.com'), - (f'notifications@{self.alias_domain}', f'{self.alias_domain}'), - (f'notifications@{self.alias_domain}', f'{self.alias_domain}, example_2.com'), + ('notifications', self.mail_alias_domain.name), + ('notifications', f'{self.mail_alias_domain.name}, example_2.com'), # default relies on "odoo" - (False, 'example.com'), + (False, self.mail_alias_domain.name), # fallback on user email if no from_filter - ('example_2.com', ' '), - ('example_2.com', ','), - ('example_2.com', False), + ('notifications', ' '), + ('notifications', ','), + ('notifications', False), (False, False), ], [ - 'noreply@example_2.com', - 'notifications@example_2.com', 'full_email@example_2.com', - 'full_email@example_2.com', - f'notifications@{self.alias_domain}', - f'notifications@{self.alias_domain}', - 'odoo@example.com', + f'notifications@{self.mail_alias_domain.name}', + f'notifications@{self.mail_alias_domain.name}', + f'odoo@{self.mail_alias_domain.name}', self.env.user.email, self.env.user.email, self.env.user.email, @@ -170,7 +163,7 @@ class TestIrMailServer(MailCommon): ], ): with self.subTest(default_from=default_from, from_filter=from_filter): - self.env['ir.config_parameter'].set_param('mail.default.from', default_from) + self.mail_alias_domain.default_from = default_from test_server.from_filter = from_filter email_from = test_server._get_test_email_from() self.assertEqual(email_from, expected_test_email) @@ -299,42 +292,3 @@ class TestIrMailServer(MailCommon): message_from=f'"Name" <{self.default_from}@{self.alias_domain}>', mail_server=self.mail_server_domain, ) - - def test_mail_server_send_email_default_from(self): - """ Test the case when the "mail.default.from" contains a full email address - and not just the local part the domain of this default email address can be - different than the catchall domain """ - other_default_from = 'notifications.other.test@custom_domain.com' - self.env['ir.config_parameter'].sudo().set_param('mail.default.from', other_default_from) - custom_server = self.env['ir.mail_server'].create({ - 'from_filter': 'custom_domain.com', - 'name': 'Custom Domain Server', - 'smtp_host': 'smtp_host', - 'smtp_encryption': 'none', - }) - - for mail_from, (expected_smtp_from, expected_msg_from, expected_mail_server) in zip( - [ - # custom server domain - '"Custom Server Name" ', - # unknown domain - '"Name" ', - # notification server domain - '"Name" ' - ], [ - ('customer.test@custom_domain.com', '"Custom Server Name" ', custom_server), - (other_default_from, '"Name" ', custom_server), - (self.default_bounce_address, '"Name" ', self.mail_server_domain), - ], - ): - with self.subTest(mail_from=mail_from): - with self.mock_smtplib_connection(): - message = self._build_email(mail_from=mail_from) - self.env['ir.mail_server'].send_email(message) - - self.assertSMTPEmailsSent( - smtp_from=expected_smtp_from, - smtp_to_list=['dest@xn--example--i1a.com'], - message_from=expected_msg_from, - mail_server=expected_mail_server, - ) diff --git a/addons/mass_mailing/tests/test_mailing_retry.py b/addons/mass_mailing/tests/test_mailing_retry.py index 01b3d61c654..55bff7db9f9 100644 --- a/addons/mass_mailing/tests/test_mailing_retry.py +++ b/addons/mass_mailing/tests/test_mailing_retry.py @@ -27,7 +27,7 @@ class TestMailingRetry(MassMailCommon, CronMixinCase): mailing.action_launch() # force email sending to fail to test our retry mechanism - def patched_mail_mail_send(mail_records, auto_commit=False, raise_exception=False, smtp_session=None): + def patched_mail_mail_send(mail_records, auto_commit=False, raise_exception=False, smtp_session=None, alias_domain_id=False): mail_records.write({'state': 'exception', 'failure_reason': 'forced_failure'}) with patch('odoo.addons.mail.models.mail_mail.MailMail._send', patched_mail_mail_send): diff --git a/addons/project/models/project_task.py b/addons/project/models/project_task.py index 4a7d6a87787..04b6e34b450 100644 --- a/addons/project/models/project_task.py +++ b/addons/project/models/project_task.py @@ -1425,8 +1425,8 @@ class Task(models.Model): task._message_add_suggested_recipient(recipients, partner=task.partner_id, reason=reason) return recipients - def _notify_by_email_get_headers(self): - headers = super(Task, self)._notify_by_email_get_headers() + def _notify_by_email_get_headers(self, headers=None): + headers = super(Task, self)._notify_by_email_get_headers(headers=headers) if self.project_id: current_objects = [h for h in headers.get('X-Odoo-Objects', '').split(',') if h] current_objects.insert(0, 'project.project-%s, ' % self.project_id.id) diff --git a/addons/test_mail/models/test_mail_models.py b/addons/test_mail/models/test_mail_models.py index a8ad806a0f8..186460aa041 100644 --- a/addons/test_mail/models/test_mail_models.py +++ b/addons/test_mail/models/test_mail_models.py @@ -25,8 +25,8 @@ class MailTestSimple(models.Model): _a = super()._notify_by_email_get_final_mail_values(*args, **kwargs) return _a - def _notify_by_email_get_headers(self): - headers = super()._notify_by_email_get_headers() + def _notify_by_email_get_headers(self, headers=None): + headers = super()._notify_by_email_get_headers(headers=headers) headers['X-Custom'] = 'Done' return headers diff --git a/addons/test_mail/tests/test_mail_composer.py b/addons/test_mail/tests/test_mail_composer.py index 3becd823fad..f683689ee04 100644 --- a/addons/test_mail/tests/test_mail_composer.py +++ b/addons/test_mail/tests/test_mail_composer.py @@ -1297,7 +1297,7 @@ class TestComposerInternals(TestMailComposer): new_partners.unlink() -@tagged('mail_composer', 'multi_lang') +@tagged('mail_composer', 'multi_lang', 'multi_company') class TestComposerResultsComment(TestMailComposer, CronMixinCase): """ Test global output of composer used in comment mode. Test notably notification and emails generated during this process. """ @@ -1492,6 +1492,7 @@ class TestComposerResultsComment(TestMailComposer, CronMixinCase): # update test configuration test_records = self.test_records test_companies = self.company_admin + self.company_2 + self.company_2.alias_domain_id = self.mail_alias_domain for company, record in zip(test_companies, test_records): record.company_id = company.id @@ -1529,7 +1530,7 @@ class TestComposerResultsComment(TestMailComposer, CronMixinCase): # update mail config default_from = mail_config.get('default_from', self.default_from) from_filter = mail_config.get('from_filter', self.default_from_filter) - self.env['ir.config_parameter'].sudo().set_param('mail.default.from', default_from) + self.mail_alias_domain.default_from = default_from self.env['ir.config_parameter'].sudo().set_param('mail.default.from_filter', from_filter) for email_from, exp_smtp_from, exp_msg_from in zip(emails_from, exp_smtp_from_lst, exp_msg_from_lst): @@ -1963,13 +1964,14 @@ class TestComposerResultsComment(TestMailComposer, CronMixinCase): mail_message=message, email_values={ 'headers': { - 'Return-Path': f'{self.alias_bounce}@{self.alias_domain}', + 'Return-Path': f'{exp_alias_domain.bounce_email}', 'X-Odoo-Objects': f'{record._name}-{record.id}', }, 'subject': f'TemplateSubject {record.name}', }, fields_values={ 'headers': { + 'Return-Path': f'{exp_alias_domain.bounce_email}', 'X-Odoo-Objects': f'{record._name}-{record.id}', }, 'mail_server_id': self.env['ir.mail_server'], @@ -1983,16 +1985,19 @@ class TestComposerResultsComment(TestMailComposer, CronMixinCase): smtp_to_list = ['"test.to.1@test.example.com"', 'test.to.1@test.example.com'] else: smtp_to_list = [recipient.email_normalized] - if recipient != record.customer_id: - emails_count = len(test_records) # not distinguishable using this assert + if exp_alias_domain == self.mail_alias_domain: + self.assertSMTPEmailsSent( + smtp_from=f'{self.default_from}@{self.alias_domain}', + smtp_to_list=smtp_to_list, + mail_server=self.mail_server_notification, + emails_count=1, + ) else: - emails_count = 1 - self.assertSMTPEmailsSent( - smtp_from=f'{self.default_from}@{self.alias_domain}', - smtp_to_list=smtp_to_list, - mail_server=self.mail_server_notification, - emails_count=emails_count, - ) + self.assertSMTPEmailsSent( + smtp_from=exp_alias_domain.bounce_email, + smtp_to_list=smtp_to_list, + emails_count=1, + ) @users('employee') @mute_logger('odoo.addons.mail.models.mail_mail') @@ -2695,7 +2700,6 @@ class TestComposerResultsMass(TestMailComposer): # self.assertIn(exp_button_es, sent_mail['body']) self.assertIn(exp_button_en, sent_mail['body']) - @users('employee') @mute_logger('odoo.addons.mail.models.mail_mail') def test_mail_composer_wtpl_mc(self): @@ -2722,10 +2726,11 @@ class TestComposerResultsMass(TestMailComposer): attachs = self.env['ir.attachment'].search([('name', 'in', [a['name'] for a in attachment_data])]) self.assertEqual(len(attachs), 2) - for companies, expected_companies in [ + for companies, expected_companies, expected_alias_domains in [ ( self.company_admin + self.company_2, self.company_admin + self.company_2, + self.mail_alias_domain + self.mail_alias_domain_c2, ), ]: with self.subTest(companies=companies): @@ -2751,8 +2756,8 @@ class TestComposerResultsMass(TestMailComposer): new_partner = self.env['res.partner'].search([('email_normalized', '=', 'test.to.1@test.example.com')]) self.assertEqual(len(new_partner), 1) # check output, company-specific values mainly for this test - for record, exp_company in zip( - test_records, expected_companies + for record, exp_company, exp_alias_domain in zip( + test_records, expected_companies, expected_alias_domains ): # message copy is kept message = record.message_ids[0] @@ -2764,16 +2769,19 @@ class TestComposerResultsMass(TestMailComposer): mail_message=message, email_values={ 'headers': { - 'Return-Path': f'{self.alias_bounce}@{self.alias_domain}', + 'Return-Path': f'{exp_alias_domain.bounce_email}', 'X-Odoo-Objects': f'{record._name}-{record.id}', }, 'subject': f'TemplateSubject {record.name}', }, fields_values={ 'headers': { + 'Return-Path': f'{exp_alias_domain.bounce_email}', 'X-Odoo-Objects': f'{record._name}-{record.id}', }, 'mail_server_id': self.env['ir.mail_server'], + 'record_alias_domain_id': exp_alias_domain, + 'record_company_id': exp_company, 'subject': f'TemplateSubject {record.name}', }, ) @@ -2783,16 +2791,19 @@ class TestComposerResultsMass(TestMailComposer): smtp_to_list = ['"test.to.1@test.example.com"', 'test.to.1@test.example.com'] else: smtp_to_list = [recipient.email_normalized] - if recipient != record.customer_id: - emails_count = len(test_records) # not distinguishable using this assert + if exp_alias_domain == self.mail_alias_domain: + self.assertSMTPEmailsSent( + smtp_from=f'{self.default_from}@{self.alias_domain}', + smtp_to_list=smtp_to_list, + mail_server=self.mail_server_notification, + emails_count=1, + ) else: - emails_count = 1 - self.assertSMTPEmailsSent( - smtp_from=f'{self.default_from}@{self.alias_domain}', - smtp_to_list=smtp_to_list, - mail_server=self.mail_server_notification, - emails_count=emails_count, - ) + self.assertSMTPEmailsSent( + smtp_from=exp_alias_domain.bounce_email, + smtp_to_list=smtp_to_list, + emails_count=1, + ) @users('employee') @mute_logger('odoo.models.unlink', 'odoo.addons.mail.models.mail_mail') diff --git a/addons/test_mail/tests/test_mail_mail.py b/addons/test_mail/tests/test_mail_mail.py index aee64987025..4b050ad9de2 100644 --- a/addons/test_mail/tests/test_mail_mail.py +++ b/addons/test_mail/tests/test_mail_mail.py @@ -294,7 +294,7 @@ class TestMailMail(MailCommon): # MailServer.build_email(): invalid from (missing) for default_from in [False, '']: - self.env['ir.config_parameter'].set_param('mail.default.from', default_from) + self.mail_alias_domain.default_from = default_from self._reset_data() with self.mock_mail_gateway(), mute_logger('odoo.addons.mail.models.mail_mail'): mail.send(raise_exception=False) @@ -311,8 +311,9 @@ class TestMailMail(MailCommon): self.assertEqual(notification.notification_status, 'exception') # MailServer.send_email(): _prepare_email_message: unexpected ASCII / Malformed 'Return-Path' or 'From' address - # Force catchall domain to void otherwise bounce is set to postmaster-odoo@domain - self.env['ir.config_parameter'].set_param('mail.catchall.domain', '') + # Force bounce alias to void, will force usage of email_from + self.mail_alias_domain.bounce_alias = False + self.env.company.invalidate_recordset(fnames={'bounce_email', 'bounce_formatted'}) for email_from in ['strange@example¢¡.com', 'robert']: self._reset_data() mail.write({'email_from': email_from}) @@ -326,20 +327,6 @@ class TestMailMail(MailCommon): self.assertEqual(notification.failure_type, 'mail_from_invalid') self.assertEqual(notification.notification_status, 'exception') - # MailServer.send_email(): _prepare_email_message: unexpected ASCII based on catchall domain in bounce - self.env['ir.config_parameter'].set_param('mail.catchall.domain', 'domain¢¡.com') - self._reset_data() - mail.write({'email_from': 'test.user@example.com'}) - with self.mock_mail_gateway(): - mail.send(raise_exception=False) - self.assertEqual(self._mails[0]['email_from'], 'test.user@example.com') - self.assertIn("Malformed 'Return-Path' or 'From' address: bounce.test@domain¢¡.com", mail.failure_reason) - self.assertEqual(mail.failure_type, 'mail_from_invalid') - self.assertEqual(mail.state, 'exception') - self.assertEqual(notification.failure_reason, "Malformed 'Return-Path' or 'From' address: bounce.test@domain¢¡.com - It should contain one valid plain ASCII email") - self.assertEqual(notification.failure_type, 'mail_from_invalid') - self.assertEqual(notification.notification_status, 'exception') - @mute_logger('odoo.addons.mail.models.mail_mail') def test_mail_mail_send_exceptions_recipients_emails(self): """ Test various use case with exceptions and errors and see how they are diff --git a/addons/test_mail/tests/test_message_post.py b/addons/test_mail/tests/test_message_post.py index 13feeb12c2b..084b19cff25 100644 --- a/addons/test_mail/tests/test_message_post.py +++ b/addons/test_mail/tests/test_message_post.py @@ -973,11 +973,12 @@ class TestMessagePost(TestMessagePostCommon, CronMixinCase): 'content': 'Body', 'email_values': { 'headers': { - 'Return-Path': f'{self.alias_bounce}@{self.alias_domain}', + 'Return-Path': f'{expected_alias_domain.bounce_alias}@{expected_alias_domain.name}', }, }, 'mail_mail_values': { 'headers': { + 'Return-Path': f'{expected_alias_domain.bounce_alias}@{expected_alias_domain.name}', 'X-Odoo-Objects': f'{record._name}-{record.id}', }, }, @@ -1301,7 +1302,7 @@ class TestMessagePost(TestMessagePostCommon, CronMixinCase): ) self.assertEqual(msg.subject, '1st line 2nd line') - @mute_logger('odoo.addons.mail.models.mail_mail') + @mute_logger('odoo.addons.base.models.ir_model', 'odoo.addons.mail.models.mail_mail') def test_portal_acls(self): self.test_record.message_subscribe((self.partner_1 | self.user_employee.partner_id).ids) diff --git a/odoo/addons/base/models/ir_mail_server.py b/odoo/addons/base/models/ir_mail_server.py index 03529e9e2f9..6f364ae88ab 100644 --- a/odoo/addons/base/models/ir_mail_server.py +++ b/odoo/addons/base/models/ir_mail_server.py @@ -363,8 +363,7 @@ class IrMailServer(models.Model): if mail_server: from_filter = mail_server.from_filter else: - from_filter = self.env['ir.config_parameter'].sudo().get_param( - 'mail.default.from_filter', tools.config.get('from_filter')) + from_filter = self.env['ir.mail_server']._get_default_from_filter() smtp_encryption = encryption if smtp_encryption is None and tools.config.get('smtp_ssl'): @@ -474,7 +473,7 @@ class IrMailServer(models.Model): :rtype: email.message.EmailMessage :return: the new RFC2822 email message """ - email_from = email_from or self._get_default_from_address() + email_from = email_from or self.env.context.get('domain_notifications_email') or self._get_default_from_address() assert email_from, self.NO_FOUND_FROM headers = headers or {} # need valid dict later @@ -540,6 +539,19 @@ class IrMailServer(models.Model): """ return tools.config.get("email_from") + @api.model + def _get_default_from_filter(self): + """ Computes the default from_filter. It is used when no specific + ir.mail_server is used when sending emails, hence having no value for + from_filter. + + :return str/None: defaults to 'mail.default.from_filter', then + ``--from-filter`` CLI/config parameter. + """ + return self.env['ir.config_parameter'].sudo().get_param( + 'mail.default.from_filter', tools.config.get('from_filter') + ) + def _prepare_email_message(self, message, smtp_session): """Prepare the SMTP information (from, to, message) before sending. @@ -554,7 +566,9 @@ class IrMailServer(models.Model): # Use the default bounce address **only if** no Return-Path was # provided by caller. Caller may be using Variable Envelope Return # Path (VERP) to detect no-longer valid email addresses. - bounce_address = message['Return-Path'] or self._get_default_bounce_address() or message['From'] + # context may force a value, e.g. mail.alias.domain usage + bounce_address = self.env.context.get('domain_bounce_address') or message['Return-Path'] or self._get_default_bounce_address() or message['From'] + smtp_from = message['From'] or bounce_address assert smtp_from, self.NO_FOUND_SMTP_FROM @@ -579,11 +593,13 @@ class IrMailServer(models.Model): del message['To'] # avoid multiple To: headers! message['To'] = x_forge_to - # Try to not spoof the mail from headers + # Try to not spoof the mail from headers; fetch session-based or contextualized + # values for encapsulation computation from_filter = getattr(smtp_session, 'from_filter', False) smtp_from = getattr(smtp_session, 'smtp_from', False) or smtp_from - - notifications_email = email_normalize(self._get_default_from_address()) + notifications_email = email_normalize( + self.env.context.get('domain_notifications_email') or self._get_default_from_address() + ) if notifications_email and smtp_from == notifications_email and message['From'] != notifications_email: smtp_from = encapsulate_email(message['From'], notifications_email) @@ -699,7 +715,7 @@ class IrMailServer(models.Model): """ email_from_normalized = email_normalize(email_from) email_from_domain = email_domain_extract(email_from_normalized) - notifications_email = email_normalize(self._get_default_from_address()) + notifications_email = self.env.context.get('domain_notifications_email') or email_normalize(self._get_default_from_address()) notifications_domain = email_domain_extract(notifications_email) if mail_servers is None: @@ -741,8 +757,7 @@ class IrMailServer(models.Model): return mail_servers[0], email_from # 5: SMTP config in odoo-bin arguments - from_filter = self.env['ir.config_parameter'].sudo().get_param( - 'mail.default.from_filter', tools.config.get('from_filter')) + from_filter = self.env['ir.mail_server']._get_default_from_filter() if self._match_from_filter(email_from, from_filter): return None, email_from diff --git a/odoo/addons/base/tests/test_ir_mail_server.py b/odoo/addons/base/tests/test_ir_mail_server.py index b9c328f5f91..827b8584256 100644 --- a/odoo/addons/base/tests/test_ir_mail_server.py +++ b/odoo/addons/base/tests/test_ir_mail_server.py @@ -291,6 +291,36 @@ class TestIrMailServer(TransactionCase, MockSmtplibCase): from_filter=False, ) + @mute_logger('odoo.models.unlink') + def test_mail_server_send_email_context_force(self): + """ Allow to force notifications_email / bounce_address from context + to allow higher-level apps to send values until end of mail stack + without hacking too much models. """ + # custom notification / bounce email from context + context_server = self.env['ir.mail_server'].create({ + 'from_filter': 'context.example.com', + 'name': 'context', + 'smtp_host': 'test', + }) + IrMailServer = self.env["ir.mail_server"].with_context( + domain_notifications_email="notification@context.example.com", + domain_bounce_address="bounce@context.example.com", + ) + with self.mock_smtplib_connection(): + mail_server, smtp_from = IrMailServer._find_mail_server(email_from='"Name" ') + self.assertEqual(mail_server, context_server) + self.assertEqual(smtp_from, "notification@context.example.com") + smtp_session = IrMailServer.connect(smtp_from=smtp_from) + message = self._build_email(mail_from='"Name" ') + IrMailServer.send_email(message, smtp_session=smtp_session) + + self.assertEqual(len(self.emails), 1) + self.assertSMTPEmailsSent( + smtp_from="bounce@context.example.com", + message_from='"Name" ', + from_filter=context_server.from_filter, + ) + @mute_logger('odoo.models.unlink') def test_mail_server_send_email_IDNA(self): """ Test that the mail from / recipient envelop are encoded using IDNA """ From 11def7563ee998b8da2abc8ffacb15a2f594ec6e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Mon, 25 Oct 2021 15:32:01 +0200 Subject: [PATCH 15/18] [IMP] base_setup, mail, hr: allow to configure company alias domains 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 Update settings to configure your company's current alias domain instead of the global 'alias.catchall.domain' configuration parameter. As domain configuration is globally done per company, this is now a related on the company. Advanced configuration and management can still be done manually in settings. Field 'external_email_server_default' triggering display of mail configuration is moved to mail. It is defined in 'base_setup' but used only in mail hence moving the field to be coherent. Task-36879 (Mail: Support Multi Domains Aliases) Part-of: odoo/odoo#76734 --- addons/base_setup/models/res_config_settings.py | 3 --- .../views/res_config_settings_views.xml | 11 ++++++----- addons/mail/models/res_config_settings.py | 8 ++++++-- addons/mail/views/res_config_settings_views.xml | 16 ++++++++++++---- 4 files changed, 24 insertions(+), 14 deletions(-) diff --git a/addons/base_setup/models/res_config_settings.py b/addons/base_setup/models/res_config_settings.py index 32981b403fb..387b4a6cc5e 100644 --- a/addons/base_setup/models/res_config_settings.py +++ b/addons/base_setup/models/res_config_settings.py @@ -15,9 +15,6 @@ class ResConfigSettings(models.TransientModel): user_default_rights = fields.Boolean( "Default Access Rights", config_parameter='base_setup.default_user_rights') - external_email_server_default = fields.Boolean( - "Use Custom Email Servers", - config_parameter='base_setup.default_external_email_server') module_base_import = fields.Boolean("Allow users to import data from CSV/XLS/XLSX/ODS files") module_google_calendar = fields.Boolean( string='Allow the users to synchronize their calendar with Google Calendar') diff --git a/addons/hr_expense/views/res_config_settings_views.xml b/addons/hr_expense/views/res_config_settings_views.xml index 45e108356c5..a0f5a714729 100644 --- a/addons/hr_expense/views/res_config_settings_views.xml +++ b/addons/hr_expense/views/res_config_settings_views.xml @@ -14,18 +14,19 @@ help="Create expenses from incoming emails" title="Send an email to this email alias with the receipt in attachment to create an expense in one click. If the first word of the mail subject contains the category's internal reference or the category name, the corresponding category will automatically be set. Type the expense amount in the mail subject to set it on the expense too."> -
-
+
+
-
+
diff --git a/addons/mail/models/res_config_settings.py b/addons/mail/models/res_config_settings.py index 470200d2532..c57b86d15e3 100644 --- a/addons/mail/models/res_config_settings.py +++ b/addons/mail/models/res_config_settings.py @@ -12,9 +12,13 @@ class ResConfigSettings(models.TransientModel): the alias domain. """ _inherit = 'res.config.settings' + external_email_server_default = fields.Boolean( + "Use Custom Email Servers", + config_parameter='base_setup.default_external_email_server') fail_counter = fields.Integer('Fail Mail', compute="_compute_fail_counter") - alias_domain = fields.Char( - 'Alias Domain', config_parameter='mail.catchall.domain', + alias_domain_id = fields.Many2one( + 'mail.alias.domain', 'Alias Domain', + readonly=False, related='company_id.alias_domain_id', help="If you have setup a catch-all email domain redirected to the Odoo server, enter the domain name here.") module_google_gmail = fields.Boolean('Support Gmail Authentication') module_microsoft_outlook = fields.Boolean('Support Outlook Authentication') diff --git a/addons/mail/views/res_config_settings_views.xml b/addons/mail/views/res_config_settings_views.xml index 767acd38c3e..ddb40d0ae2a 100644 --- a/addons/mail/views/res_config_settings_views.xml +++ b/addons/mail/views/res_config_settings_views.xml @@ -33,7 +33,18 @@
- + + @ + + +
Save this page and come back here to set up the feature.
@@ -51,9 +62,6 @@
- - @ -
From c68ddce1b936a23da8abef0ece6e81d1daab5c46 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Fri, 22 Oct 2021 16:48:00 +0200 Subject: [PATCH 16/18] [IMP] various: update alias management in views 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 Update fields used in form views. Use 'alias_domain_id' instead of domain that is now simply a related on 'alias_domain_id.name'. To ease UI we add a placeholder on this field as it is now writable e.g. in multi domains environment. Avoid unwanted configuration change by making it generally no_open / no_create_edit. In project, remove an unnecessary field adding complexity for few real use case now that aliases are more open to configuration. Task-36879 (Mail: Support Multi Domains Aliases) Part-of: odoo/odoo#76734 --- .../views/account_journal_dashboard_view.xml | 4 +-- .../account/views/account_journal_views.xml | 17 +++++++++---- addons/crm/views/crm_team_views.xml | 19 ++++++-------- addons/hr_recruitment/views/hr_job_views.xml | 25 +++++++++++-------- addons/mail_group/views/mail_group_views.xml | 19 +++++++------- .../maintenance/views/maintenance_views.xml | 6 +++-- addons/project/data/digest_data.xml | 2 +- addons/project/models/project_project.py | 10 -------- .../project/views/project_project_views.xml | 18 ++++++------- 9 files changed, 60 insertions(+), 60 deletions(-) diff --git a/addons/account/views/account_journal_dashboard_view.xml b/addons/account/views/account_journal_dashboard_view.xml index 96111a366f9..75a31af4c4a 100644 --- a/addons/account/views/account_journal_dashboard_view.xml +++ b/addons/account/views/account_journal_dashboard_view.xml @@ -12,7 +12,7 @@ - +
@@ -45,7 +45,7 @@ -
-
+
diff --git a/addons/account/views/account_journal_views.xml b/addons/account/views/account_journal_views.xml index f446d3490f7..926c055f575 100644 --- a/addons/account/views/account_journal_views.xml +++ b/addons/account/views/account_journal_views.xml @@ -143,18 +143,25 @@ - + - +
- Configure Email Servers + + Choose or Configure Email Servers +
- + diff --git a/addons/crm/views/crm_team_views.xml b/addons/crm/views/crm_team_views.xml index 08620447bfd..2645aad2e09 100644 --- a/addons/crm/views/crm_team_views.xml +++ b/addons/crm/views/crm_team_views.xml @@ -159,18 +159,13 @@ - + diff --git a/addons/project/data/digest_data.xml b/addons/project/data/digest_data.xml index 8be2e75226c..9908e34cb53 100644 --- a/addons/project/data/digest_data.xml +++ b/addons/project/data/digest_data.xml @@ -28,7 +28,7 @@
- +

Tip: Create tasks from incoming emails

Emails sent to will generate tasks in your project.

diff --git a/addons/project/models/project_project.py b/addons/project/models/project_project.py index 170e7de20f8..3d664bafc05 100644 --- a/addons/project/models/project_project.py +++ b/addons/project/models/project_project.py @@ -136,7 +136,6 @@ class Project(models.Model): domain=[('state', 'not in', list(CLOSED_STATES))]) color = fields.Integer(string='Color Index') user_id = fields.Many2one('res.users', string='Project Manager', default=lambda self: self.env.user, tracking=True) - alias_enabled = fields.Boolean(string='Use Email Alias', compute='_compute_alias_enabled', readonly=False) alias_id = fields.Many2one(help="Internal email associated with this project. Incoming emails are automatically synchronized " "with Tasks (or optionally Issues if the Issue Tracker module is installed).") privacy_visibility = fields.Selection([ @@ -215,11 +214,6 @@ class Project(models.Model): ('project_date_greater', 'check(date >= date_start)', "The project's start date must be before its end date.") ] - @api.onchange('alias_enabled') - def _onchange_alias_name(self): - if not self.alias_enabled: - self.alias_name = False - @api.onchange('company_id') def _onchange_company_id(self): if (self.env.user.has_group('project.group_project_stages') and self.stage_id.company_id @@ -230,10 +224,6 @@ class Project(models.Model): limit=1, ).id - def _compute_alias_enabled(self): - for project in self: - project.alias_enabled = bool(project.alias_email) - def _compute_access_url(self): super(Project, self)._compute_access_url() for project in self: diff --git a/addons/project/views/project_project_views.xml b/addons/project/views/project_project_views.xml index 81aed92a234..31b8f7fa3be 100644 --- a/addons/project/views/project_project_views.xml +++ b/addons/project/views/project_project_views.xml @@ -108,19 +108,20 @@ -
+
-
+ invisible="not alias_email"/> @@ -335,11 +336,12 @@
-
+
@@ -392,8 +394,6 @@ - - From ab8c0e4c6f70a990eb5bfcbfed548f63c625470f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Wed, 23 Aug 2023 18:27:01 +0200 Subject: [PATCH 17/18] [REF] mail, various: remove now dead configuration parameters 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 Now that alias domains are used in Odoo codebase there is no usage anymore for the old config parameters. So long and thanks for all the fish ! Task-36879 (Mail: Support Multi Domains Aliases) Part-of: odoo/odoo#76734 --- addons/mail/__manifest__.py | 1 - addons/mail/data/ir_config_parameter.xml | 9 ---- addons/mail/data/ir_config_parameter_data.xml | 20 ++------- addons/mail/models/ir_config_parameter.py | 3 -- addons/mail/tests/common.py | 4 -- addons/test_mail/tests/test_mail_alias.py | 43 ------------------- addons/test_mail/tests/test_mail_message.py | 1 - odoo/addons/base/tests/test_ir_mail_server.py | 4 +- 8 files changed, 4 insertions(+), 81 deletions(-) delete mode 100644 addons/mail/data/ir_config_parameter.xml diff --git a/addons/mail/__manifest__.py b/addons/mail/__manifest__.py index f87807e5ddd..09e9b82a92a 100644 --- a/addons/mail/__manifest__.py +++ b/addons/mail/__manifest__.py @@ -99,7 +99,6 @@ For more specific needs, you may also assign custom-defined actions 'data/mail_activity_data.xml', 'data/security_notifications_templates.xml', 'data/ir_cron_data.xml', - 'data/ir_config_parameter.xml', 'security/mail_security.xml', 'security/ir.model.access.csv', 'views/discuss_public_templates.xml', diff --git a/addons/mail/data/ir_config_parameter.xml b/addons/mail/data/ir_config_parameter.xml deleted file mode 100644 index f8523283883..00000000000 --- a/addons/mail/data/ir_config_parameter.xml +++ /dev/null @@ -1,9 +0,0 @@ - - - - - mail.activity.gc.delete_overdue_years - 3 - - - diff --git a/addons/mail/data/ir_config_parameter_data.xml b/addons/mail/data/ir_config_parameter_data.xml index 69b5800e8d4..a523ab92b83 100644 --- a/addons/mail/data/ir_config_parameter_data.xml +++ b/addons/mail/data/ir_config_parameter_data.xml @@ -1,23 +1,9 @@ - - - mail.catchall.alias - catchall + + mail.activity.gc.delete_overdue_years + 3 - - - - mail.bounce.alias - bounce - - - - - mail.default.from - notifications - - diff --git a/addons/mail/models/ir_config_parameter.py b/addons/mail/models/ir_config_parameter.py index 9112fd6ed27..a94f2549b22 100644 --- a/addons/mail/models/ir_config_parameter.py +++ b/addons/mail/models/ir_config_parameter.py @@ -20,9 +20,6 @@ class IrConfigParameter(models.Model): # remove existing users, including inactive template user # admin will regain the right via implied_ids on group_system group_user._remove_group(group_mail_template_editor) - # sanitize, normalize; note that False = unlink ICP - elif key in {'mail.bounce.alias', 'mail.catchall.alias'} and value: - value = self.env['mail.alias']._sanitize_alias_name(value) # sanitize and normalize allowed catchall domains elif key == 'mail.catchall.domain.allowed' and value: value = self.env['mail.alias']._sanitize_allowed_domains(value) diff --git a/addons/mail/tests/common.py b/addons/mail/tests/common.py index 0149d0cfec2..fc8c4b7cc32 100644 --- a/addons/mail/tests/common.py +++ b/addons/mail/tests/common.py @@ -101,10 +101,6 @@ class MockEmail(common.BaseCase, MockSmtplibCase): cls.alias_bounce = 'bounce.test' cls.default_from = 'notifications.test' cls.default_from_filter = False - cls.env['ir.config_parameter'].set_param('mail.bounce.alias', cls.alias_bounce) - cls.env['ir.config_parameter'].set_param('mail.catchall.domain', cls.alias_domain) - cls.env['ir.config_parameter'].set_param('mail.catchall.alias', cls.alias_catchall) - cls.env['ir.config_parameter'].set_param('mail.default.from', cls.default_from) cls.env['ir.config_parameter'].set_param('mail.default.from_filter', cls.default_from_filter) # ensure global alias domain for tests: to ease tests, search or create diff --git a/addons/test_mail/tests/test_mail_alias.py b/addons/test_mail/tests/test_mail_alias.py index fe9ffeeb463..af639075c6a 100644 --- a/addons/test_mail/tests/test_mail_alias.py +++ b/addons/test_mail/tests/test_mail_alias.py @@ -46,49 +46,6 @@ class TestMailAlias(TestMailAliasCommon): self.env['ir.config_parameter'].set_param('mail.catchall.domain.allowed', value) self.assertEqual(self.env['ir.config_parameter'].get_param('mail.catchall.domain.allowed'), expected) - @users('admin') - @mute_logger('odoo.models.unlink') - def test_alias_domain_parameters(self): - """ Check the validation of ``mail.bounce.alias`` and ``mail.catchall.alias`` - parameters. """ - ICP = self.env['ir.config_parameter'] - # sanitization - for (bounce_value, catchall_value), (expected_bounce, expected_catchall) in zip( - [ - ('bounce+b4r=*R3wl_#_-$€{}[]()~|\\/!?&%^\'"`~', 'catchall+b4r=*R3wl_#_-$€{}[]()~|\\/!?&%^\'"`~'), - ('bounce+😊', 'catchall+😊'), - ('Bouncâïde 😊', 'Catchôïee 😊'), - ('ぁ', 'ぁぁ'), - ], - [ - ('bounce+b4r=*r3wl_#_-$-{}-~|-/!?&%^\'-`~', 'catchall+b4r=*r3wl_#_-$-{}-~|-/!?&%^\'-`~'), - ('bounce+-', 'catchall+-'), - ('bouncaide-', 'catchoiee-'), - ('?', '??'), - ] - ): - with self.subTest(bounce_value=bounce_value): - ICP.set_param('mail.bounce.alias', bounce_value) - self.assertEqual(ICP.get_param('mail.bounce.alias'), expected_bounce) - with self.subTest(catchall_value=catchall_value): - ICP.set_param('mail.catchall.alias', catchall_value) - self.assertEqual(ICP.get_param('mail.catchall.alias'), expected_catchall) - - # falsy values - for config_value in [False, None, '', ' ']: - with self.subTest(config_value=config_value): - ICP.set_param('mail.bounce.alias', config_value) - self.assertFalse(ICP.get_param('mail.bounce.alias')) - ICP.set_param('mail.catchall.alias', config_value) - self.assertFalse(ICP.get_param('mail.catchall.alias')) - - # check successive param set, should not raise for unicity against itself - for _ in range(2): - ICP.set_param('mail.bounce.alias', 'bounce+double.test') - ICP.set_param('mail.catchall.alias', 'catchall+double.test') - self.assertEqual(ICP.get_param('mail.bounce.alias'), 'bounce+double.test') - self.assertEqual(ICP.get_param('mail.catchall.alias'), 'catchall+double.test') - @users('erp_manager') def test_alias_domain_company_check(self): """ Check constraint trying to avoid ill-defined company setup aka diff --git a/addons/test_mail/tests/test_mail_message.py b/addons/test_mail/tests/test_mail_message.py index 126bb9fd909..f54dbff8557 100644 --- a/addons/test_mail/tests/test_mail_message.py +++ b/addons/test_mail/tests/test_mail_message.py @@ -460,7 +460,6 @@ class TestMessageAccess(MailCommon): def test_mail_message_access_create_reply(self): # TDE FIXME: should it really work ? not sure - catchall makes crash (aka, post will crash also) - self.env['ir.config_parameter'].set_param('mail.catchall.domain', False) self.message.write({'partner_ids': [(4, self.user_employee.partner_id.id)]}) self.env['mail.message'].with_user(self.user_employee).create({'model': 'discuss.channel', 'res_id': self.private_group.id, 'body': 'Test', 'parent_id': self.message.id}) diff --git a/odoo/addons/base/tests/test_ir_mail_server.py b/odoo/addons/base/tests/test_ir_mail_server.py index 827b8584256..cfc4d986c7f 100644 --- a/odoo/addons/base/tests/test_ir_mail_server.py +++ b/odoo/addons/base/tests/test_ir_mail_server.py @@ -51,9 +51,7 @@ class TestIrMailServer(TransactionCase, MockSmtplibCase): @classmethod def setUpClass(cls): super().setUpClass() - # TEMPORARY - for icp in {'mail.default.from', 'mail.catchall.alias', 'mail.bounce.alias', 'mail.catchall.domain', 'mail.default.from_filter'}: - cls.env['ir.config_parameter'].sudo().set_param(icp, False) + cls.env['ir.config_parameter'].sudo().set_param('mail.default.from_filter', False) cls._init_mail_servers() cls.default_bounce_address = 'CACA' cls.default_from_address = 'PROUT' From 162b74e40270a4abbe4d4b291f2769a782b444d6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Fri, 20 Oct 2023 16:10:51 +0200 Subject: [PATCH 18/18] [FIX] various: update query counters Update counters after all changes. Notably storage of environment variables (company, alias domain) during mail creation and sending process as well as computing values depending on alias domain (e.g. reply_to, return-path) lead to some additional queries to read companies and their alias domains. Task-36879 (Mail: Support Multi Domains Aliases) Part-of: odoo/odoo#76734 --- .../crm/tests/test_crm_lead_convert_mass.py | 4 +- addons/crm/tests/test_performances.py | 2 +- .../tests/test_performance.py | 6 +-- addons/test_mail/tests/test_performance.py | 44 +++++++++---------- .../tests/test_mail_performance.py | 2 +- addons/test_mail_full/tests/test_rating.py | 4 +- .../tests/test_sms_performance.py | 8 ++-- .../tests/test_performance.py | 4 +- 8 files changed, 37 insertions(+), 37 deletions(-) diff --git a/addons/crm/tests/test_crm_lead_convert_mass.py b/addons/crm/tests/test_crm_lead_convert_mass.py index 0a3010fc3fa..0da42cebb26 100644 --- a/addons/crm/tests/test_crm_lead_convert_mass.py +++ b/addons/crm/tests/test_crm_lead_convert_mass.py @@ -42,7 +42,7 @@ class TestLeadConvertMass(crm_common.TestLeadConvertMassCommon): with self.assertQueryCount(user_sales_manager=0): test_leads = self.env['crm.lead'].browse(test_leads.ids) - with self.assertQueryCount(user_sales_manager=581): # crm 579 / com 581 / ent 581 + with self.assertQueryCount(user_sales_manager=583): # crm 543 / com 545 / ent 583 test_leads._handle_salesmen_assignment(user_ids=user_ids, team_id=team_id) self.assertEqual(test_leads.team_id, self.sales_team_convert) @@ -167,7 +167,7 @@ class TestLeadConvertMass(crm_common.TestLeadConvertMassCommon): user_ids = self.assign_users.ids # randomness: at least 1 query - with self.assertQueryCount(user_sales_manager=1800): # crm 1503 / com 1790 / ent 1798 + with self.assertQueryCount(user_sales_manager=1800): # crm 1503 / com 1790 / ent 1800 mass_convert = self.env['crm.lead2opportunity.partner.mass'].with_context({ 'active_model': 'crm.lead', 'active_ids': test_leads.ids, diff --git a/addons/crm/tests/test_performances.py b/addons/crm/tests/test_performances.py index acc771fc3ef..6b15d99a402 100644 --- a/addons/crm/tests/test_performances.py +++ b/addons/crm/tests/test_performances.py @@ -178,7 +178,7 @@ class TestLeadAssignPerf(TestLeadAssignCommon): # randomness: add 2 queries with self.with_user('user_sales_manager'): - with self.assertQueryCount(user_sales_manager=6066): # crm 6048 / com 6052 / ent 6054 + with self.assertQueryCount(user_sales_manager=6066): # crm 6048 / com 6052 / ent 6055 self.env['crm.team'].browse(sales_teams.ids)._action_assign_leads(work_days=30) # teams assign diff --git a/addons/hr_work_entry_holidays/tests/test_performance.py b/addons/hr_work_entry_holidays/tests/test_performance.py index 7b81bbfa362..31fe439f7b9 100644 --- a/addons/hr_work_entry_holidays/tests/test_performance.py +++ b/addons/hr_work_entry_holidays/tests/test_performance.py @@ -32,7 +32,7 @@ class TestWorkEntryHolidaysPerformance(TestWorkEntryHolidaysBase): self.richard_emp.generate_work_entries(date(2018, 1, 1), date(2018, 1, 2)) leave = self.create_leave(datetime(2018, 1, 1, 7, 0), datetime(2018, 1, 1, 18, 0)) - with self.assertQueryCount(__system__=100, admin=103): # com 96/97 + with self.assertQueryCount(__system__=101, admin=103): # com 96/97 leave.action_validate() leave.action_refuse() @@ -48,7 +48,7 @@ class TestWorkEntryHolidaysPerformance(TestWorkEntryHolidaysBase): @users('__system__', 'admin') @warmup def test_performance_leave_create(self): - with self.assertQueryCount(__system__=59, admin=59): + with self.assertQueryCount(__system__=60, admin=60): leave = self.create_leave(datetime(2018, 1, 1, 7, 0), datetime(2018, 1, 1, 18, 0)) leave.action_refuse() @@ -57,7 +57,7 @@ class TestWorkEntryHolidaysPerformance(TestWorkEntryHolidaysBase): def test_performance_leave_confirm(self): leave = self.create_leave(datetime(2018, 1, 1, 7, 0), datetime(2018, 1, 1, 18, 0)) leave.action_draft() - with self.assertQueryCount(__system__=42, admin=41): + with self.assertQueryCount(__system__=43, admin=42): leave.action_confirm() leave.state = 'refuse' diff --git a/addons/test_mail/tests/test_performance.py b/addons/test_mail/tests/test_performance.py index 3b0a293f18a..64eca45c41b 100644 --- a/addons/test_mail/tests/test_performance.py +++ b/addons/test_mail/tests/test_performance.py @@ -224,19 +224,19 @@ class TestBaseMailPerformance(BaseMailPerformance): @warmup def test_create_mail_with_tracking(self): """ Create records inheriting from 'mail.thread' (with field tracking). """ - with self.assertQueryCount(admin=8, demo=8): + with self.assertQueryCount(admin=9, demo=9): self.env['mail.performance.thread'].create({'name': 'X'}) @users('admin', 'employee') @warmup def test_create_mail_simple(self): - with self.assertQueryCount(admin=7, employee=7): + with self.assertQueryCount(admin=8, employee=8): self.env['mail.test.simple'].create({'name': 'Test'}) @users('admin', 'employee') @warmup def test_create_mail_simple_multi(self): - with self.assertQueryCount(admin=7, employee=7): + with self.assertQueryCount(admin=8, employee=8): self.env['mail.test.simple'].create([{'name': 'Test'}] * 5) @users('admin', 'employee') @@ -264,7 +264,7 @@ class TestBaseAPIPerformance(BaseMailPerformance): def test_adv_activity(self): model = self.env['mail.test.activity'] - with self.assertQueryCount(admin=7, employee=7): + with self.assertQueryCount(admin=8, employee=8): model.create({'name': 'Test'}) @users('admin', 'employee') @@ -400,7 +400,7 @@ class TestBaseAPIPerformance(BaseMailPerformance): composer_form.attachment_ids.add(attachment) composer = composer_form.save() - with self.assertQueryCount(admin=54, employee=54): # tm+com 53/53 + with self.assertQueryCount(admin=55, employee=55): # tm+com 53/53 composer._action_send_mail() # notifications @@ -454,7 +454,7 @@ class TestBaseAPIPerformance(BaseMailPerformance): test_record, test_template = self._create_test_records() test_template.write({'attachment_ids': [(5, 0)]}) - with self.assertQueryCount(admin=26, employee=26): # tm 15/15 / com 25/25 + with self.assertQueryCount(admin=27, employee=27): # tm 15/15 / com 25/25 composer = self.env['mail.compose.message'].with_context({ 'default_composition_mode': 'comment', 'default_model': test_record._name, @@ -478,7 +478,7 @@ class TestBaseAPIPerformance(BaseMailPerformance): def test_mail_composer_w_template_attachments(self): test_record, test_template = self._create_test_records() - with self.assertQueryCount(admin=27, employee=27): # tm 16/16 / com 26/26 + with self.assertQueryCount(admin=28, employee=28): # tm 16/16 / com 26/26 composer = self.env['mail.compose.message'].with_context({ 'default_composition_mode': 'comment', 'default_model': test_record._name, @@ -507,7 +507,7 @@ class TestBaseAPIPerformance(BaseMailPerformance): test_template.write({'attachment_ids': [(5, 0)]}) customer = self.env['res.partner'].browse(self.customer.ids) - with self.assertQueryCount(admin=33, employee=33): # tm 22/22 / com 32/32 + with self.assertQueryCount(admin=34, employee=34): # tm 22/22 / com 32/32 composer_form = Form( self.env['mail.compose.message'].with_context({ 'default_composition_mode': 'comment', @@ -518,7 +518,7 @@ class TestBaseAPIPerformance(BaseMailPerformance): ) composer = composer_form.save() - with self.assertQueryCount(admin=50, employee=50): # com 44/44 + with self.assertQueryCount(admin=51, employee=51): # com 45/45 composer._action_send_mail() # notifications @@ -537,7 +537,7 @@ class TestBaseAPIPerformance(BaseMailPerformance): test_record, test_template = self._create_test_records() customer = self.env['res.partner'].browse(self.customer.ids) - with self.assertQueryCount(admin=33, employee=33): # tm 22/22 / com 32/32 + with self.assertQueryCount(admin=34, employee=34): # tm 22/22 / com 32/32 composer_form = Form( self.env['mail.compose.message'].with_context({ 'default_composition_mode': 'comment', @@ -548,7 +548,7 @@ class TestBaseAPIPerformance(BaseMailPerformance): ) composer = composer_form.save() - with self.assertQueryCount(admin=69, employee=69): # com 61/61 + with self.assertQueryCount(admin=70, employee=70): # com 62/62 composer._action_send_mail() # notifications @@ -575,7 +575,7 @@ class TestBaseAPIPerformance(BaseMailPerformance): # use another user already pre-defined with the email notification type, # so the ormcache is preserved. record = self.env['mail.test.track'].create({'name': 'Test'}) - with self.assertQueryCount(admin=41, employee=41): + with self.assertQueryCount(admin=42, employee=42): record.write({ 'user_id': self.user_test_email.id, }) @@ -801,7 +801,7 @@ class TestMailAPIPerformance(BaseMailPerformance): 'mail_message_id': message.id, 'recipient_ids': [(4, pid) for pid in self.partners.ids], }) - with self.assertQueryCount(admin=7, employee=7): + with self.assertQueryCount(admin=8, employee=8): self.env['mail.mail'].sudo().browse(mail.ids).send() @mute_logger('odoo.tests', 'odoo.addons.mail.models.mail_mail', 'odoo.models.unlink') @@ -840,7 +840,7 @@ class TestMailAPIPerformance(BaseMailPerformance): unlinked_mails |= set(records.ids) unlinked_mails = set() - with self.assertQueryCount(admin=30, employee=30), \ + with self.assertQueryCount(admin=31, employee=31), \ patch.object(type(self.env['mail.mail']), 'unlink', _patched_unlink): self.env['mail.mail'].sudo().browse(mails.ids).send() @@ -860,7 +860,7 @@ class TestMailAPIPerformance(BaseMailPerformance): record = self.container.with_user(self.env.user) # about 20 (19?) queries per additional customer group - with self.assertQueryCount(admin=58, employee=57): # com 55/54 + with self.assertQueryCount(admin=61, employee=60): # com 55/54 record.message_post( body=Markup('

Test Post Performances

'), message_type='comment', @@ -878,7 +878,7 @@ class TestMailAPIPerformance(BaseMailPerformance): template = self.env.ref('test_mail.mail_test_container_tpl') # about 20 (19 ?) queries per additional customer group - with self.assertQueryCount(admin=67, employee=66): # com 62/61 + with self.assertQueryCount(admin=70, employee=69): # com 62/61 record.message_post_with_source( template, message_type='comment', @@ -968,7 +968,7 @@ class TestMailAPIPerformance(BaseMailPerformance): }) rec1 = rec.with_context(active_test=False) # to see inactive records self.assertEqual(rec1.message_partner_ids, self.partners | self.env.user.partner_id) - with self.assertQueryCount(admin=41, employee=41): + with self.assertQueryCount(admin=43, employee=43): rec.write({'user_id': self.user_portal.id}) self.assertEqual(rec1.message_partner_ids, self.partners | self.env.user.partner_id | self.user_portal.partner_id) # write tracking message @@ -988,7 +988,7 @@ class TestMailAPIPerformance(BaseMailPerformance): customer_id = self.customer.id user_id = self.user_portal.id - with self.assertQueryCount(admin=98, employee=98): + with self.assertQueryCount(admin=102, employee=102): rec = self.env['mail.test.ticket'].create({ 'name': 'Test', 'container_id': container_id, @@ -1017,7 +1017,7 @@ class TestMailAPIPerformance(BaseMailPerformance): rec1 = rec.with_context(active_test=False) # to see inactive records self.assertEqual(rec1.message_partner_ids, self.user_portal.partner_id | self.env.user.partner_id) self.assertEqual(len(rec1.message_ids), 1) - with self.assertQueryCount(admin=61, employee=61): + with self.assertQueryCount(admin=63, employee=63): rec.write({ 'name': 'Test2', 'container_id': self.container.id, @@ -1054,7 +1054,7 @@ class TestMailAPIPerformance(BaseMailPerformance): rec1 = rec.with_context(active_test=False) # to see inactive records self.assertEqual(rec1.message_partner_ids, self.user_portal.partner_id | self.env.user.partner_id) - with self.assertQueryCount(admin=67, employee=67): + with self.assertQueryCount(admin=70, employee=70): rec.write({ 'name': 'Test2', 'container_id': container_id, @@ -1087,7 +1087,7 @@ class TestMailAPIPerformance(BaseMailPerformance): rec1 = rec.with_context(active_test=False) # to see inactive records self.assertEqual(rec1.message_partner_ids, self.partners | self.env.user.partner_id | self.user_portal.partner_id) - with self.assertQueryCount(admin=33, employee=33): + with self.assertQueryCount(admin=34, employee=34): rec.write({ 'name': 'Test2', 'customer_id': customer_id, @@ -1364,7 +1364,7 @@ class TestPerformance(BaseMailPerformance): attachments = self.env['ir.attachment'].with_user(self.env.user).create(self.test_attachments_vals) # enable_logging = self.cr._enable_logging() if self.warm else nullcontext() # with self.assertQueryCount(employee=63), enable_logging: - with self.assertQueryCount(employee=66): # com 60 + with self.assertQueryCount(employee=69): # com 63 record_container.with_context({}).message_post( body=Markup('

Test body

'), subject='Test Subject', diff --git a/addons/test_mail_full/tests/test_mail_performance.py b/addons/test_mail_full/tests/test_mail_performance.py index fa05fb676cb..81e95ad9abc 100644 --- a/addons/test_mail_full/tests/test_mail_performance.py +++ b/addons/test_mail_full/tests/test_mail_performance.py @@ -91,7 +91,7 @@ class TestMailPerformance(FullBaseMailPerformance): record_ticket = self.env['mail.test.ticket.mc'].browse(self.record_ticket.ids) attachments = self.env['ir.attachment'].create(self.test_attachments_vals) - with self.assertQueryCount(employee=96): # test_mail_full: 95 + with self.assertQueryCount(employee=101): # test_mail_full: 95 new_message = record_ticket.message_post( attachment_ids=attachments.ids, body=Markup('

Test Content

'), diff --git a/addons/test_mail_full/tests/test_rating.py b/addons/test_mail_full/tests/test_rating.py index 8d2cc0c9498..055f180e3c8 100644 --- a/addons/test_mail_full/tests/test_rating.py +++ b/addons/test_mail_full/tests/test_rating.py @@ -172,7 +172,7 @@ class TestRatingPerformance(TestRatingCommon): @users('employee') @warmup def test_rating_last_value_perfs(self): - with self.assertQueryCount(employee=1616): # tmf 1313 / com 1313 + with self.assertQueryCount(employee=1617): # tmf 1313 / com 1313 self.create_ratings('mail.test.rating.thread') with self.assertQueryCount(employee=2101): # tmf 1901 @@ -184,7 +184,7 @@ class TestRatingPerformance(TestRatingCommon): @users('employee') @warmup def test_rating_last_value_perfs_with_rating_mixin(self): - with self.assertQueryCount(employee=1723): # tmf 1419 / com 1419 + with self.assertQueryCount(employee=1724): # tmf 1419 / com 1419 self.create_ratings('mail.test.rating') with self.assertQueryCount(employee=2304): # tmf 2104 diff --git a/addons/test_mail_sms/tests/test_sms_performance.py b/addons/test_mail_sms/tests/test_sms_performance.py index 061caba1048..83732903b3b 100644 --- a/addons/test_mail_sms/tests/test_sms_performance.py +++ b/addons/test_mail_sms/tests/test_sms_performance.py @@ -35,7 +35,7 @@ class TestSMSPerformance(BaseMailPerformance, sms_common.SMSCase): def test_message_sms_record_1_partner(self): record = self.test_record.with_user(self.env.user) pids = self.customer.ids - with self.subTest("QueryCount"), self.mockSMSGateway(sms_allow_unlink=True), self.assertQueryCount(employee=23): + with self.subTest("QueryCount"), self.mockSMSGateway(sms_allow_unlink=True), self.assertQueryCount(employee=27): messages = record._message_sms( body='Performance Test', partner_ids=pids, @@ -50,7 +50,7 @@ class TestSMSPerformance(BaseMailPerformance, sms_common.SMSCase): def test_message_sms_record_10_partners(self): record = self.test_record.with_user(self.env.user) pids = self.partners.ids - with self.subTest("QueryCount"), self.mockSMSGateway(sms_allow_unlink=True), self.assertQueryCount(employee=23): + with self.subTest("QueryCount"), self.mockSMSGateway(sms_allow_unlink=True), self.assertQueryCount(employee=27): messages = record._message_sms( body='Performance Test', partner_ids=pids, @@ -64,7 +64,7 @@ class TestSMSPerformance(BaseMailPerformance, sms_common.SMSCase): @warmup def test_message_sms_record_default(self): record = self.test_record.with_user(self.env.user) - with self.subTest("QueryCount"), self.mockSMSGateway(sms_allow_unlink=True), self.assertQueryCount(employee=25): + with self.subTest("QueryCount"), self.mockSMSGateway(sms_allow_unlink=True), self.assertQueryCount(employee=29): messages = record._message_sms( body='Performance Test', ) @@ -133,5 +133,5 @@ class TestSMSMassPerformance(BaseMailPerformance, sms_common.MockSMS): 'mass_keep_log': True, }) - with self.mockSMSGateway(sms_allow_unlink=True), self.assertQueryCount(employee=57): + with self.mockSMSGateway(sms_allow_unlink=True), self.assertQueryCount(employee=58): composer.action_send_sms() diff --git a/addons/test_mass_mailing/tests/test_performance.py b/addons/test_mass_mailing/tests/test_performance.py index b726baaa26e..ada1251811a 100644 --- a/addons/test_mass_mailing/tests/test_performance.py +++ b/addons/test_mass_mailing/tests/test_performance.py @@ -47,7 +47,7 @@ class TestMassMailPerformance(TestMassMailPerformanceBase): }) # runbot needs +51 compared to local - with self.assertQueryCount(__system__=1523, marketing=1524): + with self.assertQueryCount(__system__=1622, marketing=1623): mailing.action_send_mail() self.assertEqual(mailing.sent, 50) @@ -90,7 +90,7 @@ class TestMassMailBlPerformance(TestMassMailPerformanceBase): }) # runbot needs +51 compared to local - with self.assertQueryCount(__system__=1595, marketing=1596): + with self.assertQueryCount(__system__=1694, marketing=1695): mailing.action_send_mail() self.assertEqual(mailing.sent, 50)