From 8d3d4c79ff7da4356d019868cfe8b8a6dd739954 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicolas=20=C3=89vrard?= Date: Wed, 21 Dec 2022 22:29:24 +0100 Subject: [PATCH] Do not validate domain on empty fields unless they're required --- sao/CHANGELOG | 1 + sao/src/model.js | 19 ++++++++++++++----- tryton/CHANGELOG | 1 + .../gui/window/view_form/model/field.py | 17 ++++++++++++----- trytond/CHANGELOG | 1 + trytond/doc/ref/fields.rst | 2 ++ trytond/trytond/model/modelstorage.py | 4 +++- trytond/trytond/tests/modelstorage.py | 9 +++++++++ trytond/trytond/tests/test_modelstorage.py | 16 ++++++++++++++++ 9 files changed, 59 insertions(+), 11 deletions(-) diff --git a/sao/CHANGELOG b/sao/CHANGELOG index 654863cd5a..e16c14b8b9 100644 --- a/sao/CHANGELOG +++ b/sao/CHANGELOG @@ -1,3 +1,4 @@ +* Do not validate domain and enforce uniqueness for empty fields unless they're required * Display the number of selected records on xxx2Many Version 6.6.0 - 2022-10-31 diff --git a/sao/src/model.js b/sao/src/model.js index 48c94aebbe..48f66effa3 100644 --- a/sao/src/model.js +++ b/sao/src/model.js @@ -1651,7 +1651,9 @@ return true; } var invalid = false; - this.get_state_attrs(record).domain_readonly = false; + var state_attrs = this.get_strate_attrs(record); + var is_required = Boolean(parseInt(state_attrs.required, 10)); + state_attrs.domain_readonly = false; var inversion = new Sao.common.DomainInversion(); var domain = inversion.simplify(this.validation_domains(record, pre_validate)); @@ -1667,11 +1669,19 @@ } else if (Sao.common.compare(domain, [['id', '=', null]])) { invalid = 'domain'; } else { + let [screen_domain, _] = this.get_domains( + record, pre_validate); var uniques = inversion.unique_value(domain); var unique = uniques[0]; var leftpart = uniques[1]; var value = uniques[2]; - if (unique) { + let unique_from_screen = inversion.unique_value( + screen_domain)[0]; + if (this._is_empty(record) && + !is_required && + !unique_from_screen) { + // Do nothing + } else if (unique) { // If the inverted domain is so constraint that only one // value is possible we should use it. But we must also pay // attention to the fact that the original domain might be @@ -1706,8 +1716,7 @@ } if (setdefault && !pre_validate) { this.set_client(record, value); - this.get_state_attrs(record).domain_readonly = - domain_readonly; + state_attrs.domain_readonly = domain_readonly; } } if (!inversion.eval_domain(domain, @@ -1715,7 +1724,7 @@ invalid = domain; } } - this.get_state_attrs(record).invalid = invalid; + state_attrs.invalid = invalid; return !invalid; } }); diff --git a/tryton/CHANGELOG b/tryton/CHANGELOG index b7c65a5eb8..9d39742752 100644 --- a/tryton/CHANGELOG +++ b/tryton/CHANGELOG @@ -1,3 +1,4 @@ +* Do not validate domain and enforce uniqueness for empty fields unless they're required * Display the number of selected records on xxx2Many Version 6.6.0 - 2022-10-31 diff --git a/tryton/tryton/gui/window/view_form/model/field.py b/tryton/tryton/gui/window/view_form/model/field.py index 99b124c8c5..d499ae6527 100644 --- a/tryton/tryton/gui/window/view_form/model/field.py +++ b/tryton/tryton/gui/window/view_form/model/field.py @@ -95,8 +95,10 @@ class Field(object): def validate(self, record, softvalidation=False, pre_validate=None): if self.attrs.get('readonly'): return True + state_attrs = self.get_state_attrs(record) + is_required = bool(int(state_attrs.get('required') or 0)) invalid = False - self.get_state_attrs(record)['domain_readonly'] = False + state_attrs['domain_readonly'] = False domain = simplify(self.validation_domains(record, pre_validate)) if not softvalidation: if not self.check_required(record): @@ -107,8 +109,14 @@ class Field(object): elif domain == [('id', '=', None)]: invalid = 'domain' else: + screen_domain, _ = self.domains_get(record, pre_validate) unique, leftpart, value = unique_value(domain) - if unique: + unique_from_screen, _, _ = unique_value(screen_domain) + if (self._is_empty(record) + and not is_required + and not unique_from_screen): + pass + elif unique: # If the inverted domain is so constraint that only one value # is possible we should use it. But we must also pay attention # to the fact that the original domain might be a 'OR' domain @@ -132,11 +140,10 @@ class Field(object): setdefault = False if setdefault and not pre_validate: self.set_client(record, value) - self.get_state_attrs(record)['domain_readonly'] = ( - domain_readonly) + state_attrs['domain_readonly'] = domain_readonly if not eval_domain(domain, EvalEnvironment(record)): invalid = domain - self.get_state_attrs(record)['invalid'] = invalid + state_attrs['invalid'] = invalid return not invalid def set(self, record, value): diff --git a/trytond/CHANGELOG b/trytond/CHANGELOG index 026ea959f9..0da7d120ba 100644 --- a/trytond/CHANGELOG +++ b/trytond/CHANGELOG @@ -1,3 +1,4 @@ +* Do not validate domain for empty fields * Warn about implicit save() of unsaved relation * Raise error message with record name and field in check_xml_record * Cache RPC call for class method selection diff --git a/trytond/doc/ref/fields.rst b/trytond/doc/ref/fields.rst index 0cfd87e6d1..dc480cef82 100644 --- a/trytond/doc/ref/fields.rst +++ b/trytond/doc/ref/fields.rst @@ -49,6 +49,8 @@ All are optional except :attr:`~Field.string`. A :ref:`domain ` constraint that is applied on the field value. + The domain is enforced unless the field value is ``None``. + .. note:: For :class:`Reference` field it is a dictionary that contains the domain diff --git a/trytond/trytond/model/modelstorage.py b/trytond/trytond/model/modelstorage.py index 0cf814540d..96d910bd33 100644 --- a/trytond/trytond/model/modelstorage.py +++ b/trytond/trytond/model/modelstorage.py @@ -1292,7 +1292,9 @@ class ModelStorage(Model): relations.update(*(getattr(r, field.name) for r in records)) else: # Cache alignment is not a problem - relations = set(records) + relations = { + r for r in records + if getattr(r, field.name) is not None} relations.discard(None) return relations diff --git a/trytond/trytond/tests/modelstorage.py b/trytond/trytond/tests/modelstorage.py index a3c5364f7d..d8ab39ab05 100644 --- a/trytond/trytond/tests/modelstorage.py +++ b/trytond/trytond/tests/modelstorage.py @@ -194,6 +194,14 @@ class ModelStorageEvalEnvironment(ModelStorage_): 'test.modelstorage.eval_environment', 'many2one', "One2Many") +class ModelStorageDomainNotRequired(ModelSQL): + "Model for domain on not required field" + __name__ = 'test.modelstorage.domain_not_required' + + domain_not_required = fields.Integer( + "Domain Not Required", domain=[('domain_not_required', '>', 0)]) + + def register(module): Pool.register( ModelStorage, @@ -213,4 +221,5 @@ def register(module): ModelStorageRelationDomain2, ModelStorageRelationDomain2Target, ModelStorageEvalEnvironment, + ModelStorageDomainNotRequired, module=module, type_='model') diff --git a/trytond/trytond/tests/test_modelstorage.py b/trytond/trytond/tests/test_modelstorage.py index 90c4159283..1a196bad01 100644 --- a/trytond/trytond/tests/test_modelstorage.py +++ b/trytond/trytond/tests/test_modelstorage.py @@ -581,6 +581,22 @@ class ModelStorageTestCase(unittest.TestCase): self.assertTrue( cm.exception.domain[1]['relation2']['relation_fields']['value']) + @with_transaction() + def test_domain_not_required_validation(self): + "Test domain validation on not required" + pool = Pool() + Model = pool.get('test.modelstorage.domain_not_required') + Model.create([{'domain_not_required': None}]) + + @with_transaction() + def test_domain_not_required_invalid(self): + "Test invalid domain on not required" + pool = Pool() + Model = pool.get('test.modelstorage.domain_not_required') + + with self.assertRaises(DomainValidationError): + Model.create([{'domain_not_required': 0}]) + @with_transaction() def test_check_xml_record_without_record(self): "Test check_xml_record without record"