diff --git a/CHANGELOG b/CHANGELOG index fd163501f1..6548d897a8 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,3 +1,4 @@ +* Replace _constraints with validate in ModelStorage * Add selection_change_with on Selection and Reference fields * Add Dict fields * Remove unique constraint on model and field access diff --git a/doc/ref/models/models.rst b/doc/ref/models/models.rst index 9a64c0f15f..eb67101b2d 100644 --- a/doc/ref/models/models.rst +++ b/doc/ref/models/models.rst @@ -211,6 +211,9 @@ Class attributes are: .. attribute:: ModelStorage._constraints + .. warning:: + Deprecated, use :class:`trytond.model.ModelStorage.validate` instead. + The list of constraints that each record must respect. The definition is: [ ('function name', 'error keyword'), ... ] @@ -327,6 +330,12 @@ CLass methods: Helper method that checks if there is no recursion in the tree composed with ``parent`` as parent field name. +.. classmethod:: ModelStorage.validate(records) + + Validate the integrity of records after creation and modification. This + method must be overridden to add validation and must raise an exception if + validation fails. + Instance methods: .. method:: ModelStorage.get_rec_name(name) diff --git a/trytond/ir/action.py b/trytond/ir/action.py index aca2aee96a..e14c88930d 100644 --- a/trytond/ir/action.py +++ b/trytond/ir/action.py @@ -116,12 +116,10 @@ class ActionKeyword(ModelSQL, ModelView): def __setup__(cls): super(ActionKeyword, cls).__setup__() cls.__rpc__.update({'get_keyword': RPC()}) - cls._constraints += [ - ('check_wizard_model', 'wrong_wizard_model'), - ] cls._error_messages.update({ - 'wrong_wizard_model': 'Wrong wizard model!', - }) + 'wrong_wizard_model': ('Wrong wizard model in keyword action ' + '"%s".'), + }) @classmethod def __register__(cls, module_name): @@ -137,6 +135,12 @@ class ActionKeyword(ModelSQL, ModelView): def search_groups(cls, name, clause): return [('action.groups',) + tuple(clause[1:])] + @classmethod + def validate(cls, actions): + super(ActionKeyword, cls).validate(actions) + for action in actions: + action.check_wizard_model() + def check_wizard_model(self): ActionWizard = Pool().get('ir.action.wizard') if self.action.type == 'ir.action.wizard': @@ -145,8 +149,8 @@ class ActionKeyword(ModelSQL, ModelView): ], limit=1) if action_wizard.model: if self.model.__name__ != action_wizard.model: - return False - return True + self.raise_user_error('wrong_wizard_model', ( + action_wizard.rec_name,)) @staticmethod def _convert_vals(vals): @@ -327,11 +331,8 @@ class ActionReport(ModelSQL, ModelView): ('report_name_module_uniq', 'UNIQUE(report_name, module)', 'The internal name must be unique by module!'), ] - cls._constraints += [ - ('check_email', 'invalid_email'), - ] cls._error_messages.update({ - 'invalid_email': 'Invalid email!', + 'invalid_email': 'Invalid email definition on report "%s".', }) @classmethod @@ -414,6 +415,11 @@ class ActionReport(ModelSQL, ModelView): def default_module(): return Transaction().context.get('module') or '' + @classmethod + def validate(cls, reports): + super(ActionReport, cls).validate(reports) + cls.check_email(reports) + @classmethod def check_email(cls, reports): "Check email" @@ -422,14 +428,14 @@ class ActionReport(ModelSQL, ModelView): try: value = safe_eval(report.email, CONTEXT) except Exception: - return False + value = None if isinstance(value, dict): inkeys = set(value) if not inkeys <= EMAIL_REFKEYS: - return False + cls.raise_user_error('invalid_email', ( + report.rec_name,)) else: - return False - return True + cls.raise_user_error('invalid_email', (report.rec_name,)) @classmethod def get_report_content(cls, reports, name): @@ -589,16 +595,14 @@ class ActionActWindow(ModelSQL, ModelView): @classmethod def __setup__(cls): super(ActionActWindow, cls).__setup__() - cls._constraints += [ - ('check_views', 'invalid_views'), - ('check_domain', 'invalid_domain'), - ('check_context', 'invalid_context'), - ] cls._error_messages.update({ - 'invalid_views': 'Invalid views!', - 'invalid_domain': 'Invalid domain or search criteria!', - 'invalid_context': 'Invalid context!', - }) + 'invalid_views': ('Invalid view "%(view)s" for action ' + '"%(action)s".'), + 'invalid_domain': ('Invalid domain or search criteria ' + '"%(domain)s" on action "%(action)s".'), + 'invalid_context': ('Invalid context "%(context)s" on action ' + '"%(action)s".'), + }) cls.__rpc__.update({ 'get': RPC(), }) @@ -638,6 +642,13 @@ class ActionActWindow(ModelSQL, ModelView): def default_search_value(): return '[]' + @classmethod + def validate(cls, actions): + super(ActionActWindow, cls).validate(actions) + cls.check_views(actions) + cls.check_domain(actions) + cls.check_context(actions) + @classmethod def check_views(cls, actions): "Check views" @@ -646,17 +657,28 @@ class ActionActWindow(ModelSQL, ModelView): for act_window_view in action.act_window_views: view = act_window_view.view if view.model != action.res_model: - return False + cls.raise_user_error('invalid_views', { + 'view': view.rec_name, + 'action': action.rec_name, + }) if view.type == 'board': - return False + cls.raise_user_error('invalid_views', { + 'view': view.rec_name, + 'action': action.rec_name, + }) else: for act_window_view in action.act_window_views: view = act_window_view.view if view.model: - return False + cls.raise_user_error('invalid_views', { + 'view': view.rec_name, + 'action': action.rec_name, + }) if view.type != 'board': - return False - return True + cls.raise_user_error('invalid_views', { + 'view': view.rec_name, + 'action': action.rec_name, + }) @classmethod def check_domain(cls, actions): @@ -668,18 +690,29 @@ class ActionActWindow(ModelSQL, ModelView): try: value = safe_eval(domain, CONTEXT) except Exception: - return False + cls.raise_user_error('invalid_domain', { + 'domain': domain, + 'action': action.rec_name, + }) if isinstance(value, PYSON): if not value.types() == set([list]): - return False + cls.raise_user_error('invalid_domain', { + 'domain': domain, + 'action': action.rec_name, + }) elif not isinstance(value, list): - return False + cls.raise_user_error('invalid_domain', { + 'domain': domain, + 'action': action.rec_name, + }) else: try: fields.domain_validate(value) except Exception: - return False - return True + cls.raise_user_error('invalid_domain', { + 'domain': domain, + 'action': action.rec_name, + }) @classmethod def check_context(cls, actions): @@ -689,18 +722,29 @@ class ActionActWindow(ModelSQL, ModelView): try: value = safe_eval(action.context, CONTEXT) except Exception: - return False + cls.raise_user_error('invalid_context', { + 'context': action.context, + 'action': action.rec_name, + }) if isinstance(value, PYSON): if not value.types() == set([dict]): - return False + cls.raise_user_error('invalid_context', { + 'context': action.context, + 'action': action.rec_name, + }) elif not isinstance(value, dict): - return False + cls.raise_user_error('invalid_context', { + 'context': action.context, + 'action': action.rec_name, + }) else: try: fields.context_validate(value) except Exception: - return False - return True + cls.raise_user_error('invalid_context', { + 'context': action.context, + 'action': action.rec_name, + }) def get_views(self, name): return [(view.view.id, view.view.type) diff --git a/trytond/ir/lang.py b/trytond/ir/lang.py index 0042712b9f..9a7728815c 100644 --- a/trytond/ir/lang.py +++ b/trytond/ir/lang.py @@ -45,23 +45,20 @@ class Lang(ModelSQL, ModelView): @classmethod def __setup__(cls): super(Lang, cls).__setup__() - cls._constraints += [ - ('check_grouping', 'invalid_grouping'), - ('check_date', 'invalid_date'), - ('check_translatable', 'default_translatable'), - ] cls._sql_constraints += [ ('check_decimal_point_thousands_sep', 'CHECK(decimal_point != thousands_sep)', 'decimal_point and thousands_sep must be different!'), - ] + ] cls._error_messages.update({ - 'invalid_grouping': 'Invalid Grouping!', - 'invalid_date': 'The date format is not valid!', - 'default_translatable': - 'The default language must be translatable', - 'delete_default': 'Default language can not be deleted', - }) + 'invalid_grouping': ('Invalid grouping "%(grouping)s" on ' + '"%(language)s" language.'), + 'invalid_date': ('Invalid date format "%(format)s" on ' + '"%(language)s" language.'), + 'default_translatable': ('The default language must be ' + 'translatable.'), + 'delete_default': ('Default language can not be deleted.'), + }) @classmethod def search_rec_name(cls, name, clause): @@ -121,6 +118,13 @@ class Lang(ModelSQL, ModelView): def default_thousands_sep(): return ',' + @classmethod + def validate(cls, languages): + super(Lang, cls).validate(languages) + cls.check_grouping(languages) + cls.check_date(languages) + cls.check_translatable(languages) + @classmethod def check_grouping(cls, langs): ''' @@ -131,10 +135,12 @@ class Lang(ModelSQL, ModelView): grouping = safe_eval(lang.grouping) for i in grouping: if not isinstance(i, int): - return False + raise except Exception: - return False - return True + cls.raise_user_error('invalid_grouping', { + 'grouping': lang.grouping, + 'language': lang.rec_name, + }) @classmethod def check_date(cls, langs): @@ -146,25 +152,27 @@ class Lang(ModelSQL, ModelView): datetime_strftime(datetime.datetime.now(), lang.date.encode('utf-8')) except Exception: - return False - if '%Y' not in lang.date: - return False - if '%b' not in lang.date \ - and '%B' not in lang.date \ - and '%m' not in lang.date \ - and '%-m' not in lang.date: - return False - if '%d' not in lang.date \ - and '%-d' not in lang.date \ - and '%j' not in lang.date \ - and '%-j' not in lang.date: - return False - if '%x' in lang.date \ - or '%X' in lang.date \ - or '%c' in lang.date \ - or '%Z' in lang.date: - return False - return True + cls.raise_user_error('invalid_date', { + 'format': lang.date, + 'language': lang.rec_name, + }) + if (('%Y' not in lang.date) + or ('%b' not in lang.date + and '%B' not in lang.date + and '%m' not in lang.date + and '%-m' not in lang.date) + or ('%d' not in lang.date + and '%-d' not in lang.date + and '%j' not in lang.date + and '%-j' not in lang.date) + or ('%x' in lang.date + or '%X' in lang.date + or '%c' in lang.date + or '%Z' in lang.date)): + cls.raise_user_error('invalid_date', { + 'format': lang.date, + 'language': lang.rec_name, + }) @classmethod def check_translatable(cls, langs): @@ -177,8 +185,7 @@ class Lang(ModelSQL, ModelView): for lang in langs: if (lang.code == Config.get_language() and not lang.translatable): - return False - return True + cls.raise_user_error('default_translatable') @staticmethod def check_xml_record(langs, values): diff --git a/trytond/ir/model.py b/trytond/ir/model.py index 21f3b294fc..19b3afb4ad 100644 --- a/trytond/ir/model.py +++ b/trytond/ir/model.py @@ -56,19 +56,22 @@ class Model(ModelSQL, ModelView): cls._sql_constraints += [ ('model_uniq', 'UNIQUE(model)', 'The model must be unique!'), - ] - cls._constraints += [ - ('check_module', 'invalid_module'), - ] + ] cls._error_messages.update({ - 'invalid_module': 'Module Name must be a python identifier!', - }) + 'invalid_module': ('Module name "%s" is not a valid python ' + 'identifier.'), + }) cls._order.insert(0, ('model', 'ASC')) cls.__rpc__.update({ 'list_models': RPC(), 'global_search': RPC(), }) + @classmethod + def validate(cls, models): + super(Model, cls).validate(models) + cls.check_module(models) + @classmethod def check_module(cls, models): ''' @@ -76,8 +79,7 @@ class Model(ModelSQL, ModelView): ''' for model in models: if model.module and not IDENTIFIER.match(model.module): - return False - return True + cls.raise_user_error('invalid_module', (module.rec_name,)) @classmethod def list_models(cls): @@ -200,13 +202,11 @@ class ModelField(ModelSQL, ModelView): cls._sql_constraints += [ ('name_model_uniq', 'UNIQUE(name, model)', 'The field name in model must be unique!'), - ] - cls._constraints += [ - ('check_name', 'invalid_name'), - ] + ] cls._error_messages.update({ - 'invalid_name': 'Model Field Name must be a python identifier!', - }) + 'invalid_name': ('Model Field name "%s" is not a valid python ' + 'identifier.'), + }) cls._order.insert(0, ('name', 'ASC')) @staticmethod @@ -217,6 +217,11 @@ class ModelField(ModelSQL, ModelView): def default_field_description(): return 'No description available' + @classmethod + def validate(cls, fields): + super(ModelField, cls).validate(fields) + cls.check_name(fields) + @classmethod def check_name(cls, fields): ''' @@ -224,8 +229,7 @@ class ModelField(ModelSQL, ModelView): ''' for field in fields: if not IDENTIFIER.match(field.name): - return False - return True + cls.raise_user_error('invalid_name', (field.name,)) @classmethod def read(cls, ids, fields_names=None): diff --git a/trytond/ir/rule.py b/trytond/ir/rule.py index 6940fb284c..496b1a59ec 100644 --- a/trytond/ir/rule.py +++ b/trytond/ir/rule.py @@ -105,11 +105,8 @@ class Rule(ModelSQL, ModelView): @classmethod def __setup__(cls): super(Rule, cls).__setup__() - cls._constraints += [ - ('check_domain', 'invalid_domain'), - ] cls._error_messages.update({ - 'invalid_domain': 'Invalid domain', + 'invalid_domain': 'Invalid domain in rule "%s".', }) @classmethod @@ -122,6 +119,11 @@ class Rule(ModelSQL, ModelView): table.not_null_action('operator', action='remove') table.not_null_action('operand', action='remove') + @classmethod + def validate(cls, rules): + super(Rule, cls).validate(rules) + cls.check_domain(rules) + @classmethod def check_domain(cls, rules): ctx = cls._get_context() @@ -131,13 +133,12 @@ class Rule(ModelSQL, ModelView): except Exception: return False if not isinstance(value, list): - return False + cls.raise_user_error('invalid_domain', (rule.rec_name,)) else: try: fields.domain_validate(value) except Exception: - return False - return True + cls.raise_user_error('invalid_domain', (rule.rec_name,)) @staticmethod def _get_context(): diff --git a/trytond/ir/sequence.py b/trytond/ir/sequence.py index 1bfff43bb2..b0ba873c54 100644 --- a/trytond/ir/sequence.py +++ b/trytond/ir/sequence.py @@ -83,19 +83,19 @@ class Sequence(ModelSQL, ModelView): @classmethod def __setup__(cls): super(Sequence, cls).__setup__() - cls._constraints += [ - ('check_prefix_suffix', 'invalid_prefix_suffix'), - ('check_last_timestamp', 'future_last_timestamp'), - ] cls._sql_constraints += [ ('check_timestamp_rounding', 'CHECK(timestamp_rounding > 0)', 'Timestamp rounding should be greater than 0'), ] cls._error_messages.update({ - 'missing': 'Missing sequence!', - 'invalid_prefix_suffix': 'Invalid prefix/suffix!', - 'future_last_timestamp': 'Last Timestamp could not be in future!', - }) + 'missing': 'Missing sequence.', + 'invalid_prefix': ('Invalid prefix "%(prefix)s" on sequence ' + '"%(sequence)s".'), + 'invalid_suffix': ('Invalid suffix "%(suffix)s" on sequence ' + '"%(sequence)s".'), + 'future_last_timestamp': ('Last Timestamp cannot be in the ' + 'future on sequence "%s".'), + }) @classmethod def __register__(cls, module_name): @@ -202,17 +202,27 @@ class Sequence(ModelSQL, ModelView): sequence_types = SequenceType.search([]) return [(x.code, x.name) for x in sequence_types] + @classmethod + def validate(cls, sequences): + super(Sequence, cls).validate(sequences) + cls.check_prefix_suffix(sequences) + cls.check_last_timestamp(sequences) + @classmethod def check_prefix_suffix(cls, sequences): "Check prefix and suffix" for sequence in sequences: - try: - cls._process(sequence.prefix) - cls._process(sequence.suffix) - except Exception: - return False - return True + for fix, error_message in ((sequence.prefix, 'invalid_prefix'), + (sequence.suffix, 'invalid_suffix')): + try: + cls._process(sequence.prefix) + cls._process(sequence.suffix) + except Exception: + cls.raise_user_error(error_message, { + 'prefix': fix, + 'sequence': sequence.rec_name, + }) @classmethod def check_last_timestamp(cls, sequences): @@ -221,8 +231,8 @@ class Sequence(ModelSQL, ModelView): for sequence in sequences: next_timestamp = cls._timestamp(sequence) if sequence.last_timestamp > next_timestamp: - return False - return True + cls.raise_user_error('future_last_timestamp', ( + sequence.rec_name,)) @property def _sql_sequence_name(self): diff --git a/trytond/ir/translation.py b/trytond/ir/translation.py index 74232826be..a8ad63c5a2 100644 --- a/trytond/ir/translation.py +++ b/trytond/ir/translation.py @@ -1069,6 +1069,7 @@ class TranslationClean(Wizard): 'search_function_missing', 'time_format_validation_record', 'selection_value_notfound', + 'model_recusion_error', ): return False if model_name in pool.object_name_list(): diff --git a/trytond/ir/translation.xml b/trytond/ir/translation.xml index dc82cf1b8d..c02556d00a 100644 --- a/trytond/ir/translation.xml +++ b/trytond/ir/translation.xml @@ -164,9 +164,9 @@ this repository contains the full copyright notices and license terms. --> access_error en_US error - You try to bypass an access rule! + You try to bypass an access rule. (Document type: %s) - You try to bypass an access rule! + You try to bypass an access rule. (Document type: %s) ir @@ -175,9 +175,9 @@ this repository contains the full copyright notices and license terms. --> read_error en_US error - You try to read records that don't exist anymore! + You try to read records that don't exist anymore. (Document type: %s) - You try to read records that don't exist anymore! + You try to read records that don't exist anymore. (Document type: %s) ir @@ -186,9 +186,9 @@ this repository contains the full copyright notices and license terms. --> write_error en_US error - You try to write on records that don't exist anymore! + You try to write on records that don't exist anymore. (Document type: %s) - You try to write on records that don't exist anymore! + You try to write on records that don't exist anymore. (Document type: %s) ir @@ -239,6 +239,15 @@ this repository contains the full copyright notices and license terms. --> ir + + recursion_error + en_US + error + Recursion error: Record "%(rec_name)s" with parent "%(parent_rec_name)s" was configured as ancestor of itself. + Recursion error: Record "%(rec_name)s" with parent "%(parent_rec_name)s" was configured as ancestor of itself. + ir + + ir.translation.set.start diff --git a/trytond/ir/trigger.py b/trytond/ir/trigger.py index 1c5b9ee922..8be1bf71d6 100644 --- a/trytond/ir/trigger.py +++ b/trytond/ir/trigger.py @@ -60,15 +60,18 @@ class Trigger(ModelSQL, ModelView): ('on_exclusive', 'CHECK(NOT(on_time AND (on_create OR on_write OR on_delete)))', '"On Time" and others are mutually exclusive!'), - ] - cls._constraints += [ - ('check_condition', 'invalid_condition'), - ] + ] cls._error_messages.update({ - 'invalid_condition': 'Condition must be a python expression!', - }) + 'invalid_condition': ('Condition "%(condition)s" is not a ' + 'valid python expression on trigger "%(trigger)s".'), + }) cls._order.insert(0, ('name', 'ASC')) + @classmethod + def validate(cls, triggers): + super(Trigger, cls).validate(triggers) + cls.check_condition(triggers) + @classmethod def check_condition(cls, triggers): ''' @@ -78,8 +81,10 @@ class Trigger(ModelSQL, ModelView): try: compile(trigger.condition, '', 'eval') except (SyntaxError, TypeError): - return False - return True + cls.raise_user_error('invalid_condition', { + 'condition': trigger.condition, + 'trigger': trigger.rec_name, + }) @staticmethod def default_active(): diff --git a/trytond/ir/ui/menu.py b/trytond/ir/ui/menu.py index 3f5b9fa575..4658c96421 100644 --- a/trytond/ir/ui/menu.py +++ b/trytond/ir/ui/menu.py @@ -96,14 +96,12 @@ class UIMenu(ModelSQL, ModelView): def __setup__(cls): super(UIMenu, cls).__setup__() cls._order.insert(0, ('sequence', 'ASC')) - cls._constraints += [ - ('check_recursion', 'recursive_menu'), - ('check_name', 'wrong_name'), - ] cls._error_messages.update({ - 'recursive_menu': 'You can not create recursive menu!', - 'wrong_name': 'You can not use "%s" in name field!' % SEPARATOR, - }) + 'wrong_name': ('"%%s" is not a valid menu name because it is ' + 'not allowed to contain "%s".' % SEPARATOR), + 'recursion_error': ('Recursion error: Menu entry "%s" was ' + 'configured as ancestor of itself.'), + }) @staticmethod def default_icon(): @@ -124,10 +122,16 @@ class UIMenu(ModelSQL, ModelView): return sorted(CLIENT_ICONS + [(name, name) for _, name in Icon.list_icons()]) + @classmethod + def validate(cls, menus): + super(UIMenu, cls).validate(menus) + cls.check_recursion(menus) + for menu in menus: + menu.check_name() + def check_name(self): if SEPARATOR in self.name: - return False - return True + self.raise_user_error('wrong_name', (self.name,)) def get_rec_name(self, name): if self.parent: diff --git a/trytond/ir/ui/view.py b/trytond/ir/ui/view.py index c150b895a3..45e6102af0 100644 --- a/trytond/ir/ui/view.py +++ b/trytond/ir/ui/view.py @@ -57,12 +57,9 @@ class View(ModelSQL, ModelView): @classmethod def __setup__(cls): super(View, cls).__setup__() - cls._constraints += [ - ('check_xml', 'invalid_xml'), - ] cls._error_messages.update({ - 'invalid_xml': 'Invalid XML for View!', - }) + 'invalid_xml': 'Invalid XML for view "%s".', + }) cls._order.insert(0, ('priority', 'ASC')) cls._buttons.update({ 'show': {}, @@ -108,6 +105,11 @@ class View(ModelSQL, ModelView): cls._get_rng_cache.set(key, rng) return rng + @classmethod + def validate(cls, views): + super(View, cls).validate(views) + cls.check_xml(views) + @classmethod def check_xml(cls, views): "Check XML" @@ -128,7 +130,7 @@ class View(ModelSQL, ModelView): validator.error_log.filter_from_errors()) logger.error('Invalid xml view:\n%s' % (str(error_log) + '\n' + xml)) - return False + cls.raise_user_error('invalid_xml', (view.rec_name,)) root_element = tree.getroottree().getroot() # validate pyson attributes @@ -159,7 +161,7 @@ class View(ModelSQL, ModelView): return False return True if not encode(root_element): - return False + cls.raise_user_error('invalid_xml', (view.rec_name,)) return True diff --git a/trytond/model/modelsql.py b/trytond/model/modelsql.py index 0117436018..fb183689c1 100644 --- a/trytond/model/modelsql.py +++ b/trytond/model/modelsql.py @@ -2213,8 +2213,8 @@ class ModelSQL(ModelStorage): 'WHERE ' + red_child_sql, red_child_ids) @classmethod - def _validate(cls, records): - super(ModelSQL, cls)._validate(records) + def validate(cls, records): + super(ModelSQL, cls).validate(records) cursor = Transaction().cursor if cursor.has_constraint(): return diff --git a/trytond/model/modelstorage.py b/trytond/model/modelstorage.py index 7d199e28dd..2c770a4b48 100644 --- a/trytond/model/modelstorage.py +++ b/trytond/model/modelstorage.py @@ -8,6 +8,7 @@ import contextlib import traceback import sys import csv +import warnings try: import cStringIO as StringIO except ImportError: @@ -828,7 +829,7 @@ class ModelStorage(Model): return True @classmethod - def check_recursion(cls, records, parent='parent'): + def check_recursion(cls, records, parent='parent', rec_name='rec_name'): ''' Function that checks if there is no recursion in the tree composed with parent as parent field name. @@ -856,19 +857,25 @@ class ModelStorage(Model): for walk in walker: walked.add(walk.id) if walk.id == record.id: - return False + cls.raise_user_error('recursion_error', { + 'rec_name': getattr(record, rec_name), + 'parent_rec_name': getattr(getattr(record, + parent), rec_name) + }) walker = list(chain(*(getattr(walk, parent) for walk in walker if walk.id not in visited))) else: walked.add(walker.id) if walker.id == record.id: - return False + cls.raise_user_error('recursion_error', { + 'rec_name': getattr(record, rec_name), + 'parent_rec_name': getattr(getattr(record, + parent), rec_name) + }) walker = (getattr(walker, parent) not in visited and getattr(walker, parent)) visited.update(walked) - return True - @classmethod def _get_error_args(cls, field_name): pool = Pool() @@ -885,6 +892,10 @@ class ModelStorage(Model): model_field.model.name) return error_args + @classmethod + def validate(cls, records): + pass + @classmethod def _validate(cls, records): pool = Pool() @@ -897,9 +908,6 @@ class ModelStorage(Model): with Transaction().set_user(0, set_context=True): records = cls.browse(records) - for record in records: - record.pre_validate() - def call(name): method = getattr(cls, name) if not hasattr(method, 'im_self') or method.im_self: @@ -907,6 +915,8 @@ class ModelStorage(Model): else: return all(method(r) for r in records) for field in cls._constraints: + warnings.warn('_constraints is deprecated, override validate instead', + DeprecationWarning, stacklevel=2) if not call(field[0]): cls.raise_user_error(field[1]) @@ -1141,6 +1151,12 @@ class ModelStorage(Model): format_test(getattr(record, field_name), field.format, field_name) + for record in records: + record.pre_validate() + + cls.validate(records) + + @classmethod def _clean_defaults(cls, defaults): pool = Pool() diff --git a/trytond/test/mptt.py b/trytond/test/mptt.py index bb244db8fa..f4b67baab2 100644 --- a/trytond/test/mptt.py +++ b/trytond/test/mptt.py @@ -26,12 +26,11 @@ class MPTT(ModelSQL, ModelView): def __setup__(cls): super(MPTT, cls).__setup__() cls._order.insert(0, ('sequence', 'ASC')) - cls._constraints += [ - ('check_recursion', 'recursive_mptt'), - ] - cls._error_messages.update({ - 'recursive_mptt': 'You can not create recursive Tree!', - }) + + @classmethod + def validate(cls, record): + super(MPTT, cls).validate(record) + cls.check_recursion(record) @staticmethod def default_active(): diff --git a/trytond/test/mptt.xml b/trytond/test/mptt.xml new file mode 100644 index 0000000000..23e3d6ae4b --- /dev/null +++ b/trytond/test/mptt.xml @@ -0,0 +1,16 @@ + + + + + + test.mptt + en_US + error + recursion_error + Recursion error: MPTT "%(rec_name)s" with parent "%(parent_rec_name)s" was configured as ancestor of itself. + test + + + + diff --git a/trytond/test/tryton.cfg b/trytond/test/tryton.cfg index 6bb32ecae3..492eb69127 100644 --- a/trytond/test/tryton.cfg +++ b/trytond/test/tryton.cfg @@ -7,3 +7,4 @@ xml: sequence.xml workflow.xml wizard.xml + mptt.xml diff --git a/trytond/webdav/webdav.py b/trytond/webdav/webdav.py index d9e5a79494..c6dc36cffd 100644 --- a/trytond/webdav/webdav.py +++ b/trytond/webdav/webdav.py @@ -55,16 +55,10 @@ class Collection(ModelSQL, ModelView): ('name_parent_uniq', 'UNIQUE (name, parent)', 'The collection name must be unique inside a collection!'), ] - cls._constraints += [ - ('check_recursion', 'recursive_collections'), - ('check_attachment', 'collection_file_name'), - ] cls._error_messages.update({ - 'recursive_collections': ('You can not create recursive ' - 'collections!'), - 'collection_file_name': ('You can not create a collection\n' - 'in a collection with the name of an ' - 'existing file!'), + 'collection_file_name': ('You can not create a collection ' + 'named "%(parent)s" in collection "%(child)s" because ' + 'there is already a file with that name.'), }) cls.ext2mime = { '.png': 'image/png', @@ -82,6 +76,12 @@ class Collection(ModelSQL, ModelView): else: return self.name + @classmethod + def validate(cls, collections): + super(Collection, cls).validate(collections) + cls.check_recursion(collections, rec_name='name') + cls.check_attachment(collections) + @classmethod def check_attachment(cls, collections): pool = Pool() @@ -94,8 +94,10 @@ class Collection(ModelSQL, ModelView): ]) for attachment in attachments: if attachment.name == collection.name: - return False - return True + cls.raise_user_error('collection_file_name', { + 'parent': collection.parent.rec_name, + 'child': collection.rec_name, + }) @classmethod def _uri2object(cls, uri, object_name=__name__, object_id=None, @@ -697,14 +699,17 @@ class Attachment(ModelSQL, ModelView): @classmethod def __setup__(cls): super(Attachment, cls).__setup__() - cls._constraints += [ - ('check_collection', 'collection_attachment_name'), - ] cls._error_messages.update({ - 'collection_attachment_name': ('You can not create an attachment\n' - 'in a collection with the name\n' - 'of an existing child collection!'), - }) + 'collection_attachment_name': ('You can not create an ' + 'attachment named "%(attachment)s in collection ' + '"%(collection)s" because there is already a collection ' + 'with that name.') + }) + + @classmethod + def validate(cls, attachments): + super(Attachment, cls).validate(attachments) + cls.check_collection(attachments) @classmethod def check_collection(cls, attachments): @@ -718,8 +723,11 @@ class Attachment(ModelSQL, ModelView): collection = Collection(int(record_id)) for child in collection.childs: if child.name == attachment.name: - return False - return True + cls.raise_user_error( + 'collection_attachment_name', { + 'attachment': attachment.rec_name, + 'collection': collection.rec_name, + }) @classmethod def get_path(cls, attachments, name): diff --git a/trytond/webdav/webdav.xml b/trytond/webdav/webdav.xml index 881ae6ec7b..b3d00d33c8 100644 --- a/trytond/webdav/webdav.xml +++ b/trytond/webdav/webdav.xml @@ -140,7 +140,17 @@ this repository contains the full copyright notices and license terms. --> form attachment_form - + + + + webdav.collection + en_US + error + recursion_error + Recursion error: WebDAV Collection "%(rec_name)s" with parent "%(parent_rec_name)s" was configured as ancestor of itself. + webdav + +