From 61307c162d54ae5ce12cb5844b201513c7ccd604 Mon Sep 17 00:00:00 2001 From: FrancoisGe Date: Fri, 1 Sep 2023 09:43:06 +0200 Subject: [PATCH] [FIX] web: use evalContextWithVirtualIds for client expression Before this commit, all python expressions in js were evaluated with the same evalContext. Unfortunately, this is not possible for expressions based on an x2many. The value of the x2many depends on whether the expression will be sent to the server or not. All expressions that are only used client-side, such as modifiers (readonly, required, invisible), decorations, etc., must take virtual records into account. For example, I define a button that must be visible when I have at least one record in my x2many. So when I create my virtual record, I want the button to become visible and not have to wait for the record to be actually created. For expressions sent to the server, such as domains and contexts, we don't want them to take virtual records into account, as these are not known by the server and could cause crashes. Solution: We have evalContext for expressions sent to the server and evalContextWithVirtualIds for client-side expressions. closes odoo/odoo#133718 Related: odoo/enterprise#46604 Signed-off-by: Aaron Bohy (aab) --- .../views/list/list_renderer.js | 2 +- .../src/model/relational_model/record.js | 36 +++++++++---- .../calendar_common_popover.js | 2 +- addons/web/static/src/views/debug_items.js | 2 +- .../src/views/fields/badge/badge_field.js | 2 +- .../copy_clipboard/copy_clipboard_field.js | 7 ++- addons/web/static/src/views/fields/field.js | 30 ++++++----- .../views/fields/many2one/many2one_field.js | 2 +- .../static/src/views/form/form_compiler.js | 6 +-- .../static/src/views/list/list_renderer.js | 14 +++-- addons/web/static/src/views/view_compiler.js | 2 +- addons/web/static/src/views/widgets/widget.js | 6 ++- .../views/fields/one2many_field_tests.js | 52 +++++++++++++------ .../tests/views/form/form_compiler_tests.js | 6 +-- 14 files changed, 110 insertions(+), 59 deletions(-) diff --git a/addons/project/static/src/project_sharing/views/list/list_renderer.js b/addons/project/static/src/project_sharing/views/list/list_renderer.js index b584239ad8b..23bec9b3771 100644 --- a/addons/project/static/src/project_sharing/views/list/list_renderer.js +++ b/addons/project/static/src/project_sharing/views/list/list_renderer.js @@ -19,7 +19,7 @@ export class ProjectSharingListRenderer extends ListRenderer { const allColumns = []; const firstRecord = this.props.list.records[0]; for (const column of columns) { - if (evaluateBooleanExpr(column.column_invisible, firstRecord.evalContext)) { + if (evaluateBooleanExpr(column.column_invisible, firstRecord.evalContextWithVirtualIds)) { continue; } allColumns.push(column); diff --git a/addons/web/static/src/model/relational_model/record.js b/addons/web/static/src/model/relational_model/record.js index 3879db768c7..66503949143 100644 --- a/addons/web/static/src/model/relational_model/record.js +++ b/addons/web/static/src/model/relational_model/record.js @@ -46,8 +46,14 @@ export class Record extends DataPoint { return parentRecord.evalContext; }, }; + this.evalContextWithVirtualIds = { + get parent() { + return parentRecord.evalContextWithVirtualIds; + }, + }; } else { this.evalContext = {}; + this.evalContextWithVirtualIds = {}; } const missingFields = this.fieldNames.filter((fieldName) => !(fieldName in data)); data = { ...this._getDefaultValues(missingFields), ...data }; @@ -384,14 +390,21 @@ export class Record extends DataPoint { _computeDataContext() { const dataContext = {}; + const x2manyDataContext = { + withVirtualIds: {}, + withoutVirtualIds: {}, + }; const data = toRaw(this.data); for (const fieldName in data) { const value = data[fieldName]; const field = this.fields[fieldName]; if (["char", "text", "html"].includes(field.type)) { dataContext[fieldName] = this._textValues[fieldName]; - } else if (["one2many", "many2many"].includes(field.type)) { - dataContext[fieldName] = value.currentIds.filter((id) => typeof id === "number"); + } else if (field.type === "one2many" || field.type === "many2many") { + x2manyDataContext.withVirtualIds[fieldName] = value.currentIds; + x2manyDataContext.withoutVirtualIds[fieldName] = value.currentIds.filter( + (id) => typeof id === "number" + ); } else if (value && field.type === "date") { dataContext[fieldName] = serializeDate(value); } else if (value && field.type === "datetime") { @@ -409,7 +422,10 @@ export class Record extends DataPoint { } } dataContext.id = this.resId || false; - return dataContext; + return { + withVirtualIds: { ...dataContext, ...x2manyDataContext.withVirtualIds }, + withoutVirtualIds: { ...dataContext, ...x2manyDataContext.withoutVirtualIds }, + }; } _createStaticListDatapoint(data, fieldName) { @@ -558,17 +574,17 @@ export class Record extends DataPoint { _isInvisible(fieldName) { const invisible = this.activeFields[fieldName].invisible; - return invisible ? evaluateBooleanExpr(invisible, this.evalContext) : false; + return invisible ? evaluateBooleanExpr(invisible, this.evalContextWithVirtualIds) : false; } _isReadonly(fieldName) { const readonly = this.activeFields[fieldName].readonly; - return readonly ? evaluateBooleanExpr(readonly, this.evalContext) : false; + return readonly ? evaluateBooleanExpr(readonly, this.evalContextWithVirtualIds) : false; } _isRequired(fieldName) { const required = this.activeFields[fieldName].required; - return required ? evaluateBooleanExpr(required, this.evalContext) : false; + return required ? evaluateBooleanExpr(required, this.evalContextWithVirtualIds) : false; } async _load(nextConfig = {}) { @@ -871,14 +887,16 @@ export class Record extends DataPoint { * be uselessly re-rendered if we replace it by a brand new object. */ _setEvalContext() { - Object.assign(this.evalContext, { + const evalContext = { ...this.context, active_id: this.resId || false, active_ids: this.resId ? [this.resId] : [], active_model: this.resModel, current_company_id: this.model.company.currentCompany.id, - ...this._computeDataContext(), - }); + }; + const dataContext = this._computeDataContext(); + Object.assign(this.evalContext, evalContext, dataContext.withoutVirtualIds); + Object.assign(this.evalContextWithVirtualIds, evalContext, dataContext.withVirtualIds); for (const [fieldName, value] of Object.entries(toRaw(this.data))) { if ( diff --git a/addons/web/static/src/views/calendar/calendar_common/calendar_common_popover.js b/addons/web/static/src/views/calendar/calendar_common/calendar_common_popover.js index 6eaf44f708c..d0df770ae12 100644 --- a/addons/web/static/src/views/calendar/calendar_common/calendar_common_popover.js +++ b/addons/web/static/src/views/calendar/calendar_common/calendar_common_popover.js @@ -34,7 +34,7 @@ export class CalendarCommonPopover extends Component { } isInvisible(fieldNode, record) { - return evaluateBooleanExpr(fieldNode.invisible, record.evalContext); + return evaluateBooleanExpr(fieldNode.invisible, record.evalContextWithVirtualIds); } computeDateTimeAndDuration() { diff --git a/addons/web/static/src/views/debug_items.js b/addons/web/static/src/views/debug_items.js index 3e9c07635fc..e4eb00d1fd9 100644 --- a/addons/web/static/src/views/debug_items.js +++ b/addons/web/static/src/views/debug_items.js @@ -220,7 +220,7 @@ class SetDefaultDialog extends Component { const valueDisplayed = this.display(fieldInfo, this.fieldsValues[fieldName]); const value = valueDisplayed[0]; const displayed = valueDisplayed[1]; - const evalContext = this.props.record.evalContext; + const evalContext = this.props.record.evalContextWithVirtualIds; // ignore fields which are empty, invisible, readonly, o2m or m2m if ( !value || diff --git a/addons/web/static/src/views/fields/badge/badge_field.js b/addons/web/static/src/views/fields/badge/badge_field.js index 73448efc5ca..d388d785708 100644 --- a/addons/web/static/src/views/fields/badge/badge_field.js +++ b/addons/web/static/src/views/fields/badge/badge_field.js @@ -26,7 +26,7 @@ export class BadgeField extends Component { } get classFromDecoration() { - const evalContext = this.props.record.evalContext; + const evalContext = this.props.record.evalContextWithVirtualIds; for (const decorationName in this.props.decorations) { if (evaluateBooleanExpr(this.props.decorations[decorationName], evalContext)) { return `text-bg-${decorationName}`; diff --git a/addons/web/static/src/views/fields/copy_clipboard/copy_clipboard_field.js b/addons/web/static/src/views/fields/copy_clipboard/copy_clipboard_field.js index aeb22522ab1..dfccf430d8a 100644 --- a/addons/web/static/src/views/fields/copy_clipboard/copy_clipboard_field.js +++ b/addons/web/static/src/views/fields/copy_clipboard/copy_clipboard_field.js @@ -36,7 +36,12 @@ class CopyClipboardField extends Component { return this.props.record.fields[this.props.name].type; } get disabled() { - return this.props.disabledExpr ? evaluateBooleanExpr(this.props.disabledExpr, this.props.record.evalContext) : false; + return this.props.disabledExpr + ? evaluateBooleanExpr( + this.props.disabledExpr, + this.props.record.evalContextWithVirtualIds + ) + : false; } } diff --git a/addons/web/static/src/views/fields/field.js b/addons/web/static/src/views/fields/field.js index a906921530f..dec588ccf9c 100644 --- a/addons/web/static/src/views/fields/field.js +++ b/addons/web/static/src/views/fields/field.js @@ -5,11 +5,7 @@ import { evaluateExpr, evaluateBooleanExpr } from "@web/core/py_js/py"; import { registry } from "@web/core/registry"; import { utils } from "@web/core/ui/ui_service"; import { getFieldContext } from "@web/model/relational_model/utils"; -import { - archParseBoolean, - getClassNameFromDecoration, - X2M_TYPES, -} from "@web/views/utils"; +import { archParseBoolean, getClassNameFromDecoration, X2M_TYPES } from "@web/views/utils"; import { getTooltipInfo } from "./field_tooltip"; import { Component, xml } from "@odoo/owl"; @@ -43,8 +39,8 @@ export function getFieldFromRegistry(fieldType, widget, viewType, jsClass) { } export function fieldVisualFeedback(field, record, fieldName, fieldInfo) { - const readonly = evaluateBooleanExpr(fieldInfo.readonly, record.evalContext); - const required = evaluateBooleanExpr(fieldInfo.required, record.evalContext); + const readonly = evaluateBooleanExpr(fieldInfo.readonly, record.evalContextWithVirtualIds); + const required = evaluateBooleanExpr(fieldInfo.required, record.evalContextWithVirtualIds); const inEdit = record.isInEdition; let empty = !record.isNew; @@ -154,9 +150,11 @@ export class Field extends Component { // only handle the text-decoration. if (fieldInfo && fieldInfo.decorations) { const { decorations } = fieldInfo; - const evalContext = record.evalContext; for (const decoName in decorations) { - const value = evaluateBooleanExpr(decorations[decoName], evalContext); + const value = evaluateBooleanExpr( + decorations[decoName], + record.evalContextWithVirtualIds + ); classNames[getClassNameFromDecoration(decoName)] = value; } } @@ -174,9 +172,10 @@ export class Field extends Component { let propsFromNode = {}; if (this.props.fieldInfo) { - const evalContext = record.getEvalContext?.(false) || record.evalContext; let fieldInfo = this.props.fieldInfo; - readonly = readonly || evaluateBooleanExpr(fieldInfo.readonly, evalContext); + readonly = + readonly || + evaluateBooleanExpr(fieldInfo.readonly, record.evalContextWithVirtualIds); if (this.field.extractProps) { if (this.props.attrs) { @@ -191,7 +190,7 @@ export class Field extends Component { return getFieldContext(record, fieldInfo.name, fieldInfo.context); }, domain() { - const evalContext = record.getEvalContext?.(true) || record.evalContext; + const evalContext = record.evalContext; if (fieldInfo.domain) { return new Domain(evaluateExpr(fieldInfo.domain, evalContext)).toList(); } @@ -200,7 +199,10 @@ export class Field extends Component { ? new Domain(evaluateExpr(domain, evalContext)).toList() : domain || []; }, - required: evaluateBooleanExpr(fieldInfo.required, evalContext), + required: evaluateBooleanExpr( + fieldInfo.required, + record.evalContextWithVirtualIds + ), readonly: readonly, }; propsFromNode = this.field.extractProps(fieldInfo, dynamicInfo); @@ -296,7 +298,7 @@ Field.parseFieldNode = function (node, models, modelName, viewType, jsClass) { } } if (name === "id") { - fieldInfo.readonly = 'True'; + fieldInfo.readonly = "True"; } if (widget === "handle") { diff --git a/addons/web/static/src/views/fields/many2one/many2one_field.js b/addons/web/static/src/views/fields/many2one/many2one_field.js index 38a670e4325..aeb88a676fa 100644 --- a/addons/web/static/src/views/fields/many2one/many2one_field.js +++ b/addons/web/static/src/views/fields/many2one/many2one_field.js @@ -166,7 +166,7 @@ export class Many2OneField extends Component { return makeContext([context], evalContext); } get classFromDecoration() { - const evalContext = this.props.record.evalContext; + const evalContext = this.props.record.evalContextWithVirtualIds; for (const decorationName in this.props.decorations) { if (evaluateBooleanExpr(this.props.decorations[decorationName], evalContext)) { return `text-${decorationName}`; diff --git a/addons/web/static/src/views/form/form_compiler.js b/addons/web/static/src/views/form/form_compiler.js index 7156e3324c4..3a10ca21bb4 100644 --- a/addons/web/static/src/views/form/form_compiler.js +++ b/addons/web/static/src/views/form/form_compiler.js @@ -146,7 +146,7 @@ export class FormCompiler extends ViewCompiler { } else { isVisibleExpr = `!__comp__.evaluateBooleanExpr(${JSON.stringify( invisible - )},__comp__.props.record.evalContext)`; + )},__comp__.props.record.evalContextWithVirtualIds)`; } const mainSlot = createElement("t", { "t-set-slot": `slot_${slotId++}`, @@ -349,7 +349,7 @@ export class FormCompiler extends ViewCompiler { } else { isVisibleExpr = `!__comp__.evaluateBooleanExpr(${JSON.stringify( invisible - )},__comp__.props.record.evalContext)`; + )},__comp__.props.record.evalContextWithVirtualIds)`; } mainSlot.setAttribute("isVisible", isVisibleExpr); if (itemSpan > 0) { @@ -549,7 +549,7 @@ export class FormCompiler extends ViewCompiler { } else { isVisibleExpr = `!__comp__.evaluateBooleanExpr(${JSON.stringify( invisible - )},__comp__.props.record.evalContext)`; + )},__comp__.props.record.evalContextWithVirtualIds)`; } pageSlot.setAttribute("isVisible", isVisibleExpr); diff --git a/addons/web/static/src/views/list/list_renderer.js b/addons/web/static/src/views/list/list_renderer.js index ae3b99e0bd5..0da9c2e0f28 100644 --- a/addons/web/static/src/views/list/list_renderer.js +++ b/addons/web/static/src/views/list/list_renderer.js @@ -814,7 +814,9 @@ export class ListRenderer extends Component { getRowClass(record) { // classnames coming from decorations const classNames = this.props.archInfo.decorations - .filter((decoration) => evaluateBooleanExpr(decoration.condition, record.evalContext)) + .filter((decoration) => + evaluateBooleanExpr(decoration.condition, record.evalContextWithVirtualIds) + ) .map((decoration) => decoration.class); if (record.selected) { classNames.push("table-info"); @@ -858,7 +860,7 @@ export class ListRenderer extends Component { } const classNames = [...this.cellClassByColumn[column.id]]; if (column.type === "field") { - if (evaluateBooleanExpr(column.required, record.evalContext)) { + if (evaluateBooleanExpr(column.required, record.evalContextWithVirtualIds)) { classNames.push("o_required_modifier"); } if (record.isFieldInvalid(column.name)) { @@ -873,7 +875,9 @@ export class ListRenderer extends Component { // only handle the text-decoration. const { decorations } = column; for (const decoName in decorations) { - if (evaluateBooleanExpr(decorations[decoName], record.evalContext)) { + if ( + evaluateBooleanExpr(decorations[decoName], record.evalContextWithVirtualIds) + ) { classNames.push(getClassNameFromDecoration(decoName)); } } @@ -895,7 +899,7 @@ export class ListRenderer extends Component { return !!( this.isRecordReadonly(record) || (column.relatedPropertyField && record.selected && record.model.multiEdit) || - evaluateBooleanExpr(column.readonly, record.evalContext) + evaluateBooleanExpr(column.readonly, record.evalContextWithVirtualIds) ); } @@ -928,7 +932,7 @@ export class ListRenderer extends Component { } evalInvisible(invisible, record) { - return evaluateBooleanExpr(invisible, record.evalContext); + return evaluateBooleanExpr(invisible, record.evalContextWithVirtualIds); } evalColumnInvisible(columnInvisible) { diff --git a/addons/web/static/src/views/view_compiler.js b/addons/web/static/src/views/view_compiler.js index 83c8f73f071..b480b9920ee 100644 --- a/addons/web/static/src/views/view_compiler.js +++ b/addons/web/static/src/views/view_compiler.js @@ -234,7 +234,7 @@ export class ViewCompiler { const recordExpr = params.recordExpr || "__comp__.props.record"; let isVisileExpr = `!__comp__.evaluateBooleanExpr(${JSON.stringify( invisible - )},${recordExpr}.evalContext)`; + )},${recordExpr}.evalContextWithVirtualIds)`; if (compiled.hasAttribute("t-if")) { const formerTif = compiled.getAttribute("t-if"); isVisileExpr = `( ${formerTif} ) and ${isVisileExpr}`; diff --git a/addons/web/static/src/views/widgets/widget.js b/addons/web/static/src/views/widgets/widget.js index 63903d4ed62..3b6bdf67b06 100644 --- a/addons/web/static/src/views/widgets/widget.js +++ b/addons/web/static/src/views/widgets/widget.js @@ -38,13 +38,15 @@ export class Widget extends Component { } get widgetProps() { const record = this.props.record; - const evalContext = record.evalContext; let readonlyFromModifiers = false; let propsFromNode = {}; if (this.props.widgetInfo) { const widgetInfo = this.props.widgetInfo; - readonlyFromModifiers = evaluateBooleanExpr(widgetInfo.attrs.readonly, evalContext); + readonlyFromModifiers = evaluateBooleanExpr( + widgetInfo.attrs.readonly, + record.evalContextWithVirtualIds + ); propsFromNode = this.widget.extractProps ? this.widget.extractProps(widgetInfo) : {}; } diff --git a/addons/web/static/tests/views/fields/one2many_field_tests.js b/addons/web/static/tests/views/fields/one2many_field_tests.js index 728bc831100..fb9d47146a3 100644 --- a/addons/web/static/tests/views/fields/one2many_field_tests.js +++ b/addons/web/static/tests/views/fields/one2many_field_tests.js @@ -4361,14 +4361,7 @@ QUnit.module("Fields", (hooks) => { await clickSave(target); assert.containsNone(target, "tr.o_data_row"); - assert.verifySteps([ - "get_views", - "web_read", - "onchange", - "onchange", - "write", - "web_read", - ]); + assert.verifySteps(["get_views", "web_read", "onchange", "onchange", "write", "web_read"]); }); QUnit.test("discard O2M field with close button", async function (assert) { @@ -4823,14 +4816,7 @@ QUnit.module("Fields", (hooks) => { "9" ); - assert.verifySteps([ - "get_views", - "web_read", - "onchange", - "onchange", - "write", - "web_read", - ]); + assert.verifySteps(["get_views", "web_read", "onchange", "onchange", "write", "web_read"]); }); QUnit.test("editable o2m, pressing ESC discard current changes", async function (assert) { @@ -14064,4 +14050,38 @@ QUnit.module("Fields", (hooks) => { await clickSave(target.querySelector(".o_dialog")); await clickSave(target); }); + + QUnit.test("modifiers based on x2many", async function (assert) { + await makeView({ + type: "form", + resModel: "partner", + serverData, + arch: ` +
+ + + + + + + +