From 0ff280ec6dfb6a3c3000859cf42f80ebd5b2d9cd Mon Sep 17 00:00:00 2001 From: Mathieu Duckerts-Antoine Date: Thu, 26 Oct 2023 11:22:23 +0200 Subject: [PATCH] [IMP] web: ExpressionEditor: better conversion of in/not in operators MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When foo is not an x2many, a condition of the form ("foo", "in", []) was transformed by the ExpressionEditor into "set([foo]).intersection([])" while it can be better expressed as "foo in []" In this commit, we improve the conversion of conditions of that kind and similar other conditions. Note that we also ideally want domains and their corresponding expressions to be evaluated the same ways on records (at least on good examples). For this we want for instance, the condition ("foo", "in", 1) to be translated to "foo in [1]" and not "foo in 1" which is an invalid Python expression. closes odoo/odoo#140140 Signed-off-by: Rémy Voet (ryv) --- .../src/core/tree_editor/condition_tree.js | 17 ++-- .../static/tests/core/condition_tree_tests.js | 93 ++++++++++++++++--- 2 files changed, 89 insertions(+), 21 deletions(-) diff --git a/addons/web/static/src/core/tree_editor/condition_tree.js b/addons/web/static/src/core/tree_editor/condition_tree.js index d2274334a5a..37b18c15097 100644 --- a/addons/web/static/src/core/tree_editor/condition_tree.js +++ b/addons/web/static/src/core/tree_editor/condition_tree.js @@ -639,13 +639,7 @@ function _expressionFromTree(tree, options, isRoot = false) { return formatAST(operator === "=" ? not(pathAST) : pathAST); } - if ( - pathAST.type === 5 && - isValidPath(pathAST, options) && - ["in", "not in"].includes(operator) - ) { - const setIteratorAST = isX2Many(pathAST, options) ? pathAST : { type: 4, value: [pathAST] }; - + if (pathAST.type === 5 && isX2Many(pathAST, options) && ["in", "not in"].includes(operator)) { const valueAST = toAST(value); const otherIteratorAST = [4, 10].includes(valueAST.type) ? valueAST @@ -656,7 +650,7 @@ function _expressionFromTree(tree, options, isRoot = false) { fn: { type: 15, obj: { - args: [setIteratorAST], + args: [pathAST], type: 8, fn: { type: 5, @@ -670,13 +664,18 @@ function _expressionFromTree(tree, options, isRoot = false) { return formatAST(operator === "not in" ? not(ast) : ast); } + let valueAST = toAST(value); + if (["in", "not in"].includes(operator) && ![4, 10].includes(valueAST.type)) { + valueAST = { type: 4, value: [valueAST] }; + } + // add case true for boolean fields return formatAST({ type: 7, op, left: pathAST, - right: toAST(value), + right: valueAST, }); } diff --git a/addons/web/static/tests/core/condition_tree_tests.js b/addons/web/static/tests/core/condition_tree_tests.js index d46d9788152..484f9fdea74 100644 --- a/addons/web/static/tests/core/condition_tree_tests.js +++ b/addons/web/static/tests/core/condition_tree_tests.js @@ -1,5 +1,7 @@ /** @odoo-module **/ +import { Domain } from "@web/core/domain"; +import { evaluateExpr } from "@web/core/py_js/py"; import { complexCondition, condition, @@ -300,7 +302,7 @@ QUnit.test("expressionFromTree", function (assert) { }, { expressionTree: condition("foo", "in", []), - result: `set([foo]).intersection([])`, + result: `foo in []`, }, { expressionTree: condition(expression("expr"), "in", []), @@ -308,11 +310,11 @@ QUnit.test("expressionFromTree", function (assert) { }, { expressionTree: condition("foo", "in", [1]), - result: `set([foo]).intersection([1])`, + result: `foo in [1]`, }, { expressionTree: condition("foo", "in", 1), - result: `set([foo]).intersection([1])`, + result: `foo in [1]`, }, { expressionTree: condition("y", "in", []), @@ -324,7 +326,7 @@ QUnit.test("expressionFromTree", function (assert) { }, { expressionTree: condition("y", "in", 1), - result: `"y" in 1`, + result: `"y" in [1]`, }, { expressionTree: condition("foo_ids", "not in", []), @@ -340,15 +342,15 @@ QUnit.test("expressionFromTree", function (assert) { }, { expressionTree: condition("foo", "not in", []), - result: `not set([foo]).intersection([])`, + result: `foo not in []`, }, { expressionTree: condition("foo", "not in", [1]), - result: `not set([foo]).intersection([1])`, + result: `foo not in [1]`, }, { expressionTree: condition("foo", "not in", 1), - result: `not set([foo]).intersection([1])`, + result: `foo not in [1]`, }, { expressionTree: condition("y", "not in", []), @@ -360,7 +362,7 @@ QUnit.test("expressionFromTree", function (assert) { }, { expressionTree: condition("y", "not in", 1), - result: `"y" not in 1`, + result: `"y" not in [1]`, }, ]; for (const { expressionTree, result, extraOptions } of toTest) { @@ -755,7 +757,7 @@ QUnit.test("expressionFromTree . treeFromExpression", function (assert) { }, { expression: `set([foo]).intersection([1, 2])`, - result: `set([foo]).intersection([1, 2])`, + result: `foo in [1, 2]`, }, { expression: `set().intersection([foo])`, @@ -763,7 +765,7 @@ QUnit.test("expressionFromTree . treeFromExpression", function (assert) { }, { expression: `set([1, 2]).intersection([foo])`, - result: `set([foo]).intersection([1, 2])`, + result: `foo in [1, 2]`, }, { expression: `not set([foo]).intersection()`, @@ -771,7 +773,7 @@ QUnit.test("expressionFromTree . treeFromExpression", function (assert) { }, { expression: `not set([foo]).intersection([1, 2])`, - result: `not set([foo]).intersection([1, 2])`, + result: `foo not in [1, 2]`, }, { expression: `not set().intersection([foo])`, @@ -779,7 +781,7 @@ QUnit.test("expressionFromTree . treeFromExpression", function (assert) { }, { expression: `not set([1, 2]).intersection([foo])`, - result: `not set([foo]).intersection([1, 2])`, + result: `foo not in [1, 2]`, }, ]; for (const { expression, result, extraOptions } of toTest) { @@ -820,3 +822,70 @@ QUnit.test("expressionFromDomain", function (assert) { assert.deepEqual(expressionFromDomain(domain, o), result); } }); + +QUnit.test("evaluation . expressionFromTree = contains . domainFromTree", function (assert) { + const options = { + getFieldDef: (name) => { + if (name === "foo") { + return {}; // any field + } + if (name === "foo_ids") { + return { type: "many2many" }; + } + return null; + }, + }; + + const record = { foo: 1, foo_ids: [1, 2], uid: 7, expr: "abc" }; + + function toBool(val) { + if (val instanceof Set) { + return Boolean(val.size); + } + if (Array.isArray(val)) { + return Boolean(val.length); + } + if (val === null) { + return false; + } + if (typeof val === "object") { + return Boolean(Object.keys(val).length); + } + return Boolean(val); + } + + const toTest = [ + condition("foo", "=", false), + condition("foo", "=", false, true), + condition("foo", "!=", false), + condition("foo", "!=", false, true), + condition("y", "=", false), + condition("foo", "between", [1, 3]), + condition("foo", "between", [1, expression("uid")], true), + condition("foo_ids", "in", []), + condition("foo_ids", "in", [1]), + condition("foo_ids", "in", 1), + condition("foo", "in", []), + condition(expression("expr"), "in", []), + condition("foo", "in", [1]), + condition("foo", "in", 1), + condition("y", "in", []), + condition("y", "in", [1]), + condition("y", "in", 1), + condition("foo_ids", "not in", []), + condition("foo_ids", "not in", [1]), + condition("foo_ids", "not in", 1), + condition("foo", "not in", []), + condition("foo", "not in", [1]), + condition("foo", "not in", 1), + condition("y", "not in", []), + condition("y", "not in", [1]), + condition("y", "not in", 1), + ]; + for (const tree of toTest) { + assert.strictEqual( + toBool(evaluateExpr(expressionFromTree(tree, options), record)), + new Domain(domainFromTree(tree)).contains(record) + ); + } +});