From df037e295971650e8843b2ee668501e7aa3e7df6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Tue, 6 Dec 2016 12:55:10 +0100 Subject: [PATCH] [FIX] mail: correctly handle replies to thread containing external emails When Odoo sends emails their Message-Ids are either generated by Odoo or use the original Message-Id in the case of an incoming email going through the mail gateway. When the mailgateway handles replies to those emails it checks their References or In-Reply-To in order to find matching Message-Ids and find the reply's parent. Currently it is limited to Message-Ids matching the Odoo pattern. However if an external email began a discussion thread, references will not hold any odoo-like pattern. Previously to commit 0028c42136f02fcb7a9a81ed5763396290ffdcb8 this was hidden due to a bug when managing private messages. This code was also managing replies to external emails due to a probably undesired side effect. Correct way of checking if an incoming email is a reply to a thread is to check that References or In-Reply-To Message-Ids match some stored Message-Ids of mail.message. They don't have to match Odoo patterN --- addons/mail/models/mail_thread.py | 54 +++++++++++++------------- addons/mail/tests/test_mail_gateway.py | 14 +++++++ 2 files changed, 42 insertions(+), 26 deletions(-) diff --git a/addons/mail/models/mail_thread.py b/addons/mail/models/mail_thread.py index de66fbc50e2..8249f5354b1 100644 --- a/addons/mail/models/mail_thread.py +++ b/addons/mail/models/mail_thread.py @@ -1097,7 +1097,12 @@ class MailThread(models.AbstractModel): message_id, email_from, email_to) return [] - # 1. message is a reply to an existing message (exact match of message_id or compat-mode) + # 1. Check if message is a reply on a thread + msg_references = [ref for ref in tools.mail_header_msgid_re.findall(thread_references) if 'reply_to' not in ref] + mail_messages = MailMessage.sudo().search([('message_id', 'in', msg_references)], limit=1) + is_a_reply = bool(mail_messages) + + # 1.1 Handle forward to an alias with a different model: do not consider it as a reply if reply_model and reply_thread_id: other_alias = Alias.search([ '&', @@ -1105,35 +1110,30 @@ class MailThread(models.AbstractModel): ('alias_name', '=', email_to_localpart) ]) if other_alias and other_alias.alias_model_id.model != reply_model: - reply_match = False + is_a_reply = False - if reply_match: - msg_references = tools.mail_header_msgid_re.findall(thread_references) - mail_messages = MailMessage.sudo().search([('message_id', 'in', msg_references)], limit=1) + if is_a_reply: + model, thread_id = mail_messages.model, mail_messages.res_id + if not reply_private: # TDE note: not sure why private mode as no alias search, copying existing behavior + dest_aliases = Alias.search([('alias_name', 'in', rcpt_tos_localparts)], limit=1) - if mail_messages: - model, thread_id = mail_messages.model, mail_messages.res_id - if not reply_private: # TDE note: not sure why private mode as no alias search, copying existing behavior - dest_aliases = Alias.search([('alias_name', 'in', rcpt_tos_localparts)], limit=1) + route = self.message_route_verify( + message, message_dict, + (model, thread_id, custom_values, self._uid, dest_aliases), + update_author=True, assert_model=reply_private, create_fallback=True, + allow_private=reply_private, drop_alias=True) + 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, model, thread_id, custom_values, self._uid) + return [route] + elif route is False: + return [] - route = self.message_route_verify( - message, message_dict, - (model, thread_id, custom_values, self._uid, dest_aliases), - update_author=True, assert_model=reply_private, create_fallback=True, - allow_private=reply_private, drop_alias=True) - 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, model, thread_id, custom_values, self._uid) - return [route] - elif route is False: - return [] - - # no route found for a matching reference (or reply), so parent is invalid - message_dict.pop('parent_id', None) - - # 4. Look for a matching mail.alias entry + # 2. Look for a matching mail.alias entry if rcpt_tos_localparts: + # no route found for a matching reference (or reply), so parent is invalid + message_dict.pop('parent_id', None) dest_aliases = Alias.search([('alias_name', 'in', rcpt_tos_localparts)]) if dest_aliases: routes = [] @@ -1160,6 +1160,8 @@ class MailThread(models.AbstractModel): # 5. Fallback to the provided parameters, if they work if fallback_model: + # no route found for a matching reference (or reply), so parent is invalid + message_dict.pop('parent_id', None) route = self.message_route_verify( message, message_dict, (fallback_model, thread_id, custom_values, self._uid, None), diff --git a/addons/mail/tests/test_mail_gateway.py b/addons/mail/tests/test_mail_gateway.py index b729a105b98..fd2f80ce103 100644 --- a/addons/mail/tests/test_mail_gateway.py +++ b/addons/mail/tests/test_mail_gateway.py @@ -541,6 +541,20 @@ class TestMailgateway(TestMail): self.assertEqual(len(self.group_public.message_ids), 2, 'message_process: group should contain one new message') self.assertEqual(len(self.fake_email.child_ids), 1, 'message_process: new message should be children of the existing one') + def test_message_process_references_external(self): + """ Incoming email being a reply to an external email processed by odoo should update thread accordingly """ + new_message_id = '' + self.fake_email.write({ + 'message_id': new_message_id + }) + self.format_and_process( + MAIL_TEMPLATE, to='erroneous@example.com', + extra='References: <2233@a.com>\r\n\t<3edss_dsa@b.com> %s' % self.fake_email.message_id, + msg_id='<1198923581.41972151344608186800.JavaMail.4@agrolait.com>') + + self.assertEqual(len(self.group_public.message_ids), 2, 'message_process: group should contain one new message') + self.assertEqual(len(self.fake_email.child_ids), 1, 'message_process: new message should be children of the existing one') + @mute_logger('odoo.addons.mail.models.mail_thread', 'odoo.models') def test_message_process_references_forward(self): """ Incoming email using references but with alias forward should not go into references destination """