[FIX] mail: correctly update new and removed subtypes when updating subscription

Currently, the subtype checked selection is not saved when the user adds and
removes subtypes. Indeed either an addition is performed, either a removal
but having both is not correctly taken into account when updating an existing
subscription.

How to reproduce
  * click on the pencil icon to edit the subscription of the follower;
  * as an example default "Discussions" will be the only one selected;
  * check "Notes" and save;
  * -> this works;
  * click again on the pencil icon and uncheck "Notes" and check "Activities"
    then save;
  * -> this does not work as only removal is performed (Activities is not
    checked);

The problem only occurs when you unchecked subtypes and checked other ones.

This commit fixes that behavior by taking into consideration both new
subtypes and subtypes to remove.

LINKS

Task ID-2205643
Task ID-2241688
COM PR #56775

X-Original-Commit odoo/odoo@d16b03663823ea5a8192ef863c9975af0f4b246d

closes odoo/odoo#65481

X-original-commit: 9df7ff52d7c4d3900a8165d8054c920cdc72a7c4
Signed-off-by: Thibault Delavallee (tde) <tde@openerp.com>
Co-authored-by: Thibault Delavallee <tde@odoo.com>
Co-authored-by: Shahista Pathan <sat@odoo.com>

2 files changed, 31 insertions, 7 deletions

diff --git a/addons/mail/models/mail_followers.py b/addons/mail/models/mail_followers.py
index 6c526375584..8995de70acf 100644
--- a/addons/mail/models/mail_followers.py
+++ b/addons/mail/models/mail_followers.py
@@ -366,11 +366,15 @@ GROUP BY fol.id%s%s""" % (
                 elif existing_policy in ('replace', 'update'):
                     fol_id, sids = next(((key, val[3]) for key, val in data_fols.items() if val[0] == res_id and val[1] == partner_id), (False, []))
                     new_sids = set(partner_subtypes[partner_id]) - set(sids)
-                    old_sids = set(sids  if sids[0] is not None else []) - set(partner_subtypes[partner_id])
+                    old_sids = set(sids) - set(partner_subtypes[partner_id])
+                    update_cmd = []
                     if fol_id and new_sids:
-                        update[fol_id] = {'subtype_ids': [(4, sid) for sid in new_sids]}
+                        update_cmd += [(4, sid) for sid in new_sids]
                     if fol_id and old_sids and existing_policy == 'replace':
-                        update[fol_id] = {'subtype_ids': [(3, sid) for sid in old_sids]}
+                        update_cmd += [(3, sid) for sid in old_sids]
+                    if update_cmd:
+                        update[fol_id] = {'subtype_ids': update_cmd}
+
             for channel_id in set(channel_ids or []):
                 if channel_id not in doc_cids[res_id]:
                     new.setdefault(res_id, list()).append({
@@ -382,9 +386,12 @@ GROUP BY fol.id%s%s""" % (
                     fol_id, sids = next(((key, val[3]) for key, val in data_fols.items() if val[0] == res_id and val[2] == channel_id), (False, []))
                     new_sids = set(channel_subtypes[channel_id]) - set(sids)
                     old_sids = set(sids) - set(channel_subtypes[channel_id])
+                    update_cmd = []
                     if fol_id and new_sids:
-                        update[fol_id] = {'subtype_ids': [(4, sid) for sid in new_sids]}
+                        update_cmd += [(4, sid) for sid in new_sids]
                     if fol_id and old_sids and existing_policy == 'replace':
-                        update[fol_id] = {'subtype_ids': [(3, sid) for sid in old_sids]}
+                        update_cmd += [(3, sid) for sid in old_sids]
+                    if update_cmd:
+                        update[fol_id] = {'subtype_ids': update_cmd}
 
         return new, update
diff --git a/addons/test_mail/tests/test_mail_followers.py b/addons/test_mail/tests/test_mail_followers.py
index de827326d6b..739c210d3e3 100644
--- a/addons/test_mail/tests/test_mail_followers.py
+++ b/addons/test_mail/tests/test_mail_followers.py
@@ -3,10 +3,10 @@
 
 from psycopg2 import IntegrityError
 
-from odoo.tests import tagged, users
 from odoo.addons.test_mail.tests.common import TestMailCommon
-from odoo.tools.misc import mute_logger
 from odoo.tests import tagged
+from odoo.tests import users
+from odoo.tools.misc import mute_logger
 
 
 @tagged('mail_followers')
@@ -101,6 +101,7 @@ class BaseFollowersTest(TestMailCommon):
         self.assertEqual(test_record.message_follower_ids.subtype_ids, self.mt_mg_nodef | self.mt_al_nodef)
 
     def test_followers_multiple_subscription_noforce(self):
+        """ Calling message_subscribe without subtypes on an existing subscription should not do anything (default < existing) """
         test_record = self.test_record.with_user(self.user_employee)
 
         test_record.message_subscribe(partner_ids=[self.user_admin.partner_id.id], subtype_ids=[self.mt_mg_nodef.id, self.mt_al_nodef.id])
@@ -114,6 +115,22 @@ class BaseFollowersTest(TestMailCommon):
         self.assertEqual(test_record.message_channel_ids, self.env['mail.channel'])
         self.assertEqual(test_record.message_follower_ids.subtype_ids, self.mt_mg_nodef | self.mt_al_nodef)
 
+    def test_followers_multiple_subscription_update(self):
+        """ Calling message_subscribe with subtypes on an existing subscription should replace them (new > existing) """
+        test_record = self.test_record.with_user(self.user_employee)
+        test_record.message_subscribe(partner_ids=[self.user_employee.partner_id.id], subtype_ids=[self.mt_mg_def.id, self.mt_cl_def.id])
+        self.assertEqual(test_record.message_partner_ids, self.user_employee.partner_id)
+        follower = self.env['mail.followers'].search([
+            ('res_model', '=', 'mail.test.simple'),
+            ('res_id', '=', test_record.id),
+            ('partner_id', '=', self.user_employee.partner_id.id)])
+        self.assertEqual(follower, test_record.message_follower_ids)
+        self.assertEqual(follower.subtype_ids, self.mt_mg_def | self.mt_cl_def)
+
+        # remove one subtype `mt_mg_def` and set new subtype `mt_al_def`
+        test_record.message_subscribe(partner_ids=[self.user_employee.partner_id.id], subtype_ids=[self.mt_cl_def.id, self.mt_al_def.id])
+        self.assertEqual(follower.subtype_ids, self.mt_cl_def | self.mt_al_def)
+
     def test_followers_no_DID(self):
         """Test that a follower cannot suffer from dissociative identity disorder.
            It cannot be both a partner and a channel.