From 66d022a4dc0f724ee6f317f8d5c4ee32ae44c83c Mon Sep 17 00:00:00 2001 From: "Maruan Aguerdouh (magm)" Date: Fri, 16 Jun 2023 12:54:24 +0000 Subject: [PATCH] [FIX] web: avoid crash when deleting a property multiple times Steps to reproduce the bug: - Go to Apps, and install the Knowledge App. - Go to Knowledge and create an article, and a child article. - Inside the child article, go to Properties tab and add a property. - Go to edit the property, and inside the popover,click on the delete button, atleast 2 times. - Now we got more than 1 popover to confirm the delete of the property, click delete on all of them. Issue: Once the first popover is confirmed, we properly delete the property but now we are going to get an error because the property is already deleted and we won't be able to find the property to delete. Solution: We need to check if we can find the property we want to delete before doing any further actions. opw-3371753 closes odoo/odoo#127584 X-original-commit: 7f5aed12fb143928cc82eeeea712363cb79b1171 Signed-off-by: Maruan Aguerdouh Mohtar (magm) --- .../fields/properties/properties_field.js | 9 ++-- .../views/fields/properties_field_tests.js | 41 +++++++++++++++++++ 2 files changed, 47 insertions(+), 3 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 959e2175b5b..e48829225dc 100644 --- a/addons/web/static/src/views/fields/properties/properties_field.js +++ b/addons/web/static/src/views/fields/properties/properties_field.js @@ -294,10 +294,13 @@ export class PropertiesField extends Component { confirm: () => { this.popover.close(); const propertiesDefinitions = this.propertiesList; - propertiesDefinitions.find( + const foundPropertyDefinition = propertiesDefinitions.find( (property) => property.name === propertyName - ).definition_deleted = true; - this.props.record.update({ [this.props.name]: propertiesDefinitions }); + ); + if (foundPropertyDefinition) { + foundPropertyDefinition.definition_deleted = true; + this.props.record.update({ [this.props.name]: propertiesDefinitions }); + } }, cancel: () => {}, }; 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 68c0ff4ad82..8893e69836f 100644 --- a/addons/web/static/tests/views/fields/properties_field_tests.js +++ b/addons/web/static/tests/views/fields/properties_field_tests.js @@ -1772,4 +1772,45 @@ QUnit.module("Fields", (hooks) => { assert.ok(target.querySelector(".o_test_properties_not_empty")); } ); + + QUnit.test( + "properties: confirm the delete of an already deleted property does not throw an error", + async function (assert) { + async function mockRPC(route, { method, model, kwargs }) { + if (["check_access_rights", "check_access_rule"].includes(method)) { + return true; + } + } + await makeView({ + type: "form", + resModel: "partner", + resId: 3, + serverData, + arch: ` +
+ + + + + + + +
`, + mockRPC, + }); + + // We open the property popover + await click(target, ".o_property_field:first-child .o_field_property_open_popover"); + + // We click on the delete button 2 times to get 2 confirmation popovers + await click(target, ".o_field_property_definition_delete"); + await click(target, ".o_field_property_definition_delete"); + + // We confirm both confirmation popovers + await click(target.querySelector(".modal-content .btn-primary")); + await click(target.querySelector(".modal-content .btn-primary")); + + assert.containsNone(target, ".modal-content"); + } + ); });