[FIX] web: editable list: don't create invalid records

Clicking multiple times on 'Create' (in main list views) or 'Add an
item' (in o2m lists) might create invalid rows (i.e. rows with
required fields unset).

Creating a new record requires two sequential RPCs: a default_get
and an onchange (only if there is an onchange of one of the
record's field). When clicking twice to create a new record, that
sequence of RPCs is done twice in parallel, after unselecting the
current row (i.e. saving it if it is valid). However, in both
cases, there is no line to unselect yet (as both operations are
done basically simultaneously). When the first onchange returns,
a first new record is added to the list. When the second returns,
the second one is added and takes the edition, leaving the first
one, invalid, in the list.

This rev. ensures this doesn't happen by preventing concurrent
record creation.

5 files changed, 114 insertions, 76 deletions

diff --git a/addons/web/static/src/js/fields/relational_fields.js b/addons/web/static/src/js/fields/relational_fields.js
index f223fca68a4..a5f9b443ae0 100644
--- a/addons/web/static/src/js/fields/relational_fields.js
+++ b/addons/web/static/src/js/fields/relational_fields.js
@@ -914,6 +914,16 @@ var FieldOne2Many = FieldX2Many.extend({
     className: 'o_field_one2many',
     supportedFieldTypes: ['one2many'],
 
+    /**
+     * @override
+     */
+    init: function () {
+        this._super.apply(this, arguments);
+
+        // boolean used to prevent concurrent record creation
+        this.creatingRecord = false;
+    },
+
     //--------------------------------------------------------------------------
     // Public
     //--------------------------------------------------------------------------
@@ -931,6 +941,7 @@ var FieldOne2Many = FieldX2Many.extend({
                     var index = self.editable === 'top' ? 0 : self.value.data.length - 1;
                     var newID = self.value.data[index].id;
                     self.renderer.editRecord(newID);
+                    self.creatingRecord = false;
                 }
             }
         });
@@ -984,10 +995,13 @@ var FieldOne2Many = FieldX2Many.extend({
         ev.stopPropagation();
 
         if (this.editable) {
-            this._setValue({
-                operation: 'CREATE',
-                position: this.editable,
-            });
+            if (!this.creatingRecord) {
+                this.creatingRecord = true;
+                this._setValue({
+                    operation: 'CREATE',
+                    position: this.editable,
+                });
+            }
         } else {
             var self = this;
             this._openFormDialog({
diff --git a/addons/web/static/src/js/views/basic/basic_controller.js b/addons/web/static/src/js/views/basic/basic_controller.js
index 9f767799851..53e5be8ff6e 100644
--- a/addons/web/static/src/js/views/basic/basic_controller.js
+++ b/addons/web/static/src/js/views/basic/basic_controller.js
@@ -310,6 +310,26 @@ var BasicController = AbstractController.extend(FieldManagerMixin, {
         }
     },
     /**
+     * Disables buttons so that they can't be clicked anymore.
+     *
+     * @private
+     */
+    _disableButtons: function () {
+        if (this.$buttons) {
+            this.$buttons.find('button').attr('disabled', true);
+        }
+    },
+    /**
+     * Enables buttons so they can be clicked again.
+     *
+     * @private
+     */
+    _enableButtons: function () {
+        if (this.$buttons) {
+            this.$buttons.find('button').removeAttr('disabled');
+        }
+    },
+    /**
      * Returns the new sidebar env
      *
      * @private
diff --git a/addons/web/static/src/js/views/form/form_controller.js b/addons/web/static/src/js/views/form/form_controller.js
index 8ae66848b75..6adb474303d 100644
--- a/addons/web/static/src/js/views/form/form_controller.js
+++ b/addons/web/static/src/js/views/form/form_controller.js
@@ -73,26 +73,6 @@ var FormController = BasicController.extend({
         });
     },
     /**
-     * Disable buttons so that they can't be clicked anymore
-     *
-     */
-    disableButtons: function () {
-        if (this.$buttons) {
-            this.$buttons.find('button').attr('disabled', true);
-        }
-        this.renderer.disableButtons();
-    },
-    /**
-     * Enable buttons so they can be clicked again
-     *
-     */
-    enableButtons: function () {
-        if (this.$buttons) {
-            this.$buttons.find('button').removeAttr('disabled');
-        }
-        this.renderer.enableButtons();
-    },
-    /**
      * Returns the current res_id, wrapped in a list. This is only used by the
      * sidebar (and the debugmanager)
      *
@@ -253,6 +233,26 @@ var FormController = BasicController.extend({
         }
     },
     /**
+     * Override to disable buttons in the renderer.
+     *
+     * @override
+     * @private
+     */
+    _disableButtons: function () {
+        this._super.apply(this, arguments);
+        this.renderer.disableButtons();
+    },
+    /**
+     * Override to enable buttons in the renderer.
+     *
+     * @override
+     * @private
+     */
+    _enableButtons: function () {
+        this._super.apply(this, arguments);
+        this.renderer.enableButtons();
+    },
+    /**
      * Hook method, called when record(s) has been deleted.
      *
      * @override
@@ -347,7 +347,7 @@ var FormController = BasicController.extend({
         var self = this;
         var def;
 
-        this.disableButtons();
+        this._disableButtons();
 
         var attrs = event.data.attrs;
         if (attrs.confirm) {
@@ -380,9 +380,7 @@ var FormController = BasicController.extend({
             });
         }
 
-        def.always(function() {
-            self.enableButtons();
-        });
+        def.always(this._enableButtons.bind(this));
     },
     /**
      * Called when the user wants to create a new record -> @see createRecord
diff --git a/addons/web/static/src/js/views/list/list_controller.js b/addons/web/static/src/js/views/list/list_controller.js
index 3c86b7c222f..7c83022cd60 100644
--- a/addons/web/static/src/js/views/list/list_controller.js
+++ b/addons/web/static/src/js/views/list/list_controller.js
@@ -194,19 +194,25 @@ var ListController = BasicController.extend({
         }
     },
     /**
-     * Add a record to the list
+     * Adds a record to the list.
+     * Disables the buttons to prevent concurrent record creation or edition.
      *
      * @todo make record creation a basic controller feature
      * @private
      */
     _addRecord: function () {
         var self = this;
-        this.model.addDefaultRecord(this.handle, {position: this.editable}).then(function (recordID) {
+        this._disableButtons();
+        return this.renderer.unselectRow().then(function () {
+            return self.model.addDefaultRecord(self.handle, {
+                position: self.editable,
+            });
+        }).then(function (recordID) {
             self._toggleNoContentHelper(false);
             var state = self.model.get(self.handle);
             self.renderer.updateState(state, {});
             self.renderer.editRecord(recordID);
-        });
+        }).always(this._enableButtons.bind(this));
     },
     /**
      * Archive the current selection
diff --git a/addons/web/static/src/js/views/list/list_editable_renderer.js b/addons/web/static/src/js/views/list/list_editable_renderer.js
index a65d4f685b7..a037c12a96e 100644
--- a/addons/web/static/src/js/views/list/list_editable_renderer.js
+++ b/addons/web/static/src/js/views/list/list_editable_renderer.js
@@ -224,6 +224,44 @@ ListRenderer.include({
 
         return $.when.apply($, defs);
     },
+    /**
+     * This method is called whenever we click/move outside of a row that was
+     * in edit mode. This is the moment we save all accumulated changes on that
+     * row, if needed (@see BasicController.saveRecord).
+     *
+     * Note that we have to disable the focusable elements (inputs, ...) to
+     * prevent subsequent editions. These edits would be lost, because the list
+     * view only saves records when unselecting a row.
+     *
+     * @returns {Deferred} The deferred resolves if the row was unselected (and
+     *   possibly removed). If may be rejected, when the row is dirty and the
+     *   user refuses to discard its changes.
+     */
+    unselectRow: function () {
+        // Protect against calling this method when no row is selected
+        if (this.currentRow === null) {
+            return $.when();
+        }
+
+        var record = this.state.data[this.currentRow];
+        var recordWidgets = this.allFieldWidgets[record.id];
+        toggleWidgets(true);
+
+        var def = $.Deferred();
+        this.trigger_up('save_line', {
+            recordID: record.id,
+            onSuccess: def.resolve.bind(def),
+            onFailure: def.reject.bind(def),
+        });
+        return def.fail(toggleWidgets.bind(null, false));
+
+        function toggleWidgets(disabled) {
+            _.each(recordWidgets, function (widget) {
+                var $el = widget.getFocusableElement();
+                $el.prop('disabled', disabled);
+            });
+        }
+    },
 
     //--------------------------------------------------------------------------
     // Private
@@ -263,7 +301,7 @@ ListRenderer.include({
         if (this.currentRow > 0) {
             this._selectCell(this.currentRow - 1, this.columns.length - 1);
         } else {
-            this._unselectRow().then(this.trigger_up.bind(this, 'add_record'));
+            this.unselectRow().then(this.trigger_up.bind(this, 'add_record'));
         }
     },
     /**
@@ -282,7 +320,7 @@ ListRenderer.include({
         if (this.currentRow < this.state.data.length - 1) {
             this._selectCell(this.currentRow + 1, 0);
         } else {
-            this._unselectRow().then(this.trigger_up.bind(this, 'add_record'));
+            this.unselectRow().then(this.trigger_up.bind(this, 'add_record'));
         }
     },
     /**
@@ -427,7 +465,7 @@ ListRenderer.include({
 
         // To select a row, the currently selected one must be unselected first
         var self = this;
-        return this._unselectRow().then(function () {
+        return this.unselectRow().then(function () {
             // Notify the controller we want to make a record editable
             var record = self.state.data[rowIndex];
             var def = $.Deferred();
@@ -438,44 +476,6 @@ ListRenderer.include({
             return def;
         });
     },
-    /**
-     * This method is called whenever we click/move outside of a row that was
-     * in edit mode. This is the moment we save all accumulated changes on that
-     * row, if needed (@see BasicController.saveRecord).
-     *
-     * Note that we have to disable the focusable elements (inputs, ...) to
-     * prevent subsequent editions. These edits would be lost, because the list
-     * view only saves records when unselecting a row.
-     *
-     * @returns {Deferred} The deferred resolves if the row was unselected (and
-     *   possibly removed). If may be rejected, when the row is dirty and the
-     *   user refuses to discard its changes.
-     */
-    _unselectRow: function () {
-        // Protect against calling this method when no row is selected
-        if (this.currentRow === null) {
-            return $.when();
-        }
-
-        var record = this.state.data[this.currentRow];
-        var recordWidgets = this.allFieldWidgets[record.id];
-        toggleWidgets(true);
-
-        var def = $.Deferred();
-        this.trigger_up('save_line', {
-            recordID: record.id,
-            onSuccess: def.resolve.bind(def),
-            onFailure: def.reject.bind(def),
-        });
-        return def.fail(toggleWidgets.bind(null, false));
-
-        function toggleWidgets(disabled) {
-            _.each(recordWidgets, function (widget) {
-                var $el = widget.getFocusableElement();
-                $el.prop('disabled', disabled);
-            });
-        }
-    },
 
     //--------------------------------------------------------------------------
     // Handlers
@@ -497,7 +497,7 @@ ListRenderer.include({
 
         // but we do want to unselect current row
         var self = this;
-        this._unselectRow().then(function () {
+        this.unselectRow().then(function () {
             self.trigger_up('add_record'); // TODO write a test, the deferred was not considered
         });
     },
@@ -523,7 +523,7 @@ ListRenderer.include({
      * We need to manually unselect row, because noone else would do it
      */
     _onEmptyRowClick: function () {
-        this._unselectRow();
+        this.unselectRow();
     },
     /**
      * Clicking on a footer should unselect (and save) the currently selected
@@ -531,7 +531,7 @@ ListRenderer.include({
      * and _onWindowClicked ignore those clicks.
      */
     _onFooterClick: function () {
-        this._unselectRow();
+        this.unselectRow();
     },
     /**
      * Handles the keyboard navigation according to events triggered by field
@@ -677,7 +677,7 @@ ListRenderer.include({
             return;
         }
 
-        this._unselectRow();
+        this.unselectRow();
     },
 });