From e5f2b9d16499694be5701b39be304e029ff2f2ba Mon Sep 17 00:00:00 2001 From: Guillaume-gdi Date: Thu, 4 Apr 2024 08:43:58 +0000 Subject: [PATCH] [FIX] website: prevent void visibility comparator on form fields [This first commit] made it possible to have an error when there was no comparator for a field with conditional visibility. [This second commit] prevented an error from occurring in this case. The purpose of this commit is to prevent the user from getting a conditional visibility configuration for a form field where there is no comparator. Steps to reproduce the problem: - Go to /contactus. - Edit page. - Click on the "Your Company" field. - Select "Visible Only If" for the "Visibility" option. - Click on "Visible Only If" again. => The comparator is not defined. Another way to have the issue was: - Drop a form on a page. - Click on the "Your Company" field. - Select "Visible Only If" for the "Visibility" option. - Set visible only if Your Name is equal to "test" as condition. - Click on Your Name field. - Change the field type to Radio Buttons. => The comparator is not defined. This commit fixes those two cases. Technical information: When we change the field's visibility to conditional (`setVisibility`), we add a default visibility dependency (`_setVisibilityDependency`). At this point, the comparator is removed and added in `_renderCustomXML`. `_renderCustomXML` was only called if the visibility dependency had changed. [This first commit]: https://github.com/odoo/odoo/commit/910897fc97d87b08f01627094ec8c159f5267628 [This second commit]: https://github.com/odoo/odoo/commit/808780c89cfba940957f7410f787de31e31bda27 opw-3806409 closes odoo/odoo#161527 X-original-commit: 770cc162d6de82818d8e29bf156bb01f65ca8e2c Signed-off-by: Outagant Mehdi (mou) Signed-off-by: Guillaume Dieleman (gdi) --- .../src/snippets/s_website_form/options.js | 15 ++++++----- .../static/tests/tours/website_form_editor.js | 27 +++++++++++++++++++ 2 files changed, 35 insertions(+), 7 deletions(-) diff --git a/addons/website/static/src/snippets/s_website_form/options.js b/addons/website/static/src/snippets/s_website_form/options.js index 8c4555dad53..da8bfdc5874 100644 --- a/addons/website/static/src/snippets/s_website_form/options.js +++ b/addons/website/static/src/snippets/s_website_form/options.js @@ -1485,7 +1485,9 @@ options.registry.WebsiteFieldEditor = FieldEditor.extend({ const inputEl = this.$target[0].querySelector('input'); const dataFillWith = inputEl ? inputEl.dataset.fillWith : undefined; const hasConditionalVisibility = this.$target[0].classList.contains('s_website_form_field_hidden_if'); - const previousName = this.$target[0].querySelector('.s_website_form_input').name; + const previousInputEl = this.$target[0].querySelector('.s_website_form_input'); + const previousName = previousInputEl.name; + const previousType = previousInputEl.type; [...this.$target[0].childNodes].forEach(node => node.remove()); [...fieldEl.childNodes].forEach(node => this.$target[0].appendChild(node)); [...fieldEl.attributes].forEach(el => this.$target[0].removeAttribute(el.nodeName)); @@ -1494,8 +1496,10 @@ options.registry.WebsiteFieldEditor = FieldEditor.extend({ this.$target[0].classList.add('s_website_form_field_hidden_if', 'd-none'); } const dependentFieldEls = this.formEl.querySelectorAll(`.s_website_form_field[data-visibility-dependency="${previousName}"]`); - const newName = this.$target[0].querySelector('.s_website_form_input').name; - if (previousName !== newName && dependentFieldEls) { + const newFormInputEl = this.$target[0].querySelector('.s_website_form_input'); + const newName = newFormInputEl.name; + const newType = newFormInputEl.type; + if ((previousName !== newName || previousType !== newType) && dependentFieldEls) { // In order to keep the visibility conditions consistent, // when the name has changed, it means that the type has changed so // all fields whose visibility depends on this field must be updated so that @@ -1517,10 +1521,7 @@ options.registry.WebsiteFieldEditor = FieldEditor.extend({ _setVisibilityDependency(value) { delete this.$target[0].dataset.visibilityCondition; delete this.$target[0].dataset.visibilityComparator; - const previousDependency = this._getDependencyEl(); - if (this.formEl.querySelector(`.s_website_form_input[name="${value}"]`).type !== (previousDependency && previousDependency.type)) { - this.rerender = true; - } + this.rerender = true; this.$target[0].dataset.visibilityDependency = value; }, /** diff --git a/addons/website/static/tests/tours/website_form_editor.js b/addons/website/static/tests/tours/website_form_editor.js index 3cb1a46c18f..c1a429bba49 100644 --- a/addons/website/static/tests/tours/website_form_editor.js +++ b/addons/website/static/tests/tours/website_form_editor.js @@ -227,6 +227,23 @@ trigger: "iframe .s_website_form_field:contains('Test conditional visibility') .s_website_form_field_description", isCheck: true, }, + // Check that visibility condition is deleted on dependency type change. + ...addCustomField("char", "text", "dependent", false, {visibility: CONDITIONALVISIBILITY}), + ...addCustomField("selection", "radio", "dependency", false), + ...selectFieldByLabel("dependent"), + ...selectButtonByData('data-set-visibility-dependency="dependency"'), + ...selectFieldByLabel("dependency"), + ...selectButtonByData('data-custom-field="char"'), + ...selectFieldByLabel("dependent"), + { + content: "Open the select", + trigger: 'we-select:has(we-button[data-set-visibility="visible"]) we-toggler', + }, + { + content: "Check that the field no longer has conditional visibility", + trigger: "we-select we-button[data-set-visibility='visible'].active", + isCheck: true, + }, ...addExistingField('date', 'text', 'Test Date', true), @@ -459,6 +476,16 @@ ...addCustomField("char", "text", "field C", false), ...selectFieldByLabel("field B"), ...selectButtonByText(CONDITIONALVISIBILITY), + ...selectButtonByText(CONDITIONALVISIBILITY), + { + content: "Check that there is a comparator after two clicks on 'Visible only if'", + trigger: "[data-attribute-name='visibilityComparator']", + run: function () { + if (!this.$anchor[0].querySelector("we-button.active")) { + console.error("A default comparator should be set"); + } + }, + }, ...selectButtonByData('data-set-visibility-dependency="field C"'), ...selectButtonByData('data-select-data-attribute="set"'), ...wTourUtils.clickOnSave(),