From c832452590941075cc14fb85fdf2302b1ff2ddd9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Tue, 24 Apr 2018 18:34:02 +0200 Subject: [PATCH] [IMP] mail: update subscription instead of unlink / create new followers In some cases when subscribing followers code may ask to force subtypes of the subscription. It means existing followers should have only asked subtypes independently from existing subtypes. Previously it was done by unlinking and creating new followers for same partner or channel. This commit proposes to instead update missing subtypes and remove extra subtypes. It has a small impact in real life as forcing subtype is not done often. Some tests are improved thanks to this commit. This commit is linked to task ID 1839043. Closes #24417. --- addons/mail/models/mail_followers.py | 11 +++++++++-- addons/mail/models/mail_thread.py | 2 +- addons/test_mail/tests/test_performance.py | 4 ++-- 3 files changed, 12 insertions(+), 5 deletions(-) diff --git a/addons/mail/models/mail_followers.py b/addons/mail/models/mail_followers.py index 4040de72276..469ab1d747c 100644 --- a/addons/mail/models/mail_followers.py +++ b/addons/mail/models/mail_followers.py @@ -198,6 +198,7 @@ GROUP BY fol.id%s""" % ( * skip: simply skip existing followers, do not touch them; * force: update existing with given subtypes only; + * replace: replace existing with nex subtypes (like force without old / new follower); * update: gives an update dict allowing to add missing subtypes (no subtype removal); """ _res_ids = res_ids or [0] @@ -224,11 +225,14 @@ GROUP BY fol.id%s""" % ( 'partner_id': partner_id, 'subtype_ids': [(6, 0, partner_subtypes[partner_id])], }) - elif existing_policy == 'update': + 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) - set(partner_subtypes[partner_id]) if fol_id and new_sids: update[fol_id] = {'subtype_ids': [(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]} for channel_id in set(channel_ids or []): if channel_id not in doc_cids[res_id]: new.setdefault(res_id, list()).append({ @@ -236,10 +240,13 @@ GROUP BY fol.id%s""" % ( 'channel_id': channel_id, 'subtype_ids': [(6, 0, channel_subtypes[channel_id])], }) - elif existing_policy == 'update': + 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[2] == channel_id), (False, [])) new_sids = set(channel_subtypes[channel_id]) - set(sids) + old_sids = set(sids) - set(channel_subtypes[channel_id]) if fol_id and new_sids: update[fol_id] = {'subtype_ids': [(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]} return new, update diff --git a/addons/mail/models/mail_thread.py b/addons/mail/models/mail_thread.py index 9af7ea554ef..f92b8006fc3 100644 --- a/addons/mail/models/mail_thread.py +++ b/addons/mail/models/mail_thread.py @@ -2082,7 +2082,7 @@ class MailThread(models.AbstractModel): self._name, self.ids, partner_ids, dict((pid, subtype_ids) for pid in partner_ids), channel_ids, dict((cid, subtype_ids) for cid in channel_ids), - customer_ids=customer_ids, check_existing=True, existing_policy='force') + customer_ids=customer_ids, check_existing=True, existing_policy='replace') return True diff --git a/addons/test_mail/tests/test_performance.py b/addons/test_mail/tests/test_performance.py index 3ce8126440b..2b3442608c0 100644 --- a/addons/test_mail/tests/test_performance.py +++ b/addons/test_mail/tests/test_performance.py @@ -291,7 +291,7 @@ class TestAdvMailPerformance(TransactionCase): with self.assertQueryCount(admin=5, emp=5): # test_mail only: 5 - 5 record.message_subscribe(partner_ids=self.user_test.partner_id.ids, subtype_ids=subtype_ids) - with self.assertQueryCount(admin=12, emp=12): # test_mail only: 12 - 12 + with self.assertQueryCount(admin=1, emp=1): # test_mail only: 1 - 1 record.message_subscribe(partner_ids=self.user_test.partner_id.ids, subtype_ids=subtype_ids) @@ -449,7 +449,7 @@ class TestHeavyMailPerformance(TransactionCase): self.assertEqual(rec.message_channel_ids, self.channel) # subscribe existing and new followers with force=True, meaning all will have the same subtypes - with self.assertQueryCount(admin=42, emp=42): # test_mail only: 42 - 42 + with self.assertQueryCount(admin=13, emp=13): # test_mail only: 13 - 13 rec.message_subscribe( partner_ids=pids, channel_ids=cids,