diff --git a/addons/account/i18n/account.pot b/addons/account/i18n/account.pot index f02dee6340f2c..91346b0d6a500 100644 --- a/addons/account/i18n/account.pot +++ b/addons/account/i18n/account.pot @@ -688,7 +688,6 @@ msgstr "" #: code:addons/account/models/res_users.py:22 #, python-format msgid "A user cannot have both Tax B2B and Tax B2C.\n" -"Problematic user(s): %s\n" "You should go in General Settings, and choose to display Product Prices\n" "either in 'Tax-Included' or in 'Tax-Excluded' mode\n" "(or switch twice the mode if you are already in the desired one)." diff --git a/addons/account/models/res_users.py b/addons/account/models/res_users.py index 8030423e74d3b..013acde54ffbe 100644 --- a/addons/account/models/res_users.py +++ b/addons/account/models/res_users.py @@ -13,14 +13,11 @@ class Users(models.Model): def _check_one_user_type(self): super(Users, self)._check_one_user_type() - users_with_both_groups = self.filtered(lambda user: - user.has_group('account.group_show_line_subtotals_tax_included') and - user.has_group('account.group_show_line_subtotals_tax_excluded') - ) - if users_with_both_groups: - names = ", ".join(users_with_both_groups.mapped('name')) + g1 = self.env.ref('account.group_show_line_subtotals_tax_included') + g2 = self.env.ref('account.group_show_line_subtotals_tax_excluded') + + if self._has_multiple_groups([g1.id, g2.id]): raise ValidationError(_("A user cannot have both Tax B2B and Tax B2C.\n" - "Problematic user(s): %s\n" "You should go in General Settings, and choose to display Product Prices\n" "either in 'Tax-Included' or in 'Tax-Excluded' mode\n" - "(or switch twice the mode if you are already in the desired one).") % names) + "(or switch twice the mode if you are already in the desired one).")) diff --git a/odoo/addons/base/models/res_users.py b/odoo/addons/base/models/res_users.py index b524a6770ea11..002693ee7815f 100644 --- a/odoo/addons/base/models/res_users.py +++ b/odoo/addons/base/models/res_users.py @@ -409,10 +409,43 @@ def _check_action_id(self): @api.multi @api.constrains('groups_id') def _check_one_user_type(self): - for user in self: - if len(user.groups_id.filtered(lambda x: x.category_id.xml_id == 'base.module_category_user_type')) > 1: + """We check that no users are both portal and users (same with public). + This could typically happen because of implied groups. + """ + user_types_category = self.env.ref('base.module_category_user_type', raise_if_not_found=False) + user_types_groups = self.env['res.groups'].search( + [('category_id', '=', user_types_category.id)]) if user_types_category else False + if user_types_groups: # needed at install + if self._has_multiple_groups(user_types_groups.ids): raise ValidationError(_('The user cannot have more than one user types.')) + @api.multi + def _has_multiple_groups(self, group_ids): + """The method is not fast if the list of ids is very long; + so we rather check all users than limit to the size of the group + :param group_ids: list of group ids + :return: boolean: is there at least a user in at least 2 of the provided groups + """ + if group_ids: + args = [tuple(group_ids)] + if len(self.ids) == 1: + where_clause = "AND r.uid = %s" + args.append(self.id) + else: + where_clause = "" # default; we check ALL users (actually pretty efficient) + query = """ + SELECT 1 FROM res_groups_users_rel WHERE EXISTS( + SELECT r.uid + FROM res_groups_users_rel r + WHERE r.gid IN %s""" + where_clause + """ + GROUP BY r.uid HAVING COUNT(r.gid) > 1 + ) + """ + self.env.cr.execute(query, args) + return bool(self.env.cr.fetchall()) + else: + return False + @api.multi def toggle_active(self): for user in self: @@ -955,6 +988,7 @@ def write(self, values): JOIN group_imply i ON (r.gid = i.hid) WHERE i.gid = %(gid)s """, dict(gid=group.id)) + self._check_one_user_type() return res class UsersImplied(models.Model): diff --git a/odoo/addons/base/tests/test_user_has_group.py b/odoo/addons/base/tests/test_user_has_group.py index 0151bd2354c64..98e699a071841 100644 --- a/odoo/addons/base/tests/test_user_has_group.py +++ b/odoo/addons/base/tests/test_user_has_group.py @@ -143,3 +143,21 @@ def test_two_user_types(self): }) with self.assertRaises(ValidationError): self.grp_internal.users = [(4, test_user.id)] + + def test_two_user_types_implied_groups(self): + """Contrarily to test_two_user_types, we simply add an implied_id to a group. + This will trigger the addition of the relevant users to the relevant groups; + if, say, this was done in SQL and thus bypassing the ORM, it would bypass the constraints + and thus give us a case uncovered by the aforementioned test. + """ + grp_test = self.env["res.groups"].create( + {"name": "test", "implied_ids": [(6, 0, [self.grp_internal.id])]}) + + test_user = self.env['res.users'].create({ + 'login': 'test_user_portal', + 'name': "Test User with one user types", + 'groups_id': [(6, 0, [grp_test.id])] + }) + + with self.assertRaises(ValidationError): + grp_test.write({'implied_ids': [(4, self.grp_portal.id)]})