[FIX] [FIX] mail: fixed subscription of an already subscribed follower erasing already existing subtypes + added tests

bzr revid: tde@openerp.com-20130508102304-p52bq0qux9s7rses

2 files changed, 79 insertions, 11 deletions

diff --git a/addons/mail/mail_thread.py b/addons/mail/mail_thread.py
index 83a1df96bdf..b869a032ad5 100644
--- a/addons/mail/mail_thread.py
+++ b/addons/mail/mail_thread.py
@@ -1163,15 +1163,28 @@ class mail_thread(osv.AbstractModel):
         else:
             self.check_access_rights(cr, uid, 'write')
 
+        # subscribe partners
         self.write(cr, SUPERUSER_ID, ids, {'message_follower_ids': [(4, pid) for pid in partner_ids]}, context=context)
-        # if subtypes are not specified (and not set to a void list), fetch default ones
-        if subtype_ids is None:
-            subtype_obj = self.pool.get('mail.message.subtype')
-            subtype_ids = subtype_obj.search(cr, uid, [('default', '=', True), '|', ('res_model', '=', self._name), ('res_model', '=', False)], context=context)
-        # update the subscriptions
+
+        # subtype specified: update the subscriptions
         fol_obj = self.pool.get('mail.followers')
-        fol_ids = fol_obj.search(cr, SUPERUSER_ID, [('res_model', '=', self._name), ('res_id', 'in', ids), ('partner_id', 'in', partner_ids)], context=context)
-        fol_obj.write(cr, SUPERUSER_ID, fol_ids, {'subtype_ids': [(6, 0, subtype_ids)]}, context=context)
+        if subtype_ids is not None:
+            fol_ids = fol_obj.search(cr, SUPERUSER_ID, [('res_model', '=', self._name), ('res_id', 'in', ids), ('partner_id', 'in', partner_ids)], context=context)
+            fol_obj.write(cr, SUPERUSER_ID, fol_ids, {'subtype_ids': [(6, 0, subtype_ids)]}, context=context)
+        # no subtypes: default ones for new subscription, do not update existing subscriptions
+        else:
+            # search new subscriptions: subtype_ids is False
+            fol_ids = fol_obj.search(cr, SUPERUSER_ID, [
+                            ('res_model', '=', self._name),
+                            ('res_id', 'in', ids),
+                            ('partner_id', 'in', partner_ids),
+                            ('subtype_ids', '=', False)
+                        ], context=context)
+            if fol_ids:
+                subtype_obj = self.pool.get('mail.message.subtype')
+                subtype_ids = subtype_obj.search(cr, uid, [('default', '=', True), '|', ('res_model', '=', self._name), ('res_model', '=', False)], context=context)
+                fol_obj.write(cr, SUPERUSER_ID, fol_ids, {'subtype_ids': [(6, 0, subtype_ids)]}, context=context)
+
         return True
 
     def message_unsubscribe_users(self, cr, uid, ids, user_ids=None, context=None):
diff --git a/addons/mail/tests/test_mail_features.py b/addons/mail/tests/test_mail_features.py
index 7eea66b3b90..e1bef837010 100644
--- a/addons/mail/tests/test_mail_features.py
+++ b/addons/mail/tests/test_mail_features.py
@@ -116,18 +116,66 @@ class test_mail(TestMailBase):
         # CASE1: test subscriptions with subtypes
         # ----------------------------------------
 
-        # Do: Subscribe Raoul three times (niak niak) through message_subscribe_users
+        # Do: subscribe Raoul, should have default subtypes
+        group_pigs.message_subscribe_users([user_raoul.id])
+        group_pigs.refresh()
+        # Test: 2 followers (Admin and Raoul)
+        follower_ids = [follower.id for follower in group_pigs.message_follower_ids]
+        self.assertEqual(set(follower_ids), set([user_raoul.partner_id.id, user_admin.partner_id.id]),
+                        'message_subscribe: Admin and Raoul should be the only 2 Pigs fans')
+        # Raoul follows default subtypes
+        fol_ids = self.mail_followers.search(cr, uid, [
+                        ('res_model', '=', 'mail.group'),
+                        ('res_id', '=', self.group_pigs_id),
+                        ('partner_id', '=', user_raoul.partner_id.id)
+                    ])
+        fol_obj = self.mail_followers.browse(cr, uid, fol_ids)[0]
+        fol_subtype_ids = set([subtype.id for subtype in fol_obj.subtype_ids])
+        self.assertEqual(set(fol_subtype_ids), set(default_group_subtypes),
+                        'message_subscribe: Raoul subscription subtypes are incorrect, should be all default ones')
+
+        # Do: subscribe Raoul with specified new subtypes
+        group_pigs.message_subscribe_users([user_raoul.id], subtype_ids=[mt_mg_nodef])
+        # Test: 2 followers (Admin and Raoul)
+        follower_ids = [follower.id for follower in group_pigs.message_follower_ids]
+        self.assertEqual(set(follower_ids), set([user_raoul.partner_id.id, user_admin.partner_id.id]),
+                        'message_subscribe: Admin and Raoul should be the only 2 Pigs fans')
+        # Test: 2 lines in mail.followers (no duplicate for Raoul)
+        fol_ids = self.mail_followers.search(cr, uid, [
+                        ('res_model', '=', 'mail.group'),
+                        ('res_id', '=', self.group_pigs_id),
+                    ])
+        self.assertEqual(len(fol_ids), 2,
+                        'message_subscribe: subscribing an already-existing follower should not create new entries in mail.followers')
+        # Test: Raoul follows only specified subtypes
+        fol_ids = self.mail_followers.search(cr, uid, [
+                        ('res_model', '=', 'mail.group'),
+                        ('res_id', '=', self.group_pigs_id),
+                        ('partner_id', '=', user_raoul.partner_id.id)
+                    ])
+        fol_obj = self.mail_followers.browse(cr, uid, fol_ids)[0]
+        fol_subtype_ids = set([subtype.id for subtype in fol_obj.subtype_ids])
+        self.assertEqual(set(fol_subtype_ids), set([mt_mg_nodef]),
+                        'message_subscribe: Raoul subscription subtypes are incorrect, should be only specified')
+
+        # Do: Subscribe Raoul without specified subtypes: should not erase existing subscription subtypes
         group_pigs.message_subscribe_users([user_raoul.id, user_raoul.id])
         group_pigs.message_subscribe_users([user_raoul.id])
         group_pigs.refresh()
         # Test: 2 followers (Admin and Raoul)
         follower_ids = [follower.id for follower in group_pigs.message_follower_ids]
-        self.assertEqual(set(follower_ids), set([user_raoul.partner_id.id, user_admin.partner_id.id]), 'Admin and Raoul should be the only 2 Pigs fans')
+        self.assertEqual(set(follower_ids), set([user_raoul.partner_id.id, user_admin.partner_id.id]),
+                        'message_subscribe: Admin and Raoul should be the only 2 Pigs fans')
         # Test: Raoul follows default subtypes
-        fol_ids = self.mail_followers.search(cr, uid, [('res_model', '=', 'mail.group'), ('res_id', '=', self.group_pigs_id), ('partner_id', '=', user_raoul.partner_id.id)])
+        fol_ids = self.mail_followers.search(cr, uid, [
+                        ('res_model', '=', 'mail.group'),
+                        ('res_id', '=', self.group_pigs_id),
+                        ('partner_id', '=', user_raoul.partner_id.id)
+                    ])
         fol_obj = self.mail_followers.browse(cr, uid, fol_ids)[0]
         fol_subtype_ids = set([subtype.id for subtype in fol_obj.subtype_ids])
-        self.assertEqual(set(fol_subtype_ids), set(default_group_subtypes), 'subscription subtypes are incorrect')
+        self.assertEqual(set(fol_subtype_ids), set([mt_mg_nodef]),
+                        'message_subscribe: Raoul subscription subtypes are incorrect, should be only specified')
 
         # Do: Unsubscribe Raoul twice through message_unsubscribe_users
         group_pigs.message_unsubscribe_users([user_raoul.id, user_raoul.id])
@@ -135,6 +183,13 @@ class test_mail(TestMailBase):
         # Test: 1 follower (Admin)
         follower_ids = [follower.id for follower in group_pigs.message_follower_ids]
         self.assertEqual(follower_ids, [user_admin.partner_id.id], 'Admin must be the only Pigs fan')
+        # Test: 1 lines in mail.followers (no duplicate for Raoul)
+        fol_ids = self.mail_followers.search(cr, uid, [
+                        ('res_model', '=', 'mail.group'),
+                        ('res_id', '=', self.group_pigs_id)
+                    ])
+        self.assertEqual(len(fol_ids), 1,
+                        'message_subscribe: group should have only 1 entry in mail.follower for 1 follower')
 
         # Do: subscribe Admin with subtype_ids
         group_pigs.message_subscribe_users([uid], [mt_mg_nodef, mt_all_nodef])