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 67a4fa504d8..82dc6f90d26 100644 --- a/addons/web/static/src/js/views/basic/basic_controller.js +++ b/addons/web/static/src/js/views/basic/basic_controller.js @@ -104,39 +104,15 @@ var BasicController = AbstractController.extend(FieldManagerMixin, { return true; }, /** - * Discards the changes made to the record whose ID is given, if necessary. - * Automatically leaves to default mode for the given record. + * Waits for the mutex to be unlocked and then calls _.discardChanges. + * This ensures that the confirm dialog isn't displayed directly if there is + * a pending 'write' rpc. * - * @param {string} [recordID] - default to main recordID - * @param {Object} [options] - * @param {boolean} [options.readonlyIfRealDiscard=false] - * After discarding record changes, the usual option is to make the - * record readonly. However, the view manager calls this function - * at inappropriate times in the current code and in that case, we - * don't want to go back to readonly if there is nothing to discard - * (e.g. when switching record in edit mode in form view, we expect - * the new record to be in edit mode too, but the view manager calls - * this function as the URL changes...) @todo get rid of this when - * the view manager is improved. - * @returns {Deferred} + * @see _.discardChanges */ discardChanges: function (recordID, options) { - var self = this; - recordID = recordID || this.handle; - return this.canBeDiscarded(recordID).then(function (needDiscard) { - if (options && options.readonlyIfRealDiscard && !needDiscard) { - return; - } - - if (needDiscard) { // Just some optimization - self.model.discardChanges(recordID); - } - if (self.model.isNew(recordID)) { - self._abandonRecord(recordID); - return; - } - return self._confirmSave(recordID); - }); + return this.mutex.exec(function () {}) + .then(this._discardChanges.bind(this, recordID || this.handle, options)); }, /** * Method that will be overriden by the views with the ability to have selected ids @@ -326,6 +302,42 @@ var BasicController = AbstractController.extend(FieldManagerMixin, { this.$buttons.find('button').attr('disabled', true); } }, + /** + * Discards the changes made to the record whose ID is given, if necessary. + * Automatically leaves to default mode for the given record. + * + * @private + * @param {string} [recordID] - default to main recordID + * @param {Object} [options] + * @param {boolean} [options.readonlyIfRealDiscard=false] + * After discarding record changes, the usual option is to make the + * record readonly. However, the view manager calls this function + * at inappropriate times in the current code and in that case, we + * don't want to go back to readonly if there is nothing to discard + * (e.g. when switching record in edit mode in form view, we expect + * the new record to be in edit mode too, but the view manager calls + * this function as the URL changes...) @todo get rid of this when + * the view manager is improved. + * @returns {Deferred} + */ + _discardChanges: function (recordID, options) { + var self = this; + recordID = recordID || this.handle; + return this.canBeDiscarded(recordID) + .then(function (needDiscard) { + if (options && options.readonlyIfRealDiscard && !needDiscard) { + return; + } + if (needDiscard) { // Just some optimization + self.model.discardChanges(recordID); + } + if (self.model.isNew(recordID)) { + self._abandonRecord(recordID); + return; + } + return self._confirmSave(recordID); + }); + }, /** * Enables buttons so they can be clicked again. * @@ -501,11 +513,8 @@ var BasicController = AbstractController.extend(FieldManagerMixin, { var self = this; ev.stopPropagation(); var recordID = ev.data.recordID; - this.discardChanges(recordID) + this._discardChanges(recordID) .done(function () { - if (self.model.isNew(recordID)) { - self._abandonRecord(recordID); - } // TODO this will tell the renderer to rerender the widget that // asked for the discard but will unfortunately lose the click // made on another row if any 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 8a42168a098..2b607496e85 100644 --- a/addons/web/static/src/js/views/form/form_controller.js +++ b/addons/web/static/src/js/views/form/form_controller.js @@ -409,7 +409,7 @@ var FormController = BasicController.extend({ * @private */ _onDiscard: function () { - this.discardChanges(); + this._discardChanges(); }, /** * Called when the user clicks on 'Duplicate Record' in the sidebar 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 ee0fd323ce1..2ec9cf3181e 100644 --- a/addons/web/static/src/js/views/list/list_controller.js +++ b/addons/web/static/src/js/views/list/list_controller.js @@ -49,27 +49,6 @@ var ListController = BasicController.extend({ // Public //-------------------------------------------------------------------------- - /** - * To improve performance, list view must not be rerendered if it is asked - * to discard all its changes. Indeed, only the in-edition row needs to be - * discarded in that case. - * - * @override - * @param {string} [recordID] - default to main recordID - * @returns {Deferred} - */ - discardChanges: function (recordID) { - if ((recordID || this.handle) === this.handle) { - recordID = this.renderer.getEditableRecordID(); - if (recordID === null) { - return $.when(); - } - } - var self = this; - return this._super(recordID).then(function () { - self._updateButtons('readonly'); - }); - }, /** * Calculate the active domain of the list view. This should be done only * if the header checkbox has been checked. This is done by evaluating the @@ -245,6 +224,28 @@ var ListController = BasicController.extend({ return this.renderer.updateState(state, {noRender: true}) .then(this._setMode.bind(this, 'readonly', id)); }, + /** + * To improve performance, list view must not be rerendered if it is asked + * to discard all its changes. Indeed, only the in-edition row needs to be + * discarded in that case. + * + * @override + * @private + * @param {string} [recordID] - default to main recordID + * @returns {Deferred} + */ + _discardChanges: function (recordID) { + if ((recordID || this.handle) === this.handle) { + recordID = this.renderer.getEditableRecordID(); + if (recordID === null) { + return $.when(); + } + } + var self = this; + return this._super(recordID).then(function () { + self._updateButtons('readonly'); + }); + }, /** * @override * @private @@ -362,7 +363,7 @@ var ListController = BasicController.extend({ */ _onDiscard: function (ev) { ev.stopPropagation(); // So that it is not considered as a row leaving - this.discardChanges(); + this._discardChanges(); }, /** * Called when the user asks to edit a row -> Updates the controller buttons diff --git a/addons/web/static/tests/views/form_tests.js b/addons/web/static/tests/views/form_tests.js index b15e341adb6..e9716603d60 100644 --- a/addons/web/static/tests/views/form_tests.js +++ b/addons/web/static/tests/views/form_tests.js @@ -2883,7 +2883,7 @@ QUnit.module('Views', { }); QUnit.test('onchanges that complete after discarding', function (assert) { - assert.expect(4); + assert.expect(6); var def1 = $.Deferred(); @@ -2922,14 +2922,65 @@ QUnit.module('Views', { // discard changes form.$buttons.find('.o_form_button_cancel').click(); + assert.strictEqual(form.$('.o_field_widget[name="foo"]').val(), "1234", + "field foo should still contain new value"); + assert.strictEqual($('.modal').length, 0, + "Confirm dialog should not be displayed yet"); + + // complete the onchange + def1.resolve(); + assert.strictEqual($('.modal').length, 1, + "Confirm dialog should be displayed"); $('.modal .modal-footer .btn-primary').click(); assert.strictEqual(form.$('span[name="foo"]').text(), "blip", "field foo should still be displayed to initial value"); - // complete the onchange - def1.resolve(); - assert.strictEqual(form.$('span[name="foo"]').text(), "blip", - "field foo should still be displayed to initial value"); + form.destroy(); + }); + + QUnit.test('discarding before save returns', function (assert) { + assert.expect(4); + + var def = $.Deferred(); + + var form = createView({ + View: FormView, + model: 'partner', + data: this.data, + arch: '
' + + '' + + '
', + res_id: 2, + mockRPC: function (route, args) { + var result = this._super.apply(this, arguments); + if (args.method === 'write') { + return def.then(_.constant(result)); + } + return result; + }, + viewOptions: { + mode: 'edit', + }, + }); + + form.$('input').val("1234").trigger('input'); + + // save the value and discard directly + form.$buttons.find('.o_form_button_save').click(); + form.discardChanges(); // Simulate click on breadcrumb + + assert.strictEqual(form.$('.o_field_widget[name="foo"]').val(), "1234", + "field foo should still contain new value"); + assert.strictEqual($('.modal').length, 0, + "Confirm dialog should not be displayed"); + + // complete the write + def.resolve(); + + assert.strictEqual($('.modal').length, 0, + "Confirm dialog should not be displayed"); + assert.strictEqual(form.$('.o_field_widget[name="foo"]').text(), "1234", + "value should have been saved and rerendered in readonly"); form.destroy(); });