pengyuthon/odoo_source
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

[FIX] web: basic_controller: wait for the mutex before discard changes

Browse Source 
Before this commit, the discard confirm dialog was directly shown
without waiting for the write RPC. For instance, it the user clicked on 'Save'
after editing a record, and then directly clicked on another menu or on the
breadcrumb before the write RPC returned, the record was still considered
as dirty and thus the confirm dialog was displayed. We now wait for the RPC
to return before checking if the record is dirty, so that the confirm dialog
is only displayed when the record is actually dirty.
This commit is contained in:
Adrien Dieudonne
2017-06-28 10:40:35 +02:00
parent df45cf65a8
commit ceff4d4608
4 changed files with 123 additions and 62 deletions
Download Patch File Download Diff File Expand all files Collapse all files
addons/web/static/src/js/views/basic/basic_controller.js
+43 -34
View File
@@ -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
addons/web/static/src/js/views/form/form_controller.js
+1 -1
View File
@@ -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
addons/web/static/src/js/views/list/list_controller.js
+23 -22
View File
@@ -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
addons/web/static/tests/views/form_tests.js
+56 -5
View File
@@ -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: '<form>' +
'<group><field name="foo"/></group>' +
'</form>',
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();
});