From ad67d744249b8f78f7a8ef1ab11354d53a19836d Mon Sep 17 00:00:00 2001
From: "Robin Lejeune (role)" <role@odoo.com>
Date: Tue, 23 May 2023 08:49:10 +0000
Subject: [PATCH] [FIX] website: trigger visibility with prefilled form input
Form fields can be prefilled since [1] and their visibility
conditionally linked to other fields since [2].
1. This commit ensures prepopulated fields trigger the conditional
visibility mechanism (A), including when said field is first hidden,
then conditionally displayed (B).
2. Until [3], with a chain of visibility conditions, when one of them
was "contains", "does not contain" or a date-related condition, a
traceback was raised (C). This commit corrects the fix from [3] to
prevent this behavior now that the reason has been identified.
3. Tests are also set up to enforce these fixes.
Steps to reproduce (A):
- Drop a form
- Make field A depend on field B being set
- Have field B be autopopulated from user mail
- Save and check the form: A won't be shown despite B being set
- Removing one letter or typing anything in the input B will trigger
the visibility of A.
- Expected result: B should immediately appear.
Steps to reproduce (B):
- Drop a form
- Make field A depend on field B being set
- Have field B be autopopulated and depend on field C being set
- Field C is empty by default
- Save: only field C is shown. Type a letter: field B appears, but not
field A.
- Expected result: both should appear.
Note: this bug only occurs when field A is before field B.
Steps to reproduce (C) (until [3]):
- Drop a form
- Field A depends on B containing "abc"
- Field B depends on C being set
- Save and type something in C: there is a traceback.
task-3335536
[1]: https://github.com/odoo/odoo/commit/54873d2
[2]: https://github.com/odoo/odoo/commit/2dcbfec
[3]: https://github.com/odoo/odoo/commit/2714469
closes odoo/odoo#122044
Signed-off-by: Benoit Socias (bso) <bso@odoo.com>
---
.../static/src/snippets/s_website_form/000.js | 42 +++----
.../static/tests/tours/website_form_editor.js | 112 +++++++++++++++++-
2 files changed, 128 insertions(+), 26 deletions(-)
diff --git a/addons/website/static/src/snippets/s_website_form/000.js b/addons/website/static/src/snippets/s_website_form/000.js
index d1f92d26e719..aa3c40da0e2f 100644
--- a/addons/website/static/src/snippets/s_website_form/000.js
+++ b/addons/website/static/src/snippets/s_website_form/000.js
@@ -91,7 +91,6 @@ odoo.define('website.s_website_form', function (require) {
for (const [name, funcs] of visibilityFunctionsByFieldName.entries()) {
this._visibilityFunctionByFieldName.set(name, () => funcs.some(func => func()));
}
- this._updateFieldsVisibility();
this._onFieldInputDebounced = _.debounce(this._onFieldInput.bind(this), 400);
this.$el.on('input.s_website_form', '.s_website_form_field', this._onFieldInputDebounced);
@@ -177,6 +176,7 @@ odoo.define('website.s_website_form', function (require) {
}
}
}
+ this._updateFieldsVisibility();
// Check disabled states
this.inputEls = this.$target[0].querySelectorAll('.s_website_form_field.s_website_form_field_hidden_if .s_website_form_input');
@@ -582,32 +582,17 @@ odoo.define('website.s_website_form', function (require) {
* @returns {boolean}
*/
_compareTo(comparator, value = '', comparable, between) {
- const valueWasNull = (value === null);
- if (valueWasNull) {
- // TODO One customer apparently reached that case of receiving
- // a `null` value (actually possible when retrieving a form
- // data which is an unchecked checkbox for example) but combined
- // with an operator which really requires the received value to
- // be a string... but not sure how this is possible. In any
- // case, it should not make the form crash: it's just a
- // problematic *visibility* dependency, it should not prevent
- // using the form. So if the caller sends a `null` value, treat
- // it as an empty string. For now, let's also add an error in
- // the console so we may investigate the issue if it ever shows
- // up in a test (grep: COMPARE_TO_INVALID_OPERATOR)
+ // Value can be null when the compared field is supposed to be
+ // visible, but is not yet retrievable from the FormData() because
+ // the field was conditionally hidden. It can be considered empty.
+ if (value === null) {
value = '';
}
switch (comparator) {
case 'contains':
- if (valueWasNull) { // grep: COMPARE_TO_INVALID_OPERATOR
- console.error("This field value is null but also uses an invalid operator");
- }
return value.includes(comparable);
case '!contains':
- if (valueWasNull) { // grep: COMPARE_TO_INVALID_OPERATOR
- console.error("This field value is null but also uses an invalid operator");
- }
return !value.includes(comparable);
case 'equal':
case 'selected':
@@ -633,9 +618,6 @@ odoo.define('website.s_website_form', function (require) {
return value.name === '';
}
// Date & Date Time comparison requires formatting the value
- if (valueWasNull) { // grep: COMPARE_TO_INVALID_OPERATOR
- console.error("This field value is null but also uses an invalid operator");
- }
if (value.includes(':')) {
const datetimeFormat = time.getLangDatetimeFormat();
value = moment(value, datetimeFormat)._d.getTime() / 1000;
@@ -693,8 +675,20 @@ odoo.define('website.s_website_form', function (require) {
* Calculates the visibility for each field with conditional visibility
*/
_updateFieldsVisibility() {
+ let anyFieldVisibilityUpdated = false;
for (const [fieldEl, visibilityFunction] of this._visibilityFunctionByFieldEl.entries()) {
- this._updateFieldVisibility(fieldEl, visibilityFunction());
+ const wasVisible = !fieldEl.closest(".s_website_form_field")
+ .classList.contains("d-none");
+ const isVisible = !!visibilityFunction();
+ this._updateFieldVisibility(fieldEl, isVisible);
+ anyFieldVisibilityUpdated |= wasVisible !== isVisible;
+ }
+ // Recursive check needed in case of a field (C) that
+ // conditionally displays a prefilled field (B), which in turn
+ // triggers a conditional visibility on another field (A),
+ // registered before B.
+ if (anyFieldVisibilityUpdated) {
+ this._updateFieldsVisibility();
}
},
/**
diff --git a/addons/website/static/tests/tours/website_form_editor.js b/addons/website/static/tests/tours/website_form_editor.js
index 077ed2fa7a36..544934dae88e 100644
--- a/addons/website/static/tests/tours/website_form_editor.js
+++ b/addons/website/static/tests/tours/website_form_editor.js
@@ -37,6 +37,15 @@ odoo.define('website.tour.form_editor', function (require) {
.replaceAll(/`/g, character => `‘`);
};
+ const getFieldByLabel = (label) => {
+ return `.s_website_form_field:has(label:contains("${label}"))`;
+ };
+ const selectFieldByLabel = (label) => {
+ return [{
+ content: `Select field "${label}"`,
+ trigger: getFieldByLabel(label),
+ }];
+ };
const selectButtonByText = function (text) {
return [{
content: "Open the select",
@@ -359,6 +368,21 @@ odoo.define('website.tour.form_editor', function (require) {
trigger: 'we-input[data-attribute-name="value"] input',
run: 'text John Smith',
},
+
+ // Add two fields: the 1st one's visibility is tied to the 2nd one
+ // being set, and the 2nd one is autopopulated. As a result, both
+ // should be visible by default.
+ ...addCustomField("char", "text", "field A", false, {visibility: CONDITIONALVISIBILITY}),
+ ...addCustomField("char", "text", "field B", false),
+ ...selectFieldByLabel("field A"),
+ ...selectButtonByData('data-set-visibility-dependency="field B"'),
+ ...selectButtonByData('data-select-data-attribute="set"'),
+ ...selectFieldByLabel("field B"),
+ {
+ content: "Insert default value",
+ trigger: 'we-input[data-attribute-name="value"] input',
+ run: "text prefilled",
+ },
{
content: "Save the page",
trigger: "button[data-action=save]",
@@ -371,7 +395,21 @@ odoo.define('website.tour.form_editor', function (require) {
content: 'Verify that phone field is still auto-fillable',
trigger: '.s_website_form_field input[data-fill-with="phone"]:propValue("+1 555-555-5555")',
},
- // Check that if we edit again and save again the default value is not deleted.
+ // Check that the resulting form behavior is correct.
+ {
+ content: "Check that field B prefill text is set",
+ trigger: `${getFieldByLabel("field B")}:has(input[value="prefilled"])`,
+ run: () => null, // it's a check
+ }, {
+ content: "Check that field A is visible",
+ trigger: `.s_website_form:has(${getFieldByLabel("field A")}:visible)`,
+ run: () => null, // it's a check
+ },
+ // A) Check that if we edit again and save again the default value is
+ // not deleted.
+ // B) Add a 3rd field. Field A's visibility is tied to field B being set,
+ // field B is autopopulated and its visibility is tied to field C being
+ // set, and field C is empty.
{
content: 'Enter in edit mode again',
trigger: 'a[data-action="edit"]',
@@ -383,15 +421,85 @@ odoo.define('website.tour.form_editor', function (require) {
extra_trigger: 'button[data-action="save"]',
run: 'click',
},
- ...addCustomField('many2one', 'select', 'Select Field', true),
+ ...addCustomField("char", "text", "field C", false),
+ ...selectFieldByLabel("field B"),
+ ...selectButtonByText(CONDITIONALVISIBILITY),
+ ...selectButtonByData('data-set-visibility-dependency="field C"'),
+ ...selectButtonByData('data-select-data-attribute="set"'),
{
content: 'Save the page',
trigger: 'button[data-action=save]',
run: 'click',
},
+
+ // Check that the resulting form behavior is correct.
{
content: 'Verify that the value has not been deleted',
trigger: '.s_website_form_field:eq(0) input[value="John Smith"]',
+ }, {
+ content: "Check that fields A and B are not visible and that field B's prefill text is still set",
+ trigger: `.s_website_form:has(${getFieldByLabel("field A")}:not(:visible))` +
+ `:has(${getFieldByLabel("field B")}:has(input[value="prefilled"]):not(:visible))`,
+ run: () => null, // it's a check
+ }, {
+ content: "Type something in field C",
+ trigger: `${getFieldByLabel("field C")} input`,
+ run: "text Sesame",
+ }, {
+ content: "Check that fields A and B are visible",
+ trigger: `.s_website_form:has(${getFieldByLabel("field B")}:visible)` +
+ `:has(${getFieldByLabel("field A")}:visible)`,
+ run: () => null, // it's a check
+ },
+
+ // Have field A's visibility tied to field B containing something,
+ // while field B's visibility is also tied to another field.
+ {
+ content: "Enter edit mode",
+ trigger: "a[data-action=edit]",
+ }, {
+ content: "Check that we are in edit mode",
+ trigger: "#oe_snippets.o_loaded",
+ run: () => null,
+ },
+ ...selectFieldByLabel("field A"),
+ {
+ content: "Verify that the form editor appeared",
+ trigger: ".o_we_customize_panel .snippet-option-WebsiteFormEditor",
+ run: () => null,
+ },
+ ...selectButtonByData('data-select-data-attribute="contains"'),
+ {
+ content: "Tie the visibility of field A to field B containing 'peek-a-boo'",
+ trigger: "we-input[data-name=hidden_condition_additional_text] input",
+ run: "text peek-a-boo",
+ }, {
+ content: "Save the page",
+ trigger: "button[data-action=save]",
+ },
+
+ // Check that the resulting form works and does not raise an error.
+ {
+ content: "Wait for page reload",
+ trigger: 'body:not(.editor_enable) [data-snippet="s_website_form"]',
+ run: () => null,
+ }, {
+ content: "Write anything in C",
+ trigger: `${getFieldByLabel("field C")} input`,
+ run: "text Mellon",
+ }, {
+ content: "Check that field B is visible, but field A is not",
+ trigger: `.s_website_form:has(${getFieldByLabel("field B")}:visible)` +
+ `:has(${getFieldByLabel("field A")}:not(:visible))`,
+ run: () => null, // it's a check
+ }, {
+ content: "Insert 'peek-a-boo' in field B",
+ trigger: `${getFieldByLabel("field B")} input`,
+ run: "text peek-a-boo",
+ }, {
+ content: "Check that field A is visible",
+ trigger: `.s_website_form:has(${getFieldByLabel("field A")}:visible)`,
+ run: () => null, // it's a check
},
{
content: 'Enter in edit mode again',
--
GitLab