From 06e13555d51aace43ea6b2b36d717e58f6c2ba7f Mon Sep 17 00:00:00 2001 From: Olivier Dony Date: Thu, 30 Aug 2012 14:21:12 +0200 Subject: [PATCH] [FIX] mail: simplify test and cleanup some code + fix plaintext signature append bzr revid: odo@openerp.com-20120830122112-y2lbgcazy0opvbs8 --- addons/mail/mail_followers.py | 24 ++--- addons/mail/mail_message.py | 20 ++-- addons/mail/mail_thread.py | 15 ++- addons/mail/tests/test_mail.py | 174 ++++++++++++--------------------- 4 files changed, 94 insertions(+), 139 deletions(-) diff --git a/addons/mail/mail_followers.py b/addons/mail/mail_followers.py index b18832fa9cd..10d33536136 100644 --- a/addons/mail/mail_followers.py +++ b/addons/mail/mail_followers.py @@ -79,19 +79,19 @@ class mail_notification(osv.Model): if context.get('mail_noemail') or not partner_ids: return True - mail_mail_obj = self.pool.get('mail.mail') - msg_obj = self.pool.get('mail.message') - msg = msg_obj.browse(cr, uid, msg_id, context=context) + mail_mail = self.pool.get('mail.mail') + msg = self.pool.get('mail.message').browse(cr, uid, msg_id, context=context) # add signature body_html = msg.body signature = msg.author_id and msg.author_id.user_ids[0].signature or '' - insertion_point = body_html.find('') - if insertion_point > -1: + if signature: + signature_block = u'\n
%s
\n' % signature insertion_point = body_html.find('') - body_html = body_html[:insertion_point] + '
%s
' % tools.ustr(signature) + body_html[insertion_point:] - else: - body_html = body_html + '
%s
' % tools.ustr(signature) + if insertion_point > -1: + body_html = body_html[:insertion_point] + signature_block + body_html[insertion_point:] + else: + body_html += signature_block towrite = { 'mail_message_id': msg.id, @@ -118,8 +118,8 @@ class mail_notification(osv.Model): continue if partner.email not in towrite['email_to']: towrite['email_to'].append(partner.email) - towrite['email_to'] = ', '.join(towrite['email_to']) - - email_notif_id = mail_mail_obj.create(cr, uid, towrite, context=context) - mail_mail_obj.send(cr, uid, [email_notif_id], context=context) + if towrite['email_to']: + towrite['email_to'] = ', '.join(towrite['email_to']) + email_notif_id = mail_mail.create(cr, uid, towrite, context=context) + mail_mail.send(cr, uid, [email_notif_id], context=context) return True diff --git a/addons/mail/mail_message.py b/addons/mail/mail_message.py index 97473886e1a..413606b9ed2 100644 --- a/addons/mail/mail_message.py +++ b/addons/mail/mail_message.py @@ -330,19 +330,19 @@ class mail_message(osv.Model): Call mail_notification.notify to manage the email sending """ message = self.browse(cr, uid, newid, context=context) - partners_to_notify = [] + partners_to_notify = set([]) # add all partner_ids of the message if message.partner_ids: - for partner in message.partner_ids: - if partner.id not in partners_to_notify: - partners_to_notify.append(partner.id) - # add all followers and set them as partner_ids + partners_to_notify |= set(partner.id for partner in message.partner_ids) + # add all followers and set add them in partner_ids if message.model and message.res_id: - modobj = self.pool.get(message.model) - for follower in modobj.browse(cr, uid, message.res_id, context=context).message_follower_ids: - partners_to_notify.append(follower.id) - self.write(cr, uid, [newid], {'partner_ids': [(4, follower.id)]}, context=context) - self.pool.get('mail.notification').notify(cr, uid, partners_to_notify, newid, context=context) + record = self.pool.get(message.model).browse(cr, uid, message.res_id, context=context) + extra_notified = set(partner.id for partner in record.message_follower_ids) + missing_notified = extra_notified - partners_to_notify + if missing_notified: + message.write({'partner_ids': [(4, p_id) for p_id in missing_notified]}) + partners_to_notify |= extra_notified + self.pool.get('mail.notification').notify(cr, uid, list(partners_to_notify), newid, context=context) def read(self, cr, uid, ids, fields_to_read=None, context=None, load='_classic_read'): self.check(cr, uid, ids, 'read', context=context) diff --git a/addons/mail/mail_thread.py b/addons/mail/mail_thread.py index eb9b48bbc5f..5c4ecbd48f6 100644 --- a/addons/mail/mail_thread.py +++ b/addons/mail/mail_thread.py @@ -475,16 +475,25 @@ class mail_thread(osv.Model): # plain text is wrapped in 
 to preserve formatting
                     text = u'
%s
' % tools.ustr(part.get_payload(decode=True),
                                                          encoding, errors='replace')
-                    body = body[:insertion_point] + text + body[:insertion_point]
+                    if insertion_point != -1:
+                        body = body[:insertion_point] + text + body[insertion_point:]
+                    else:
+                        body += text
                 # 3) text/html -> raw
                 elif part.get_content_type() == 'text/html':
                     html = tools.ustr(part.get_payload(decode=True), encoding, errors='replace')
                     if alternative:
                         body = html
+                        # force  tag to lowercase, for easier matching
+                        body = re.sub(r'(?i)', r'', body)
                     else:
-                        html = re.sub('(?i)', '', html)
+                        # strip enclosing html/body tags and append to existing body
+                        html = re.sub(r'(?i)(|)', '', html)
                         insertion_point = body.find('')
-                        body = body[:insertion_point] + text + body[:insertion_point]
+                        if insertion_point != -1:
+                            body = body[:insertion_point] + html + body[insertion_point:]
+                        else:
+                            body += html
                 # 4) Anything else -> attachment
                 else:
                     attachments.append((filename or 'attachment', part.get_payload(decode=True)))
diff --git a/addons/mail/tests/test_mail.py b/addons/mail/tests/test_mail.py
index 858e5e868ad..86f3997c6b7 100644
--- a/addons/mail/tests/test_mail.py
+++ b/addons/mail/tests/test_mail.py
@@ -150,69 +150,47 @@ class test_mail(common.TransactionCase):
             {'res_model': 'mail.group', 'res_id': group_dummy_id, 'partner_id': partner_bert_id})
 
         # Pigs just created: should be only Admin as follower
-        follower_ids = [follower.id for follower in group_pigs.message_follower_ids]
-        self.assertTrue(len(follower_ids) == 1,
-            'Newly created group should have only 1 follower')
-        self.assertTrue(user_admin.partner_id.id in follower_ids,
-            'Admin should be the only Pigs group follower')
+        follower_ids = set([follower.id for follower in group_pigs.message_follower_ids])
+        self.assertEqual(follower_ids, set([user_admin.partner_id.id]), 'Admin should be the only Pigs fan')
 
         # Subscribe Bert through a '4' command
         group_pigs.write({'message_follower_ids': [(4, partner_bert_id)]})
         group_pigs.refresh()
-        follower_ids = [follower.id for follower in group_pigs.message_follower_ids]
-        self.assertTrue(len(follower_ids) == 2,
-            'Pigs group should have 2 followers after linking Bert')
-        self.assertTrue(all(id in follower_ids for id in [partner_bert_id, user_admin.partner_id.id]),
-            'Bert and Admin should be the 2 Pigs group followers')
+        follower_ids = set([follower.id for follower in group_pigs.message_follower_ids])
+        self.assertEqual(follower_ids, set([partner_bert_id, user_admin.partner_id.id]), 'Bert and Admin should be the only Pigs fans')
 
         # Unsubscribe Bert through a '3' command
         group_pigs.write({'message_follower_ids': [(3, partner_bert_id)]})
         group_pigs.refresh()
-        follower_ids = [follower.id for follower in group_pigs.message_follower_ids]
-        self.assertTrue(len(follower_ids) == 1,
-            'Pigs group should have 1 follower after unlinking Bert')
-        self.assertTrue(all(id in follower_ids for id in [user_admin.partner_id.id]),
-            'Admin should be the only Pigs group follower')
+        follower_ids = set([follower.id for follower in group_pigs.message_follower_ids])
+        self.assertEqual(follower_ids, set([user_admin.partner_id.id]), 'Admin should be the only Pigs fan')
 
         # Set followers through a '6' command
         group_pigs.write({'message_follower_ids': [(6, 0, [partner_bert_id])]})
         group_pigs.refresh()
-        follower_ids = [follower.id for follower in group_pigs.message_follower_ids]
-        self.assertTrue(len(follower_ids) == 1,
-            'Pigs group should have 1 follower after replacing followers')
-        self.assertTrue(follower_ids == [partner_bert_id],
-            'Bert should be the only Pigs group follower')
+        follower_ids = set([follower.id for follower in group_pigs.message_follower_ids])
+        self.assertEqual(follower_ids, set([partner_bert_id]), 'Bert should be the only Pigs fan')
 
         # Add a follower created on the fly through a '0' command
         group_pigs.write({'message_follower_ids': [(0, 0, {'name': 'Patrick Fiori'})]})
         partner_patrick_id = self.res_partner.search(cr, uid, [('name', '=', 'Patrick Fiori')])[0]
         group_pigs.refresh()
-        follower_ids = [follower.id for follower in group_pigs.message_follower_ids]
-        self.assertTrue(len(follower_ids) == 2,
-            'Pigs group should have 2 followers after linking to new partner')
-        self.assertTrue(all(id in follower_ids for id in [partner_bert_id, partner_patrick_id]),
-            'Bert and Patrick should be Pigs group followers')
+        follower_ids = set([follower.id for follower in group_pigs.message_follower_ids])
+        self.assertEqual(follower_ids, set([partner_bert_id, partner_patrick_id]), 'Bert and Patrick should be the only Pigs fans')
 
         # Finally, unlink through a '5' command
         group_pigs.write({'message_follower_ids': [(5, 0)]})
         group_pigs.refresh()
-        follower_ids = [follower.id for follower in group_pigs.message_follower_ids]
-        self.assertTrue(len(follower_ids) == 0,
-            'Pigs group should not have followers anymore')
+        follower_ids = set([follower.id for follower in group_pigs.message_follower_ids])
+        self.assertFalse(follower_ids, 'Pigs group should not have fans anymore')
 
         # Test dummy data has not been altered
         fol_obj_ids = self.mail_followers.search(cr, uid, [('res_model', '=', 'mail.thread'), ('res_id', '=', self.group_pigs_id)])
-        follower_ids = [follower.partner_id.id for follower in self.mail_followers.browse(cr, uid, fol_obj_ids)]
-        self.assertTrue(len(follower_ids) == 1,
-            'mail.thread dummy data should have 1 follower')
-        self.assertTrue(follower_ids[0] == partner_bert_id,
-            'Bert should be the follower of dummy mail.thread data')
+        follower_ids = set([follower.partner_id.id for follower in self.mail_followers.browse(cr, uid, fol_obj_ids)])
+        self.assertEqual(follower_ids, set([partner_bert_id]), 'Bert should be the follower of dummy mail.thread data')
         fol_obj_ids = self.mail_followers.search(cr, uid, [('res_model', '=', 'mail.group'), ('res_id', '=', group_dummy_id)])
-        follower_ids = [follower.partner_id.id for follower in self.mail_followers.browse(cr, uid, fol_obj_ids)]
-        self.assertTrue(len(follower_ids) == 2,
-            'mail.group dummy data should have 2 followers')
-        self.assertTrue(all(id in follower_ids for id in [partner_bert_id, user_admin.partner_id.id]),
-            'Bert and Admin should be the followers of dummy mail.group data')
+        follower_ids = set([follower.partner_id.id for follower in self.mail_followers.browse(cr, uid, fol_obj_ids)])
+        self.assertEqual(follower_ids,set([partner_bert_id, user_admin.partner_id.id]), 'Bert and Admin should be the followers of dummy mail.group data')
 
     def test_11_message_followers(self):
         """ Tests designed for the subscriber API. """
@@ -227,21 +205,14 @@ class test_mail(common.TransactionCase):
         # Subscribe Raoul twice (niak niak) through message_subscribe_users
         group_pigs.message_subscribe_users([user_raoul_id, user_raoul_id])
         group_pigs.refresh()
-        follower_ids = [follower.id for follower in group_pigs.message_follower_ids]
-        self.assertTrue(len(follower_ids) == 2,
-            'Pigs group should have 2 followers after having subscribed Raoul. Subscribing twice '\
-            'the same user should create only one follower.')
-        self.assertTrue(all(id in follower_ids for id in [user_raoul.partner_id.id, user_admin.partner_id.id]),
-            'Admin and Raoul should be the 2 Pigs group followers')
+        follower_ids = set([follower.id for follower in group_pigs.message_follower_ids])
+        self.assertEqual(follower_ids, set([user_raoul.partner_id.id, user_admin.partner_id.id]), 'Admin and Raoul should be the only 2 Pigs group followers')
 
         # Unsubscribe Raoul twice through message_unsubscribe_users
         group_pigs.message_unsubscribe_users([user_raoul_id, user_raoul_id])
         group_pigs.refresh()
-        follower_ids = [follower.id for follower in group_pigs.message_follower_ids]
-        self.assertTrue(len(follower_ids) == 1,
-            'Pigs group should have 1 follower after unsubscribing Raoul')
-        self.assertTrue(all(id in follower_ids for id in [user_admin.partner_id.id]),
-            'Admin the only Pigs group followers')
+        follower_ids = set([follower.id for follower in group_pigs.message_follower_ids])
+        self.assertEqual(follower_ids,  set([user_admin.partner_id.id]), 'Admin must be the only Pigs fan')
 
     def test_20_message_post(self):
         """ Tests designed for message_post. """
@@ -266,97 +237,72 @@ class test_mail(common.TransactionCase):
         _subject = 'Pigs'
         _mail_subject = '%s posted on %s' % (user_admin.name, group_pigs.name)
         _body1 = 'Pigs rules'
-        _mail_body1 = 'Pigs rules
Admin
'
+        _mail_body1 = 'Pigs rules\n
Admin
\n'
         _mail_bodyalt1 = 'Pigs rules\nAdmin'
         _body2 = 'Pigs rules'
-        _mail_body2 = 'Pigs rules
Admin
'
-        _mail_bodyalt2 = 'Pigs rules\nAdmin'
+        _mail_body2 = 'Pigs rules\n
Admin
\n'
+        _mail_bodyalt2 = 'Pigs rules\nAdmin\n'
 
         # Post comment with body and subject, comment preference
         msg_id = self.mail_group.message_post(cr, uid, self.group_pigs_id, body=_body1, subject=_subject, msg_type='comment')
-        # Fetch: created mail_message, mail_mail, send_email
+        # Fetch: created mail_message, mail_mail, sent_email
         message = self.mail_message.browse(cr, uid, msg_id)
         mail_ids = self.mail_mail.search(cr, uid, [], limit=1)
         mail = self.mail_mail.browse(cr, uid, mail_ids[0])
-        send_email = self._build_email_kwargs
+        sent_email = self._build_email_kwargs
         
         # Test: mail_message: subject is _subject, body is _body1 (no formatting done)
-        self.assertEqual(message.subject, _subject,
-            'mail.message subject is %s; should be %s' % (message.subject, _subject))
-        self.assertEqual(message.body, _body1,
-            'mail.message body is %s; should be %s' % (message.body, _body1))
+        self.assertEqual(message.subject, _subject, 'mail.message subject incorrect')
+        self.assertEqual(message.body, _body1, 'mail.message body incorrect')
         # Test: mail_mail: subject is _subject, body_html is _mail_body1 (signature appended)
-        self.assertEqual(mail.subject, _subject,
-            'mail.mail subject is %s; should be %s' % (mail.subject, _subject))
-        self.assertEqual(mail.body_html, _mail_body1,
-            'mail.mail body_html is %s; should be %s' % (mail.body_html, _mail_body1))
-        self.assertTrue(mail.mail_message_id and mail.mail_message_id.id == msg_id,
-            'mail_mail.mail_message_id is not the id of its related mail_message; got %s, should be %s' % (mail.mail_message_id.id, msg_id))
-        # Test: send_email: email send by server: correct subject and body
-        self.assertEqual(send_email['subject'], _subject,
-            'send_email subject is %s; should be %s' % (send_email['subject'], _subject))
-        self.assertEqual(send_email['body'], _mail_body1,
-            'send_email body is %s; should be %s' % (send_email['body'], _mail_body1))
-        self.assertEqual(send_email['body_alternative'], _mail_bodyalt1,
-            'send_email body_alternative is %s; should be %s' % (send_email['body_alternative'], _mail_bodyalt1))
+        self.assertEqual(mail.subject, _subject, 'mail.mail subject incorrect')
+        self.assertEqual(mail.body_html, _mail_body1, 'mail.mail body_html incorrect')
+        self.assertEqual(mail.mail_message_id.id, msg_id, 'mail_mail.mail_message_id is not the id of its related mail_message)')
+        # Test: sent_email: email send by server: correct subject and body
+        self.assertEqual(sent_email['subject'], _subject, 'sent_email subject incorrect')
+        self.assertEqual(sent_email['body'], _mail_body1, 'sent_email body incorrect')
+        self.assertEqual(sent_email['body_alternative'], _mail_bodyalt1, 'sent_email body_alternative is incorrect')
         # Test: mail_message: partner_ids = group followers
-        message_pids = [partner.id for partner in message.partner_ids]
-        test_pids = [p_a_id, p_b_id, p_c_id]
-        self.assertTrue(all(pid in message_pids for pid in test_pids) and len(message_pids) == len(test_pids),
-            'mail.message partners are %s; should be %s' % (message_pids, test_pids))
+        message_pids = set([partner.id for partner in message.partner_ids])
+        test_pids = set([p_a_id, p_b_id, p_c_id])
+        self.assertEqual(test_pids, message_pids, 'mail.message partners incorrect')
         # Test: notification linked to this message = group followers = partner_ids
         notif_ids = self.mail_notification.search(cr, uid, [('message_id', '=', message.id)])
-        notif_pids = [notif.partner_id.id for notif in self.mail_notification.browse(cr, uid, notif_ids)]
-        self.assertTrue(all(pid in notif_pids for pid in test_pids) and len(notif_pids) == len(test_pids),
-            'mail.message notification partners are %s; should be %s' % (notif_pids, test_pids))
-        # Test: send_email: email_to should contain b@b, not c@c (pref email), not a@a (writer)
-        test_emails = ['b@b']
-        self.assertTrue(all(email in send_email['email_to'] for email in test_emails) and len(send_email['email_to']) == len(test_emails),
-            'send_email email_to is %s; should be %s' % (send_email['email_to'], test_emails))
+        notif_pids = set([notif.partner_id.id for notif in self.mail_notification.browse(cr, uid, notif_ids)])
+        self.assertEqual(notif_pids, test_pids, 'mail.message notification partners incorrect')
+        # Test: sent_email: email_to should contain b@b, not c@c (pref email), not a@a (writer)
+        self.assertEqual(sent_email['email_to'], ['b@b'], 'sent_email email_to is incorrect')
 
         # New post: test automatic subject, signature in html, add a partner, email preference, parent_id previous message
         msg_id2 = self.mail_group.message_post(cr, uid, self.group_pigs_id, body=_body2, msg_type='email', partner_ids=[(6, 0, [p_d_id])], parent_id=msg_id)
-        # Fetch: created mail_message, mail_mail, send_email
+        # Fetch: created mail_message, mail_mail, sent_email
         message = self.mail_message.browse(cr, uid, msg_id2)
         mail_ids = self.mail_mail.search(cr, uid, [], limit=1)
         mail = self.mail_mail.browse(cr, uid, mail_ids[0])
-        send_email = self._build_email_kwargs
+        sent_email = self._build_email_kwargs
 
         # Test: mail_message: subject is False, body is _body1 (no formatting done), parent_id is msg_id
-        self.assertEqual(message.subject, False,
-            'mail.message subject is %s; should be %s' % (message.subject, False))
-        self.assertEqual(message.body, _body2,
-            'mail.message body is %s; should be %s' % (message.body, _body2))
-        self.assertEqual(message.parent_id.id, msg_id,
-            'mail.message parent_id is %s; should be %s' % (message.parent_id.id, msg_id))
+        self.assertEqual(message.subject, False, 'mail.message subject incorrect')
+        self.assertEqual(message.body, _body2, 'mail.message body incorrect')
+        self.assertEqual(message.parent_id.id, msg_id, 'mail.message parent_id incorrect')
         # Test: mail_mail: subject is False, body_html is _mail_body1 (signature appended)
-        self.assertEqual(mail.subject, False,
-            'mail.mail subject is %s; should be %s' % (mail.subject, False))
-        self.assertEqual(mail.body_html, _mail_body2,
-            'mail.mail body_html is %s; should be %s' % (mail.body_html, _mail_body2))
-        self.assertTrue(mail.mail_message_id and mail.mail_message_id.id == msg_id2,
-            'mail_mail.mail_message_id is not the id of its related mail_message; got %s, should be %s' % (mail.mail_message_id.id, msg_id2))
-        # Test: send_email: email send by server: correct subject and body
-        self.assertEqual(send_email['subject'], _mail_subject,
-            'send_email subject is %s; should be %s' % (send_email['subject'], _mail_subject))
-        self.assertEqual(send_email['body'], _mail_body2,
-            'send_email body is %s; should be %s' % (send_email['body'], _mail_body2))
-        self.assertEqual(send_email['body_alternative'], _mail_bodyalt2,
-            'send_email body_alternative is %s; should be %s' % (send_email['body_alternative'], _mail_bodyalt2))
+        self.assertEqual(mail.subject, False, 'mail.mail subject is incorrect')
+        self.assertEqual(mail.body_html, _mail_body2, 'mail.mail body_html incorrect')
+        self.assertEqual(mail.mail_message_id.id, msg_id2, 'mail_mail.mail_message_id incorrect')
+        # Test: sent_email: email send by server: correct subject and body
+        self.assertEqual(sent_email['subject'], _mail_subject, 'sent_email subject incorrect')
+        self.assertEqual(sent_email['body'], _mail_body2, 'sent_email body incorrect')
+        self.assertEqual(sent_email['body_alternative'], _mail_bodyalt2, 'sent_email body_alternative incorrect')
         # Test: mail_message: partner_ids = group followers
-        message_pids = [partner.id for partner in message.partner_ids]
-        test_pids = [p_a_id, p_b_id, p_c_id, p_d_id]
-        self.assertTrue(all(pid in message_pids for pid in test_pids) and len(message_pids) == len(test_pids),
-            'mail.message partners are %s; should be %s' % (message_pids, test_pids))
+        message_pids = set([partner.id for partner in message.partner_ids])
+        test_pids = set([p_a_id, p_b_id, p_c_id, p_d_id])
+        self.assertEqual(message_pids, test_pids, 'mail.message partners incorrect')
         # Test: notification linked to this message = group followers = partner_ids
         notif_ids = self.mail_notification.search(cr, uid, [('message_id', '=', message.id)])
-        notif_pids = [notif.partner_id.id for notif in self.mail_notification.browse(cr, uid, notif_ids)]
-        self.assertTrue(all(pid in notif_pids for pid in test_pids) and len(notif_pids) == len(test_pids),
-            'mail.message notification partners are %s; should be %s' % (notif_pids, test_pids))
-        # Test: send_email: email_to should contain b@b, c@c, not a@a (writer)
-        test_emails = ['b@b', 'c@c']
-        self.assertTrue(all(email in send_email['email_to'] for email in test_emails) and len(send_email['email_to']) == len(test_emails),
-            'send_email email_to is %s; should be %s' % (send_email['email_to'], test_emails))
+        notif_pids = set([notif.partner_id.id for notif in self.mail_notification.browse(cr, uid, notif_ids)])
+        self.assertEqual(notif_pids, test_pids, 'mail.message notification partners incorrect')
+        # Test: sent_email: email_to should contain b@b, c@c, not a@a (writer)
+        self.assertEqual(set(sent_email['email_to']), set(['b@b', 'c@c']), 'sent_email email_to incorrect')
 
     def test_21_message_post_attachments(self):
         """ Tests designed for attachments. """