Filter Many2Many group fields on active = True

When the field is used in a domain adding this filter ensure that comparison
with None will succeed when there are only inactive groups in the field.

Closes #14369

trytond/trytond/ir/action.py

@@ -68,7 +68,10 @@ class Action(DeactivableMixin, ModelSQL, ModelView):
             'Keywords')
     icon = fields.Many2One('ir.ui.icon', 'Icon')
     groups = fields.Many2Many(
-        'ir.action-res.group', 'action', 'group', "Groups")
+        'ir.action-res.group', 'action', 'group', "Groups",
+        filter=[
+            ('active', '=', True),
+            ])
     _groups_cache = Cache('ir.action.get_groups', context=False)
 
     @classmethod

trytond/trytond/ir/export.py

@@ -24,6 +24,9 @@ class Export(_ClearCache, ModelSQL, ModelView):
         'res.user', "User", required=True, ondelete='CASCADE')
     groups = fields.Many2Many(
         'ir.export-res.group', 'export', 'group', "Groups",
+        filter=[
+            ('active', '=', True),
+            ],
         help="The user groups that can use the export.")
     write_groups = fields.Many2Many(
         'ir.export-write-res.group', 'export', 'group',

trytond/trytond/ir/model.py

@@ -975,7 +975,10 @@ class ModelButton(
             ])
     _reset_cache = Cache('ir.model.button.reset')
     groups = fields.Many2Many(
-        'ir.model.button-res.group', 'button', 'group', "Groups")
+        'ir.model.button-res.group', 'button', 'group', "Groups",
+        filter=[
+            ('active', '=', True),
+            ])
     _groups_cache = Cache('ir.model.button.groups')
     _view_attributes_cache = Cache(
         'ir.model.button.view_attributes', context=False)

trytond/trytond/ir/sequence.py

@@ -37,6 +37,9 @@ class SequenceType(ModelSQL, ModelView):
     name = fields.Char('Sequence Name', required=True, translate=True)
     groups = fields.Many2Many(
         'ir.sequence.type-res.group', 'sequence_type', 'group', "Groups",
+        filter=[
+            ('active', '=', True),
+            ],
         help="Groups allowed to edit the sequences of this type.")
 
 

trytond/trytond/ir/ui/menu.py

@@ -110,7 +110,10 @@ class UIMenu(
             ('keyword', '=', 'tree_open'),
             ])
     groups = fields.Many2Many(
-        'ir.ui.menu-res.group', 'menu', 'group', "Groups")
+        'ir.ui.menu-res.group', 'menu', 'group', "Groups",
+        filter=[
+            ('active', '=', True),
+            ])
     favorite = fields.Function(fields.Boolean('Favorite'), 'get_favorite')
     favorites = fields.Many2Many(
         'ir.ui.menu.favorite', 'menu', 'user', "Favorites")

trytond/trytond/res/user.py

@@ -618,16 +618,22 @@ class User(avatar_mixin(100, 'login'), DeactivableMixin, ModelSQL, ModelView):
         '''
         pool = Pool()
         UserGroup = pool.get('res.user-res.group')
+        Group = pool.get('res.group')
+
         transaction = Transaction()
         user = transaction.user
         groups = cls._get_groups_cache.get(user)
         if groups is not None:
             return groups
         cursor = transaction.connection.cursor()
+        group = Group.__table__()
         user_group = UserGroup.user_group_all_table()
-        cursor.execute(*user_group.select(
+        cursor.execute(*user_group
+            .join(group, 'LEFT', user_group.group == group.id)
+            .select(
                 user_group.group,
-                where=user_group.user == user,
+                where=((user_group.user == user)
+                    & (group.active == Literal(True))),
                 order_by=[user_group.group.asc]))
         groups = tuple(g for g, in cursor)
         cls._get_groups_cache.set(user, groups)

trytond/trytond/tests/test_access.py

@@ -227,6 +227,56 @@ class _ModelAccessTestCase(TestCase):
 
         self._assert_raises(record)
 
+    @with_transaction(context=_context)
+    def test_no_access_with_inactive_group(self):
+        "Test no access with inactive group"
+        pool = Pool()
+        Group = pool.get('res.group')
+        ModelAccess = pool.get('ir.model.access')
+        TestAccess = pool.get(self.model_name)
+
+        inactive_group, = Group.create([{'name': 'Test', 'active': False}])
+        record, = TestAccess.create([{}])
+        ModelAccess.create([{
+                    'model': self.model_name,
+                    'group': None,
+                    self._perm: False,
+                    }])
+        ModelAccess.create([{
+                    'model': self.model_name,
+                    'group': inactive_group.id,
+                    self._perm: True,
+                    }])
+
+        self._assert_raises(record)
+
+    @with_transaction(context=_context)
+    def test_no_access_with_member_inactive_group(self):
+        "Test no access when member of an inactive group"
+        pool = Pool()
+        Group = pool.get('res.group')
+        ModelAccess = pool.get('ir.model.access')
+        TestAccess = pool.get(self.model_name)
+
+        inactive_group, = Group.create([{
+                    'name': 'Test',
+                    'active': False,
+                    'users': [('add', [Transaction().user])],
+                    }])
+        record, = TestAccess.create([{}])
+        ModelAccess.create([{
+                    'model': self.model_name,
+                    'group': None,
+                    self._perm: False,
+                    }])
+        ModelAccess.create([{
+                    'model': self.model_name,
+                    'group': inactive_group.id,
+                    self._perm: True,
+                    }])
+
+        self._assert_raises(record)
+
     @with_transaction(context=_context)
     def test_one_access_with_other_group_no_perm(self):
         "Test one access with other group no perm"
@@ -656,6 +706,62 @@ class _ModelFieldAccessTestCase(TestCase):
         self._assert_raises1(record)
         self._assert2(record)
 
+    @with_transaction(context=_context)
+    def test_no_access_with_inactive_group(self):
+        "Test no access with inactive group"
+        pool = Pool()
+        Group = pool.get('res.group')
+        FieldAccess = pool.get('ir.model.field.access')
+        TestAccess = pool.get('test.access')
+
+        inactive_group, = Group.create([{'name': 'Test', 'active': False}])
+        record, = TestAccess.create([{}])
+        FieldAccess.create([{
+                    'model': 'test.access',
+                    'field': 'field1',
+                    'group': None,
+                    self._perm: False,
+                    }])
+        FieldAccess.create([{
+                    'model': 'test.access',
+                    'field': 'field1',
+                    'group': inactive_group.id,
+                    self._perm: True,
+                    }])
+
+        self._assert_raises1(record)
+        self._assert2(record)
+
+    @with_transaction(context=_context)
+    def test_no_access_with_member_inactive_group(self):
+        "Test no access when member of an inactive group"
+        pool = Pool()
+        Group = pool.get('res.group')
+        FieldAccess = pool.get('ir.model.field.access')
+        TestAccess = pool.get('test.access')
+
+        inactive_group, = Group.create([{
+                    'name': 'Test',
+                    'active': False,
+                    'users': [('add', [Transaction().user])],
+                    }])
+        record, = TestAccess.create([{}])
+        FieldAccess.create([{
+                    'model': 'test.access',
+                    'field': 'field1',
+                    'group': None,
+                    self._perm: False,
+                    }])
+        FieldAccess.create([{
+                    'model': 'test.access',
+                    'field': 'field1',
+                    'group': inactive_group.id,
+                    self._perm: True,
+                    }])
+
+        self._assert_raises1(record)
+        self._assert2(record)
+
     @with_transaction(context=_context)
     def test_one_access_with_other_group_no_perm(self):
         "Test one access with other group no perm"