8000 [FIX] *: domain field: option allow_expressions · odoo/odoo@ebab15a · GitHub
[go: up one dir, main page]
Skip to content

Commit ebab15a

Browse files
Polymorphe57Polymorphe57
committed
[FIX] *: domain field: option allow_expressions
Some views use a domain widget to edit/save a domain and use it afterwards in several places where domains must have literals only. Typically, a literal_eval is used to evaluate the (string) domain. Thus expressions like "uid" or "context_today()"" should not be used in those contexts. Here we introduce an option "allow_expressions" (default False) for the domain field and use it to mark as invalid domains that contain expressions when the option is set to False. A notification is displayed when a domain contains an unwanted expressions. Since we cannot expect modules like base to be updated, we have to find another system to allow the usage of expressions in the form views for the models ir.filters and base.automation: we simply hardcode those models as allowing expressions. Since for these models, the evaluation of domains is done via safe_eval but with a restricted evaluation context, we also display a notification that alerts the user that the evaluation of expressions (although accepted by the domain field) can fail. We revert the recent commit f95c494 that introduced potentially problematic calls to safe_eval in order to allow evaluation of expressions. Forward-port-of: #208876 X-original-commit: 329867a Part-of: #210145 Related: odoo/enterprise#85620 Signed-off-by: Aaron Bohy (aab) <aab@odoo.com> Signed-off-by: Mathieu Duckerts-Antoine (dam) <dam@odoo.com>
1 parent ad4f6d4 commit ebab15a

File tree

7 files changed

+199
-37
lines changed

7 files changed

+199
-37
lines changed

‎addons/mail/tests/test_mail_tools.py

Lines changed: 0 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,6 @@
33

44
from odoo.addons.mail.tests.common import MailCommon
55
from odoo.tests import tagged, users
6-
from odoo.addons.mail.tools.parser import domain_eval
7-
from freezegun import freeze_time
86

97

108
@tagged('mail_tools', 'res_partner')
@@ -181,22 +179,6 @@ def test_mail_find_partner_from_emails_multicompany(self):
181179
found = Partner._mail_find_partner_from_emails([self._test_email], records=record)
182180
self.assertEqual(found, [expected_partner], msg)
183181

184-
@freeze_time('2030-05-24')
185-
def test_domain_eval(self):
186-
success_pairs = [
187-
("list()", []),
188-
("list(range(1, 4))", [1, 2, 3]),
189-
("['|', (1, '=', 1), (1, '>', 0)]", ['|', (1, '=', 1), (1, '>', 0)]),
190-
("[(2, '=', 1 + 1)]", [(2, '=', 2)]),
191-
(
192-
"[('create_date', '<', datetime.datetime.combine(context_today() - relativedelta(days=100), datetime.time(1, 2, 3)).to_utc().strftime('%Y-%m-%d %H:%M:%S'))]",
193-
[('create_date', '<', "2030-02-13 01:02:03")],
194-
), # use the date utils used by front-end domains
195-
]
196-
for domain_expression, domain_value in success_pairs:
197-
with self.subTest(domain_expression=domain_expression, domain_value=domain_value):
198-
self.assertEqual(domain_eval(domain_expression), domain_value)
199-
200182

201183
@tagged('mail_tools', 'mail_init')
202184
class TestMailUtils(MailCommon):

‎addons/mail/tools/parser.py

Lines changed: 1 addition & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
import ast
55

66
from odoo.exceptions import ValidationError
7-
from odoo.tools import is_list_of, safe_eval
7+
from odoo.tools import is_list_of
88

99

1010
def parse_res_ids(res_ids, env):
@@ -35,15 +35,3 @@ def parse_res_ids(res_ids, env):
3535
raise ValidationError(error_msg)
3636

3737
return res_ids
38-
39-
40-
def domain_eval(domain):
41-
domain = domain.replace('.to_utc()', '')
42-
evaluated_domain = safe_eval.safe_eval(domain, {
43-
'context_today': safe_eval.datetime.datetime.today,
44-
'datetime': safe_eval.datetime,
45-
'dateutil': safe_eval.dateutil,
46-
'relativedelta': safe_eval.dateutil.relativedelta.relativedelta,
47-
'time': safe_eval.time,
48-
})
49-
return evaluated_domain

‎addons/web/i18n/web.pot

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3291,6 +3291,13 @@ msgid ""
32913291
"dropdown at all."
32923292
msgstr ""
32933293

3294+
#. module: web
3295+
#. odoo-javascript
3296+
#: code:addons/web/static/src/views/fields/domain/domain_field.js:0
3297+
#, python-format
3298+
msgid "If true, non-literals are accepted"
3299+
msgstr ""
3300+
32943301
#. module: web
32953302
#. odoo-javascript
32963303
#: code:addons/web/static/src/core/file_viewer/file_viewer.xml:0
@@ -6452,6 +6459,20 @@ msgid ""
64526459
"characters). Please use the CSV format for this export."
64536460
msgstr ""
64546461

6462+
#. module: web
6463+
#. odoo-javascript
6464+
#: code:addons/web/static/src/views/fields/domain/domain_field.js:0
6465+
#, python-format
6466+
msgid "The domain involves non-literals. Their evaluation might fail."
6467+
msgstr ""
6468+
6469+
#. module: web
6470+
#. odoo-javascript
6471+
#: code:addons/web/static/src/views/fields/domain/domain_field.js:0
6472+
#, python-format
6473+
msgid "The domain should not involve non-literals"
6474+
msgstr ""
6475+
64556476
#. module: web
64566477
#. odoo-javascript
64576478
#: code:addons/web/static/src/core/errors/error_dialogs.xml:0

‎addons/web/static/src/core/tree_editor/condition_tree.js

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -199,6 +199,34 @@ export function complexCondition(value) {
199199
return { type: "complex_condition", value };
200200
}
201201

202+
function treeContainsExpressions(tree) {
203+
if (tree.type === "condition") {
204+
const { path, operator, value } = tree;
205+
return [path, operator, value].some(
206+
(v) =>
207+
v instanceof Expression ||
208+
(Array.isArray(v) && v.some((w) => w instanceof Expression))
209+
);
210+
}
211+
for (const child of tree.children) {
212+
if (treeContainsExpressions(child)) {
213+
return true;
214+
}
215+
}
216+
return false;
217+
}
218+
219+
export function domainContainsExpresssions(domain) {
220+
let tree;
221+
try {
222+
tree = treeFromDomain(domain);
223+
} catch {
224+
return null;
225+
}
226+
// detect expressions in the domain tree, which we know is well-formed
227+
return treeContainsExpressions(tree);
228+
}
229+
202230
/**
203231
* @param {Value} value
204232
* @returns {Value}

‎addons/web/static/src/views/fields/domain/domain_field.js

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import { standardFieldProps } from "../standard_field_props";
1111
import { useBus, useService, useOwnedDialogs } from "@web/core/utils/hooks";
1212
import { useGetTreeDescription, useMakeGetFieldDef } from "@web/core/tree_editor/utils";
1313
import { useGetDefaultLeafDomain } from "@web/core/domain_selector/utils";
14-
import { treeFromDomain } from "@web/core/tree_editor/condition_tree";
14+
import { domainContainsExpresssions, treeFromDomain } from "@web/core/tree_editor/condition_tree";
1515
import { useRecordObserver } from "@web/model/relational_model/utils";
1616

1717
export class DomainField extends Component {
@@ -25,16 +25,19 @@ export class DomainField extends Component {
2525
editInDialog: { type: Boolean, optional: true },
2626
resModel: { type: String, optional: true },
2727
isFoldable: { type: Boolean, optional: true },
28+
allowExpressions: { type: Boolean, optional: true },
2829
};
2930
static defaultProps = {
3031
editInDialog: false,
3132
isFoldable: false,
33+
allowExpressions: false,
3234
};
3335

3436
setup() {
3537
this.orm = useService("orm");
3638
this.getDomainTreeDescription = useGetTreeDescription();
3739
this.makeGetFieldDef = useMakeGetFieldDef();
40+
this.notification = useService("notification");
3841
this.getDefaultLeafDomain = useGetDefaultLeafDomain();
3942
this.addDialog = useOwnedDialogs();
4043

@@ -84,6 +87,13 @@ export class DomainField extends Component {
8487
});
8588
}
8689

90+
allowExpressions(props) {
91+
return (
92+
props.allowExpressions ||
93+
["base.automation", "ir.filters"].includes(props.record.resModel)
94+
);
95+
}
96+
8797
getContext(props = this.props) {
8898
return props.context;
8999
}
@@ -95,6 +105,20 @@ export class DomainField extends Component {
95105
getEvaluatedDomain(props = this.props) {
96106
const domainStringRepr = this.getDomain(props);
97107
const evalContext = this.getContext(props);
108+
if (domainContainsExpresssions(domainStringRepr)) {
109+
const allowExpressions = this.allowExpressions(props);
110+
if (domainStringRepr !== this.lastDomainChecked) {
111+
this.lastDomainChecked = domainStringRepr;
112+
this.notification.add(
113+
allowExpressions
114+
? _t("The domain involves non-literals. Their evaluation might fail.")
115+
: _t("The domain should not involve non-literals")
116+
);
117+
}
118+
if (!allowExpressions) {
119+
return { isInvalid: true };
120+
}
121+
}
98122
try {
99123
const domain = new Domain(domainStringRepr).toList(evalContext);
100124
// Here, there is still some incertitude on the domain validity.
@@ -275,6 +299,12 @@ export const domainField = {
275299
type: "boolean",
276300
help: _t("Display the domain using facets"),
277301
},
302+
{
303+
label: _t("Allow expressions"),
304+
name: "allow_expressions",
305+
type: "boolean",
306+
help: _t("If true, non-literals are accepted"),
307+
},
278308
{
279309
label: _t("Model"),
280310
name: "model",
@@ -287,6 +317,7 @@ export const domainField = {
287317
return {
288318
editInDialog: options.in_dialog,
289319
isFoldable: options.foldable,
320+
allowExpressions: options.allow_expressions,
290321
resModel: options.model,
291322
context: dynamicInfo.context,
292323
};

‎addons/web/static/tests/core/domain_field.test.js

Lines changed: 116 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ import {
3131
} from "@web/../tests/web_test_helpers";
3232

3333
import { WebClient } from "@web/webclient/webclient";
34+
import { registry } from "@web/core/registry";
3435

3536
class PartnerType extends models.Model {
3637
name = fields.Char({ string: "Partner Type" });
@@ -44,11 +45,29 @@ class PartnerType extends models.Model {
4445

4546
defineModels([Partner, Product, Team, Player, Country, Stage, PartnerType]);
4647

47-
test("The domain editor should not crash the view when given a dynamic filter", async function () {
48+
function replaceNotificationService() {
49+
registry.category("services").add(
50+
"notification",
51+
{
52+
start() {
53+
return {
54+
add(message) {
55+
expect.step(message);
56+
},
57+
};
58+
},
59+
},
60+
{ force: true }
61+
);
62+
}
63+
64+
test("The domain editor should not crash the view when given a dynamic filter (allow_expressions=False)", async function () {
4865
// dynamic filters (containing variables, such as uid, parent or today)
4966
// are handled by the domain editor
5067
Partner._records[0].foo = `[("int", "=", uid)]`;
5168

69+
replaceNotificationService();
70+
5271
await mountView({
5372
type: "form",
5473
resModel: "partner",
@@ -63,6 +82,30 @@ test("The domain editor should not crash the view when given a dynamic filter",
6382
expect(getCurrentValue()).toBe("uid", {
6483
message: "The widget should show the dynamic filter.",
6584
});
85+
expect(".o_field_domain").not.toHaveClass("o_field_invalid");
86+
expect.verifySteps(["The domain should not involve non-literals"]);
87+
});
88+
89+
test("The domain editor should not crash the view when given a dynamic filter (allow_expressions=True)", async function () {
90+
Partner._records[0].foo = `[("int", "=", uid)]`;
91+
92+
replaceNotificationService();
93+
94+
await mountView({
95+
type: "form",
96+
resModel: "partner",
97+
resId: 1,
98+
arch: `
99+
<form>
100+
<field name="foo" widget="domain" options="{'model': 'partner', 'allow_expressions':True}" />
101+
<field name="int" invisible="1" />
102+
</form>`,
103+
});
104+
expect(getCurrentValue()).toBe("uid", {
105+
message: "The widget should show the dynamic filter.",
106+
});
107+
expect(".o_field_domain").not.toHaveClass("o_field_invalid");
108+
expect.verifySteps(["The domain involves non-literals. Their evaluation might fail."]);
66109
});
67110

68111
test("The domain editor should not crash the view when given a dynamic filter ( datetime )", async function () {
@@ -572,7 +615,7 @@ test("domain field: edit domain with dynamic content", async function () {
572615
form: `
573616
<form>
574617
<field name="bar"/>
575-
<field name="foo" widget="domain" options="{'model': 'bar'}"/>
618+
<field name="foo" widget="domain" options="{'model': 'bar', 'allow_expressions':True}"/>
576619
</form>`,
577620
search: `<search />`,
578621
};
@@ -616,7 +659,7 @@ test("domain field: edit through selector (dynamic content)", async function ()
616659
form: `
617660
<form>
618661
<field name="bar"/>
619-
<field name="foo" widget="domain" options="{'model': 'bar'}"/>
662+
<field name="foo" widget="domain" options="{'model': 'bar', 'allow_expressions':True}"/>
620663
</form>`,
621664
search: `<search />`,
622665
};
@@ -1025,3 +1068,73 @@ test("folded domain field with withinh operator", async function () {
10251068
});
10261069
expect(`.o_field_domain .o_facet_values`).toHaveText("Datetime is within 2 months");
10271070
});
1071+
1072+
test("allow_expressions = true", async function () {
1073+
Partner._records[0].foo = "[]";
1074+
1075+
serverState.debug = "1";
1076+
replaceNotificationService();
1077+
1078+
onRpc("/web/domain/validate", () => true);
1079+
1080+
await mountView({
1081+
type: "form",
1082+
resModel: "partner",
1083+
resId: 1,
1084+
arch: `
1085+
<form>
1086+
<sheet>
1087+
<group>
1088+
<field name="foo" widget="domain" options="{'model': 'partner.type', 'allow_expressions':True}" />
1089+
</group>
1090+
</sheet>
1091+
</form>`,
1092+
context: { path: "name", name: "name" },
1093+
});
1094+
1095+
await contains(SELECTORS.debugArea).edit(`[("name", "=", [name])]`);
1096+
await animationFrame();
1097+
expect(".o_field_domain").not.toHaveClass("o_field_invalid");
1098+
expect.verifySteps(["The domain involves non-literals. Their evaluation might fail."]);
1099+
1100+
await contains(SELECTORS.debugArea).edit(
1101+
`["&", ("name", "=", "name"), (path, "=", "other name")]`
1102+
);
1103+
await animationFrame();
1104+
expect(".o_field_domain").not.toHaveClass("o_field_invalid");
1105+
expect.verifySteps(["The domain involves non-literals. Their evaluation might fail."]);
1106+
});
1107+
1108+
test("allow_expressions = false (default)", async function () {
1109+
Partner._records[0].foo = "[]";
1110+
1111+
serverState.debug = "1";
1112+
replaceNotificationService();
1113+
1114+
await mountView({
1115+
type: "form",
1116+
resModel: "partner",
1117+
resId: 1,
1118+
arch: `
1119+
<form>
1120+
<sheet>
1121+
<group>
1122+
<field name="foo" widget="domain" options="{'model': 'partner.type' }" />
1123+
</group>
1124+
</sheet>
1125+
</form>`,
1126+
context: { path: "name", name: "name" },
1127+
});
1128+
1129+
await contains(SELECTORS.debugArea).edit(`[("name", "=", [name])]`);
1130+
await animationFrame();
1131+
expect(".o_field_domain").toHaveClass("o_field_invalid");
1132+
expect.verifySteps(["The domain should not involve non-literals"]);
1133+
1134+
await contains(SELECTORS.debugArea).edit(
1135+
`["&", ("name", "=", "name"), (path, "=", "other name")]`
1136+
);
1137+
await animationFrame();
1138+
expect(".o_field_domain").toHaveClass("o_field_invalid");
1139+
expect.verifySteps(["The domain should not involve non-literals"]);
1140+
});

‎addons/website/controllers/model_page.py

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44

55
from odoo.http import Controller, request, route
66
from odoo.osv.expression import AND, OR
7-
from odoo.addons.mail.tools.parser import domain_eval
87

98

109
class ModelPageController(Controller):
@@ -44,7 +43,7 @@ def generic_model(self, page_name_slugified=None, page_number=1, record_slug=Non
4443
if not Model.has_access("read"):
4544
raise werkzeug.exceptions.Forbidden()
4645

47-
rec_domain = domain_eval(page.record_domain or "[]")
46+
rec_domain = ast.literal_eval(page.record_domain or "[]")
4847
domains = [rec_domain]
4948
implements_published_mixin = "website_published" in Model._fields
5049
if implements_published_mixin and not request.env.user.has_group('website.group_website_designer'):

0 commit comments

Comments
 (0)
0