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..0ba6214de90 100644 --- a/addons/mail/models/mail_thread.py +++ b/addons/mail/models/mail_thread.py @@ -1006,6 +1006,7 @@ class MailThread(models.AbstractModel): thread_id = False for model, thread_id, custom_values, user_id, alias in routes or (): subtype_id = False + related_user = self.env['res.users'].browse(user_id) Model = self.env[model].with_context(mail_create_nosubscribe=True, mail_create_nolog=True) if not (thread_id and hasattr(Model, 'message_update') or hasattr(Model, 'message_new')): raise ValueError( @@ -1015,7 +1016,7 @@ class MailThread(models.AbstractModel): # disabled subscriptions during message_new/update to avoid having the system user running the # email gateway become a follower of all inbound messages - ModelCtx = Model.with_user(user_id).sudo() + ModelCtx = Model.with_user(related_user).sudo() if thread_id and hasattr(ModelCtx, 'message_update'): thread = ModelCtx.browse(thread_id) thread.message_update(message_dict) @@ -1048,6 +1049,9 @@ class MailThread(models.AbstractModel): if thread._name == 'mail.thread': # message with parent_id not linked to record new_msg = thread.message_notify(**post_params) else: + # parsing should find an author independently of user running mail gateway, and ensure it is not odoobot + partner_from_found = message_dict.get('author_id') and message_dict['author_id'] != self.env['ir.model.data'].xmlid_to_res_id('base.partner_root') + thread = thread.with_context(mail_create_nosubscribe=not partner_from_found) new_msg = thread.message_post(**post_params) if new_msg and original_partner_ids: @@ -1844,8 +1848,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 +2773,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 7669394e875..fe8c5f38d96 100644 --- a/addons/test_mail/tests/test_mail_followers.py +++ b/addons/test_mail/tests/test_mail_followers.py @@ -3,7 +3,7 @@ from psycopg2 import IntegrityError -from odoo.tests import tagged +from odoo.tests import tagged, users from odoo.addons.test_mail.tests.common import TestMailCommon from odoo.tools.misc import mute_logger @@ -17,13 +17,20 @@ class BaseFollowersTest(TestMailCommon): cls._create_portal_user() cls._create_channel_listener() + # allow employee to update partners + cls.user_employee.write({'groups_id': [(4, cls.env.ref('base.group_partner_manager').id)]}) + Subtype = cls.env['mail.message.subtype'] - cls.mt_mg_def = Subtype.create({'name': 'mt_mg_def', 'default': True, 'res_model': 'mail.test.simple'}) - cls.mt_cl_def = Subtype.create({'name': 'mt_cl_def', 'default': True, 'res_model': 'mail.test.container'}) + # global cls.mt_al_def = Subtype.create({'name': 'mt_al_def', 'default': True, 'res_model': False}) - cls.mt_mg_nodef = Subtype.create({'name': 'mt_mg_nodef', 'default': False, 'res_model': 'mail.test.simple'}) cls.mt_al_nodef = Subtype.create({'name': 'mt_al_nodef', 'default': False, 'res_model': False}) - cls.mt_mg_def_int = cls.env['mail.message.subtype'].create({'name': 'mt_mg_def', 'default': True, 'res_model': 'mail.test.simple', 'internal': True}) + # mail.test.simple + cls.mt_mg_def = Subtype.create({'name': 'mt_mg_def', 'default': True, 'res_model': 'mail.test.simple'}) + cls.mt_mg_nodef = Subtype.create({'name': 'mt_mg_nodef', 'default': False, 'res_model': 'mail.test.simple'}) + cls.mt_mg_def_int = Subtype.create({'name': 'mt_mg_def', 'default': True, 'res_model': 'mail.test.simple', 'internal': True}) + # mail.test.container + cls.mt_cl_def = Subtype.create({'name': 'mt_cl_def', 'default': True, 'res_model': 'mail.test.container'}) + cls.default_group_subtypes = Subtype.search([('default', '=', True), '|', ('res_model', '=', 'mail.test.simple'), ('res_model', '=', False)]) cls.default_group_subtypes_portal = Subtype.search([('internal', '=', False), ('default', '=', True), '|', ('res_model', '=', 'mail.test.simple'), ('res_model', '=', False)]) @@ -127,6 +134,27 @@ class BaseFollowersTest(TestMailCommon): channel_ids=[self.channel_listen.id] ) + @users('employee') + def test_followers_inactive(self): + """ Test standard API does not subscribe inactive partners """ + customer = self.env['res.partner'].create({ + 'name': 'Valid Lelitre', + 'email': 'valid.lelitre@agrolait.com', + 'country_id': self.env.ref('base.be').id, + 'mobile': '0456001122', + 'active': False, + }) + document = self.env['mail.test.simple'].browse(self.test_record.id) + self.assertEqual(document.message_partner_ids, self.env['res.partner']) + document.message_subscribe(partner_ids=(self.partner_portal | customer).ids) + self.assertEqual(document.message_partner_ids, self.partner_portal) + self.assertEqual(document.message_follower_ids.partner_id, self.partner_portal) + + # works through low-level API + document._message_subscribe(partner_ids=(self.partner_portal | customer).ids) + self.assertEqual(document.message_partner_ids, self.partner_portal, 'No active test: customer not visible') + self.assertEqual(document.message_follower_ids.partner_id, self.partner_portal | customer) + class AdvancedFollowersTest(TestMailCommon): @classmethod @@ -159,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') @@ -193,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', @@ -210,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) diff --git a/addons/test_mail/tests/test_mail_gateway.py b/addons/test_mail/tests/test_mail_gateway.py index b615ceb5ff5..630ccf47ffa 100644 --- a/addons/test_mail/tests/test_mail_gateway.py +++ b/addons/test_mail/tests/test_mail_gateway.py @@ -223,19 +223,62 @@ class TestMailgateway(TestMailCommon): set(['rosaçée.gif', 'verte!µ.gif', 'orangée.gif'])) def test_message_process_followers(self): - pass - # TODO : the author of a message post should be added as follower - # currently it is not the case as otherwise Administrator would be follower of a lot of stuff - # this is a bug with mail_create_nosubscribe -> should be changed in master - # self.assertEqual(record.message_partner_ids, self.partner_1, - # 'message_process: recognized email -> added as follower') + """ Incoming email: recognized author not archived and not odoobot: added as follower """ + with self.mock_mail_gateway(): + record = self.format_and_process(MAIL_TEMPLATE, self.partner_1.email_formatted, 'groups@test.com') - # TODO : the author of a message post on mail.test should not be added as follower - # Test: author (and not recipient) added as follower - # self.assertEqual(self.test_public.message_partner_ids, self.partner_1 | self.partner_2, - # 'message_process: after reply, group should have 2 followers') - # self.assertEqual(self.test_public.message_channel_ids, self.env['mail.test.container'], - # 'message_process: after reply, group should have 2 followers (0 channels)') + self.assertEqual(record.message_ids[0].author_id, self.partner_1, + 'message_process: recognized email -> author_id') + self.assertEqual(record.message_ids[0].email_from, self.partner_1.email_formatted) + self.assertEqual(record.message_follower_ids.partner_id, self.partner_1, + 'message_process: recognized email -> added as follower') + self.assertEqual(record.message_partner_ids, self.partner_1, + 'message_process: recognized email -> added as follower') + + # just an email -> no follower + with self.mock_mail_gateway(): + record2 = self.format_and_process( + MAIL_TEMPLATE, self.email_from, 'groups@test.com', + subject='Another Email') + + self.assertEqual(record2.message_ids[0].author_id, self.env['res.partner']) + self.assertEqual(record2.message_ids[0].email_from, self.email_from) + self.assertEqual(record2.message_follower_ids.partner_id, self.env['res.partner'], + 'message_process: unrecognized email -> no follower') + self.assertEqual(record2.message_partner_ids, self.env['res.partner'], + 'message_process: unrecognized email -> no follower') + + # archived partner -> no follower + self.partner_1.active = False + self.partner_1.flush() + with self.mock_mail_gateway(): + record3 = self.format_and_process( + MAIL_TEMPLATE, self.partner_1.email_formatted, 'groups@test.com', + subject='Yet Another Email') + + self.assertEqual(record3.message_ids[0].author_id, self.env['res.partner']) + self.assertEqual(record3.message_ids[0].email_from, self.partner_1.email_formatted) + self.assertEqual(record3.message_follower_ids.partner_id, self.env['res.partner'], + 'message_process: unrecognized email -> no follower') + self.assertEqual(record3.message_partner_ids, self.env['res.partner'], + 'message_process: unrecognized email -> no follower') + + + # partner_root -> never again + odoobot = self.env.ref('base.partner_root') + odoobot.active = True + odoobot.email = 'odoobot@example.com' + with self.mock_mail_gateway(): + record4 = self.format_and_process( + MAIL_TEMPLATE, odoobot.email_formatted, 'groups@test.com', + subject='Odoobot Automatic Answer') + + self.assertEqual(record4.message_ids[0].author_id, odoobot) + self.assertEqual(record4.message_ids[0].email_from, odoobot.email_formatted) + self.assertEqual(record4.message_follower_ids.partner_id, self.env['res.partner'], + 'message_process: unrecognized email -> no follower') + self.assertEqual(record4.message_partner_ids, self.env['res.partner'], + 'message_process: unrecognized email -> no follower') # -------------------------------------------------- # Author recognition