Don't check write access on model for button with groups

issue5010
review23181002

CHANGELOG

@@ -1,3 +1,4 @@
+* Don't check write access on model for button with groups
 * Limit readonly state for xxx2Many
 * Use home directory as default path for database and web root.
 * Add count option on Action Window Domains

trytond/model/modelview.py

@@ -483,6 +483,7 @@ class ModelView(Model):
         pool = Pool()
         Translation = pool.get('ir.translation')
         ModelData = pool.get('ir.model.data')
+        ModelAccess = pool.get('ir.model.access')
         Button = pool.get('ir.model.button')
         User = pool.get('res.user')
 
@@ -562,7 +563,10 @@ class ModelView(Model):
                 states = {}
             groups = set(User.get_groups())
             button_groups = Button.get_groups(cls.__name__, button_name)
-            if button_groups and not groups & button_groups:
+            if ((button_groups and not groups & button_groups)
+                    or (not button_groups
+                        and not ModelAccess.check(
+                            cls.__name__, 'write', raise_exception=False))):
                 states = states.copy()
                 states['readonly'] = True
             element.set('states', encoder.encode(states))
@@ -614,13 +618,16 @@ class ModelView(Model):
             if ((Transaction().user != 0)
                     and Transaction().context.get('_check_access')):
                 ModelAccess.check(cls.__name__, 'read')
-                ModelAccess.check(cls.__name__, 'write')
                 groups = set(User.get_groups())
                 button_groups = Button.get_groups(cls.__name__,
                     func.__name__)
-                if button_groups and not groups & button_groups:
-                    raise UserError('Calling button %s on %s is not allowed!'
-                        % (func.__name__, cls.__name__))
+                if button_groups:
+                    if not groups & button_groups:
+                        raise UserError(
+                            'Calling button %s on %s is not allowed!'
+                            % (func.__name__, cls.__name__))
+                else:
+                    ModelAccess.check(cls.__name__, 'write')
             with Transaction().set_context(_check_access=False):
                 return func(cls, *args, **kwargs)
         return wrapper

trytond/tests/__init__.py

@@ -120,6 +120,7 @@ def register():
         UnionTree,
         ModelViewChangedValues,
         ModelViewChangedValuesTarget,
+        ModelViewButton,
         MPTT,
         ImportDataBoolean,
         ImportDataInteger,

trytond/tests/modelview.py

@@ -8,6 +8,7 @@ from trytond.model import ModelView, fields
 __all__ = [
     'ModelViewChangedValues',
     'ModelViewChangedValuesTarget',
+    'ModelViewButton',
     ]
 
 
@@ -30,3 +31,20 @@ class ModelViewChangedValuesTarget(ModelView):
     __name__ = 'test.modelview.changed_values.target'
     name = fields.Char('Name')
     parent = fields.Many2One('test.modelview.changed_values', 'Parent')
+
+
+class ModelViewButton(ModelView):
+    'ModelView Button'
+    __name__ = 'test.modelview.button'
+
+    @classmethod
+    def __setup__(cls):
+        super(ModelViewButton, cls).__setup__()
+        cls._buttons = {
+            'test': {},
+            }
+
+    @classmethod
+    @ModelView.button
+    def test(cls, records):
+        pass

trytond/tests/test_modelview.py

@@ -5,6 +5,7 @@ import unittest
 
 from trytond.tests.test_tryton import install_module, with_transaction
 from trytond.pool import Pool
+from trytond.exceptions import UserError
 
 
 class ModelView(unittest.TestCase):
@@ -72,6 +73,47 @@ class ModelView(unittest.TestCase):
                     },
                 })
 
+    @with_transaction(context={'_check_access': True})
+    def test_button_access(self):
+        'Test Button Access'
+        pool = Pool()
+        TestModel = pool.get('test.modelview.button')
+        Model = pool.get('ir.model')
+        Button = pool.get('ir.model.button')
+        ModelAccess = pool.get('ir.model.access')
+        Group = pool.get('res.group')
+
+        model, = Model.search([('model', '=', 'test.modelview.button')])
+        admin, = Group.search([('name', '=', 'Administration')])
+        test = TestModel()
+
+        # Without model/button access
+        TestModel.test([test])
+
+        # Without read access
+        access = ModelAccess(model=model, group=None, perm_read=False)
+        access.save()
+        self.assertRaises(UserError, TestModel.test, [test])
+
+        # Without write access
+        access.perm_read = True
+        access.perm_write = False
+        access.save()
+        self.assertRaises(UserError, TestModel.test, [test])
+
+        # Without write access but with button access
+        button = Button(name='test', model=model, groups=[admin])
+        button.save()
+        TestModel.test([test])
+
+        # Without button access
+        ModelAccess.delete([access])
+        no_group = Group(name='no group')
+        no_group.save()
+        button.groups = [no_group]
+        button.save()
+        self.assertRaises(UserError, TestModel.test, [test])
+
 
 def suite():
     func = unittest.TestLoader().loadTestsFromTestCase