From 39e51241709e002a9be45c212f5a474b087bb6a3 Mon Sep 17 00:00:00 2001 From: Mathieu Duckerts-Antoine Date: Tue, 24 Oct 2023 16:17:41 +0200 Subject: [PATCH] [IMP] web: support set intersections in condition tree Now that some field attributes like invisible are given by Python expressions that can involve set operations, we want to be able to easily edit those expressions in the expression editor. For this we have to improve a bit the mapping expression <-> condition tree, in order to have simple rewrittings like "set(user_ids).intersection([1, 2])" <-> condition('user_ids', 'in', [1, 2]). Part-of: odoo/odoo#139451 --- .../src/core/tree_editor/condition_tree.js | 120 +++++- .../static/tests/core/condition_tree_tests.js | 364 +++++++++++++++++- 2 files changed, 464 insertions(+), 20 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 c0c38b2af50..d2274334a5a 100644 --- a/addons/web/static/src/core/tree_editor/condition_tree.js +++ b/addons/web/static/src/core/tree_editor/condition_tree.js @@ -335,7 +335,7 @@ function not(ast) { return { ...ast, value: !ast.value }; } if (ast.type === 7 && COMPARATORS.includes(ast.op)) { - return { ...ast, op: TERM_OPERATORS_NEGATION_EXTENDED[ast.op] }; + return { ...ast, op: TERM_OPERATORS_NEGATION_EXTENDED[ast.op] }; // do not use this if ast is within a domain context! } return { type: 6, op: "not", right: isBool(ast) ? ast.args[0] : ast }; } @@ -366,14 +366,16 @@ function isNot(ast) { function is(oneParamFunc, ast) { return ( ast.type === 8 && - ast.fn && ast.fn.type === 5 && ast.fn.value === oneParamFunc && - ast.args && ast.args.length === 1 ); // improve condition? } +function isSet(ast) { + return ast.type === 8 && ast.fn.type === 5 && ast.fn.value === "set" && ast.args.length <= 1; +} + function isBool(ast) { return is("bool", ast); } @@ -386,6 +388,14 @@ function isValidPath(ast, options) { return false; } +function isX2Many(ast, options) { + if (isValidPath(ast, options)) { + const fieldDef = options.getFieldDef(ast.value); // safe: isValidPath has not returned null; + return ["many2many", "one2many"].includes(fieldDef.type); + } + return false; +} + function _getConditionFromComparator(ast, options) { if (["is", "is not"].includes(ast.op)) { // we could do something smarter here @@ -407,8 +417,9 @@ function _getConditionFromComparator(ast, options) { if (!isValidPath(left, options)) { if (operator in EXCHANGE) { - left = ast.right; - right = ast.left; + const temp = left; + left = right; + right = temp; operator = EXCHANGE[operator]; } else { return null; @@ -418,6 +429,60 @@ function _getConditionFromComparator(ast, options) { return condition(left.value, operator, toValue(right)); } +function isValidPath2(ast, options) { + if (!ast) { + return null; + } + if ([4, 10].includes(ast.type) && ast.value.length === 1) { + return isValidPath(ast.value[0], options); + } + return isValidPath(ast, options); +} + +function _getConditionFromIntersection(ast, options, negate = false) { + let left = ast.fn.obj.args[0]; + let right = ast.args[0]; + + if (!left) { + return condition(negate ? 1 : 0, "=", 1); + } + + // left/right exchange + if (isValidPath2(left, options) == isValidPath2(right, options)) { + return null; + } + if (!isValidPath2(left, options)) { + const temp = left; + left = right; + right = temp; + } + + if ([4, 10].includes(left.type) && left.value.length === 1) { + left = left.value[0]; + } + + if (!right) { + return condition(left.value, negate ? "=" : "!=", false); + } + + // try to extract the ast of an iterable + // we only make simple conversions here + if (isSet(right)) { + if (!right.args[0]) { + right = { type: 4, value: [] }; + } + if ([4, 10].includes(right.args[0].type)) { + right = right.args[0]; + } + } + + if (![4, 10].includes(right.type)) { + return null; + } + + return condition(left.value, negate ? "not in" : "in", toValue(right)); +} + /** * @param {AST} ast * @param {Options} options @@ -438,6 +503,18 @@ function _leafFromAST(ast, options, negate = false) { return condition(astValue ? 1 : 0, "=", 1); } + if ( + ast.type === 8 && + ast.fn.type === 15 /** object lookup */ && + isSet(ast.fn.obj) && + ast.fn.key === "intersection" + ) { + const tree = _getConditionFromIntersection(ast, options, negate); + if (tree) { + return tree; + } + } + if (ast.type === 7 && COMPARATORS.includes(ast.op)) { if (negate) { return _leafFromAST(not(ast), options); @@ -448,7 +525,7 @@ function _leafFromAST(ast, options, negate = false) { } } - // no conclusive way to transform ast in a condition + // no conclusive/simple way to transform ast in a condition return complexCondition(formatAST(negate ? not(ast) : ast)); } @@ -562,6 +639,37 @@ 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] }; + + const valueAST = toAST(value); + const otherIteratorAST = [4, 10].includes(valueAST.type) + ? valueAST + : { type: 4, value: [valueAST] }; + + const ast = { + type: 8, + fn: { + type: 15, + obj: { + args: [setIteratorAST], + type: 8, + fn: { + type: 5, + value: "set", + }, + }, + key: "intersection", + }, + args: [otherIteratorAST], + }; + return formatAST(operator === "not in" ? not(ast) : ast); + } + // add case true for boolean fields return formatAST({ diff --git a/addons/web/static/tests/core/condition_tree_tests.js b/addons/web/static/tests/core/condition_tree_tests.js index d21176b86ff..d46d9788152 100644 --- a/addons/web/static/tests/core/condition_tree_tests.js +++ b/addons/web/static/tests/core/condition_tree_tests.js @@ -243,41 +243,125 @@ QUnit.test("domainFromExpression", function (assert) { QUnit.test("expressionFromTree", function (assert) { const options = { - getFieldDef: (name) => (name === "x" ? {} /** any field */ : null), + getFieldDef: (name) => { + if (["foo", "bar"].includes(name)) { + return {}; // any field + } + if (["foo_ids", "bar_ids"].includes(name)) { + return { type: "many2many" }; + } + return null; + }, }; const toTest = [ { - expressionTree: condition("x", "=", false), - result: `not x`, + expressionTree: condition("foo", "=", false), + result: `not foo`, }, { - expressionTree: condition("x", "=", false, true), - result: `x`, + expressionTree: condition("foo", "=", false, true), + result: `foo`, }, { - expressionTree: condition("x", "!=", false), - result: `x`, + expressionTree: condition("foo", "!=", false), + result: `foo`, }, { - expressionTree: condition("x", "!=", false, true), - result: `not x`, + expressionTree: condition("foo", "!=", false, true), + result: `not foo`, }, { expressionTree: condition("y", "=", false), result: `not "y"`, }, { - expressionTree: condition("x", "between", [1, 3]), - result: `x >= 1 and x <= 3`, + expressionTree: condition("foo", "between", [1, 3]), + result: `foo >= 1 and foo <= 3`, }, { - expressionTree: condition("x", "between", [1, expression("uid")], true), - result: `not ( x >= 1 and x <= uid )`, + expressionTree: condition("foo", "between", [1, expression("uid")], true), + result: `not ( foo >= 1 and foo <= uid )`, }, { expressionTree: complexCondition("uid"), result: `uid`, }, + { + expressionTree: condition("foo_ids", "in", []), + result: `set(foo_ids).intersection([])`, + }, + { + expressionTree: condition("foo_ids", "in", [1]), + result: `set(foo_ids).intersection([1])`, + }, + { + expressionTree: condition("foo_ids", "in", 1), + result: `set(foo_ids).intersection([1])`, + }, + { + expressionTree: condition("foo", "in", []), + result: `set([foo]).intersection([])`, + }, + { + expressionTree: condition(expression("expr"), "in", []), + result: `expr in []`, + }, + { + expressionTree: condition("foo", "in", [1]), + result: `set([foo]).intersection([1])`, + }, + { + expressionTree: condition("foo", "in", 1), + result: `set([foo]).intersection([1])`, + }, + { + expressionTree: condition("y", "in", []), + result: `"y" in []`, + }, + { + expressionTree: condition("y", "in", [1]), + result: `"y" in [1]`, + }, + { + expressionTree: condition("y", "in", 1), + result: `"y" in 1`, + }, + { + expressionTree: condition("foo_ids", "not in", []), + result: `not set(foo_ids).intersection([])`, + }, + { + expressionTree: condition("foo_ids", "not in", [1]), + result: `not set(foo_ids).intersection([1])`, + }, + { + expressionTree: condition("foo_ids", "not in", 1), + result: `not set(foo_ids).intersection([1])`, + }, + { + expressionTree: condition("foo", "not in", []), + result: `not set([foo]).intersection([])`, + }, + { + expressionTree: condition("foo", "not in", [1]), + result: `not set([foo]).intersection([1])`, + }, + { + expressionTree: condition("foo", "not in", 1), + result: `not set([foo]).intersection([1])`, + }, + { + expressionTree: condition("y", "not in", []), + result: `"y" not in []`, + }, + { + expressionTree: condition("y", "not in", [1]), + result: `"y" not in [1]`, + }, + { + expressionTree: condition("y", "not in", 1), + result: `"y" not in 1`, + }, ]; for (const { expressionTree, result, extraOptions } of toTest) { const o = { ...options, ...extraOptions }; @@ -291,7 +375,7 @@ QUnit.test("treeFromExpression", function (assert) { if (["foo", "bar"].includes(name)) { return {}; // any field } - if (name === "foo_ids") { + if (["foo_ids", "bar_ids"].includes(name)) { return { type: "many2many" }; } return null; @@ -365,6 +449,122 @@ QUnit.test("treeFromExpression", function (assert) { ]), ]), }, + { + expression: `set()`, + result: complexCondition(`set()`), + }, + { + expression: `set([1, 2])`, + result: complexCondition(`set([1, 2])`), + }, + { + expression: `set(foo_ids).intersection([1, 2])`, + result: condition("foo_ids", "in", [1, 2]), + }, + { + expression: `set(foo_ids).intersection(set([1, 2]))`, + result: condition("foo_ids", "in", [1, 2]), + }, + { + expression: `set(foo_ids).intersection(set((1, 2)))`, + result: condition("foo_ids", "in", [1, 2]), + }, + { + expression: `set(foo_ids).intersection("ab")`, + result: complexCondition(`set(foo_ids).intersection("ab")`), + }, + { + expression: `set([1, 2]).intersection(foo_ids)`, + result: condition("foo_ids", "in", [1, 2]), + }, + { + expression: `set(set([1, 2])).intersection(foo_ids)`, + result: condition("foo_ids", "in", [1, 2]), + }, + { + expression: `set((1, 2)).intersection(foo_ids)`, + result: condition("foo_ids", "in", [1, 2]), + }, + { + expression: `set("ab").intersection(foo_ids)`, + result: complexCondition(`set("ab").intersection(foo_ids)`), + }, + { + expression: `set([2, 3]).intersection([1, 2])`, + result: complexCondition(`set([2, 3]).intersection([1, 2])`), + }, + { + expression: `set(foo_ids).intersection(bar_ids)`, + result: complexCondition(`set(foo_ids).intersection(bar_ids)`), + }, + { + expression: `set().intersection(foo_ids)`, + result: condition(0, "=", 1), + }, + { + expression: `set(foo_ids).intersection()`, + result: condition("foo_ids", "set", false), + }, + { + expression: `not set().intersection(foo_ids)`, + result: condition(1, "=", 1), + }, + { + expression: `not set(foo_ids).intersection()`, + result: condition("foo_ids", "not_set", false), + }, + { + expression: `not set(foo_ids).intersection([1, 2])`, + result: condition("foo_ids", "not in", [1, 2]), + }, + { + expression: `not set(foo_ids).intersection(set([1, 2]))`, + result: condition("foo_ids", "not in", [1, 2]), + }, + { + expression: `not set(foo_ids).intersection(set((1, 2)))`, + result: condition("foo_ids", "not in", [1, 2]), + }, + { + expression: `not set(foo_ids).intersection("ab")`, + result: complexCondition(`not set(foo_ids).intersection("ab")`), + }, + { + expression: `not set([1, 2]).intersection(foo_ids)`, + result: condition("foo_ids", "not in", [1, 2]), + }, + { + expression: `not set(set([1, 2])).intersection(foo_ids)`, + result: condition("foo_ids", "not in", [1, 2]), + }, + { + expression: `not set((1, 2)).intersection(foo_ids)`, + result: condition("foo_ids", "not in", [1, 2]), + }, + { + expression: `not set("ab").intersection(foo_ids)`, + result: complexCondition(`not set("ab").intersection(foo_ids)`), + }, + { + expression: `not set([2, 3]).intersection([1, 2])`, + result: complexCondition(`not set([2, 3]).intersection([1, 2])`), + }, + { + expression: `not set(foo_ids).intersection(bar_ids)`, + result: complexCondition(`not set(foo_ids).intersection(bar_ids)`), + }, + { + expression: `set(foo_ids).difference([1, 2])`, + result: complexCondition(`set(foo_ids).difference([1, 2])`), + }, + { + expression: `set(foo_ids).union([1, 2])`, + result: complexCondition(`set(foo_ids).union([1, 2])`), + }, + { + expression: `expr in []`, + result: complexCondition(`expr in []`), + }, ]; for (const { expression, result, extraOptions } of toTest) { const o = { ...options, ...extraOptions }; @@ -445,6 +645,142 @@ QUnit.test("expressionFromTree . treeFromExpression", function (assert) { expression: `not context.get("toto")`, result: `not context.get("toto")`, }, + { + expression: `set()`, + result: `set()`, + }, + { + expression: `set([1, 2])`, + result: `set([1, 2])`, + }, + { + expression: `set(foo_ids).intersection([1, 2])`, + result: `set(foo_ids).intersection([1, 2])`, + }, + { + expression: `set(foo_ids).intersection(set([1, 2]))`, + result: `set(foo_ids).intersection([1, 2])`, + }, + { + expression: `set(foo_ids).intersection(set((1, 2)))`, + result: `set(foo_ids).intersection([1, 2])`, + }, + { + expression: `set(foo_ids).intersection("ab")`, + result: `set(foo_ids).intersection("ab")`, + }, + { + expression: `set([1, 2]).intersection(foo_ids)`, + result: `set(foo_ids).intersection([1, 2])`, + }, + { + expression: `set(set([1, 2])).intersection(foo_ids)`, + result: `set(foo_ids).intersection([1, 2])`, + }, + { + expression: `set((1, 2)).intersection(foo_ids)`, + result: `set(foo_ids).intersection([1, 2])`, + }, + { + expression: `set("ab").intersection(foo_ids)`, + result: `set("ab").intersection(foo_ids)`, + }, + { + expression: `set([2, 3]).intersection([1, 2])`, + result: `set([2, 3]).intersection([1, 2])`, + }, + { + expression: `set(foo_ids).intersection(bar_ids)`, + result: `set(foo_ids).intersection(bar_ids)`, + }, + { + expression: `set().intersection(foo_ids)`, + result: `False`, + }, + { + expression: `set(foo_ids).intersection()`, + result: `foo_ids`, + }, + { + expression: `not set(foo_ids).intersection([1, 2])`, + result: `not set(foo_ids).intersection([1, 2])`, + }, + { + expression: `not set(foo_ids).intersection(set([1, 2]))`, + result: `not set(foo_ids).intersection([1, 2])`, + }, + { + expression: `not set(foo_ids).intersection(set((1, 2)))`, + result: `not set(foo_ids).intersection([1, 2])`, + }, + { + expression: `not set(foo_ids).intersection("ab")`, + result: `not set(foo_ids).intersection("ab")`, + }, + { + expression: `not set([1, 2]).intersection(foo_ids)`, + result: `not set(foo_ids).intersection([1, 2])`, + }, + { + expression: `not set(set([1, 2])).intersection(foo_ids)`, + result: `not set(foo_ids).intersection([1, 2])`, + }, + { + expression: `not set((1, 2)).intersection(foo_ids)`, + result: `not set(foo_ids).intersection([1, 2])`, + }, + { + expression: `not set("ab").intersection(foo_ids)`, + result: `not set("ab").intersection(foo_ids)`, + }, + { + expression: `not set([2, 3]).intersection([1, 2])`, + result: `not set([2, 3]).intersection([1, 2])`, + }, + { + expression: `not set(foo_ids).intersection(bar_ids)`, + result: `not set(foo_ids).intersection(bar_ids)`, + }, + { + expression: `set(foo_ids).difference([1, 2])`, + result: `set(foo_ids).difference([1, 2])`, + }, + { + expression: `set(foo_ids).intersection([1, 2])`, + result: `set(foo_ids).intersection([1, 2])`, + }, + { + expression: `set([foo]).intersection()`, + result: `foo`, + }, + { + expression: `set([foo]).intersection([1, 2])`, + result: `set([foo]).intersection([1, 2])`, + }, + { + expression: `set().intersection([foo])`, + result: `False`, + }, + { + expression: `set([1, 2]).intersection([foo])`, + result: `set([foo]).intersection([1, 2])`, + }, + { + expression: `not set([foo]).intersection()`, + result: `not foo`, + }, + { + expression: `not set([foo]).intersection([1, 2])`, + result: `not set([foo]).intersection([1, 2])`, + }, + { + expression: `not set().intersection([foo])`, + result: `True`, + }, + { + expression: `not set([1, 2]).intersection([foo])`, + result: `not set([foo]).intersection([1, 2])`, + }, ]; for (const { expression, result, extraOptions } of toTest) { const o = { ...options, ...extraOptions };