From c5b87e130bd69996c9121227bb6d085035c362fa Mon Sep 17 00:00:00 2001 From: FrancoisGe Date: Mon, 9 Oct 2023 10:20:35 +0200 Subject: [PATCH] [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) --- .../src/model/relational_model/record.js | 109 ++++++++++++++---- .../relational_model/relational_model.js | 11 +- .../src/model/relational_model/utils.js | 33 ------ .../src/views/calendar/calendar_model.js | 3 +- .../views/fields/properties/property_value.js | 8 +- .../views/fields/properties_field_tests.js | 68 ++++++++++- .../web/static/tests/views/list_view_tests.js | 6 +- 7 files changed, 161 insertions(+), 77 deletions(-) diff --git a/addons/web/static/src/model/relational_model/record.js b/addons/web/static/src/model/relational_model/record.js index 21fd9dfe6cf..ccb314fade6 100644 --- a/addons/web/static/src/model/relational_model/record.js +++ b/addons/web/static/src/model/relational_model/record.js @@ -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]); } diff --git a/addons/web/static/src/model/relational_model/relational_model.js b/addons/web/static/src/model/relational_model/relational_model.js index 90b1817e185..98bdd1be5db 100644 --- a/addons/web/static/src/model/relational_model/relational_model.js +++ b/addons/web/static/src/model/relational_model/relational_model.js @@ -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 */ 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); } /** diff --git a/addons/web/static/src/model/relational_model/utils.js b/addons/web/static/src/model/relational_model/utils.js index 9017c2650fc..9ff071e6f4a 100644 --- a/addons/web/static/src/model/relational_model/utils.js +++ b/addons/web/static/src/model/relational_model/utils.js @@ -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) { diff --git a/addons/web/static/src/views/calendar/calendar_model.js b/addons/web/static/src/views/calendar/calendar_model.js index 589add42901..126efbaed7d 100644 --- a/addons/web/static/src/views/calendar/calendar_model.js +++ b/addons/web/static/src/views/calendar/calendar_model.js @@ -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); diff --git a/addons/web/static/src/views/fields/properties/property_value.js b/addons/web/static/src/views/fields/properties/property_value.js index d2fc87b4017..f0c39f0985b 100644 --- a/addons/web/static/src/views/fields/properties/property_value.js +++ b/addons/web/static/src/views/fields/properties/property_value.js @@ -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 { diff --git a/addons/web/static/tests/views/fields/properties_field_tests.js b/addons/web/static/tests/views/fields/properties_field_tests.js index 2d8ef26dbef..c46f076194b 100644 --- a/addons/web/static/tests/views/fields/properties_field_tests.js +++ b/addons/web/static/tests/views/fields/properties_field_tests.js @@ -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: ` +
+ + + + + + +
`, + 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"] + ); + }); }); diff --git a/addons/web/static/tests/views/list_view_tests.js b/addons/web/static/tests/views/list_view_tests.js index 818e0264643..b7efa7b57e8 100644 --- a/addons/web/static/tests/views/list_view_tests.js +++ b/addons/web/static/tests/views/list_view_tests.js @@ -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"); });