[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
2023-10-24 19:24:50 +00:00
parent b99d16670c
commit c53aae845d
5 changed files with 102 additions and 70 deletions
@@ -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)

@@ -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}"
@@ -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):
@@ -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']

@@ -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