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

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

Browse Source 
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.
This commit is contained in:
Aaron Bohy
2017-05-31 20:53:55 +02:00
parent 23613b411f
commit f9a1241721
7 changed files with 196 additions and 76 deletions
Download Patch File Download Diff File Expand all files Collapse all files
addons/web/static/src/js/fields/relational_fields.js
+18 -4
View File
@@ -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({
addons/web/static/src/js/views/basic/basic_controller.js
+20
View File
@@ -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
*
addons/web/static/src/js/views/form/form_controller.js
+22 -24
View File
@@ -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
addons/web/static/src/js/views/list/list_controller.js
+9 -3
View File
@@ -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
addons/web/static/src/js/views/list/list_editable_renderer.js
+45 -45
View File
@@ -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();
},
});
addons/web/static/tests/fields/relational_fields_tests.js
+45
View File
@@ -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: '<form string="Partners">' +
'<field name="turtles">' +
'<tree editable="bottom">' +
'<field name="turtle_trululu" required="1"/>' +
'</tree>' +
'</field>' +
'</form>',
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) {
addons/web/static/tests/views/list_tests.js
+37
View File
@@ -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: '<tree editable="top"><field name="m2o" required="1"/></tree>',
mockRPC: function (route, args) {
var result = this._super.apply(this, arguments);
if (args.method === 'onchange') {
return $.when(def).then(_.constant(result));
}
return result;
},
});
assert.strictEqual(list.$('.o_data_row').length, 4,
"should contain 4 records");
// click on Add twice, and delay the onchange
list.$buttons.find('.o_list_button_add').click();
list.$buttons.find('.o_list_button_add').click();
def.resolve();
assert.strictEqual(list.$('.o_data_row').length, 5,
"only one record should have been created");
list.destroy();
});
});
});