[IMP] mail: add message_id in references to help thread formation

Issue: when using Amazon's SES SMTP service they rewrite the Message-Id of all
outgoing messages. When someone replies, In-Reply-To contains the SES Message-Id
which we don't know. Threading is therefore broken. Amazon requires to remember
new Message-Id and handle it ourself, which is complicated [1].

Possible solution: copy the Message-Id to References in outgoing message as if
we were pretending that the message is part of a pre-existing thread with
itself. Since Amazon does not alter References it is kept during transport.
Replies will therefore contain both Amazon Message-Id and Odoo Message-Id as
well as original parent Message-Id in case of nested replies.

  * ``In-Reply-To``: Amazon Msg-Id
  * ``References``: Odoo Parent Msg-Id Odoo Msg-Id

Task-2643114 (Mail: Message-Id in references to ease thread formation)

[1] https://docs.aws.amazon.com/ses/latest/DeveloperGuide/header-fields.html

closes #81901

Signed-off-by: Thibault Delavallee (tde) <tde@openerp.com>

addons/mail/models/mail_thread.py

@@ -1473,14 +1473,20 @@ def message_parse(self, message, save_original=False):
             msg_dict['date'] = stored_date.strftime(tools.DEFAULT_SERVER_DATETIME_FORMAT)
 
         if msg_dict['in_reply_to']:
-            parent_ids = self.env['mail.message'].search([('message_id', '=', msg_dict['in_reply_to'])], limit=1)
+            parent_ids = self.env['mail.message'].search(
+                [('message_id', '=', msg_dict['in_reply_to'])],
+                order='create_date DESC, id DESC',
+                limit=1)
             if parent_ids:
                 msg_dict['parent_id'] = parent_ids.id
                 msg_dict['internal'] = parent_ids.subtype_id and parent_ids.subtype_id.internal or False
 
         if msg_dict['references'] and 'parent_id' not in msg_dict:
             references_msg_id_list = tools.mail_header_msgid_re.findall(msg_dict['references'])
-            parent_ids = self.env['mail.message'].search([('message_id', 'in', [x.strip() for x in references_msg_id_list])], limit=1)
+            parent_ids = self.env['mail.message'].search([
+                ('message_id', 'in', [x.strip() for x in references_msg_id_list])],
+                order='create_date DESC, id DESC',
+                limit=1)
             if parent_ids:
                 msg_dict['parent_id'] = parent_ids.id
                 msg_dict['internal'] = parent_ids.subtype_id and parent_ids.subtype_id.internal or False
@@ -2270,13 +2276,21 @@ def _notify_record_by_email(self, message, recipients_data, msg_vals=False,
         mail_subject = message.subject or (message.record_name and 'Re: %s' % message.record_name) # in cache, no queries
         # Replace new lines by spaces to conform to email headers requirements
         mail_subject = ' '.join((mail_subject or '').splitlines())
+        # compute references: set references to the parent and add current message just to
+        # have a fallback in case replies mess with Messsage-Id in the In-Reply-To (e.g. amazon
+        # SES SMTP may replace Message-Id and In-Reply-To refers an internal ID not stored in Odoo)
+        message_sudo = message.sudo()
+        if message_sudo.parent_id:
+            references = f'{message_sudo.parent_id.message_id} {message_sudo.message_id}'
+        else:
+            references = message_sudo.message_id
         # prepare notification mail values
         base_mail_values = {
             'mail_message_id': message.id,
             'mail_server_id': message.mail_server_id.id, # 2 query, check acces + read, may be useless, Falsy, when will it be used?
             'auto_delete': mail_auto_delete,
             # due to ir.rule, user have no right to access parent message if message is not published
-            'references': message.parent_id.sudo().message_id if message.parent_id else False,
+            'references': references,
             'subject': mail_subject,
         }
         base_mail_values = self._notify_by_email_add_values(base_mail_values)

addons/test_mail/tests/test_mail_message.py

@@ -217,6 +217,7 @@ def test_mail_message_values_fromto_no_auto_thread(self):
         self.assertNotIn('-%d-' % self.alias_record.id, msg.message_id.split('@')[0])
 
 
+@tagged('mail_message')
 class TestMessageAccess(common.BaseFunctionalTest, common.MockEmails):
 
     @classmethod
@@ -415,7 +416,7 @@ def test_mail_message_access_create_wo_parent_access(self):
 
         new_mail = self.env['mail.mail'].search([
             ('mail_message_id', '=', new_msg.id),
-            ('references', '=', message.message_id),
+            ('references', '=', f'{message.message_id} {new_msg.message_id}'),
         ])
 
         self.assertTrue(new_mail)

addons/test_mail/tests/test_message_post.py

@@ -210,7 +210,7 @@ def test_post_notifications(self):
             [[self.partner_1], [self.partner_2], [self.user_admin.partner_id]],
             reply_to=msg.reply_to, subject=_subject,
             body_content=_body, body_alt_content=_body_alt,
-            references=False)
+            references=msg.message_id)
 
     @mute_logger('odoo.addons.mail.models.mail_mail')
     def test_post_notifications_keep_emails(self):
@@ -281,27 +281,51 @@ def test_post_answer(self):
         self.assertEqual(parent_msg.partner_ids, self.env['res.partner'])
         self.assertEmails(self.user_employee.partner_id, [])
 
+        # post a first reply
+        self._init_mock_build_email()
         msg = self.test_record.with_user(self.user_employee).message_post(
             body='<p>Test Answer</p>',
-            message_type='comment', subtype='mt_comment',
+            message_type='comment',
+            subject='Welcome',
+            subtype='mt_comment',
+            parent_id=parent_msg.id,
             partner_ids=[self.partner_1.id],
-            parent_id=parent_msg.id)
+        )
 
         self.assertEqual(msg.parent_id.id, parent_msg.id)
         self.assertEqual(msg.partner_ids, self.partner_1)
         self.assertEqual(parent_msg.partner_ids, self.env['res.partner'])
 
-        # check notification emails: references
-        self.assertEmails(self.user_employee.partner_id, [[self.partner_1]], ref_content='openerp-%d-mail.test.simple' % self.test_record.id)
-        # self.assertTrue(all('openerp-%d-mail.test.simple' % self.test_record.id in m['references'] for m in self._mails))
+        # check notification emails, among other references
+        self.assertEmails(
+            self.user_employee.partner_id,
+            [[self.partner_1]],
+            subject='Welcome',
+            ref_content='openerp-%d-mail.test.simple' % self.test_record.id,
+            # references should be sorted from the oldest to the newest
+            references='%s %s' % (parent_msg.message_id, msg.message_id),
+        )
 
+        # post a reply to the reply: check parent is the first one
+        self._init_mock_build_email()
         new_msg = self.test_record.with_user(self.user_employee).message_post(
             body='<p>Test Answer Bis</p>',
-            message_type='comment', subtype='mt_comment',
-            parent_id=msg.id)
+            message_type='comment',
+            subtype='mt_comment',
+            parent_id=msg.id,
+            partner_ids=[self.partner_2.id],
+        )
 
         self.assertEqual(new_msg.parent_id.id, parent_msg.id, 'message_post: flatten error')
-        self.assertEqual(new_msg.partner_ids, self.env['res.partner'])
+        self.assertEqual(new_msg.partner_ids, self.partner_2)
+        self.assertEmails(
+            self.user_employee.partner_id, [[self.partner_2]],
+            body_content='<p>Test Answer Bis</p>',
+            reply_to=msg.reply_to,
+            subject='Re: %s' % self.test_record.name,
+            ref_content='openerp-%d-mail.test.simple' % self.test_record.id,
+            references='%s %s' % (parent_msg.message_id, new_msg.message_id),
+        )
 
     @mute_logger('odoo.addons.mail.models.mail_mail')
     def test_post_email_with_multiline_subject(self):