From 3158279cb38d9a5040ecae96035616b5dd47c092 Mon Sep 17 00:00:00 2001
From: Sergi Almacellas Abellana <sergi@koolpi.com>
Date: Wed, 1 Aug 2018 10:44:06 +0200
Subject: [PATCH] Remove button if it is not allowed to access some of its
 dependant fields

issue7502
review47511002
---
 CHANGELOG                       |  1 +
 trytond/model/modelview.py      | 43 +++++++++++++++++++++------------
 trytond/tests/__init__.py       |  1 +
 trytond/tests/modelview.py      | 21 ++++++++++++++++
 trytond/tests/modelview.xml     | 13 ++++++++++
 trytond/tests/test_modelview.py | 38 +++++++++++++++++++++++++++++
 6 files changed, 101 insertions(+), 16 deletions(-)

diff --git a/CHANGELOG b/CHANGELOG
index d682048ad1..204d0e2b09 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -1,3 +1,4 @@
+* Remove button if it is not allowed to access some of its dependant fields
 * Add support for Python 3.7
 * Add transactional queue and workers
 * Add __table_handler__ to ModelSQL
diff --git a/trytond/model/modelview.py b/trytond/model/modelview.py
index c417fee37a..0194add78c 100644
--- a/trytond/model/modelview.py
+++ b/trytond/model/modelview.py
@@ -409,22 +409,33 @@ class ModelView(Model):
                         fields_to_remove.add(name)
             checked |= to_check
 
-        # Remove field without read access
-        for field in fields_to_remove:
-            xpath = ('//field[@name="%(field)s"] | //label[@name="%(field)s"]'
-                ' | //page[@name="%(field)s"] | //group[@name="%(field)s"]'
-                ' | //separator[@name="%(field)s"]') % {'field': field}
-            for i, element in enumerate(tree.xpath(xpath)):
-                if type == 'tree' or element.tag == 'page':
-                    parent = element.getparent()
-                    parent.remove(element)
-                elif type == 'form':
-                    element.tag = 'label'
-                    colspan = element.attrib.get('colspan')
-                    element.attrib.clear()
-                    element.attrib['id'] = 'hidden %s-%s' % (field, i)
-                    if colspan is not None:
-                        element.attrib['colspan'] = colspan
+        buttons_to_remove = set()
+        for name, definition in cls._buttons.items():
+            if fields_to_remove & set(definition.get('depends', [])):
+                buttons_to_remove.add(name)
+
+        field_xpath = ('//field[@name="%(name)s"]'
+            '| //label[@name="%(name)s"] | //page[@name="%(name)s"]'
+            '| //group[@name="%(name)s"] | //separator[@name="%(name)s"]')
+        button_xpath = '//button[@name="%(name)s"]'
+        # Remove field and button without read acces
+        for xpath, names in (
+                (field_xpath, fields_to_remove),
+                (button_xpath, buttons_to_remove),
+                ):
+            for name in names:
+                path = xpath % {'name': name}
+                for i, element in enumerate(tree.xpath(path)):
+                    if type == 'tree' or element.tag == 'page':
+                        parent = element.getparent()
+                        parent.remove(element)
+                    elif type == 'form':
+                        element.tag = 'label'
+                        colspan = element.attrib.get('colspan')
+                        element.attrib.clear()
+                        element.attrib['id'] = 'hidden %s-%s' % (name, i)
+                        if colspan is not None:
+                            element.attrib['colspan'] = colspan
 
         # Remove empty pages
         if type == 'form':
diff --git a/trytond/tests/__init__.py b/trytond/tests/__init__.py
index 5263f6b7ee..fd213dabfc 100644
--- a/trytond/tests/__init__.py
+++ b/trytond/tests/__init__.py
@@ -51,6 +51,7 @@ def register():
         ModelViewChangedValues,
         ModelViewChangedValuesTarget,
         ModelViewButton,
+        ModelViewButtonDepends,
         ModelViewRPC,
         ModelViewEmptyPage,
         ModelViewCircularDepends,
diff --git a/trytond/tests/modelview.py b/trytond/tests/modelview.py
index d97b275f9b..529793708a 100644
--- a/trytond/tests/modelview.py
+++ b/trytond/tests/modelview.py
@@ -9,6 +9,7 @@ __all__ = [
     'ModelViewChangedValues',
     'ModelViewChangedValuesTarget',
     'ModelViewButton',
+    'ModelViewButtonDepends',
     'ModelViewRPC',
     'ModelViewEmptyPage',
     'ModelViewCircularDepends',
@@ -58,6 +59,26 @@ class ModelViewButton(ModelView):
         pass
 
 
+class ModelViewButtonDepends(ModelView):
+    'ModelView Button Depends'
+    __name__ = 'test.modelview.button_depends'
+    value = fields.Integer("Value")
+
+    @classmethod
+    def __setup__(cls):
+        super(ModelViewButtonDepends, cls).__setup__()
+        cls._buttons = {
+            'test': {
+                'depends': ['value'],
+                },
+            }
+
+    @classmethod
+    @ModelView.button
+    def test(cls, records):
+        pass
+
+
 class ModelViewRPC(ModelView):
     'ModelView RPC'
     __name__ = 'test.modelview.rpc'
diff --git a/trytond/tests/modelview.xml b/trytond/tests/modelview.xml
index b7060034e6..d4e3add2f7 100644
--- a/trytond/tests/modelview.xml
+++ b/trytond/tests/modelview.xml
@@ -16,4 +16,17 @@
             ]]>
         </field>
     </record>
+    <record model="ir.ui.view" id="test_modelview_button_view_form">
+        <field name="model">test.modelview.button_depends</field>
+        <field name="type">form</field>
+        <field name="arch" type="xml">
+            <![CDATA[
+            <form>
+                <label name="value"/>
+                <field name="value"/>
+                <button name="test"/>
+            </form>
+            ]]>
+        </field>
+    </record>
 </tryton>
diff --git a/trytond/tests/test_modelview.py b/trytond/tests/test_modelview.py
index dd6965ce81..ce66ca9091 100644
--- a/trytond/tests/test_modelview.py
+++ b/trytond/tests/test_modelview.py
@@ -350,6 +350,44 @@ class ModelView(unittest.TestCase):
 
         self.assertEqual(fields, {})
 
+    @with_transaction(context={'_check_access': True})
+    def test_button_depends_access(self):
+        "Testing buttons are not removed when dependant fields are accesible"
+        pool = Pool()
+        Button = pool.get('test.modelview.button_depends')
+
+        arch = Button.fields_view_get(view_type='form')['arch']
+        parser = etree.XMLParser()
+        tree = etree.fromstring(arch, parser=parser)
+        buttons = tree.xpath('//button')
+
+        self.assertEqual(len(buttons), 1)
+
+    @with_transaction(context={'_check_access': True})
+    def test_button_depends_no_access(self):
+        "Testing buttons are removed when dependant fields are not accesible"
+        pool = Pool()
+        Button = pool.get('test.modelview.button_depends')
+        Field = pool.get('ir.model.field')
+        FieldAccess = pool.get('ir.model.field.access')
+
+        field, = Field.search([
+                ('model.model', '=', Button.__name__),
+                ('name', '=', 'value'),
+                ])
+        FieldAccess.create([{
+            'field': field.id,
+            'group': None,
+            'perm_read': False,
+            }])
+
+        arch = Button.fields_view_get(view_type='form')['arch']
+        parser = etree.XMLParser()
+        tree = etree.fromstring(arch, parser=parser)
+        buttons = tree.xpath('//button')
+
+        self.assertEqual(len(buttons), 0)
+
 
 def suite():
     func = unittest.TestLoader().loadTestsFromTestCase