From f9be31007f563b96c5c9079ca05a422938104e28 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Krier?= Date: Mon, 26 Dec 2022 13:18:22 +0100 Subject: [PATCH] Raise error message with record name and field in check_xml_record Closes #11867 --- modules/currency/currency.py | 4 +-- modules/party/configuration.py | 2 +- modules/product/uom.py | 2 +- modules/stock/location.py | 2 +- trytond/CHANGELOG | 1 + trytond/doc/ref/models.rst | 7 ++-- trytond/trytond/ir/cron.py | 6 ++-- trytond/trytond/ir/lang.py | 6 ++-- trytond/trytond/ir/message.xml | 4 +-- trytond/trytond/ir/model.py | 12 +++---- trytond/trytond/model/modelstorage.py | 41 +++++++++------------- trytond/trytond/tests/test_modelstorage.py | 30 +++++----------- 12 files changed, 51 insertions(+), 66 deletions(-) diff --git a/modules/currency/currency.py b/modules/currency/currency.py index f4aa67d27d..97239871ec 100644 --- a/modules/currency/currency.py +++ b/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) diff --git a/modules/party/configuration.py b/modules/party/configuration.py index 0906209dde..f5c9337db8 100644 --- a/modules/party/configuration.py +++ b/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): diff --git a/modules/product/uom.py b/modules/product/uom.py index 4a9fc7aaab..4b3b9fbaa6 100644 --- a/modules/product/uom.py +++ b/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(): diff --git a/modules/stock/location.py b/modules/stock/location.py index 2439190722..c84acf6746 100644 --- a/modules/stock/location.py +++ b/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 diff --git a/trytond/CHANGELOG b/trytond/CHANGELOG index fa365d2664..35c4673465 100644 --- a/trytond/CHANGELOG +++ b/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 diff --git a/trytond/doc/ref/models.rst b/trytond/doc/ref/models.rst index 0b77c55de9..86ad287a36 100644 --- a/trytond/doc/ref/models.rst +++ b/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. diff --git a/trytond/trytond/ir/cron.py b/trytond/trytond/ir/cron.py index 1ca3e0e139..200c364b7b 100644 --- a/trytond/trytond/ir/cron.py +++ b/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): diff --git a/trytond/trytond/ir/lang.py b/trytond/trytond/ir/lang.py index b970ffc023..33c02e9e7c 100644 --- a/trytond/trytond/ir/lang.py +++ b/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): diff --git a/trytond/trytond/ir/message.xml b/trytond/trytond/ir/message.xml index 6eaa457c69..b217b73574 100644 --- a/trytond/trytond/ir/message.xml +++ b/trytond/trytond/ir/message.xml @@ -137,10 +137,10 @@ this repository contains the full copyright notices and license terms. --> ID must be positive. - You are not allowed to modify this record. + You are not allowed to modify the field "%(field)s" in "%(record)s" of "%(model)s". - You are not allowed to delete this record. + You are not allowed to delete the record "%(record)s" of "%(model)s". This record is part of the base configuration. diff --git a/trytond/trytond/ir/model.py b/trytond/trytond/ir/model.py index 3ff644f51e..32e6d89749 100644 --- a/trytond/trytond/ir/model.py +++ b/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(): diff --git a/trytond/trytond/model/modelstorage.py b/trytond/trytond/model/modelstorage.py index 4e1c1d42a0..419c4fc4f4 100644 --- a/trytond/trytond/model/modelstorage.py +++ b/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): diff --git a/trytond/trytond/tests/test_modelstorage.py b/trytond/trytond/tests/test_modelstorage.py index bdd22d3cb8..1714ef5174 100644 --- a/trytond/trytond/tests/test_modelstorage.py +++ b/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):