From 9e2a3ff662b5fea7af526064b222550b18693ea7 Mon Sep 17 00:00:00 2001 From: std-odoo Date: Mon, 5 Sep 2022 11:51:06 +0000 Subject: [PATCH] [IMP] web: clean the code about property attributes reset Purpose ======= When changing the type or the model of a property, the name is re-generated, so the others record values are reset. If we change back to the original model / type, the original property name is restored. This is a super important behavior of the properties component and it should be fully tested. Reset the model and model description when changing the type. Now that the form view is always in edit mode, even if we changed the model / type of a properties, then save, we are able to restore the old properties as long as we don't refresh the page. Task-2965523 X-original-commit: 54ae82974b836a0badab3c4c767916a1daf88f76 Part-of: odoo/odoo#101487 --- .../fields/properties/properties_field.js | 61 +++++---- .../fields/properties/property_definition.js | 4 + .../views/fields/properties_field_tests.js | 119 +++++++++++++++--- 3 files changed, 142 insertions(+), 42 deletions(-) diff --git a/addons/web/static/src/views/fields/properties/properties_field.js b/addons/web/static/src/views/fields/properties/properties_field.js index 3a4c2633b1b..628d7a1117b 100644 --- a/addons/web/static/src/views/fields/properties/properties_field.js +++ b/addons/web/static/src/views/fields/properties/properties_field.js @@ -13,7 +13,7 @@ import { usePopover } from "@web/core/popover/popover_hook"; import { sprintf } from "@web/core/utils/strings"; import { ConfirmationDialog } from "@web/core/confirmation_dialog/confirmation_dialog"; -const { Component, useRef, useState, useEffect, onWillStart, onWillUpdateProps } = owl; +const { Component, useRef, useState, useEffect, onWillStart } = owl; export class PropertiesField extends Component { setup() { @@ -37,13 +37,6 @@ export class PropertiesField extends Component { await this._checkDefinitionAccess(); }); - onWillUpdateProps(async () => { - // reset the initials properties values if we saved the record - if (this.props.record.mode === "readonly") { - this._saveInitialPropertiesValues(); - } - }); - useEffect(() => { this._movePopoverIfNeeded(); @@ -223,24 +216,7 @@ export class PropertiesField extends Component { (property) => property.name === propertyDefinition.name ); - // if the type / model are the same, restore the original name to not reset the children - // otherwise, generate a new value so all value of the record are reset - const initialValues = this.initialValues[propertyDefinition.name]; - if ( - initialValues && - propertyDefinition.type === initialValues.type && - propertyDefinition.comodel === initialValues.comodel - ) { - // restore the original name - propertyDefinition.name = initialValues.name; - } else if (initialValues && initialValues.name === propertyDefinition.name) { - // generate a new new to reset all values on other records - // store the new generated name to be able to restore it - // if needed - const newName = uuid(); - this.initialValues[newName] = initialValues; - propertyDefinition.name = newName; - } + this._regeneratePropertyName(propertyDefinition); propertiesValues[propertyIndex] = propertyDefinition; this.props.update(propertiesValues); @@ -375,10 +351,41 @@ export class PropertiesField extends Component { this.state.canChangeDefinition = await this.orm.call( definitionRecordModel, "check_access_rights", - ["write", false] + ["write", false], ); } + /** + * Regenerate a new name if needed or restore the original one. + * (see @_saveInitialPropertiesValues). + * + * If the type / model are the same, restore the original name to not reset the + * children otherwise, generate a new value so all value of the record are reset. + * + * @param {object} propertyDefinition + */ + _regeneratePropertyName(propertyDefinition) { + const initialValues = this.initialValues[propertyDefinition.name]; + if ( + initialValues && + propertyDefinition.type === initialValues.type && + propertyDefinition.comodel === initialValues.comodel + ) { + // restore the original name + propertyDefinition.name = initialValues.name; + } else if (initialValues && initialValues.name === propertyDefinition.name) { + // Generate a new name to reset all values on other records. + // because the name has been changed on the definition, + // the old name on others record won't match the name on the definition + // and the python field will just ignore the old value. + // Store the new generated name to be able to restore it + // if needed. + const newName = uuid(); + this.initialValues[newName] = initialValues; + propertyDefinition.name = newName; + } + } + /** * If we change the type / model of a property, we will regenerate it's name * (like if it was a new property) in order to reset the value of the children. diff --git a/addons/web/static/src/views/fields/properties/property_definition.js b/addons/web/static/src/views/fields/properties/property_definition.js index ef22f45c7a5..9ac8ec19cc4 100644 --- a/addons/web/static/src/views/fields/properties/property_definition.js +++ b/addons/web/static/src/views/fields/properties/property_definition.js @@ -141,8 +141,12 @@ export class PropertyDefinition extends Component { value: false, }; + delete propertyDefinition.comodel; + this.props.onChange(propertyDefinition); this.state.propertyDefinition = propertyDefinition; + this.state.resModel = ''; + this.state.resModelDescription = ''; this.state.typeLabel = this._typeLabel(newType); } 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 84e1ff6ea2a..799f6461028 100644 --- a/addons/web/static/tests/views/fields/properties_field_tests.js +++ b/addons/web/static/tests/views/fields/properties_field_tests.js @@ -20,6 +20,14 @@ async function closePopover(target) { await click(document, "html"); } +async function changeType(target, propertyType) { + const TYPES_INDEX = {"datetime": 6, "selection": 7, "tags": 8, "many2one": 9, "many2many": 10}; + const propertyTypeIndex = TYPES_INDEX[propertyType]; + await click(target, ".o_field_property_definition_type input"); + await nextTick(); + await click(target, `.o_field_property_definition_type .dropdown-item:nth-child(${propertyTypeIndex})`); +} + QUnit.module("Fields", (hooks) => { hooks.beforeEach(() => { target = getFixture(); @@ -242,11 +250,7 @@ QUnit.module("Fields", (hooks) => { // Change the property type to "Date & Time" await editInput(target, ".o_field_property_definition_header input", "My Datetime"); - await click(target, ".o_field_property_definition_type button"); - await click( - target, - ".o_field_property_definition_type .dropdown-menu .dropdown-item:nth-child(6)" - ); + await changeType(target, "datetime"); assert.strictEqual(type.value, "Date & Time", "Should have changed the property type"); // Choosing a date in the date picker should not close the definition popover @@ -606,9 +610,7 @@ QUnit.module("Fields", (hooks) => { await click(target, ".o_property_field:nth-child(2) .o_field_property_open_popover"); let popover = target.querySelector(".o_property_field_popover"); // Select the tags type - await click(popover, ".o_field_property_definition_type input"); - await nextTick(); - await click(popover, ".o_field_property_definition_type .dropdown-item:nth-child(8)"); + await changeType(target, "tags"); // Create 3 tags const tagsInputSelector = ".o_property_field_popover .o_field_property_dropdown_menu input"; @@ -738,9 +740,7 @@ QUnit.module("Fields", (hooks) => { await click(target, ".o_property_field:nth-child(2) .o_field_property_open_popover"); const popover = target.querySelector(".o_property_field_popover"); // Select the many2one type - await click(popover, ".o_field_property_definition_type input"); - await nextTick(); - await click(popover, ".o_field_property_definition_type .dropdown-item:nth-child(9)"); + await changeType(target, "many2one"); // Choose the "User" model await click(popover, ".o_field_property_definition_model input"); @@ -790,7 +790,7 @@ QUnit.module("Fields", (hooks) => { { model: "res.users", display_name: "User" }, ]; } else if (method === "display_name_for" && model === "ir.model" && args[0][0] === "res.users") { - return [{"display_name": "User", "model": "res.users"}] + return [{"display_name": "User", "model": "res.users"}]; } else if (method === "name_create" && model === "res.users") { // Add a prefix to check that "name_create" // has been called with the right parameters @@ -827,9 +827,7 @@ QUnit.module("Fields", (hooks) => { await click(target, ".o_property_field:nth-child(2) .o_field_property_open_popover"); const popover = target.querySelector(".o_property_field_popover"); // Select the many2many type - await click(popover, ".o_field_property_definition_type input"); - await nextTick(); - await click(popover, ".o_field_property_definition_type .dropdown-item:nth-child(10)"); + await changeType(target, "many2many"); // Choose the "User" model await click(popover, ".o_field_property_definition_model input"); @@ -963,4 +961,95 @@ QUnit.module("Fields", (hooks) => { await clickSave(target); assert.verifySteps(["write", "read"]); }); + + /** + * Changing the type or the model of a property must regenerate it's name. + * (so if we change the type / model, all other property values on other records + * are set to False). + * Resetting the old model / type should reset the original name. + */ + QUnit.test("properties: name reset", async function (assert) { + async function mockRPC(route, { method, model, kwargs }) { + if (method === "check_access_rights") { + return true; + } else if (method === "get_available_models" && model === "ir.model") { + return [ + { model: "res.partner", display_name: "Partner" }, + { model: "res.users", display_name: "User" }, + ]; + } else if (method === "display_name_for" && model === "ir.model") { + return [ + {"display_name": "User", "model": "res.users"}, + {"display_name": "Partner", "model": "res.partner"}, + ]; + } else if (method === "search_count") { + return 5; + } + } + + await makeView({ + type: "form", + resModel: "partner", + resId: 1, + serverData, + arch: ` +
+ + + + + + +
`, + mockRPC, + }); + + assert.ok(target.querySelector('.o_property_field[property-name="property_2"]')); + + // open the definition popover + await click(target, ".o_property_field:nth-child(2) .o_field_property_open_popover"); + + // change the type to "many2one" + await changeType(target, "many2one"); + + // select the "User" model + await click(target, ".o_field_property_definition_model input"); + await click(target, ".o_field_property_definition_model .ui-menu-item:nth-child(2)"); + + await closePopover(target); + + // check that the name has been regenerated + let property = target.querySelector(".o_property_field:nth-child(2)"); + const propertyName2 = property.getAttribute("property-name"); + assert.ok(propertyName2 !== "property_2", "Name must have been regenerated"); + + // change back to "Selection" and verify that the original name is restored + await click(target, ".o_property_field:nth-child(2) .o_field_property_open_popover"); + await changeType(target, "selection"); + await closePopover(target); + property = target.querySelector(".o_property_field:nth-child(2)"); + const propertyName3 = property.getAttribute("property-name"); + assert.strictEqual(propertyName3, "property_2", "Name must have been restored"); + + // re-select many2one user + await click(target, ".o_property_field:nth-child(2) .o_field_property_open_popover"); + await changeType(target, "many2one"); + await click(target, ".o_field_property_definition_model input"); + await click(target, ".o_field_property_definition_model .ui-menu-item:nth-child(2)"); + property = target.querySelector(".o_property_field:nth-child(2)"); + const propertyName4 = property.getAttribute("property-name"); + + // save (if we do not save, the name will be the same even if + // we change the model, because it would be useless to regenerate it again) + await closePopover(target); + + // restore the model "User", and check that the name has been restored + await click(target, ".o_property_field:nth-child(2) .o_field_property_open_popover"); + await click(target, ".o_field_property_definition_model input"); + await click(target, ".o_field_property_definition_model .ui-menu-item:nth-child(2)"); + await closePopover(target); + property = target.querySelector(".o_property_field:nth-child(2)"); + const propertyName6 = property.getAttribute("property-name"); + assert.strictEqual(propertyName4, propertyName6); + }); });