From df08cacb38624d2a9f756178bacdfed9e3f7bb47 Mon Sep 17 00:00:00 2001 From: Denis Ledoux Date: Tue, 13 Sep 2022 09:59:57 +0000 Subject: [PATCH] [FIX] base: evaluate context dependent modifiers after cache A feature allows to set the modifiers invisible, readonly, required according to a key in the context e.g. ```xml ``` Following odoo/odoo#99417, back-end views are now cached. These expressions are currently evaluated server-side, before serving the view to the web client. Hence, the modifier becomes for instance `invisible="1"` or `invisible="0"` according to the context passed when calling `get_view`. The key used to cache the views doesn't take into account such context keys. Hence, when you asked for a view using for instance `invisible="context.get('quotation_only')"` A first time using the context `{'quotation_only': True}` and a second time using the context `{'quotation_only': False}`, on the second time, you received the view from the first time you requested the view, where the modifier is evaluated as if the the context was `{'quotation_only': True}`. As we do not want to store a cached version of the view for each possible key in the context, postprocess the evaluation of the modifiers using the context after retrieving the view from the cache. A better alternative would be to delegate this evaluation to the web client, because it already has the information it needs, it has the context value. Modifiers using domains are already evaluated client-side, it would make sense modifiers using context would too. Nevertheless, as this bug has been introduced by odoo/odoo#99417, and as we are close to the release, solve this server-side, to keep a similar behavior than before. We might re-consider the implementation of this later on. closes odoo/odoo#100130 Signed-off-by: Denis Ledoux (dle) --- odoo/addons/base/models/ir_ui_view.py | 73 ++++++++++++++++++--------- odoo/addons/base/tests/test_views.py | 26 ++++++++++ odoo/models.py | 1 + 3 files changed, 77 insertions(+), 23 deletions(-) diff --git a/odoo/addons/base/models/ir_ui_view.py b/odoo/addons/base/models/ir_ui_view.py index 22ae309fac8..4f62326680c 100644 --- a/odoo/addons/base/models/ir_ui_view.py +++ b/odoo/addons/base/models/ir_ui_view.py @@ -23,7 +23,7 @@ from odoo import api, fields, models, tools, _ from odoo.exceptions import ValidationError, AccessError from odoo.http import request from odoo.modules.module import get_resource_from_path, get_resource_path -from odoo.tools import config, ConstantMapping, get_diff, pycompat, apply_inheritance_specs, locate_node +from odoo.tools import config, ConstantMapping, get_diff, pycompat, apply_inheritance_specs, locate_node, str2bool from odoo.tools.convert import _fix_multiple_roots from odoo.tools import safe_eval, lazy, lazy_property, frozendict from odoo.tools.view_validation import valid_view, get_variable_names, get_domain_identifiers, get_dict_asts @@ -53,20 +53,6 @@ ref_re = re.compile(r""" """, re.VERBOSE) -def quick_eval(expr, globals_dict): - """ Functionally identical to safe_eval(), but optimized with special-casing. """ - # most (~95%) elements are 1/True/0/False - if expr == '1': - return 1 - if expr == 'True': - return True - if expr == '0': - return 0 - if expr == 'False': - return False - return safe_eval.safe_eval(expr, globals_dict) - - def att_names(name): yield name yield f"t-att-{name}" @@ -91,10 +77,8 @@ def transfer_field_to_modifiers(field, modifiers): modifiers[attr] = default_value -def transfer_node_to_modifiers(node, modifiers, context=None): +def transfer_node_to_modifiers(node, modifiers): # Don't deal with groups, it is done by check_group(). - # Need the context to evaluate the invisible attribute on tree views. - # For non-tree views, the context shouldn't be given. attrs = node.attrib.pop('attrs', None) if attrs: modifiers.update(ast.literal_eval(attrs.strip())) @@ -108,21 +92,37 @@ def transfer_node_to_modifiers(node, modifiers, context=None): else: modifiers['invisible'] = [('state', 'not in', states)] + context_dependent_modifiers = [] for attr in ('invisible', 'readonly', 'required'): value_str = node.attrib.pop(attr, None) if value_str: - value = bool(quick_eval(value_str, {'context': context or {}})) + if (attr == 'invisible' and any(parent.tag == 'tree' for parent in node.iterancestors()) and not any(parent.tag == 'header' for parent in node.iterancestors())): # Invisible in a tree view has a specific meaning, make it a # new key in the modifiers attribute. - modifiers['column_invisible'] = value - elif value or (attr not in modifiers or not isinstance(modifiers[attr], list)): + attr = 'column_invisible' + + # TODO: for invisible="context.get('...')", delegate to the web client. + try: + # most (~95%) elements are 1/True/0/False + value = str2bool(value_str) + except ValueError: + # if str2bool fails, it means it's something else than 1/True/0/False, + # meaning most-likely `context.get('...')`, + # which should be evaluated after retrieving the view arch from the cache + value = value_str + context_dependent_modifiers.append(attr) + + if value or (attr not in modifiers or not isinstance(modifiers[attr], list)): # Don't set the attribute to False if a dynamic value was # provided (i.e. a domain from attrs or states). modifiers[attr] = value + if context_dependent_modifiers: + node.set('context-dependent-modifiers', ','.join(context_dependent_modifiers)) + def simplify_modifiers(modifiers): for a in ('column_invisible', 'invisible', 'readonly', 'required'): @@ -1076,6 +1076,29 @@ actual arch. return tree + def _postprocess_context_dependent(self, tree): + """ + Evaluate the modifiers which depends on the context after retrieving the view from the cache. + + e.g. + + """ + for node in tree.xpath('//*[@context-dependent-modifiers]'): + modifiers = json.loads(node.attrib.pop('modifiers')) + for attr in node.attrib.pop('context-dependent-modifiers').split(','): + modifiers[attr] = bool(safe_eval.safe_eval(modifiers[attr], {'context': self._context})) + transfer_modifiers_to_node(modifiers, node) + return tree + def _postprocess_view(self, node, model_name, editable=True, parent_name_manager=None, **options): """ Process the given architecture, modifying it in-place to add and remove stuff. @@ -1119,7 +1142,7 @@ actual arch. # the node has been removed, stop processing here continue - transfer_node_to_modifiers(node, node_info['modifiers'], self._context) + transfer_node_to_modifiers(node, node_info['modifiers']) transfer_modifiers_to_node(node_info['modifiers'], node) # if present, iterate on node_info['children'] instead of node @@ -1458,7 +1481,11 @@ actual arch. for attribute in ('invisible', 'readonly', 'required'): val = node.get(attribute) if val: - res = quick_eval(val, {'context': self._context}) + try: + # most (~95%) elements are 1/True/0/False + res = str2bool(val) + except ValueError: + res = safe_eval.safe_eval(val, {'context': self._context}) if res not in (1, 0, True, False, None): msg = _( 'Attribute %(attribute)s evaluation expects a boolean, got %(value)s', diff --git a/odoo/addons/base/tests/test_views.py b/odoo/addons/base/tests/test_views.py index 5e07bd81754..8574b3e4b18 100644 --- a/odoo/addons/base/tests/test_views.py +++ b/odoo/addons/base/tests/test_views.py @@ -1,6 +1,7 @@ # -*- coding: utf-8 -*- # Part of Odoo. See LICENSE file for full copyright and licensing details. import ast +import json import logging import time @@ -2187,6 +2188,31 @@ class TestViews(ViewCase): "Attribute readonly evaluation expects a boolean, got [('model', '=', '1')]", ) + def test_modifier_attribute_using_context(self): + view = self.assertValid(""" +
+ + + """) + + for context, expected in [ + ({}, {}), + ({'foo': True}, {'invisible': True}), + ({'bar': True}, {'readonly': True}), + ({'baz': True}, {'required': True}), + ({'foo': True, 'bar': True}, {'invisible': True, 'readonly': True}), + ]: + arch = self.View.with_context(**context).get_view(view.id)['arch'] + field_node = etree.fromstring(arch).xpath('//field[@name="name"]')[0] + modifiers = json.loads(field_node.get('modifiers') or '{}') + self.assertEqual(modifiers.get('invisible'), expected.get('invisible')) + self.assertEqual(modifiers.get('readonly'), expected.get('readonly')) + self.assertEqual(modifiers.get('required'), expected.get('required')) + @mute_logger('odoo.addons.base.models.ir_ui_view') def test_domain_in_filter(self): arch = """ diff --git a/odoo/models.py b/odoo/models.py index 85e7ff960df..55e7227249b 100644 --- a/odoo/models.py +++ b/odoo/models.py @@ -1832,6 +1832,7 @@ class BaseModel(metaclass=MetaModel): node = etree.fromstring(result['arch']) node = self.env['ir.ui.view']._postprocess_access_rights(node) + node = self.env['ir.ui.view']._postprocess_context_dependent(node) result['arch'] = etree.tostring(node, encoding="unicode").replace('\t', '') return result