diff --git a/addons/mail/models/ir_config_parameter.py b/addons/mail/models/ir_config_parameter.py index 09b6080f33b..7fe7407e413 100644 --- a/addons/mail/models/ir_config_parameter.py +++ b/addons/mail/models/ir_config_parameter.py @@ -30,10 +30,8 @@ class IrConfigParameter(models.Model): group_user.implied_ids |= group_mail_template_editor elif value and group_mail_template_editor in group_user.implied_ids: - group_user.implied_ids -= group_mail_template_editor # remove existing users, including inactive template user # admin will regain the right via implied_ids on group_system - group_mail_template_editor.with_context(active_test=False).users.write( - {'groups_id': [(3, group_mail_template_editor.id)]}) + group_user._remove_group(group_mail_template_editor) return super(IrConfigParameter, self).set_param(key, value) diff --git a/addons/mail/tests/test_mail_template.py b/addons/mail/tests/test_mail_template.py index 14b0ffdaf75..4d26f315b80 100644 --- a/addons/mail/tests/test_mail_template.py +++ b/addons/mail/tests/test_mail_template.py @@ -53,7 +53,9 @@ class TestMailTemplate(MailCommon): def test_mail_template_acl(self): # Sanity check self.assertTrue(self.user_admin.has_group('mail.group_mail_template_editor')) + self.assertTrue(self.user_admin.has_group('base.group_sanitize_override')) self.assertFalse(self.user_employee.has_group('mail.group_mail_template_editor')) + self.assertFalse(self.user_employee.has_group('base.group_sanitize_override')) # Group System can create / write / unlink mail template mail_template = self.env['mail.template'].with_user(self.user_admin).create({'name': 'Test template'}) @@ -186,3 +188,17 @@ class TestMailTemplateReset(MailCommon): # subject is not there in the data file template, so it should be set to False self.assertFalse(mail_template.subject, "Subject should be set to False") + +@tagged('-at_install', 'post_install') +class TestConfigRestrictEditor(MailCommon): + + def test_switch_icp_value(self): + # Sanity check + self.assertTrue(self.user_employee.has_group('mail.group_mail_template_editor')) + self.assertFalse(self.user_employee.has_group('base.group_system')) + + self.env['ir.config_parameter'].set_param('mail.restrict.template.rendering', True) + self.assertFalse(self.user_employee.has_group('mail.group_mail_template_editor')) + + self.env['ir.config_parameter'].set_param('mail.restrict.template.rendering', False) + self.assertTrue(self.user_employee.has_group('mail.group_mail_template_editor')) diff --git a/odoo/addons/base/models/res_users.py b/odoo/addons/base/models/res_users.py index ee8a6d60457..3efa10cda13 100644 --- a/odoo/addons/base/models/res_users.py +++ b/odoo/addons/base/models/res_users.py @@ -1252,8 +1252,17 @@ class GroupsImplied(models.Model): groups = self.filtered(lambda g: implied_group in g.implied_ids) if groups: groups.write({'implied_ids': [Command.unlink(implied_group.id)]}) - if groups.users: - implied_group.write({'users': [Command.unlink(user.id) for user in groups.users]}) + # if user belongs to implied_group thanks to another group, don't remove him + # this avoids readding the template user and triggering the mechanism at 121cd0d6084cb28 + users_to_unlink = [ + user + for user in groups.with_context(active_test=False).users + if implied_group not in (user.groups_id - implied_group).trans_implied_ids + ] + if users_to_unlink: + # do not remove inactive users (e.g. default) + implied_group.with_context(active_test=False).write( + {'users': [Command.unlink(user.id) for user in users_to_unlink]}) class UsersImplied(models.Model): _inherit = 'res.users' diff --git a/odoo/addons/base/tests/test_base.py b/odoo/addons/base/tests/test_base.py index 34eb98ad28f..a68ec268587 100644 --- a/odoo/addons/base/tests/test_base.py +++ b/odoo/addons/base/tests/test_base.py @@ -789,22 +789,52 @@ class TestGroups(TransactionCase): self.assertIn(a, c.implied_ids) def test_remove_groups(self): - u = self.env['res.users'].create({'login': 'u', 'name': 'U'}) + u1 = self.env['res.users'].create({'login': 'u1', 'name': 'U1'}) + u2 = self.env['res.users'].create({'login': 'u2', 'name': 'U2'}) + default = self.env.ref('base.default_user') + portal = self.env.ref('base.group_portal') + p = self.env['res.users'].create({'login': 'p', 'name': 'P', 'groups_id': [Command.set([portal.id])]}) - a = self.env['res.groups'].create({'name': 'A', 'users': [Command.set(u.ids)]}) - b = self.env['res.groups'].create({'name': 'B', 'users': [Command.set(u.ids)]}) - c = self.env['res.groups'].create({'name': 'C', 'implied_ids': [Command.set(a.ids)]}) + a = self.env['res.groups'].create({'name': 'A', 'users': [Command.set(u1.ids)]}) + b = self.env['res.groups'].create({'name': 'B', 'users': [Command.set(u1.ids)]}) + c = self.env['res.groups'].create({'name': 'C', 'implied_ids': [Command.set(a.ids)], 'users': [Command.set([p.id, u2.id, default.id])]}) + d = self.env['res.groups'].create({'name': 'D', 'implied_ids': [Command.set(a.ids)], 'users': [Command.set([u2.id, default.id])]}) + + def assertUsersEqual(users, group): + self.assertEqual( + sorted([r.login for r in users]), + sorted([r.login for r in group.with_context(active_test=False).users]) + ) + # sanity checks + assertUsersEqual([u1, u2, p, default], a) + assertUsersEqual([u1], b) + assertUsersEqual([u2, p, default], c) + assertUsersEqual([u2, default], d) # C already implies A, we want none of B+C to imply A (b + c)._remove_group(a) self.assertNotIn(a, b.implied_ids) self.assertNotIn(a, c.implied_ids) + self.assertIn(a, d.implied_ids) - # Since B didn't imply A, removing A from the implied groups of (B+C) - # should not remove user U from A, even though C implied A, since C does - # not have U as a user - self.assertIn(u, a.users) + # - Since B didn't imply A, removing A from the implied groups of (B+C) + # should not remove user U1 from A, even though C implied A, since C does + # not have U1 as a user + # - P should be removed as was only added via inheritance to C + # - U2 should not be removed from A since it is implied via C but also via D + assertUsersEqual([u1, u2, default], a) + assertUsersEqual([u1], b) + assertUsersEqual([u2, p, default], c) + assertUsersEqual([u2, default], d) + + # When adding the template user to a new group, it should add it to existing internal users + e = self.env['res.groups'].create({'name': 'E'}) + default.write({'groups_id': [Command.link(e.id)]}) + self.assertIn(u1, e.users) + self.assertIn(u2, e.users) + self.assertIn(default, e.with_context(active_test=False).users) + self.assertNotIn(p, e.users) class TestUsers(TransactionCase):