From 2eece10cd4631763b1ca2cc00b2d8f84280f6d1d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Wed, 7 Aug 2013 16:24:18 +0200 Subject: [PATCH] [FIX] mail_mail: computation of reply_to after create to avoid interfering with automatic message_id creation of mail_message [FIX] mail_thread: fixed issues with private messages through mailgateway (wrong route checking, variable erasing) [TEST] mail: added test for the second bug bzr revid: tde@openerp.com-20130807142418-3h5qxdt3ekosj9x6 --- addons/mail/mail_mail.py | 10 +++- addons/mail/mail_thread.py | 20 +++++--- addons/mail/tests/__init__.py | 8 +-- addons/mail/tests/test_mail_gateway.py | 67 ++++++++++++++++++++------ 4 files changed, 76 insertions(+), 29 deletions(-) diff --git a/addons/mail/mail_mail.py b/addons/mail/mail_mail.py index bf76216e7e4..b105d6677bd 100644 --- a/addons/mail/mail_mail.py +++ b/addons/mail/mail_mail.py @@ -139,10 +139,16 @@ class mail_mail(osv.Model): # notification field: if not set, set if mail comes from an existing mail.message if 'notification' not in values and values.get('mail_message_id'): values['notification'] = True + mail_id = super(mail_mail, self).create(cr, uid, values, context=context) + # reply_to: if not set, set with default values that require creation values + # but delegate after creation because of mail_message.message_id automatic + # creation using existence of reply_to if not values.get('reply_to'): - values['reply_to'] = self._get_reply_to(cr, uid, values, context=context) - return super(mail_mail, self).create(cr, uid, values, context=context) + reply_to = self._get_reply_to(cr, uid, values, context=context) + if reply_to: + self.write(cr, uid, [mail_id], {'reply_to': reply_to}, context=context) + return mail_id def unlink(self, cr, uid, ids, context=None): # cascade-delete the parent message for all mails that are not created for a notification diff --git a/addons/mail/mail_thread.py b/addons/mail/mail_thread.py index 306e5fd5905..94e9126f020 100644 --- a/addons/mail/mail_thread.py +++ b/addons/mail/mail_thread.py @@ -682,6 +682,12 @@ class mail_thread(osv.AbstractModel): assert thread_id == 0, 'Routing: posting a message without model should be with a null res_id (private message).' _warn('posting a message without model should be with a null res_id (private message), resetting thread_id') thread_id = 0 + # Private message: should have a parent_id (only answers) + if not model and not message_dict.get('parent_id'): + if assert_model: + assert message_dict.get('parent_id'), 'Routing: posting a message without model should be with a parent_id (private mesage).' + _warn('posting a message without model should be with a parent_id (private mesage), skipping') + return () # Existing Document: check if exists; if not, fallback on create if allowed if thread_id and not model_pool.exists(cr, uid, thread_id): @@ -695,7 +701,7 @@ class mail_thread(osv.AbstractModel): return () # Existing Document: check model accepts the mailgateway - if thread_id and not hasattr(model_pool, 'message_update'): + if thread_id and model and not hasattr(model_pool, 'message_update'): if create_fallback: _warn('model %s does not accept document update, fall back on document creation' % model) thread_id = None @@ -706,7 +712,7 @@ class mail_thread(osv.AbstractModel): return () # New Document: check model accepts the mailgateway - if not thread_id and not hasattr(model_pool, 'message_new'): + if not thread_id and model and not hasattr(model_pool, 'message_new'): if assert_model: assert hasattr(model_pool, 'message_new'), 'Model %s does not accept document creation, crashing' % model _warn('model %s does not accept document creation, skipping' % model) @@ -798,16 +804,16 @@ class mail_thread(osv.AbstractModel): # 2. Reply to a private message if in_reply_to: - message_ids = self.pool.get('mail.message').search(cr, uid, [ + mail_message_ids = self.pool.get('mail.message').search(cr, uid, [ ('message_id', '=', in_reply_to), '!', ('message_id', 'ilike', 'reply_to') ], limit=1, context=context) - if message_ids: - message = self.pool.get('mail.message').browse(cr, uid, message_ids[0], context=context) + if mail_message_ids: + mail_message = self.pool.get('mail.message').browse(cr, uid, mail_message_ids[0], context=context) _logger.info('Routing mail from %s to %s with Message-Id %s: direct reply to a private message: %s, custom_values: %s, uid: %s', - email_from, email_to, message_id, message.id, custom_values, uid) + email_from, email_to, message_id, mail_message.id, custom_values, uid) route = self.message_route_verify(cr, uid, message, message_dict, - (message.model, message.res_id, custom_values, uid, None), + (mail_message.model, mail_message.res_id, custom_values, uid, None), update_author=True, assert_model=True, create_fallback=True, context=context) return route and [route] or [] diff --git a/addons/mail/tests/__init__.py b/addons/mail/tests/__init__.py index ff1080b0580..6dcc333a4cd 100644 --- a/addons/mail/tests/__init__.py +++ b/addons/mail/tests/__init__.py @@ -22,11 +22,11 @@ from . import test_mail_message, test_mail_features, test_mail_gateway, test_message_read, test_invite checks = [ - test_mail_message, - test_mail_features, + # test_mail_message, + # test_mail_features, test_mail_gateway, - test_message_read, - test_invite, + # test_message_read, + # test_invite, ] # vim:expandtab:smartindent:tabstop=4:softtabstop=4:shiftwidth=4: diff --git a/addons/mail/tests/test_mail_gateway.py b/addons/mail/tests/test_mail_gateway.py index 2e0ba336dc4..9aa1e21d7c1 100644 --- a/addons/mail/tests/test_mail_gateway.py +++ b/addons/mail/tests/test_mail_gateway.py @@ -633,12 +633,19 @@ class TestMailgateway(TestMailBase): """ Testing private discussion between partners. """ cr, uid = self.cr, self.uid + def format(template, to='Pretty Pigs , other@gmail.com', subject='Re: 1', + extra='', email_from='Sylvie Lelitre ', + msg_id='<1198923581.41972151344608186760.JavaMail@agrolait.com>'): + return template.format(to=to, subject=subject, extra=extra, email_from=email_from, msg_id=msg_id) + # Do: Raoul writes to Bert and Administrator, with a thread_model in context that should not be taken into account msg1_pids = [self.partner_admin_id, self.partner_bert_id] - msg1_id = self.mail_thread.message_post(cr, self.user_raoul_id, False, - partner_ids=msg1_pids, - subtype='mail.mt_comment', - context={'thread_model': 'mail.group'}) + msg1_id = self.mail_thread.message_post( + cr, self.user_raoul_id, False, + partner_ids=msg1_pids, + subtype='mail.mt_comment', + context={'thread_model': 'mail.group'} + ) # Test: message recipients msg = self.mail_message.browse(cr, uid, msg1_id) @@ -647,16 +654,26 @@ class TestMailgateway(TestMailBase): test_pids = msg1_pids test_nids = msg1_pids self.assertEqual(set(msg_pids), set(test_pids), - 'message_post: private discussion: incorrect recipients') + 'message_post: private discussion: incorrect recipients') self.assertEqual(set(msg_nids), set(test_nids), - 'message_post: private discussion: incorrect notified recipients') + 'message_post: private discussion: incorrect notified recipients') self.assertEqual(msg.model, False, - 'message_post: private discussion: context key "thread_model" not correctly ignored when having no res_id') + 'message_post: private discussion: context key "thread_model" not correctly ignored when having no res_id') + # Test: message reply_to and message-id + self.assertFalse(msg.reply_to, + 'message_post: private discussion: initial message should not have any reply_to specified') + self.assertIn('openerp-private', msg.message_id, + 'message_post: private discussion: message-id should contain the private keyword') # Do: Bert replies through mailgateway (is a customer) - msg2_id = self.mail_thread.message_post(cr, uid, False, - author_id=self.partner_bert_id, - parent_id=msg1_id, subtype='mail.mt_comment') + reply_message = format(MAIL_TEMPLATE, to='not_important@mydomain.com', + email_from='bert@bert.fr', + extra='In-Reply-To: %s' % msg.message_id, + msg_id='') + self.mail_thread.message_process(cr, uid, None, reply_message) + + # Test: last mail_message created + msg2_id = self.mail_message.search(cr, uid, [], limit=1)[0] # Test: message recipients msg = self.mail_message.browse(cr, uid, msg2_id) @@ -664,14 +681,32 @@ class TestMailgateway(TestMailBase): msg_nids = [p.id for p in msg.notified_partner_ids] test_pids = [self.partner_admin_id, self.partner_raoul_id] test_nids = test_pids + self.assertEqual(msg.author_id.id, self.partner_bert_id, + 'message_post: private discussion: wrong author through mailgatewya based on email') self.assertEqual(set(msg_pids), set(test_pids), - 'message_post: private discussion: incorrect recipients when replying') + 'message_post: private discussion: incorrect recipients when replying') self.assertEqual(set(msg_nids), set(test_nids), - 'message_post: private discussion: incorrect notified recipients when replying') + 'message_post: private discussion: incorrect notified recipients when replying') + + # Do: Bert replies through chatter (is a customer) + msg3_id = self.mail_thread.message_post( + cr, uid, False, + author_id=self.partner_bert_id, + parent_id=msg1_id, subtype='mail.mt_comment') + + # Test: message recipients + msg = self.mail_message.browse(cr, uid, msg3_id) + msg_pids = [p.id for p in msg.partner_ids] + msg_nids = [p.id for p in msg.notified_partner_ids] + test_pids = [self.partner_admin_id, self.partner_raoul_id] + test_nids = test_pids + self.assertEqual(set(msg_pids), set(test_pids), + 'message_post: private discussion: incorrect recipients when replying') + self.assertEqual(set(msg_nids), set(test_nids), + 'message_post: private discussion: incorrect notified recipients when replying') # Do: Administrator replies - msg3_id = self.mail_thread.message_post(cr, uid, False, - parent_id=msg2_id, subtype='mail.mt_comment') + msg3_id = self.mail_thread.message_post(cr, uid, False, parent_id=msg3_id, subtype='mail.mt_comment') # Test: message recipients msg = self.mail_message.browse(cr, uid, msg3_id) @@ -680,6 +715,6 @@ class TestMailgateway(TestMailBase): test_pids = [self.partner_bert_id, self.partner_raoul_id] test_nids = test_pids self.assertEqual(set(msg_pids), set(test_pids), - 'message_post: private discussion: incorrect recipients when replying') + 'message_post: private discussion: incorrect recipients when replying') self.assertEqual(set(msg_nids), set(test_nids), - 'message_post: private discussion: incorrect notified recipients when replying') + 'message_post: private discussion: incorrect notified recipients when replying')