[FIX] web: update properties field

The purpose of this commit is to resolve two issues relating to the properties field:

Issue 1:
========
In a list view, when you edit a property and then select another field
in the same record, the edition disappears.

How to resolve:
--------------
Do not delete the changes linked to the property in record.update.

How to reproduce:
----------------
- Go to a list view with a properties field containing a property char
- Insert a value in the char property
- Click on another field in the same record

Before this commit:
    The inserted value disappears

After this commit:
    The inserted value is still present

Issue 2:
========
The fields/activeFields only take into account properties received
during the initial web_read and not those added by an update or an onChange.

How to resolve:
--------------
Generate property fields and activeFields during parseServerValue
(web_read and onChange) and during an update.

How to reproduce:
----------------
- Go to a form view with a properties field
- Editing a field with an onChange
- The onChange returns a new property for the properties field

Before this commit:
    A crash is displayed

After this commit:
    The new property is displayed correctly

closes odoo/odoo#137989

Signed-off-by: Aaron Bohy (aab) <aab@odoo.com>
This commit is contained in:
FrancoisGe
2023-10-12 13:16:44 +00:00
parent b2faa9ea8c
commit c5b87e130b
7 changed files with 161 additions and 77 deletions
@@ -11,7 +11,7 @@ import { escape } from "@web/core/utils/strings";
import { DataPoint } from "./datapoint";
import {
FetchRecordError,
applyProperties,
createPropertyActiveField,
getBasicEvalContext,
getFieldContext,
getFieldsSpec,
@@ -451,6 +451,9 @@ export class Record extends DataPoint {
for (const fieldName in data) {
const value = data[fieldName];
const field = this.fields[fieldName];
if (field.relatedPropertyField) {
continue;
}
if (["char", "text", "html"].includes(field.type)) {
dataContext[fieldName] = this._textValues[fieldName];
} else if (field.type === "one2many" || field.type === "many2many") {
@@ -655,6 +658,69 @@ export class Record extends DataPoint {
});
}
/**
* This function extracts all properties and adds them to fields and activeFields.
* @param {Object[]} properties the list of properties to be extracted
* @param {string} fieldName name of the field containing the properties
* @param {Array} parent Array with ['id, 'display_name'], representing the record to which the definition of properties is linked
* @param {Object} currentValues current values of the record
* @returns An object containing as key `${fieldName}.${property.name}` and as value the value of the property
*/
_processProperties(properties, fieldName, parent, currentValues = {}) {
const data = {};
const relatedPropertyField = {
fieldName,
};
if (parent) {
relatedPropertyField.id = parent[0];
relatedPropertyField.displayName = parent[1];
}
const hasCurrentValues = Object.keys(currentValues).length > 0;
for (const property of properties) {
const propertyFieldName = `${fieldName}.${property.name}`;
// Add Unknown Property Field and ActiveField
if (hasCurrentValues || !this.fields[propertyFieldName]) {
this.fields[propertyFieldName] = {
...property,
name: propertyFieldName,
relatedPropertyField,
propertyName: property.name,
relation: property.comodel,
};
}
if (hasCurrentValues || !this.activeFields[propertyFieldName]) {
this.activeFields[propertyFieldName] = createPropertyActiveField(property);
}
// Extract property data
if (property.type === "many2many") {
let staticList = currentValues[propertyFieldName];
if (!staticList) {
staticList = this._createStaticListDatapoint(
(property.value || []).map((record) => ({
id: record[0],
display_name: record[1],
})),
propertyFieldName
);
}
data[propertyFieldName] = staticList;
} else if (property.type === "many2one") {
data[propertyFieldName] =
property.value.length && property.value[1] === null
? [property.value[0], _t("No Access")]
: property.value;
} else {
data[propertyFieldName] = property.value ?? false;
}
}
return data;
}
_parseServerValues(serverValues, currentValues = {}) {
const parsedValues = {};
if (!serverValues) {
@@ -687,26 +753,16 @@ export class Record extends DataPoint {
} else {
parsedValues[fieldName] = parseServerValue(field, value);
if (field.type === "properties") {
for (const property of parsedValues[fieldName]) {
const fieldPropertyName = `${fieldName}.${property.name}`;
if (property.type === "many2many") {
const staticList = this._createStaticListDatapoint(
(property.value || []).map((record) => ({
id: record[0],
display_name: record[1],
})),
fieldPropertyName
);
parsedValues[fieldPropertyName] = staticList;
} else if (property.type === "many2one") {
parsedValues[fieldPropertyName] =
property.value.length && property.value[1] === null
? [property.value[0], _t("No Access")]
: property.value;
} else {
parsedValues[fieldPropertyName] = property.value ?? false;
}
}
const parent = serverValues[field.definition_record];
Object.assign(
parsedValues,
this._processProperties(
parsedValues[fieldName],
fieldName,
parent,
currentValues
)
);
}
}
}
@@ -841,7 +897,14 @@ export class Record extends DataPoint {
_preprocessPropertiesChanges(changes) {
for (const [fieldName, value] of Object.entries(changes)) {
const field = this.fields[fieldName];
if (field && field.relatedPropertyField) {
if (field.type === "properties") {
const parent =
changes[field.definition_record] || this.data[field.definition_record];
Object.assign(
changes,
this._processProperties(value, fieldName, parent, this.data)
);
} else if (field && field.relatedPropertyField) {
const [propertyFieldName, propertyName] = field.name.split(".");
const propertiesData = this.data[propertyFieldName] || [];
if (!propertiesData.find((property) => property.name === propertyName)) {
@@ -857,7 +920,6 @@ export class Record extends DataPoint {
changes[propertyFieldName] = propertiesData.map((property) =>
property.name === propertyName ? { ...property, value } : property
);
delete changes[field.name];
}
}
}
@@ -957,7 +1019,6 @@ export class Record extends DataPoint {
}
await this.model.hooks.onRecordSaved(this, changes);
if (reload) {
applyProperties(records, this.config.activeFields, this.config.fields);
if (this.resId) {
this.model._updateSimilarRecords(this, records[0]);
}
@@ -17,7 +17,6 @@ import { Record } from "./record";
import { StaticList } from "./static_list";
import {
FetchRecordError,
applyProperties,
extractInfoFromGroupData,
getBasicEvalContext,
getFieldsSpec,
@@ -511,9 +510,7 @@ export class RelationalModel extends Model {
* @returns Promise<Object>
*/
async _loadNewRecord(config, params = {}) {
const record = await this._onchange(config, params);
applyProperties([record], config.activeFields, config.fields);
return record;
return this._onchange(config, params);
}
/**
@@ -538,7 +535,6 @@ export class RelationalModel extends Model {
throw new FetchRecordError(resIds);
}
applyProperties(records, config.activeFields, config.fields);
return records;
} else {
return resIds.map((resId) => {
@@ -564,10 +560,7 @@ export class RelationalModel extends Model {
count_limit:
config.countLimit !== Number.MAX_SAFE_INTEGER ? config.countLimit + 1 : undefined,
};
const response = await this.orm.webSearchRead(config.resModel, config.domain, kwargs);
applyProperties(response.records, config.activeFields, config.fields);
return response;
return this.orm.webSearchRead(config.resModel, config.domain, kwargs);
}
/**
@@ -76,39 +76,6 @@ export function addFieldDependencies(activeFields, fields, fieldDependencies = [
}
}
export function applyProperties(records, activeFields, fields) {
for (const record of records) {
for (const fieldName in record) {
const field = fields[fieldName];
if (fieldName !== "id" && field.type === "properties" && record[fieldName]) {
const parent = record[field.definition_record];
const relatedPropertyField = {
fieldName,
};
if (parent) {
relatedPropertyField.id = parent.id;
relatedPropertyField.displayName = parent.display_name;
}
for (const property of record[fieldName]) {
const propertyFieldName = `${fieldName}.${property.name}`;
if (!fields[propertyFieldName]) {
fields[propertyFieldName] = {
...property,
name: propertyFieldName,
relatedPropertyField,
propertyName: property.name,
relation: property.comodel,
};
}
if (!activeFields[propertyFieldName]) {
activeFields[propertyFieldName] = createPropertyActiveField(property);
}
}
}
}
}
}
function completeActiveField(activeField, extra) {
if (extra.related) {
for (const fieldName in extra.related.activeFields) {
@@ -11,7 +11,7 @@ import { _t } from "@web/core/l10n/translation";
import { registry } from "@web/core/registry";
import { KeepLast } from "@web/core/utils/concurrency";
import { Model } from "@web/model/model";
import { applyProperties, extractFieldsFromArchInfo } from "@web/model/relational_model/utils";
import { extractFieldsFromArchInfo } from "@web/model/relational_model/utils";
export class CalendarModel extends Model {
setup(params, services) {
@@ -484,7 +484,6 @@ export class CalendarModel extends Model {
*/
async loadRecords(data) {
const rawRecords = await this.fetchRecords(data);
applyProperties(rawRecords, this.meta.activeFields, this.meta.fields);
const records = {};
for (const rawRecord of rawRecords) {
records[rawRecord.id] = this.normalizeRecord(rawRecord);
@@ -12,8 +12,6 @@ import {
deserializeDateTime,
formatDate,
formatDateTime,
serializeDate,
serializeDateTime,
} from "@web/core/l10n/dates";
import { _t } from "@web/core/l10n/translation";
import { TagsList } from "@web/core/tags_list/tags_list";
@@ -204,11 +202,7 @@ export class PropertyValue extends Component {
* @param {object} newValue
*/
async onValueChange(newValue) {
if (this.props.type === "datetime") {
newValue = newValue && serializeDateTime(newValue);
} else if (this.props.type === "date") {
newValue = newValue && serializeDate(newValue);
} else if (this.props.type === "integer") {
if (this.props.type === "integer") {
try {
newValue = parseInteger(newValue) || 0;
} catch {
@@ -258,7 +258,7 @@ QUnit.module("Fields", (hooks) => {
records: [
{
id: 37,
display_name: "Company 1",
name: "Company 1",
definitions: [
{
name: "property_1",
@@ -2522,4 +2522,70 @@ QUnit.module("Fields", (hooks) => {
assert.containsNone(target, ".o_cp_action_menus span:contains(Add Properties)");
},
);
QUnit.test("properties: onChange return new properties", async function (assert) {
serverData.models.company.records[1] = {
id: 38,
display_name: "Company 2",
definitions: [
{
name: "property_2_1",
string: "My Char",
type: "char",
view_in_kanban: true,
},
],
};
serverData.models.partner.onchanges = {
company_id: (changes) => {
if (changes.company_id === 38) {
changes.properties = [
{
name: "property_2_2",
string: "My New Char",
type: "char",
value: "Hello",
},
];
}
},
};
async function mockRPC(route, { method }) {
if (["check_access_rights", "check_access_rule"].includes(method)) {
return true;
}
}
await makeView({
type: "form",
resModel: "partner",
resId: 1,
serverData,
arch: `
<form>
<sheet>
<group>
<field name="company_id"/>
<field name="properties"/>
</group>
</sheet>
</form>`,
mockRPC,
});
await editInput(target, "[name='company_id'] input", "Company 2");
await click(target.querySelector(".dropdown-menu li"));
assert.deepEqual(
[...target.querySelectorAll("[name='properties'] .o_property_field")].map(
(el) => el.textContent
),
["My New Char"]
);
assert.deepEqual(
[...target.querySelectorAll("[name='properties'] .o_property_field input")].map(
(el) => el.value
),
["Hello"]
);
});
});
@@ -18627,8 +18627,12 @@ QUnit.module("Views", (hooks) => {
await click(target.querySelector(".o_field_cell.o_char_cell"));
await editInput(target, ".o_field_cell.o_char_cell input", "TEST");
await clickSave(target);
assert.strictEqual(target.querySelector(".o_field_cell.o_char_cell input").value, "TEST");
await click(target.querySelector("[name='m2o']"));
assert.strictEqual(target.querySelector(".o_field_cell.o_char_cell input").value, "TEST");
await clickSave(target);
assert.strictEqual(target.querySelector(".o_field_cell.o_char_cell").textContent, "TEST");
});