[FW][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 odoo/odoo#89328 X-original-commit: bc568747bdf92c6bbca400ce3ba11f6487679f3a Signed-off-by: Thibault Delavallee (tde) <tde@openerp.com>
This commit is contained in:
@@ -1415,10 +1415,16 @@ class MailThread(models.AbstractModel):
|
||||
|
||||
parent_ids = False
|
||||
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 msg_dict['references'] and not parent_ids:
|
||||
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['is_internal'] = parent_ids.subtype_id and parent_ids.subtype_id.internal or False
|
||||
@@ -2490,12 +2496,19 @@ class MailThread(models.AbstractModel):
|
||||
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?
|
||||
# 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,
|
||||
}
|
||||
if additional_values:
|
||||
|
||||
@@ -785,6 +785,10 @@ class MailCase(MockEmail):
|
||||
self.assertMessageBusNotifications(message)
|
||||
|
||||
# check emails that should be sent (hint: mail.mail per group, email par recipient)
|
||||
email_values = {'body_content': mbody,
|
||||
'references_content': message.message_id}
|
||||
if message_info.get('email_values'):
|
||||
email_values.update(message_info['email_values'])
|
||||
for recipients in email_groups.values():
|
||||
partners = self.env['res.partner'].sudo().concat(*recipients)
|
||||
if all(p in mail_groups['failure'] for p in partners):
|
||||
@@ -801,12 +805,14 @@ class MailCase(MockEmail):
|
||||
partners, 'sent',
|
||||
author=message.author_id if message.author_id else message.email_from,
|
||||
mail_message=message,
|
||||
email_values={'body_content': mbody}
|
||||
email_values=email_values,
|
||||
)
|
||||
else:
|
||||
for recipient in partners:
|
||||
self.assertSentEmail(message.author_id if message.author_id else message.email_from, [recipient],
|
||||
email_values={'body_content': mbody})
|
||||
self.assertSentEmail(message.author_id if message.author_id else message.email_from,
|
||||
[recipient],
|
||||
**email_values
|
||||
)
|
||||
if not any(p for recipients in email_groups.values() for p in recipients):
|
||||
self.assertNoMail(partners, mail_message=message, author=message.author_id)
|
||||
|
||||
|
||||
@@ -251,6 +251,7 @@ class TestMessageValues(TestMailCommon):
|
||||
self.assertNotIn('-%d-' % self.alias_record.id, msg.message_id.split('@')[0])
|
||||
|
||||
|
||||
@tagged('mail_message')
|
||||
class TestMessageAccess(TestMailCommon):
|
||||
|
||||
@classmethod
|
||||
@@ -445,7 +446,7 @@ class TestMessageAccess(TestMailCommon):
|
||||
|
||||
new_mail = self.env['mail.mail'].sudo().search([
|
||||
('mail_message_id', '=', new_msg.id),
|
||||
('references', '=', message.message_id),
|
||||
('references', '=', f'{message.message_id} {new_msg.message_id}'),
|
||||
])
|
||||
|
||||
self.assertTrue(new_mail)
|
||||
|
||||
@@ -641,6 +641,7 @@ class TestMessagePost(TestMessagePostCommon):
|
||||
self.assertFalse(parent_msg.partner_ids)
|
||||
self.assertNotSentEmail()
|
||||
|
||||
# post a first reply
|
||||
with self.assertPostNotifications(
|
||||
[{'content': '<p>Test Answer</p>', 'notif': [{'partner': self.partner_1, 'type': 'email'}]}]
|
||||
):
|
||||
@@ -649,6 +650,7 @@ class TestMessagePost(TestMessagePostCommon):
|
||||
message_type='comment',
|
||||
parent_id=parent_msg.id,
|
||||
partner_ids=[self.partner_1.id],
|
||||
subject='Welcome',
|
||||
subtype_xmlid='mail.mt_comment',
|
||||
)
|
||||
self.assertEqual(msg.parent_id, parent_msg)
|
||||
@@ -657,19 +659,33 @@ class TestMessagePost(TestMessagePostCommon):
|
||||
|
||||
# check notification emails: references
|
||||
self.assertSentEmail(
|
||||
self.user_employee.partner_id, [self.partner_1],
|
||||
ref_content='openerp-%d-mail.test.simple' % self.test_record.id
|
||||
self.user_employee.partner_id,
|
||||
[self.partner_1],
|
||||
references_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
|
||||
with self.mock_mail_gateway():
|
||||
new_msg = test_record.message_post(
|
||||
body='<p>Test Answer Bis</p>',
|
||||
message_type='comment',
|
||||
parent_id=msg.id,
|
||||
subtype_xmlid='mail.mt_comment',
|
||||
parent_id=msg.id,
|
||||
partner_ids=[self.partner_2.id],
|
||||
)
|
||||
self.assertEqual(new_msg.parent_id, parent_msg, 'message_post: flatten error')
|
||||
self.assertFalse(new_msg.partner_ids)
|
||||
self.assertEqual(new_msg.partner_ids, self.partner_2)
|
||||
self.assertSentEmail(
|
||||
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,
|
||||
references_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', 'odoo.addons.mail.models.mail_thread')
|
||||
@users('employee')
|
||||
|
||||
Reference in New Issue
Block a user