From 4b7b89b04072f8bb44da45bb321c62002b6a11ed Mon Sep 17 00:00:00 2001
From: FrancoisGe <fge@odoo.com>
Date: Thu, 28 Jul 2022 09:44:05 +0000
Subject: [PATCH] [FIX] web: duplicate blank option in selection field

Before this commit, in a field selection, it was possible to have two
blank options.

To reproduce this problem, you need to have a non required selection
field and one of the selection sent by the server has the value false.

Our solution is to ignore the false selection returned by the server
and add the false option only if the field is not required.

closes odoo/odoo#96951

Signed-off-by: Aaron Bohy (aab) <aab@odoo.com>
---
 .../font_selection/font_selection_field.js    |  8 ++-
 .../font_selection/font_selection_field.xml   |  7 ---
 .../views/fields/selection/selection_field.js | 15 ++++--
 .../fields/selection/selection_field.xml      |  7 ---
 .../fields/font_selection_field_tests.js      | 17 +++++++
 .../views/fields/selection_field_tests.js     | 51 +++++++++++++++++++
 6 files changed, 86 insertions(+), 19 deletions(-)

diff --git a/addons/web/static/src/views/fields/font_selection/font_selection_field.js b/addons/web/static/src/views/fields/font_selection/font_selection_field.js
index fb10e1a7401..6b2024f8677 100644
--- a/addons/web/static/src/views/fields/font_selection/font_selection_field.js
+++ b/addons/web/static/src/views/fields/font_selection/font_selection_field.js
@@ -9,7 +9,13 @@ const { Component } = owl;
 
 export class FontSelectionField extends Component {
     get options() {
-        return Array.from(this.props.record.fields[this.props.name].selection);
+        const options = this.props.record.fields[this.props.name].selection.filter(
+            (option) => option[0] !== false && option[1] !== ""
+        );
+        if (!this.isRequired) {
+            options.unshift([false, this.props.placeholder || ""]);
+        }
+        return options;
     }
     get isRequired() {
         return this.props.record.isRequired(this.props.name);
diff --git a/addons/web/static/src/views/fields/font_selection/font_selection_field.xml b/addons/web/static/src/views/fields/font_selection/font_selection_field.xml
index 93ae586dbc1..b079da1164a 100644
--- a/addons/web/static/src/views/fields/font_selection/font_selection_field.xml
+++ b/addons/web/static/src/views/fields/font_selection/font_selection_field.xml
@@ -7,13 +7,6 @@
         </t>
         <t t-else="">
             <select class="o_input" t-on-change="onChange" t-attf-style="font-family:{{ props.value }};">
-                <t t-if="!isRequired">
-                    <option
-                        value="false"
-                        t-att-selected="!props.value"
-                        t-esc="props.placeholder"
-                    />
-                </t>
                 <t t-foreach="options" t-as="option" t-key="option[0]">
                     <option
                         t-att-selected="option[0] === value"
diff --git a/addons/web/static/src/views/fields/selection/selection_field.js b/addons/web/static/src/views/fields/selection/selection_field.js
index 8acda7141ac..b8927d7e6f6 100644
--- a/addons/web/static/src/views/fields/selection/selection_field.js
+++ b/addons/web/static/src/views/fields/selection/selection_field.js
@@ -8,14 +8,21 @@ const { Component } = owl;
 
 export class SelectionField extends Component {
     get options() {
+        let options;
         switch (this.props.record.fields[this.props.name].type) {
             case "many2one":
-                return this.props.record.preloadedData[this.props.name];
+                options = [...this.props.record.preloadedData[this.props.name]];
+                break;
             case "selection":
-                return this.props.record.fields[this.props.name].selection;
-            default:
-                return [];
+                options = this.props.record.fields[this.props.name].selection.filter(
+                    (option) => option[0] !== false && option[1] !== ""
+                );
+                break;
         }
+        if (!this.isRequired) {
+            options.unshift([false, this.props.placeholder || ""]);
+        }
+        return options;
     }
     get string() {
         switch (this.props.type) {
diff --git a/addons/web/static/src/views/fields/selection/selection_field.xml b/addons/web/static/src/views/fields/selection/selection_field.xml
index 4f6b1a17fc1..82a4abd2b60 100644
--- a/addons/web/static/src/views/fields/selection/selection_field.xml
+++ b/addons/web/static/src/views/fields/selection/selection_field.xml
@@ -7,13 +7,6 @@
         </t>
         <t t-else="">
             <select class="o_input" t-on-change="onChange">
-                <t t-if="!isRequired">
-                    <option
-                        value="false"
-                        t-att-selected="!props.value"
-                        t-esc="props.placeholder"
-                    />
-                </t>
                 <t t-foreach="options" t-as="option" t-key="option[0]">
                     <option
                         t-att-selected="option[0] === value"
diff --git a/addons/web/static/tests/views/fields/font_selection_field_tests.js b/addons/web/static/tests/views/fields/font_selection_field_tests.js
index 79296eebcda..912fffa0b2e 100644
--- a/addons/web/static/tests/views/fields/font_selection_field_tests.js
+++ b/addons/web/static/tests/views/fields/font_selection_field_tests.js
@@ -71,4 +71,21 @@ QUnit.module("Fields", (hooks) => {
             "Widget font should be updated (Oswald)"
         );
     });
+
+    QUnit.test(
+        "FontSelectionField displays one blank option (not required)",
+        async function (assert) {
+            serverData.models.partner.fields.fonts.selection = [
+                [false, ""],
+                ...serverData.models.partner.fields.fonts.selection,
+            ];
+            await makeView({
+                serverData,
+                type: "form",
+                resModel: "partner",
+                arch: '<form><field name="fonts" widget="font"/></form>',
+            });
+            assert.containsN(target.querySelector(".o_field_widget[name='fonts']"), "option", 3);
+        }
+    );
 });
diff --git a/addons/web/static/tests/views/fields/selection_field_tests.js b/addons/web/static/tests/views/fields/selection_field_tests.js
index 9d5c586d1ab..015e39da84e 100644
--- a/addons/web/static/tests/views/fields/selection_field_tests.js
+++ b/addons/web/static/tests/views/fields/selection_field_tests.js
@@ -396,6 +396,57 @@ QUnit.module("Fields", (hooks) => {
         );
     });
 
+    QUnit.test(
+        "required selection widget should have only one blank option",
+        async function (assert) {
+            serverData.models.partner.fields.feedback_value = {
+                type: "selection",
+                required: true,
+                selection: [
+                    ["good", "Good"],
+                    ["bad", "Bad"],
+                ],
+                default: "good",
+                string: "Good",
+            };
+            serverData.models.partner.fields.color.selection = [
+                [false, ""],
+                ...serverData.models.partner.fields.color.selection,
+            ];
+
+            await makeView({
+                type: "form",
+                resModel: "partner",
+                resId: 1,
+                serverData,
+                arch: `
+                <form>
+                    <field name="feedback_value" />
+                    <field name="color" attrs="{'required': [('feedback_value', '=', 'bad')]}" />
+                </form>`,
+            });
+
+            await click(target, ".o_form_button_edit");
+
+            assert.containsN(
+                target.querySelector(".o_field_widget[name='color']"),
+                "option",
+                3,
+                "Three options in non required field (one blank option)"
+            );
+
+            // change value to update widget modifier values
+            await editSelect(target, ".o_field_widget[name='feedback_value'] select", '"bad"');
+
+            assert.containsN(
+                target.querySelector(".o_field_widget[name='color']"),
+                "option",
+                2,
+                "Still three options in required selection (one blank option)"
+            );
+        }
+    );
+
     QUnit.test("selection field with placeholder", async function (assert) {
         await makeView({
             type: "form",