pengyuthon/odoo_source
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

[IMP] web: ExpressionEditor: better conversion of in/not in operators

Browse Source 
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) <ryv@odoo.com>
This commit is contained in:
Mathieu Duckerts-Antoine
2023-10-27 21:36:53 +00:00
parent 3c1d74181a
commit 0ff280ec6d
2 changed files with 89 additions and 21 deletions
Download Patch File Download Diff File Expand all files Collapse all files
addons/web/static/src/core/tree_editor/condition_tree.js
+8 -9
View File
@@ -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,
});
}
addons/web/static/tests/core/condition_tree_tests.js
+81 -12
View File
@@ -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)
);
}
});