[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

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 96a78aa6079..44527232803 100644
--- a/addons/mail/models/mail_thread.py
+++ b/addons/mail/models/mail_thread.py
@@ -1870,8 +1870,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)
@@ -2801,11 +2801,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)