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

[FIX] web: fix SelectMenu object shallowEqual

Browse Source 
With commit (1), the clear button is only displayed when a selected
value is found from the available choices. But since we used
shallowEqual to find the corresponding choice, we couldn't use objects
containing objects in its values, since a simple comparison was made
in the utility function. This was a wrong usage of the SelectMenu and
we don't want to support objects and arrays as values. The value
attribute should only be a string, and it's the parent responsability
to handle any object related to this string.

The test using an object and an array as values has been removed since
it had no purpose after the removal of this partial and wrong support.

With this fix, an assertion for the presence of the clear button has
been added instead for the import action.

(1): d760b81271

closes odoo/odoo#119298

Related: odoo/enterprise#40177
Signed-off-by: Géry Debongnie <ged@odoo.com>
This commit is contained in:
luvi
2023-04-25 09:38:00 +02:00
parent 41ae9fe011
commit 22cc9660ed
6 changed files with 223 additions and 192 deletions
Download Patch File Download Diff File Expand all files Collapse all files
addons/base_import/static/src/import_action/import_action.js
+3 -2
View File
@@ -191,8 +191,9 @@ export class ImportAction extends Component {
// Fields
//--------------------------------------------------------------------------
onFieldChanged(column, value) {
this.model.setColumnField(column, value);
onFieldChanged(column, fieldId) {
const selection = this.model.fields.find((f) => f.id === fieldId);
this.model.setColumnField(column, selection);
}
isFieldSet(column) {
addons/base_import/static/src/import_data_content/import_data_content.js
+1 -1
View File
@@ -46,7 +46,7 @@ export class ImportDataContent extends Component {
makeChoices(fields) {
return fields.map((field) => ({
label: field.label,
value: field,
value: field.id,
}));
}
addons/base_import/static/src/import_data_content/import_data_content.xml
+2 -2
View File
@@ -60,9 +60,9 @@
</td>
<td>
<SelectMenu
value="column.fieldInfo"
value="column.fieldInfo and column.fieldInfo.id"
groups="getGroups(column)"
onSelect="fieldInfo => this.props.onFieldChanged(column, fieldInfo)"
onSelect="(fieldId) => this.props.onFieldChanged(column, fieldId)"
searchPlaceholder="searchPlaceholder"
togglerClass="column.fieldInfo ? 'ps-1' : ''"
>
addons/base_import/static/tests/import_action_tests.js
+212 -124
View File
@@ -160,70 +160,70 @@ function executeFailingImport(field, isMultiline, field_path = "") {
ids: false,
messages: isMultiline
? [
{
field,
field_name: serverData.models.partner.fields[field].string,
field_path,
message: "Invalid value",
moreInfo,
record: 0,
rows: { from: 0, to: 0 },
value: "Invalid value",
priority: "info",
},
{
field,
field_name: serverData.models.partner.fields[field].string,
field_path,
message: "Duplicate value",
moreInfo,
record: 0,
rows: { from: 1, to: 1 },
priority: "error",
},
{
field,
field_name: serverData.models.partner.fields[field].string,
field_path,
message: "Wrong values",
moreInfo,
record: 0,
rows: { from: 2, to: 3 },
priority: "warning",
},
{
field,
field_name: serverData.models.partner.fields[field].string,
field_path,
message: "Bad value here",
moreInfo,
record: 0,
rows: { from: 4, to: 4 },
value: "Bad value",
priority: "warning",
},
{
field,
field_name: serverData.models.partner.fields[field].string,
field_path,
message: "Duplicate value",
moreInfo,
record: 0,
rows: { from: 5, to: 5 },
priority: "error",
},
]
{
field,
field_name: serverData.models.partner.fields[field].string,
field_path,
message: "Invalid value",
moreInfo,
record: 0,
rows: { from: 0, to: 0 },
value: "Invalid value",
priority: "info",
},
{
field,
field_name: serverData.models.partner.fields[field].string,
field_path,
message: "Duplicate value",
moreInfo,
record: 0,
rows: { from: 1, to: 1 },
priority: "error",
},
{
field,
field_name: serverData.models.partner.fields[field].string,
field_path,
message: "Wrong values",
moreInfo,
record: 0,
rows: { from: 2, to: 3 },
priority: "warning",
},
{
field,
field_name: serverData.models.partner.fields[field].string,
field_path,
message: "Bad value here",
moreInfo,
record: 0,
rows: { from: 4, to: 4 },
value: "Bad value",
priority: "warning",
},
{
field,
field_name: serverData.models.partner.fields[field].string,
field_path,
message: "Duplicate value",
moreInfo,
record: 0,
rows: { from: 5, to: 5 },
priority: "error",
},
]
: [
{
field,
field_name: serverData.models.partner.fields[field].string,
field_path,
message: "Incorrect value",
moreInfo,
record: 0,
rows: { from: 0, to: 0 },
},
],
{
field,
field_name: serverData.models.partner.fields[field].string,
field_path,
message: "Incorrect value",
moreInfo,
record: 0,
rows: { from: 0, to: 0 },
},
],
name: ["Some invalid content", "Wrong content", "Bad content"],
nextrow: 0,
};
@@ -579,7 +579,7 @@ QUnit.module("Base Import Tests", (hooks) => {
registerFakeHTTPService();
const notificationMock = (message) => {
assert.step(message);
return () => { };
return () => {};
};
registry
.category("services")
@@ -755,7 +755,6 @@ QUnit.module("Base Import Tests", (hooks) => {
"base_import.import/execute_import": (route, args) => {
if (shouldFail) {
shouldFail = false;
// return executeFailingImport(args, args[1][0]);
return executeFailingImport(args[1][0]);
}
assert.deepEqual(
@@ -1181,57 +1180,90 @@ QUnit.module("Base Import Tests", (hooks) => {
);
});
QUnit.test("Import view: relational fields correctly mapped on preview", async function (assert) {
assert.expect(4);
QUnit.test(
"Import view: relational fields correctly mapped on preview",
async function (assert) {
assert.expect(4);
registerFakeHTTPService();
await createImportAction({
"base_import.import/parse_preview": async (route, args) => {
return customParsePreview(args[1], {
fields: [
{ id: "id", name: "id", string: "External ID", fields: [], type: "id" },
{ id: "display_name", name: "display_name", string: "Display Name", fields: [], type: "id" },
{
id: "many2many_field", name: "many2many_field", string: "Many2Many", fields: [
{ id: "id", name: "id", string: "External ID", fields: [], type: "id" },
], type: "id"
registerFakeHTTPService();
await createImportAction({
"base_import.import/parse_preview": async (route, args) => {
return customParsePreview(args[1], {
fields: [
{ id: "id", name: "id", string: "External ID", fields: [], type: "id" },
{
id: "display_name",
name: "display_name",
string: "Display Name",
fields: [],
type: "id",
},
{
id: "many2many_field",
name: "many2many_field",
string: "Many2Many",
fields: [
{
id: "id",
name: "id",
string: "External ID",
fields: [],
type: "id",
},
],
type: "id",
},
],
headers: ["id", "display_name", "many2many_field/id"],
rowCount: 5,
matches: {
0: ["id"],
1: ["display_name"],
2: ["many2many_field", "id"],
},
],
headers: ["id", "display_name", "many2many_field/id"],
rowCount: 5,
matches: {
0: ["id"],
1: ["display_name"],
2: ["many2many_field", "id"]
},
preview: [
["0", "1", "2"],
["Name 1", "Name 2", "Name 3",],
["", "1", "2"],
],
});
},
"base_import.import/execute_import": (route, args) => {
assert.deepEqual(args[1], ["id", "display_name", "many2many_field/id"],
"The proper arguments are given for the import");
assert.deepEqual(args[2], ["id", "display_name", "many2many_field/id"],
"The proper arguments are given for the import");
return executeImport(args);
},
});
preview: [
["0", "1", "2"],
["Name 1", "Name 2", "Name 3"],
["", "1", "2"],
],
});
},
"base_import.import/execute_import": (route, args) => {
assert.deepEqual(
args[1],
["id", "display_name", "many2many_field/id"],
"The proper arguments are given for the import"
);
assert.deepEqual(
args[2],
["id", "display_name", "many2many_field/id"],
"The proper arguments are given for the import"
);
return executeImport(args);
},
});
// Set and trigger the change of a file for the input
const file = new File(["fake_file"], "fake_file.xls", { type: "text/plain" });
await editInput(target, "input[type='file']", file);
// Set and trigger the change of a file for the input
const file = new File(["fake_file"], "fake_file.xls", { type: "text/plain" });
await editInput(target, "input[type='file']", file);
assert.strictEqual(target.querySelector("tr:nth-child(3) .o_import_file_column_cell span.text-truncate").innerText, "many2many_field/id",
"The third row should be the relational field");
assert.strictEqual(
target.querySelector(
"tr:nth-child(3) .o_import_file_column_cell span.text-truncate"
).innerText,
"many2many_field/id",
"The third row should be the relational field"
);
assert.strictEqual(target.querySelector("tr:nth-child(3) .o_select_menu_toggler_slot span").innerText, "Many2Many / External ID",
"The relational field should be selected by default and the name should be the full path.");
assert.strictEqual(
target.querySelector("tr:nth-child(3) .o_select_menu_toggler_slot span").innerText,
"Many2Many / External ID",
"The relational field should be selected by default and the name should be the full path."
);
await click(target.querySelector(".o_control_panel button:first-child"));
});
await click(target.querySelector(".o_control_panel button:first-child"));
}
);
QUnit.test("Import view: import errors with relational fields", async function (assert) {
registerFakeHTTPService();
@@ -1244,11 +1276,27 @@ QUnit.module("Base Import Tests", (hooks) => {
return customParsePreview(args[1], {
fields: [
{ id: "id", name: "id", string: "External ID", fields: [], type: "id" },
{ id: "display_name", name: "display_name", string: "Display Name", fields: [], type: "id" },
{
id: "many2many_field", name: "many2many_field", string: "Many2Many", fields: [
{ id: "id", name: "id", string: "External ID", fields: [], type: "id" },
], type: "id"
id: "display_name",
name: "display_name",
string: "Display Name",
fields: [],
type: "id",
},
{
id: "many2many_field",
name: "many2many_field",
string: "Many2Many",
fields: [
{
id: "id",
name: "id",
string: "External ID",
fields: [],
type: "id",
},
],
type: "id",
},
],
headers: ["id", "display_name", "many2many_field/id"],
@@ -1256,11 +1304,11 @@ QUnit.module("Base Import Tests", (hooks) => {
matches: {
0: ["id"],
1: ["display_name"],
2: ["many2many_field", "id"]
2: ["many2many_field", "id"],
},
preview: [
["0", "1", "2"],
["Name 1", "Name 2", "Name 3",],
["Name 1", "Name 2", "Name 3"],
["", "1", "2"],
],
});
@@ -1283,17 +1331,30 @@ QUnit.module("Base Import Tests", (hooks) => {
"A message is shown if the import was blocked"
);
assert.strictEqual(target.querySelector("tr:nth-child(3) .o_import_file_column_cell span.text-truncate").innerText, "many2many_field/id",
"The third row should be the relational field");
assert.strictEqual(
target.querySelector("tr:nth-child(3) .o_import_file_column_cell span.text-truncate")
.innerText,
"many2many_field/id",
"The third row should be the relational field"
);
assert.strictEqual(target.querySelector("tr:nth-child(3) .o_select_menu_toggler_slot span").innerText, "Many2Many / External ID",
"The relational field is properly mapped");
assert.strictEqual(
target.querySelector("tr:nth-child(3) .o_select_menu_toggler_slot span").innerText,
"Many2Many / External ID",
"The relational field is properly mapped"
);
assert.containsOnce(target, "tr:nth-child(3) .o_import_report.alert",
"The relational field should have error messages on his row");
assert.containsOnce(
target,
"tr:nth-child(3) .o_import_report.alert",
"The relational field should have error messages on his row"
);
assert.strictEqual(target.querySelector("tr:nth-child(3) .o_import_report.alert p b").innerText, "Many2Many / External ID",
"The error should contain the full path of the relational field");
assert.strictEqual(
target.querySelector("tr:nth-child(3) .o_import_report.alert p b").innerText,
"Many2Many / External ID",
"The error should contain the full path of the relational field"
);
});
QUnit.test("Import view: date format should be converted to strftime", async function (assert) {
@@ -1340,4 +1401,31 @@ QUnit.module("Base Import Tests", (hooks) => {
await click(target.querySelector(".o_control_panel button:first-child"));
assert.verifySteps(["execute_import"]);
});
QUnit.test("Import action: field selection has a clear button", async function (assert) {
registerFakeHTTPService();
patchWithCleanup(browser, {
setTimeout: (fn) => fn(),
});
await createImportAction();
// Set and trigger the change of a file for the input
const file = new File(["fake_file"], "fake_file.xlsx", { type: "text/plain" });
await editInput(target, "input[type='file']", file);
await editSelectMenu(target, ".o_import_data_content .o_select_menu", "Bar");
assert.containsN(
target,
".o_select_menu_toggler_clear",
2,
"clear button is present for each field to unselect it"
);
await click(target.querySelector(".o_select_menu_toggler_clear"));
assert.strictEqual(
target.querySelector("tr:nth-child(2) .o_select_menu").textContent,
"To import, select a field...",
"field has been unselected"
);
});
});
addons/web/static/src/core/select_menu/select_menu.js
+1 -7
View File
@@ -4,7 +4,6 @@ import { Component, useState, useRef, onWillUpdateProps } from "@odoo/owl";
import { Dropdown } from "@web/core/dropdown/dropdown";
import { DropdownItem } from "@web/core/dropdown/dropdown_item";
import { _lt } from "@web/core/l10n/translation";
import { shallowEqual } from "@web/core/utils/objects";
import { useDebounced } from "@web/core/utils/timing";
import { scrollTo } from "@web/core/utils/scrolling";
import { fuzzyLookup } from "@web/core/utils/search";
@@ -172,12 +171,7 @@ export class SelectMenu extends Component {
getSelectedChoice(props) {
if (props.value) {
const choices = [...props.choices, ...props.groups.flatMap((g) => g.choices)];
return choices.find((c) => {
if (typeof c.value === "object" && typeof props.value === "object") {
return shallowEqual(c.value, props.value);
}
return c.value === props.value;
});
return choices.find((c) => c.value === props.value);
} else {
return undefined;
}
addons/web/static/tests/core/select_menu_tests.js
+4 -56
View File
@@ -175,58 +175,6 @@ QUnit.module("Web Components", (hooks) => {
assert.equal(document.activeElement, target.querySelector(".o_select_menu_input input"));
});
QUnit.test("value props accept array", async (assert) => {
class Parent extends Component {
setup() {
this.choices = [
{ label: "Z", value: [1, 2] },
{ label: "A", value: [1, 3] },
];
this.value = [1, 2];
}
}
Parent.components = { SelectMenu };
Parent.template = xml`
<SelectMenu
choices="this.choices"
value="this.value"
/>
`;
await mount(Parent, target, { env });
assert.equal(
target.querySelector(".o_select_menu_toggler_slot").innerText,
"Z",
`The select value shoud be "Z"`
);
});
QUnit.test("value props accept object", async (assert) => {
class Parent extends Component {
setup() {
this.choices = [
{ label: "Z", value: { hello: "world" } },
{ label: "A", value: { paper: "company" } },
];
this.value = { paper: "company" };
}
}
Parent.components = { SelectMenu };
Parent.template = xml`
<SelectMenu
choices="this.choices"
value="this.value"
/>
`;
await mount(Parent, target, { env });
assert.equal(
target.querySelector(".o_select_menu_toggler_slot").innerText,
"A",
`The select value shoud be "A"`
);
});
QUnit.test(
"Value with no corresponding choices displays as if no choice was selected",
async (assert) => {
@@ -266,10 +214,10 @@ QUnit.module("Web Components", (hooks) => {
`;
setup() {
this.choices = [
{ label: "Z", value: { hello: "world" } },
{ label: "A", value: { paper: "company" } },
{ label: "Z", value: "world" },
{ label: "A", value: "company" },
];
this.state = useState({ value: { paper: "company" } });
this.state = useState({ value: "company" });
}
setValue(newValue) {
this.state.value = newValue;
@@ -279,7 +227,7 @@ QUnit.module("Web Components", (hooks) => {
const comp = await mount(Parent, target, { env });
assert.equal(getValue(), "A", `The select value shoud be "A"`);
comp.setValue({ hello: "world" });
comp.setValue("world");
await nextTick();
assert.equal(
getValue(),