From 285aa27da47fe1983993acc0efd91d0ccbf28e99 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Wed, 26 Aug 2020 12:45:54 +0000 Subject: [PATCH] [IMP] mail: do not auto-subscribe inactive partners Tested flows * posting a message through an inactive partner (like automated actions posting a message on behalf of an inactive partner); * automatic subscription based on parent record (like an archived user and partner following a project that should not be added as follower of sub tasks); * automatic subscription based on responsible field: this is already fixed as user has to be active to receive a notification and be added in followers; LINKS Task ID-2326281 PR odoo/odoo#56560 X-original-commit: 42910ce25f06a31af3cfbc8d5d41e443ee754c34 --- addons/mail/models/mail_followers.py | 15 ++++++---- addons/mail/models/mail_thread.py | 10 +++---- addons/test_mail/tests/test_mail_followers.py | 28 +++++++++++++++++-- 3 files changed, 41 insertions(+), 12 deletions(-) diff --git a/addons/mail/models/mail_followers.py b/addons/mail/models/mail_followers.py index cdbdca86066..6c526375584 100644 --- a/addons/mail/models/mail_followers.py +++ b/addons/mail/models/mail_followers.py @@ -193,7 +193,7 @@ FROM mail_channel channel WHERE channel.id IN %s """ res = [] return res - def _get_subscription_data(self, doc_data, pids, cids, include_pshare=False): + def _get_subscription_data(self, doc_data, pids, cids, include_pshare=False, include_active=False): """ Private method allowing to fetch follower data from several documents of a given model. Followers can be filtered given partner IDs and channel IDs. @@ -202,6 +202,7 @@ FROM mail_channel channel WHERE channel.id IN %s """ :param pids: optional partner to filter; if None take all, otherwise limitate to pids :param cids: optional channel to filter; if None take all, otherwise limitate to cids :param include_pshare: optional join in partner to fetch their share status + :param include_active: optional join in partner to fetch their active flag :return: list of followers data which is a list of tuples containing follower ID, @@ -210,6 +211,7 @@ FROM mail_channel channel WHERE channel.id IN %s """ channel ID (void if partner_id), followed subtype IDs, share status of partner (void id channel_id, returned only if include_pshare is True) + active flag status of partner (void id channel_id, returned only if include_active is True) """ # base query: fetch followers of given documents where_clause = ' OR '.join(['fol.res_model = %s AND fol.res_id IN %s'] * len(doc_data)) @@ -231,17 +233,20 @@ FROM mail_channel channel WHERE channel.id IN %s """ where_clause += "AND (%s)" % " OR ".join(sub_where) query = """ -SELECT fol.id, fol.res_id, fol.partner_id, fol.channel_id, array_agg(subtype.id)%s +SELECT fol.id, fol.res_id, fol.partner_id, fol.channel_id, array_agg(subtype.id)%s%s FROM mail_followers fol %s LEFT JOIN mail_followers_mail_message_subtype_rel fol_rel ON fol_rel.mail_followers_id = fol.id LEFT JOIN mail_message_subtype subtype ON subtype.id = fol_rel.mail_message_subtype_id WHERE %s -GROUP BY fol.id%s""" % ( +GROUP BY fol.id%s%s""" % ( ', partner.partner_share' if include_pshare else '', - 'LEFT JOIN res_partner partner ON partner.id = fol.partner_id' if include_pshare else '', + ', partner.active' if include_active else '', + 'LEFT JOIN res_partner partner ON partner.id = fol.partner_id' if (include_pshare or include_active) else '', where_clause, - ', partner.partner_share' if include_pshare else '') + ', partner.partner_share' if include_pshare else '', + ', partner.active' if include_active else '' + ) self.env.cr.execute(query, tuple(where_params)) return self.env.cr.fetchall() diff --git a/addons/mail/models/mail_thread.py b/addons/mail/models/mail_thread.py index 24d85f79f70..2decfac6c44 100644 --- a/addons/mail/models/mail_thread.py +++ b/addons/mail/models/mail_thread.py @@ -1844,8 +1844,8 @@ class MailThread(models.AbstractModel): self._message_set_main_attachment_id(values['attachment_ids']) if values['author_id'] and values['message_type'] != 'notification' and not self._context.get('mail_create_nosubscribe'): - # if self.env['res.partner'].browse(values['author_id']).active: # we dont want to add odoobot/inactive as a follower - self._message_subscribe([values['author_id']]) + if self.env['res.partner'].browse(values['author_id']).active: # we dont want to add odoobot/inactive as a follower + self._message_subscribe([values['author_id']]) self._message_post_after_hook(new_message, values) self._notify_thread(new_message, values, **notif_kwargs) @@ -2769,11 +2769,11 @@ class MailThread(models.AbstractModel): if udpated_fields: doc_data = [(model, [updated_values[fname] for fname in fnames]) for model, fnames in updated_relation.items()] - res = self.env['mail.followers']._get_subscription_data(doc_data, None, None, include_pshare=True) - for fid, rid, pid, cid, subtype_ids, pshare in res: + res = self.env['mail.followers']._get_subscription_data(doc_data, None, None, include_pshare=True, include_active=True) + for fid, rid, pid, cid, subtype_ids, pshare, active in res: sids = [parent[sid] for sid in subtype_ids if parent.get(sid)] sids += [sid for sid in subtype_ids if sid not in parent and sid in def_ids or sid in int_ids] - if pid: + if pid and active: # auto subscribe only active partners new_partners[pid] = (set(sids) & set(all_ids)) - set(int_ids) if pshare else set(sids) & set(all_ids) if cid: new_channels[cid] = (set(sids) & set(all_ids)) - set(int_ids) diff --git a/addons/test_mail/tests/test_mail_followers.py b/addons/test_mail/tests/test_mail_followers.py index 5bac6d00529..fe8c5f38d96 100644 --- a/addons/test_mail/tests/test_mail_followers.py +++ b/addons/test_mail/tests/test_mail_followers.py @@ -187,6 +187,22 @@ class AdvancedFollowersTest(TestMailCommon): """ Creator of records are automatically added as followers """ self.assertEqual(self.test_track.message_partner_ids, self.user_employee.partner_id) + def test_auto_subscribe_inactive(self): + """ Test inactive are not added as followers in automated subscription """ + self.test_track.user_id = False + self.user_admin.active = False + self.user_admin.flush() + self.partner_admin.active = False + self.partner_admin.flush() + + self.test_track.with_user(self.user_admin).message_post(body='Coucou hibou', message_type='comment') + self.assertEqual(self.test_track.message_partner_ids, self.user_employee.partner_id) + self.assertEqual(self.test_track.message_follower_ids.partner_id, self.user_employee.partner_id) + + self.test_track.write({'user_id': self.user_admin.id}) + self.assertEqual(self.test_track.message_partner_ids, self.user_employee.partner_id) + self.assertEqual(self.test_track.message_follower_ids.partner_id, self.user_employee.partner_id) + def test_auto_subscribe_post(self): """ People posting a message are automatically added as followers """ self.test_track.with_user(self.user_admin).message_post(body='Coucou hibou', message_type='comment') @@ -221,13 +237,20 @@ class AdvancedFollowersTest(TestMailCommon): automatically create subscription with matching subtypes * subscribing to a sub-record as creator applies default subtype values * portal user should not have access to internal subtypes + + Inactive partners should not be auto subscribed. """ container = self.env['mail.test.container'].with_context(self._test_context).create({ 'name': 'Project-Like', }) - container.message_subscribe(partner_ids=[self.partner_portal.id]) - self.assertEqual(container.message_partner_ids, self.partner_portal) + container.message_subscribe(partner_ids=(self.partner_portal | self.partner_admin).ids) + self.assertEqual(container.message_partner_ids, self.partner_portal | self.partner_admin) + + self.user_admin.active = False + self.user_admin.flush() + self.partner_admin.active = False + self.partner_admin.flush() sub1 = self.env['mail.test.track'].with_user(self.user_employee).create({ 'name': 'Task-Like Test', @@ -238,6 +261,7 @@ class AdvancedFollowersTest(TestMailCommon): external_defaults = all_defaults.filtered(lambda subtype: not subtype.internal) self.assertEqual(sub1.message_partner_ids, self.partner_portal | self.user_employee.partner_id) + self.assertEqual(sub1.message_follower_ids.partner_id, self.partner_portal | self.user_employee.partner_id) self.assertEqual( sub1.message_follower_ids.filtered(lambda fol: fol.partner_id == self.partner_portal).subtype_ids, external_defaults | self.sub_umb1)