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

[FIX] base_import: Fix relationnal fields not being correctly mapped

Browse Source 
This commit fixes a couple of issues linked to relational fields:

- When previewing a file with relational fields/ids, they were not
correctly mapped to their respective fields in the selects, the fields
were also not correctly mapped in the arguments given for the import.
- The select label was also only showing the name of the last sub-field
while it should show the full path ("External ID" vs "Company / External
 ID").
- Some errors were not correctly displayed: wrong field name, not in a
column when it should be.

Task ID: 3210337
opw-3203704 + opw-3192403

closes odoo/odoo#114490

X-original-commit: 871690afdc2ce4da2770a144cc4fc5ed038e6f73
Signed-off-by: Aaron Bohy (aab) <aab@odoo.com>
This commit is contained in:
Bastien Fafchamps (bafa)
2023-03-07 09:03:06 +01:00
parent f8ce09276a
commit a16361f96d
6 changed files with 283 additions and 137 deletions
Download Patch File Download Diff File Expand all files Collapse all files
addons/base_import/static/src/import_data_column_error/import_data_column_error.xml
+1 -1
View File
@@ -6,7 +6,7 @@
<p>
<t t-if="props.errors[0].value">No matching records found for the following name </t>
<t t-else="">Multiple errors occurred </t>
in field <b t-esc="props.fieldInfo.name"/>:
in field <b t-esc="props.fieldInfo.label"/>:
</p>
<ul>
<t t-foreach="props.errors" t-as="error" t-key="error_index">
addons/base_import/static/src/import_data_content/import_data_content.scss
+4
View File
@@ -14,6 +14,10 @@
.o_import_preview {
width: fit-content;
}
td {
vertical-align: top;
}
}
@mixin o-import-sprite-icon($x: 0, $y: 0) {
addons/base_import/static/src/import_data_content/import_data_content.xml
+1 -1
View File
@@ -74,7 +74,7 @@
t-att-data-tooltip-info="getTooltipDetails(column.fieldInfo)"
></i>
<span class="ps-5">
<t t-esc="column.fieldInfo.string" />
<t t-esc="column.fieldInfo.label" />
</span>
</t>
<span t-else="" class="text-warning">To import, select a field...</span>
addons/base_import/static/src/import_data_options/import_data_options.js
+6 -2
View File
@@ -62,14 +62,18 @@ export class ImportDataOptions extends Component {
"import_skip_records",
].includes(ev.target.value)
) {
this.props.onOptionChanged(ev.target.value, ev.target.value, this.props.fieldInfo.name);
this.props.onOptionChanged(
ev.target.value,
ev.target.value,
this.props.fieldInfo.fieldPath
);
} else {
const value = {
fallback_value: ev.target.value,
field_model: this.currentModel,
field_type: this.props.fieldInfo.type,
};
this.props.onOptionChanged("fallback_values", value, this.props.fieldInfo.name);
this.props.onOptionChanged("fallback_values", value, this.props.fieldInfo.fieldPath);
}
}
}
addons/base_import/static/src/import_model.js
+69 -66
View File
@@ -164,7 +164,7 @@ export class BaseImportModel {
const startRow = this.importOptions.skip;
const importRes = {
ids: [],
fields: this.columns.map((e) => Boolean(e.fieldInfo) && e.fieldInfo.name),
fields: this.columns.map((e) => Boolean(e.fieldInfo) && e.fieldInfo.fieldPath),
columns: this.columns.map((e) => e.name.trim().toLowerCase()),
hasError: false,
};
@@ -179,7 +179,6 @@ export class BaseImportModel {
}
const error = await this._executeImportStep(isTest, importRes);
if (error) {
let message;
const errorData = error.data || {};
@@ -308,8 +307,9 @@ export class BaseImportModel {
async _executeImportStep(isTest, importRes) {
const importArgs = [this.id, importRes.fields, importRes.columns, this.importOptions];
const { ids, messages, nextrow, name, error } = await this._callImport(isTest, importArgs);
// Handle server errors
if (error) {
return error;
}
@@ -318,52 +318,13 @@ export class BaseImportModel {
importRes.ids = importRes.ids.concat(ids);
}
// Handle import errors
if (messages && messages.length) {
importRes.hasError = true;
this.stopImport();
if (messages[0].not_matching_error) {
this._addMessage(messages[0].type, [messages[0].message]);
if (this._handleImportErrors(messages, name)) {
return false;
}
const sortedMessages = this._groupErrorsByField(messages);
if (sortedMessages[0]) {
this._addMessage(sortedMessages[0].type, [sortedMessages[0].message]);
delete sortedMessages[0];
} else {
this._addMessage("danger", [_t("The file contains blocking errors (see below)")]);
}
for (const [columnFieldId, errors] of Object.entries(sortedMessages)) {
// Handle errors regarding specific colums.
const column = this.columns.find(
(e) => e.fieldInfo && e.fieldInfo.id === columnFieldId
);
if (column) {
column.resultNames = name;
column.errors = errors;
} else {
for (const error of errors) {
// Handle errors regarding specific records.
if (error.record !== undefined) {
this._addMessage("danger", [
error.rows.from === error.rows.to
? sprintf(
_t('Error at row %s: "%s"'),
error.record,
error.message
)
: sprintf(_t("%s at multiple rows"), error.message),
]);
}
// Handle global errors.
else {
this._addMessage("danger", [error.message]);
}
}
}
}
}
this.setOption("skip", nextrow || 0);
@@ -390,6 +351,47 @@ export class BaseImportModel {
}
}
_handleImportErrors(messages, name) {
if (messages[0].not_matching_error) {
this._addMessage(messages[0].type, [messages[0].message]);
return true;
}
const sortedMessages = this._groupErrorsByField(messages);
if (sortedMessages[0]) {
this._addMessage(sortedMessages[0].type, [sortedMessages[0].message]);
delete sortedMessages[0];
} else {
this._addMessage("danger", [_t("The file contains blocking errors (see below)")]);
}
for (const [columnFieldId, errors] of Object.entries(sortedMessages)) {
// Handle errors regarding specific colums.
const column = this.columns.find(
(e) => e.fieldInfo && e.fieldInfo.fieldPath === columnFieldId
);
if (column) {
column.resultNames = name;
column.errors = errors;
} else {
for (const error of errors) {
// Handle errors regarding specific records.
if (error.record !== undefined) {
this._addMessage("danger", [
error.rows.from === error.rows.to
? sprintf(_t('Error at row %s: "%s"'), error.record, error.message)
: sprintf(_t("%s at multiple rows"), error.message),
]);
}
// Handle global errors.
else {
this._addMessage("danger", [error.message]);
}
}
}
}
}
_groupErrorsByField(messages) {
const groupedErrors = {};
const errorsByMessage = groupBy(this._sortErrors(messages), (f) => f.message || "0");
@@ -455,20 +457,6 @@ export class BaseImportModel {
}
_getColumns(res) {
function createColumn(model, id, name, index, previews, preview) {
const fields = model._getFields(res, index);
return {
id,
name,
preview,
previews,
fields,
fieldInfo: model._findField(fields, id),
comments: [],
errors: [],
};
}
function getId(res, index) {
return res.matches && index in res.matches && res.matches[index].length > 0
? res.matches[index].join("/")
@@ -477,8 +465,8 @@ export class BaseImportModel {
if (this.importOptions.has_headers && res.headers && res.preview.length > 0) {
return res.headers.flatMap((header, index) => {
return createColumn(
this,
return this._createColumn(
res,
getId(res, index),
header,
index,
@@ -488,8 +476,8 @@ export class BaseImportModel {
});
} else if (res.preview && res.preview.length >= 2) {
return res.preview.flatMap((preview, index) =>
createColumn(
this,
this._createColumn(
res,
preview[0],
this.importOptions.has_headers ? preview[0] : preview.join(", "),
index,
@@ -501,10 +489,24 @@ export class BaseImportModel {
return [];
}
_createColumn(res, id, name, index, previews, preview) {
const fields = this._getFields(res, index);
return {
id,
name,
preview,
previews,
fields,
fieldInfo: this._findField(fields, id),
comments: [],
errors: [],
};
}
_findField(fields, id) {
return Object.entries(fields)
.flatMap((e) => e[1])
.find((field) => field.name === id);
.find((field) => field.fieldPath === id);
}
/**
@@ -584,7 +586,8 @@ export class BaseImportModel {
column.comments = [];
column.errors = [];
column.resultNames = [];
column.importOptions = column.fieldInfo && this.fieldsToHandle[column.fieldInfo.id];
column.importOptions =
column.fieldInfo && this.fieldsToHandle[column.fieldInfo.fieldPath];
if (!column.fieldInfo) {
continue;
@@ -603,7 +606,7 @@ export class BaseImportModel {
// If multiple columns are mapped on the same field, inform
// the user that they will be concatenated.
const samefieldColumns = this.columns.filter(
(col) => col.fieldInfo && col.fieldInfo.id === column.fieldInfo.id
(col) => col.fieldInfo && col.fieldInfo.fieldPath === column.fieldInfo.fieldPath
);
if (samefieldColumns.length >= 2) {
column.comments.push({
@@ -615,7 +618,7 @@ export class BaseImportModel {
} else if (updatedColumn && column.id !== updatedColumn.id && updatedColumn.fieldInfo) {
// If column is mapped on an already mapped field, remove that field
// from the old column to keep it unique.
if (updatedColumn.fieldInfo.id === column.fieldInfo.id) {
if (updatedColumn.fieldInfo.fieldPath === column.fieldInfo.fieldPath) {
column.fieldInfo = null;
}
}
addons/base_import/static/tests/import_action_tests.js
+202 -67
View File
@@ -129,9 +129,29 @@ function parsePreview(opts) {
});
}
function customParsePreview(opts, { fields, headers, rowCount, matches, preview }) {
totalRows = rowCount;
return Promise.resolve({
advanced_mode: opts.advanced,
batch: false,
fields: fields,
file_length: opts.has_headers ? totalRows - 1 : totalRows,
header_types: false,
headers: headers,
matches: matches,
options: {
...opts,
sheet: opts.sheet.length ? opts.sheet : "Template",
sheets: ["Template", "Template 2"],
},
preview: preview,
});
}
// since executing a real import would be difficult, this method simply returns
// some error messages to help testing the UI
function executeFailingImport(field, isMultiline) {
function executeFailingImport(field, isMultiline, field_path = "") {
let moreInfo = [];
if (serverData.models.partner.fields[field].type === "selection") {
moreInfo = serverData.models.partner.fields[field].selection;
@@ -140,70 +160,70 @@ function executeFailingImport(field, isMultiline) {
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,
};
@@ -559,7 +579,7 @@ QUnit.module("Base Import Tests", (hooks) => {
registerFakeHTTPService();
const notificationMock = (message) => {
assert.step(message);
return () => {};
return () => { };
};
registry
.category("services")
@@ -904,8 +924,8 @@ QUnit.module("Base Import Tests", (hooks) => {
);
// Check that errors have been sorted and grouped
assert.strictEqual(
target.querySelector(".o_import_report p").textContent.trim(),
"Multiple errors occurred in field foo:"
target.querySelector(".o_import_report p").textContent.trim().toLowerCase(),
"multiple errors occurred in field foo:"
);
assert.strictEqual(
target.querySelector(".o_import_report li:first-child").textContent.trim(),
@@ -1160,4 +1180,119 @@ QUnit.module("Base Import Tests", (hooks) => {
"the import will resume at line 1"
);
});
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"
},
],
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);
},
});
// 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_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"));
});
QUnit.test("Import view: import errors with relational fields", async function (assert) {
registerFakeHTTPService();
patchWithCleanup(browser, {
setTimeout: (fn) => fn(),
});
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"]
},
preview: [
["0", "1", "2"],
["Name 1", "Name 2", "Name 3",],
["", "1", "2"],
],
});
},
"base_import.import/execute_import": (route, args) => {
return executeFailingImport(args[1][0], true, ["many2many_field", "id"]);
},
});
// 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);
// For this test, we force the display of an error message if this field is set
await editSelectMenu(target, ".o_import_data_content .o_select_menu", "Many2Many");
await click(target.querySelector(".o_control_panel button:nth-child(2)"));
assert.strictEqual(
target.querySelector(".o_import_data_content .alert-danger").textContent,
"The file contains blocking errors (see below)",
"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_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.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");
});
});