[FIX] web: selection field with value 0

The server doesn't make a distinction between an unset selection
field, and a value 0 (integer). So when 0 is a valid key for the
field, the server returns false as value if the field is unset, or
if its value is 0, which is not really convenient.

As a consequence, when the value is 0, it appears as unset in the
interface. To circumvent this, we automatically converts false
value of selection fields to 0, if 0 is a key of the selection.

4 files changed, 97 insertions, 24 deletions

diff --git a/addons/web/static/src/js/fields/field_utils.js b/addons/web/static/src/js/fields/field_utils.js
index 98fafcef7f1..aabf5e2ae95 100644
--- a/addons/web/static/src/js/fields/field_utils.js
+++ b/addons/web/static/src/js/fields/field_utils.js
@@ -296,12 +296,12 @@ function formatMonetary(value, field, options) {
  * @param {boolean} [options.escape=false] if true, escapes the formatted value
  */
 function formatSelection(value, field, options) {
-    if (!value) {
-        return '';
-    }
     var val = _.find(field.selection, function (option) {
         return option[0] === value;
     });
+    if (!val) {
+        return '';
+    }
     value = val[1];
     if (options && options.escape) {
         value = _.escape(value);
diff --git a/addons/web/static/src/js/fields/relational_fields.js b/addons/web/static/src/js/fields/relational_fields.js
index d65532165e8..7f8cbd2eb45 100644
--- a/addons/web/static/src/js/fields/relational_fields.js
+++ b/addons/web/static/src/js/fields/relational_fields.js
@@ -1671,7 +1671,7 @@ var FieldSelection = AbstractField.extend({
     },
 
     //--------------------------------------------------------------------------
-    // Private
+    // Public
     //--------------------------------------------------------------------------
 
     /**
@@ -1681,6 +1681,12 @@ var FieldSelection = AbstractField.extend({
     getFocusableElement: function () {
         return this.$el.is('select') ? this.$el : $();
     },
+    /**
+     * @override
+     */
+    isSet: function () {
+        return this.value !== false;
+    },
 
     //--------------------------------------------------------------------------
     // Private
diff --git a/addons/web/static/src/js/views/basic/basic_model.js b/addons/web/static/src/js/views/basic/basic_model.js
index 046c810c437..eef42f19bc3 100644
--- a/addons/web/static/src/js/views/basic/basic_model.js
+++ b/addons/web/static/src/js/views/basic/basic_model.js
@@ -1093,14 +1093,8 @@ var BasicModel = AbstractModel.extend({
                     });
                     defs.push(def);
                 }
-            } else if (field.type === 'date') {
-                // process data: convert into a moment instance
-                record._changes[name] = fieldUtils.parse.date(val);
-            } else if (field.type === 'datetime') {
-                // process datetime: convert into a moment instance
-                record._changes[name] = fieldUtils.parse.datetime(val);
             } else {
-                record._changes[name] = val;
+                record._changes[name] = self._parseServerValue(field, val);
             }
         });
         return $.when.apply($, defs);
@@ -2349,14 +2343,8 @@ var BasicModel = AbstractModel.extend({
                         _.each(_.keys(many2ones), function (name) {
                             defs.push(self._fetchNameGets(x2manyList, name));
                         });
-                    } else if (field.type === 'date') {
-                        // process date: convert into a moment instance
-                        record._changes[name] = fieldUtils.parse.date(result[name], field, {isUTC: true});
-                    } else if (field.type === 'datetime') {
-                        // process datetime: convert into a moment instance
-                        record._changes[name] = fieldUtils.parse.datetime(result[name], field, {isUTC: true});
                     } else {
-                        record._changes[name] = result[name];
+                        record._changes[name] = self._parseServerValue(field, result[name]);
                     }
                 });
                 return $.when.apply($, defs)
@@ -2426,16 +2414,36 @@ var BasicModel = AbstractModel.extend({
                     // no value for the many2one
                     record[fieldName] = false;
                 }
-            } else if (field.type === 'date') {
-                // process data: convert into a moment instance
-                record[fieldName] = fieldUtils.parse.date(val, field, {isUTC: true});
-            } else if (field.type === 'datetime') {
-                // process datetime: convert into a moment instance
-                record[fieldName] = fieldUtils.parse.datetime(val, field, {isUTC: true});
+            } else {
+                record[fieldName] = self._parseServerValue(field, val);
             }
         });
     },
     /**
+     * Processes date(time) and selection field values sent by the server.
+     * Converts data(time) values to moment instances.
+     * Converts false values of selection fields to 0 if 0 is a valid key,
+     * because the server doesn't make a distinction between false and 0, and
+     * always sends false when value is 0.
+     *
+     * @param {Object} field the field description
+     * @param {*} value
+     * @returns {*} the processed value
+     */
+    _parseServerValue: function (field, value) {
+        if (field.type === 'date' || field.type === 'datetime') {
+            // process date(time): convert into a moment instance
+            value = fieldUtils.parse[field.type](value, field, {isUTC: true});
+        } else if (field.type === 'selection' && value === false) {
+            // process selection: convert false to 0, if 0 is a valid key
+            var hasKey0 = _.find(field.selection, function (option) {
+                return option[0] === 0;
+            });
+            value = hasKey0 ? 0 : value;
+        }
+        return value;
+    },
+    /**
      * This method is quite important: it is supposed to perform the /onchange
      * rpc and apply the result.
      *
diff --git a/addons/web/static/tests/fields/relational_fields_tests.js b/addons/web/static/tests/fields/relational_fields_tests.js
index 2c6d35bc915..a6abf4be4b3 100644
--- a/addons/web/static/tests/fields/relational_fields_tests.js
+++ b/addons/web/static/tests/fields/relational_fields_tests.js
@@ -4004,6 +4004,65 @@ QUnit.module('relational_fields', {
         form.destroy();
     });
 
+    QUnit.test('unset selection field with 0 as key', function (assert) {
+        // The server doesn't make a distinction between false value (the field
+        // is unset), and selection 0, as in that case the value it returns is
+        // false. So the client must convert false to value 0 if it exists.
+        assert.expect(2);
+
+        this.data.partner.fields.selection = {
+            type: "selection",
+            selection: [[0, "Value O"], [1, "Value 1"]],
+        };
+
+        var form = createView({
+            View: FormView,
+            model: 'partner',
+            data: this.data,
+            arch: '<form string="Partners">' +
+                        '<field name="selection"/>' +
+                '</form>',
+            res_id: 1,
+        });
+
+        assert.strictEqual(form.$('.o_field_widget').text(), 'Value O',
+            "the displayed value should be 'Value O'");
+        assert.notOk(form.$('.o_field_widget').hasClass('o_field_empty'),
+            "should not have class o_field_empty");
+
+        form.destroy();
+    });
+
+    QUnit.test('unset selection field with string keys', function (assert) {
+        // The server doesn't make a distinction between false value (the field
+        // is unset), and selection 0, as in that case the value it returns is
+        // false. So the client must convert false to value 0 if it exists. In
+        // this test, it doesn't exist as keys are strings.
+        assert.expect(2);
+
+        this.data.partner.fields.selection = {
+            type: "selection",
+            selection: [['0', "Value O"], ['1', "Value 1"]],
+        };
+
+        var form = createView({
+            View: FormView,
+            model: 'partner',
+            data: this.data,
+            arch: '<form string="Partners">' +
+                        '<field name="selection"/>' +
+                '</form>',
+            res_id: 1,
+        });
+
+        assert.strictEqual(form.$('.o_field_widget').text(), '',
+            "there should be no displayed value");
+        assert.ok(form.$('.o_field_widget').hasClass('o_field_empty'),
+            "should have class o_field_empty");
+
+        form.destroy();
+    });
+
     QUnit.module('FieldMany2ManyTags');
 
     QUnit.test('fieldmany2many tags: rendering and edition', function (assert) {