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 @@ -309,6 +309,26 @@ var BasicController = AbstractController.extend(FieldManagerMixin, { doIt(); } }, + /** + * 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 * 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 @@ -72,26 +72,6 @@ var FormController = BasicController.extend({ return self._setMode('edit'); }); }, - /** - * 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) @@ -252,6 +232,26 @@ var FormController = BasicController.extend({ return this.renderer.confirmChange(record, record.id, [fieldsChanged]); } }, + /** + * 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. * @@ -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(); }, }); diff --git a/addons/web/static/tests/fields/relational_fields_tests.js b/addons/web/static/tests/fields/relational_fields_tests.js index 916325b64b7..73aa28730e2 100644 --- a/addons/web/static/tests/fields/relational_fields_tests.js +++ b/addons/web/static/tests/fields/relational_fields_tests.js @@ -3699,6 +3699,51 @@ QUnit.module('relational_fields', { form.destroy(); }); + QUnit.test('editable list: multiple clicks on Add an item do not create invalid rows', function (assert) { + assert.expect(3); + + this.data.turtle.onchanges = { + turtle_trululu: function () {}, + }; + + var def; + var form = createView({ + View: FormView, + model: 'partner', + data: this.data, + arch: '
', + mockRPC: function (route, args) { + var result = this._super.apply(this, arguments); + if (args.method === 'onchange') { + return $.when(def).then(_.constant(result)); + } + return result; + }, + }); + + // click twice to add a new line + def = $.Deferred(); + form.$('.o_field_x2many_list_row_add a').click(); + form.$('.o_field_x2many_list_row_add a').click(); + assert.strictEqual(form.$('.o_data_row').length, 0, + "no row should have been created yet (waiting for the onchange)"); + + // resolve the onchange def + def.resolve(); + assert.strictEqual(form.$('.o_data_row').length, 1, + "only one row should have been created"); + assert.ok(form.$('.o_data_row:first').hasClass('o_selected_row'), + "the created row should be in edition"); + + form.destroy(); + }); + QUnit.module('FieldMany2Many'); QUnit.test('many2many kanban: edition', function (assert) { diff --git a/addons/web/static/tests/views/list_tests.js b/addons/web/static/tests/views/list_tests.js index 601e7a23533..0418158465b 100644 --- a/addons/web/static/tests/views/list_tests.js +++ b/addons/web/static/tests/views/list_tests.js @@ -2262,6 +2262,43 @@ QUnit.module('Views', { list.destroy(); }); + QUnit.test('multiple clicks on Add do not create invalid rows', function (assert) { + assert.expect(2); + + this.data.foo.onchanges = { + m2o: function () {}, + }; + + var def = $.Deferred(); + var list = createView({ + View: ListView, + model: 'foo', + data: this.data, + arch: '