[FIX] base: check group constraints when adding implied groups

Since commit 5f12e244f6, the write of implied groups is done directly in SQL,
bypassing the ORM writes, and thus bypassing the python constraints checks.
In particular such a constraint was added in f206714af0.
We add back an explicit check of this constraint;
however we rewrite it in SQL to be fast.

closes odoo/odoo#35411

closes odoo/odoo#36769

Signed-off-by: Nans Lefebvre (len) <len@odoo.com>
Signed-off-by: Nans Lefebvre (len) <len@odoo.com>

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)."

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)."))

odoo/addons/base/models/res_users.py

@@ -409,10 +409,43 @@ class Users(models.Model):
     @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 @@ class GroupsImplied(models.Model):
                            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):

odoo/addons/base/tests/test_user_has_group.py

@@ -143,3 +143,21 @@ class TestHasGroup(TransactionCase):
              })
         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)]})