Raise error message with record name and field in check_xml_record

Closes #11867

modules/currency/currency.py

@@ -87,7 +87,7 @@ class Currency(
 
     @classmethod
     def check_xml_record(cls, records, values):
-        return True
+        pass
 
     @classmethod
     def search_global(cls, text):
@@ -287,7 +287,7 @@ class CurrencyRate(ModelSQL, ModelView):
 
     @classmethod
     def check_xml_record(cls, records, values):
-        return True
+        pass
 
     def get_rec_name(self, name):
         return str(self.date)

modules/party/configuration.py

@@ -121,7 +121,7 @@ class ConfigurationSequence(_ConfigurationValue, ModelSQL, ValueMixin):
 
     @classmethod
     def check_xml_record(cls, records, values):
-        return True
+        pass
 
 
 class ConfigurationLang(_ConfigurationValue, ModelSQL, ValueMixin):

modules/product/uom.py

@@ -74,7 +74,7 @@ class Uom(SymbolMixin, DigitsMixin, DeactivableMixin, ModelSQL, ModelView):
 
     @classmethod
     def check_xml_record(cls, records, values):
-        return True
+        pass
 
     @staticmethod
     def default_rate():

modules/stock/location.py

@@ -334,7 +334,7 @@ class Location(DeactivableMixin, tree(), ModelSQL, ModelView):
 
     @classmethod
     def check_xml_record(cls, records, values):
-        return True
+        pass
 
     def get_warehouse(self, name):
         # Order by descending left to get the first one in the tree

trytond/CHANGELOG

@@ -1,3 +1,4 @@
+* Raise error message with record name and field in check_xml_record
 * Cache RPC call for class method selection
 
 Version 6.6.0 - 2022-10-31

trytond/doc/ref/models.rst

@@ -442,9 +442,12 @@ Class methods:
 
 .. classmethod:: ModelStorage.check_xml_record(records, values)
 
-   Verify if the records are originating from XML data.
+   Raise an :exc:`~trytond.model.exceptions.AccessError` if the records can not
+   be modified because they originate from XML data.
+   ``values`` is a dictionary of written values or ``None`` for deletion.
 
-   It is used to prevent modification of data coming from XML files.
+   It is used by :meth:`~ModelStorage.write` and :meth:`~ModelStorage.delete`
+   to prevent modification of data coming from XML files.
 
    .. note::
       This method must be overiden to change this behavior.

trytond/trytond/ir/cron.py

@@ -105,9 +105,9 @@ class Cron(DeactivableMixin, ModelSQL, ModelView):
     def get_timezone(self, name):
         return tz.SERVER.key
 
-    @staticmethod
-    def check_xml_record(crons, values):
-        return True
+    @classmethod
+    def check_xml_record(cls, crons, values):
+        pass
 
     @classmethod
     def view_attributes(cls):

trytond/trytond/ir/lang.py

@@ -307,9 +307,9 @@ class Lang(DeactivableMixin, ModelSQL, ModelView):
                     gettext('ir.msg_language_default_translatable',
                         language=lang.rec_name))
 
-    @staticmethod
-    def check_xml_record(langs, values):
-        return True
+    @classmethod
+    def check_xml_record(cls, langs, values):
+        pass
 
     @classmethod
     def get_translatable_languages(cls):

trytond/trytond/ir/message.xml

@@ -137,10 +137,10 @@ this repository contains the full copyright notices and license terms. -->
             <field name="text">ID must be positive.</field>
         </record>
         <record model="ir.message" id="msg_write_xml_record">
-            <field name="text">You are not allowed to modify this record.</field>
+            <field name="text">You are not allowed to modify the field "%(field)s" in "%(record)s" of "%(model)s".</field>
         </record>
         <record model="ir.message" id="msg_delete_xml_record">
-            <field name="text">You are not allowed to delete this record.</field>
+            <field name="text">You are not allowed to delete the record "%(record)s" of "%(model)s".</field>
         </record>
         <record model="ir.message" id="msg_base_config_record">
             <field name="text">This record is part of the base configuration.</field>

trytond/trytond/ir/model.py

@@ -492,9 +492,9 @@ class ModelAccess(DeactivableMixin, ModelSQL, ModelView):
                 'get_access': RPC(),
                 })
 
-    @staticmethod
-    def check_xml_record(accesses, values):
-        return True
+    @classmethod
+    def check_xml_record(cls, accesses, values):
+        pass
 
     @staticmethod
     def default_perm_read():
@@ -701,9 +701,9 @@ class ModelFieldAccess(DeactivableMixin, ModelSQL, ModelView):
     description = fields.Text('Description')
     _get_access_cache = Cache('ir_model_field_access.check', context=False)
 
-    @staticmethod
-    def check_xml_record(field_accesses, values):
-        return True
+    @classmethod
+    def check_xml_record(cls, field_accesses, values):
+        pass
 
     @staticmethod
     def default_perm_read():

trytond/trytond/model/modelstorage.py

@@ -236,10 +236,7 @@ class ModelStorage(Model):
         all_records = []
         all_fields = set()
         for records, values in zip(actions, actions):
-            if not cls.check_xml_record(records, values):
-                raise AccessError(
-                    gettext('ir.msg_write_xml_record'),
-                    gettext('ir.msg_base_config_record'))
+            cls.check_xml_record(records, values)
             all_records += records
             all_fields.update(values.keys())
 
@@ -303,10 +300,7 @@ class ModelStorage(Model):
         ModelData = pool.get('ir.model.data')
 
         ModelAccess.check(cls.__name__, 'delete')
-        if not cls.check_xml_record(records, None):
-            raise AccessError(
-                gettext('ir.msg_delete_xml_record'),
-                gettext('ir.msg_base_config_record'))
+        cls.check_xml_record(records, None)
         if ModelData.has_model(cls.__name__):
             with Transaction().set_context(_check_access=False):
                 data = []
@@ -1124,40 +1118,39 @@ class ModelStorage(Model):
 
     @classmethod
     def check_xml_record(cls, records, values):
-        """
-        Check if a list of records and their corresponding fields are
-        originating from xml data. This is used by write and delete
-        functions: if the return value is True the records can be
-        written/deleted, False otherwise. The default behaviour is to
-        forbid any modification on records/fields originating from
-        xml. Values is the dictionary of written values. If values is
-        equal to None, no field by field check is performed, False is
-        returned as soon as one of the record comes from the xml.
-        """
         ModelData = Pool().get('ir.model.data')
         # Allow root user to update/delete
         if (Transaction().user == 0
                 or not ModelData.has_model(cls.__name__)):
-            return True
+            return
+        id2record = {r.id: r for r in records}
         with Transaction().set_context(_check_access=False):
             models_data = ModelData.search([
                 ('model', '=', cls.__name__),
-                ('db_id', 'in', list(map(int, records))),
+                ('db_id', 'in', list(id2record.keys())),
                 ])
             if not models_data:
-                return True
+                return
             for model_data in models_data:
+                record = id2record[model_data.db_id]
                 if values is None:
                     if not model_data.noupdate:
-                        return False
+                        raise AccessError(
+                            gettext(
+                                'ir.msg_delete_xml_record',
+                                **cls.__names__(record=record)),
+                            gettext('ir.msg_base_config_record'))
                 else:
                     if not model_data.values or model_data.noupdate:
                         continue
                     xml_values = ModelData.load_values(model_data.values)
                     for key, val in values.items():
                         if key in xml_values and val != xml_values[key]:
-                            return False
-        return True
+                            raise AccessError(
+                                gettext(
+                                    'ir.msg_write_xml_record',
+                                    **cls.__names__(field=key, record=record)),
+                                gettext('ir.msg_base_config_record'))
 
     @classmethod
     def validate(cls, records):

trytond/trytond/tests/test_modelstorage.py

@@ -515,9 +515,7 @@ class ModelStorageTestCase(unittest.TestCase):
         Model = pool.get('test.modelstorage')
         record, = Model.create([{}])
 
-        result = Model.check_xml_record([record], {'name': "Test"})
-
-        self.assertTrue(result)
+        Model.check_xml_record([record], {'name': "Test"})
 
     @with_transaction()
     def test_check_xml_record_with_record_no_matching_values(self):
@@ -535,9 +533,7 @@ class ModelStorageTestCase(unittest.TestCase):
                     'noupdate': False,
                     }])
 
-        result = Model.check_xml_record([record], {'name': "Test"})
-
-        self.assertTrue(result)
+        Model.check_xml_record([record], {'name': "Test"})
 
     @with_transaction()
     def test_check_xml_record_with_record_matching_values(self):
@@ -555,9 +551,8 @@ class ModelStorageTestCase(unittest.TestCase):
                     'noupdate': False,
                     }])
 
-        result = Model.check_xml_record([record], {'name': "Bar"})
-
-        self.assertFalse(result)
+        with self.assertRaises(AccessError):
+            Model.check_xml_record([record], {'name': "Bar"})
 
     @with_transaction()
     def test_check_xml_record_with_record_matching_values_noupdate(self):
@@ -575,9 +570,7 @@ class ModelStorageTestCase(unittest.TestCase):
                     'noupdate': True,
                     }])
 
-        result = Model.check_xml_record([record], {'name': "Bar"})
-
-        self.assertTrue(result)
+        Model.check_xml_record([record], {'name': "Bar"})
 
     @with_transaction(user=0)
     def test_check_xml_record_with_record_matching_values_root(self):
@@ -595,9 +588,7 @@ class ModelStorageTestCase(unittest.TestCase):
                     'noupdate': False,
                     }])
 
-        result = Model.check_xml_record([record], {'name': "Bar"})
-
-        self.assertTrue(result)
+        Model.check_xml_record([record], {'name': "Bar"})
 
     @with_transaction()
     def test_check_xml_record_with_record_no_values(self):
@@ -615,9 +606,8 @@ class ModelStorageTestCase(unittest.TestCase):
                     'noupdate': False,
                     }])
 
-        result = Model.check_xml_record([record], None)
-
-        self.assertFalse(result)
+        with self.assertRaises(AccessError):
+            Model.check_xml_record([record], None)
 
     @with_transaction()
     def test_check_xml_record_with_record_no_values_noupdate(self):
@@ -635,9 +625,7 @@ class ModelStorageTestCase(unittest.TestCase):
                     'noupdate': True,
                     }])
 
-        result = Model.check_xml_record([record], None)
-
-        self.assertTrue(result)
+        Model.check_xml_record([record], None)
 
     @with_transaction()
     def test_delete_clear_db_id_model_data_noupdate(self):