diff --git a/CHANGELOG b/CHANGELOG index b7dda5d9b6..dfd5fc39bc 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,3 +1,6 @@ +* Use a set for field's depends +* Relax the constraint on a field's depends +* Include only needed fields when fetching a view definition * Make warning check re-entrant * Manage DatabaseDataError in ModelSQL * Remove support for pysqlite2 diff --git a/doc/ref/fields.rst b/doc/ref/fields.rst index 69f27e72dd..5dd81ec206 100644 --- a/doc/ref/fields.rst +++ b/doc/ref/fields.rst @@ -112,13 +112,28 @@ All are optional except :attr:`~Field.string`. .. attribute:: Field.depends - A list of field names on which the field depends. + A :py:class:`set` of extra field names on which the field depends. This means that the client read also these fields even if they are not defined on the view. :attr:`Field.depends` is used for example to ensure that :class:`~trytond.pyson.PYSON` statement could be evaluated. +.. attribute:: Field.display_depends + + A computed set of field names on which the field depends when being + displayed in a read only view. + +.. attribute:: Field.edition_depends + + A computed set of field names on which the field depends when being + displayed in a writable view. + +.. attribute:: Field.validation_depends + + A computed set of field names on which the field depends when being + validated. + .. attribute:: Field.context A dictionary which updates the current context for *relation field*. diff --git a/trytond/ir/email_.py b/trytond/ir/email_.py index 71a87df672..b54114483d 100644 --- a/trytond/ir/email_.py +++ b/trytond/ir/email_.py @@ -332,7 +332,7 @@ class EmailTemplate(ModelSQL, ModelView): ], ] ] - field.depends.append('model') + field.depends.add('model') cls.__rpc__.update({ 'get': RPC(instantiate=0), 'get_default': RPC(), diff --git a/trytond/model/active.py b/trytond/model/active.py index 51808587cd..0e0b1e16ae 100644 --- a/trytond/model/active.py +++ b/trytond/model/active.py @@ -32,8 +32,6 @@ class DeactivableMixin(Model): field.states['readonly'] |= inactive else: field.states['readonly'] = inactive - if 'active' not in field.depends: - field.depends.append('active') if issubclass(cls, ModelView): for states in cls._buttons.values(): diff --git a/trytond/model/fields/date.py b/trytond/model/fields/date.py index 0876bb8d22..ad07758444 100644 --- a/trytond/model/fields/date.py +++ b/trytond/model/fields/date.py @@ -1,13 +1,14 @@ # This file is part of Tryton. The COPYRIGHT file at the top level of # this repository contains the full copyright notices and license terms. import datetime +from functools import cached_property from sql.functions import AtTimeZone, Function from trytond import backend -from trytond.pyson import PYSONEncoder +from trytond.pyson import PYSON, PYSONEncoder -from .field import Field +from .field import Field, get_eval_fields class SQLite_Date(Function): @@ -59,6 +60,20 @@ class FormatMixin(Field): definition['format'] = encoder.encode(self.format) return definition + @cached_property + def display_depends(self): + depends = super().display_depends + if isinstance(self.format, PYSON): + depends |= get_eval_fields(self.format) + return depends + + @cached_property + def validation_depends(self): + depends = super().display_depends + if isinstance(self.format, PYSON): + depends |= get_eval_fields(self.format) + return depends + class Timestamp(FormatMixin, Field): ''' diff --git a/trytond/model/fields/field.py b/trytond/model/fields/field.py index b1f34ce602..3ce4cbbdbf 100644 --- a/trytond/model/fields/field.py +++ b/trytond/model/fields/field.py @@ -16,6 +16,7 @@ from trytond.const import OPERATORS from trytond.pool import Pool from trytond.pyson import PYSON, Eval, PYSONDecoder, PYSONEncoder from trytond.rpc import RPC +from trytond.tools import cached_property from trytond.tools.string_ import LazyString, StringPartitioned from trytond.transaction import Transaction @@ -62,7 +63,7 @@ def states_validate(value): def depends_validate(value): - assert isinstance(value, list), 'depends must be a list' + assert isinstance(value, set), 'depends must be a set' def context_validate(value): @@ -244,7 +245,7 @@ class Field(object): :param on_change_with: A list of values. Like ``on_change``, but defined the other way around. The list contains all the fields that must update the current field. - :param depends: A list of field name on which this one depends. + :param depends: A set of field name on which this one depends. :param context: A dictionary which will be given to open the relation fields. :param loading: Define how the field must be loaded: @@ -274,7 +275,7 @@ class Field(object): DeprecationWarning, stacklevel=3) self.on_change_with.update(on_change_with) self.__depends = None - self.depends = depends or [] + self.depends = depends or set() self.__context = None self.context = context or {} assert loading in ('lazy', 'eager'), \ @@ -322,11 +323,30 @@ class Field(object): return self.__depends def _set_depends(self, value): + value = set(value) depends_validate(value) self.__depends = value depends = property(_get_depends, _set_depends) + @cached_property + def display_depends(self): + depends = get_eval_fields(self.states.get('invisible')) + return self.depends | depends + + @cached_property + def edition_depends(self): + depends = get_eval_fields(self.domain) + depends |= get_eval_fields(self.states.get('readonly')) + depends |= get_eval_fields(self.states.get('required')) + return self.depends | depends + + @cached_property + def validation_depends(self): + depends = get_eval_fields(self.domain) + depends |= get_eval_fields(self.states.get('required')) + return self.depends | depends + def _get_context(self): return self.__context diff --git a/trytond/model/fields/float.py b/trytond/model/fields/float.py index 979aeb54b6..f5e2709b57 100644 --- a/trytond/model/fields/float.py +++ b/trytond/model/fields/float.py @@ -1,8 +1,9 @@ # This file is part of Tryton. The COPYRIGHT file at the top level of # this repository contains the full copyright notices and license terms. from trytond.pyson import PYSON, PYSONEncoder +from trytond.tools import cached_property -from .field import Field +from .field import Field, get_eval_fields def digits_validate(value): @@ -18,6 +19,13 @@ def digits_validate(value): 'PYSON digits must return an integer' +def _get_digits_depends(field): + if isinstance(field.digits, str): + return {field.digits} + else: + return get_eval_fields(field.digits) + + class Float(Field): ''' Define a float field (``float``). @@ -63,3 +71,15 @@ class Float(Field): definition = super().definition(model, language) definition['digits'] = encoder.encode(self.digits) return definition + + @cached_property + def display_depends(self): + return super().display_depends | _get_digits_depends(self) + + @cached_property + def edition_depends(self): + return super().edition_depends | _get_digits_depends(self) + + @cached_property + def validation_depends(self): + return super().validation_depends | _get_digits_depends(self) diff --git a/trytond/model/fields/many2many.py b/trytond/model/fields/many2many.py index 26599f38ac..c02445aa16 100644 --- a/trytond/model/fields/many2many.py +++ b/trytond/model/fields/many2many.py @@ -9,12 +9,13 @@ from sql.functions import Position, Substring from trytond.pool import Pool from trytond.pyson import PYSONEncoder -from trytond.tools import grouped_slice +from trytond.tools import cached_property, grouped_slice from trytond.transaction import Transaction from .field import ( - Field, context_validate, domain_validate, instanciate_values, - instantiate_context, search_order_validate, size_validate) + Field, context_validate, domain_validate, get_eval_fields, + instanciate_values, instantiate_context, search_order_validate, + size_validate) from .function import Function @@ -109,6 +110,18 @@ class Many2Many(Field): def add_remove(self): return self.domain + @cached_property + def edition_depends(self): + depends = super().edition_depends + depends |= get_eval_fields(self.size) + return depends + + @cached_property + def validation_depends(self): + depends = super().validation_depends + depends |= get_eval_fields(self.size) + return depends + def sql_type(self): return None diff --git a/trytond/model/fields/one2many.py b/trytond/model/fields/one2many.py index fc5b7717fc..b9f13c03d1 100644 --- a/trytond/model/fields/one2many.py +++ b/trytond/model/fields/one2many.py @@ -9,12 +9,13 @@ from sql.functions import Position, Substring from trytond.pool import Pool from trytond.pyson import PYSONEncoder -from trytond.tools import grouped_slice +from trytond.tools import cached_property, grouped_slice from trytond.transaction import Transaction from .field import ( - Field, context_validate, domain_validate, instanciate_values, - instantiate_context, search_order_validate, size_validate) + Field, context_validate, domain_validate, get_eval_fields, + instanciate_values, instantiate_context, search_order_validate, + size_validate) from .function import Function @@ -107,6 +108,17 @@ class One2Many(Field): context_validate(value) self.__search_context = value + @cached_property + def edition_depends(self): + depends = super().edition_depends + for attribute in ['add_remove', 'size']: + depends |= get_eval_fields(getattr(self, attribute)) + return depends + + @cached_property + def validation_depends(self): + return super().validation_depends | get_eval_fields(self.size) + def sql_type(self): return None diff --git a/trytond/model/fields/reference.py b/trytond/model/fields/reference.py index 16c2700eba..ab60627ddb 100644 --- a/trytond/model/fields/reference.py +++ b/trytond/model/fields/reference.py @@ -9,10 +9,11 @@ from trytond.pool import Pool from trytond.pyson import PYSONEncoder from trytond.rpc import RPC from trytond.transaction import Transaction +from trytond.tools import cached_property from .field import ( - Field, context_validate, domain_validate, instantiate_context, - search_order_validate, with_inactive_records) + Field, context_validate, domain_validate, get_eval_fields, + instantiate_context, search_order_validate, with_inactive_records) from .selection import SelectionMixin @@ -39,11 +40,6 @@ class Reference(SelectionMixin, Field): :param search_order: The order to use when searching for a record :param search_context: The context to use when searching for a record ''' - if datetime_field: - if depends: - depends.append(datetime_field) - else: - depends = [datetime_field] super(Reference, self).__init__(string=string, help=help, required=required, readonly=readonly, domain=domain, states=states, select=select, on_change=on_change, on_change_with=on_change_with, @@ -101,6 +97,13 @@ class Reference(SelectionMixin, Field): context_validate(value) self.__search_context = value + @cached_property + def display_depends(self): + depends = super().display_depends + if self.datetime_field: + depends.add(self.datetime_field) + return depends + def set_rpc(self, model): super(Reference, self).set_rpc(model) if not isinstance(self.selection, (list, tuple)): diff --git a/trytond/model/modelstorage.py b/trytond/model/modelstorage.py index ac21b59cc0..94a39c1ed9 100644 --- a/trytond/model/modelstorage.py +++ b/trytond/model/modelstorage.py @@ -1320,11 +1320,10 @@ class ModelStorage(Model): ctx_pref['active_test'] = False with Transaction().set_context(ctx_pref): for field_name, field in cls._fields.items(): - depends = set(field.depends) if (field_names and field_name not in field_names - and not (depends & field_names) - and not (depends & function_fields)): + and not (field.validation_depends & field_names) + and not (field.validation_depends & function_fields)): continue if isinstance(field, fields.Function): continue @@ -1656,7 +1655,7 @@ class ModelStorage(Model): if field.context: eval_fields = fields.get_eval_fields(field.context) for context_field_name in eval_fields: - if context_field_name not in field.depends: + if context_field_name not in field.validation_depends: continue if context_field_name not in ffields: context_field = self._fields.get(context_field_name) diff --git a/trytond/model/modelview.py b/trytond/model/modelview.py index 45a89e600f..6dcd0bd61f 100644 --- a/trytond/model/modelview.py +++ b/trytond/model/modelview.py @@ -313,6 +313,12 @@ class ModelView(Model): ModelAccess = pool.get('ir.model.access') FieldAccess = pool.get('ir.model.field.access') + tree_root = tree.getroottree().getroot() + readonly_view = ( + tree_root.tag == 'board' + or (tree_root.tag == 'tree' + and not int(tree_root.attrib.get('editable', '0')))) + encoder = PYSONEncoder() if view_depends is None: view_depends = [] @@ -320,6 +326,8 @@ class ModelView(Model): view_depends = view_depends.copy() with Transaction().set_context(view_id=view_id): for xpath, attribute, value, *extra in cls.view_attributes(): + if readonly_view and attribute in {'required', 'readonly'}: + continue depends = [] if extra: depends, = extra @@ -426,13 +434,17 @@ class ModelView(Model): fields_def.setdefault(depend, {'name': depend}) field_names = list(fields_def.keys()) - while field_names: - field_name = field_names.pop() - if field_name in cls._fields: - field = cls._fields[field_name] - else: + + for field_name in field_names: + if field_name not in cls._fields: continue - for depend in field.depends: + field = cls._fields[field_name] + field_depends = field.display_depends.copy() + if not readonly_view: + field_depends |= field.edition_depends + if 'context' in field_depends and 'context' not in cls._fields: + field_depends.discard('context') + for depend in field_depends: if depend not in fields_def: fields_def[depend] = {'name': depend} field_names.append(depend) diff --git a/trytond/tests/modelview.py b/trytond/tests/modelview.py index c4f6861023..b8b7e05ce8 100644 --- a/trytond/tests/modelview.py +++ b/trytond/tests/modelview.py @@ -260,6 +260,21 @@ class ModelViewViewAttributesDepends(ModelView): ] +class ModelViewStatesDepends(ModelView): + "ModelView States Depends" + __name__ = 'test.modelview.states_depends' + + foo = fields.Char("Foo", + states={ + 'invisible': Eval('bar', True), + 'readonly': Eval('baz', True), + }, + depends=['quux']) + bar = fields.Boolean("Bar") + baz = fields.Boolean("Baz") + quux = fields.Char("Quux") + + def register(module): Pool.register( ModelViewChangedValues, @@ -280,4 +295,5 @@ def register(module): ModeViewDependsDepends, ModelViewViewAttributes, ModelViewViewAttributesDepends, + ModelViewStatesDepends, module=module, type_='model') diff --git a/trytond/tests/modelview.xml b/trytond/tests/modelview.xml index 8622fde747..b617430dc8 100644 --- a/trytond/tests/modelview.xml +++ b/trytond/tests/modelview.xml @@ -87,4 +87,28 @@ ]]> + + + test.modelview.states_depends + tree + + + + + ]]> + + + + test.modelview.states_depends + form + + + diff --git a/trytond/tests/test_modelview.py b/trytond/tests/test_modelview.py index a1edacd1be..2c9713d67e 100644 --- a/trytond/tests/test_modelview.py +++ b/trytond/tests/test_modelview.py @@ -668,6 +668,28 @@ class ModelView(unittest.TestCase): self.assertIn('bar', fields) + @with_transaction() + def test_states_depends_display(self): + "Test that fields used in readonly states are included" + pool = Pool() + Depends = pool.get('test.modelview.states_depends') + + readonly_fields = Depends.fields_view_get(view_type='tree')['fields'] + self.assertIn('bar', readonly_fields) + self.assertNotIn('baz', readonly_fields) + self.assertIn('quux', readonly_fields) + + @with_transaction() + def test_states_depends_edition(self): + "Test that fields used in editable states are included" + pool = Pool() + Depends = pool.get('test.modelview.states_depends') + + writeable_fields = Depends.fields_view_get(view_type='form')['fields'] + self.assertIn('bar', writeable_fields) + self.assertIn('baz', writeable_fields) + self.assertIn('quux', writeable_fields) + def suite(): func = unittest.TestLoader().loadTestsFromTestCase diff --git a/trytond/tests/test_tryton.py b/trytond/tests/test_tryton.py index 49de15f0a0..91f8c36b56 100644 --- a/trytond/tests/test_tryton.py +++ b/trytond/tests/test_tryton.py @@ -343,26 +343,9 @@ class ModuleTestCase(unittest.TestCase): if not isregisteredby(model, self.module): continue for fname, field in model._fields.items(): - fields = set() - fields |= get_eval_fields(field.domain) - if hasattr(field, 'digits'): - if isinstance(field.digits, str): - fields.add(field.digits) - else: - fields |= get_eval_fields(field.digits) - if hasattr(field, 'add_remove'): - fields |= get_eval_fields(field.add_remove) - if hasattr(field, 'size'): - fields |= get_eval_fields(field.size) - fields.discard(fname) - fields.discard('context') - fields.discard('active_model') - fields.discard('active_id') - depends = set(field.depends) + depends = { + f for f in field.depends if not f.startswith('_parent_')} with self.subTest(model=mname, field=fname): - self.assertLessEqual(fields, depends, - msg='Missing depends %s in "%s"."%s"' % ( - list(fields - depends), mname, fname)) self.assertLessEqual(depends, set(model._fields), msg='Unknown depends %s in "%s"."%s"' % ( list(depends - set(model._fields)), mname, fname))