[FIX] web: proper x2m edition with pagination

Before this commit, we did not really handle gracefully editing a
one2many with many records. The main issue is that if the user was not
on the last page, creating a new record added it to the last page, which
is confusing and actually a bad idea for records with required fields.

This fix requires a way to track the current changes (because we do not
display the current state of the form view: the new record is displayed
in the first page, but is actually added at the end of the dataset).

Note that this probably does not solve all issues with pagination. This
is a quite delicate problem. We will add tests and fixes as needed.
This commit is contained in:
Géry Debongnie
2017-09-05 15:41:28 +02:00
parent fef645fcc5
commit 983c7ae22f
2 changed files with 146 additions and 12 deletions
@@ -421,15 +421,27 @@ var BasicModel = AbstractModel.extend({
if (element._keepChangesUnsorted) {
// only apply a subset (by default 0) of changes before sorting
count = element._keepChangesUnsortedCount || 0;
element = this._applyX2ManyOperations(element, {to: count});
element = this._applyX2ManyOperations(element, {to: count, position: 'bottom'});
} else {
// apply all changes before sorting
element = this._applyX2ManyOperations(element);
element = this._applyX2ManyOperations(element, {position: 'bottom'});
}
this._sortList(element);
if (element._keepChangesUnsorted) {
// apply the remaining changes after the sort
element = this._applyX2ManyOperations(element, {from: count});
element = this._applyX2ManyOperations(element, {from: count, position: 'bottom'});
}
if (element._changes) {
_.each(element._changes, function (change) {
if (change.operation === 'ADD' && change.isNew) {
element.data = _.without(element.data, change.id);
if (change.position === 'top') {
element.data.unshift(change.id);
} else {
element.data.push(change.id);
}
}
});
}
var list = {
aggregateValues: _.extend({}, element.aggregateValues),
@@ -700,6 +712,10 @@ var BasicModel = AbstractModel.extend({
if (!options.keepChanges) {
this.discardChanges(id, {rollback: false});
}
} else if (element._changes) {
_.each(element._changes, function (change) {
delete change.isNew;
});
}
if (options.context !== undefined) {
@@ -892,6 +908,10 @@ var BasicModel = AbstractModel.extend({
var list = this.localData[list_id];
if (list.type === 'record') {
return;
} else if (list._changes) {
_.each(list._changes, function (change) {
delete change.isNew;
});
}
// the user manually selected a sort order, so we sort the list with all
// the changes he made so far, even if it is editable, however, changes
@@ -991,7 +1011,7 @@ var BasicModel = AbstractModel.extend({
};
return this._makeDefaultRecord(list.model, params).then(function (id) {
var position = options && options.position || 'top';
list._changes.push({operation: 'ADD', id: id, position: position});
list._changes.push({operation: 'ADD', id: id, position: position, isNew: true});
var record = self.localData[id];
list._cache[record.res_id] = id;
return id;
@@ -1448,13 +1468,15 @@ var BasicModel = AbstractModel.extend({
*
* @private
* @param {Object} dataPoint of type list
* @param {Object} [range] the range of operations to apply
* @param {Object} [range.from=0] the index of the first operation to apply
* @param {Object} [range.to=length] the index of the last operation to apply
* @param {Object} [options] mostly contains the range of operations to apply
* @param {Object} [options.from=0] the index of the first operation to apply
* @param {Object} [options.to=length] the index of the last operation to apply
* @param {Object} [options.position] if set, each new operation will be set
* accordingly at the top or the bottom of the list
* @returns {Object} element of type list in which the commands have been
* applied
*/
_applyX2ManyOperations: function (list, range) {
_applyX2ManyOperations: function (list, options) {
if (!list.static) {
// this function only applies on x2many lists
return list;
@@ -1463,9 +1485,9 @@ var BasicModel = AbstractModel.extend({
list = _.extend({}, list);
list.res_ids = list.res_ids.slice(0);
var changes = list._changes || [];
if (range) {
var to = range.to === 0 ? 0 : (range.to || changes.length);
changes = changes.slice(range.from || 0, to);
if (options) {
var to = options.to === 0 ? 0 : (options.to || changes.length);
changes = changes.slice(options.from || 0, to);
}
_.each(changes, function (change) {
var relRecord;
@@ -1476,7 +1498,7 @@ var BasicModel = AbstractModel.extend({
case 'ADD':
list.count++;
var resID = relRecord ? relRecord.res_id : change.resID;
if (change.position === 'top') {
if (change.position === 'top' && (options ? options.position !== 'bottom' : true)) {
list.res_ids.unshift(resID);
} else {
list.res_ids.push(resID);
@@ -5407,6 +5407,118 @@ QUnit.module('relational_fields', {
});
QUnit.test('editing tabbed one2many (editable=bottom)', function (assert) {
assert.expect(11);
this.data.partner.records[0].turtles = [];
for (var i = 0; i < 42; i++) {
var id = 100 + i;
this.data.turtle.records.push({id: id, turtle_foo: 'turtle' + (id-99)});
this.data.partner.records[0].turtles.push(id);
}
var form = createView({
View: FormView,
model: 'partner',
data: this.data,
arch:'<form string="Partners">' +
'<sheet>' +
'<field name="turtles">' +
'<tree editable="bottom">' +
'<field name="turtle_foo"/>' +
'</tree>' +
'</field>' +
'</sheet>' +
'</form>',
res_id: 1,
mockRPC: function (route, args) {
assert.step(args.method);
if (args.method === 'write') {
assert.deepEqual(args.args[1].turtles[42], [0, false, {turtle_foo: 'rainbow dash'}]);
}
return this._super.apply(this, arguments);
},
});
form.$buttons.find('.o_form_button_edit').click();
form.$('.o_field_x2many_list_row_add a').click();
assert.strictEqual(form.$('tr.o_data_row').length, 41,
"should have 41 data rows on the current page");
assert.ok(form.$('tr.o_data_row').last().hasClass('o_selected_row'),
"last row should be selected");
form.$('.o_data_row input[name="turtle_foo"]').val('rainbow dash').trigger('input');
form.$buttons.find('.o_form_button_save').click();
assert.strictEqual(form.$('tr.o_data_row').length, 40,
"should have 40 data rows on the current page");
assert.verifySteps(['read', 'read', 'default_get', 'write', 'read', 'read']);
form.destroy();
});
QUnit.test('editing tabbed one2many (editable=top)', function (assert) {
assert.expect(14);
this.data.partner.records[0].turtles = [];
this.data.turtle.fields.turtle_foo.default = "default foo";
for (var i = 0; i < 42; i++) {
var id = 100 + i;
this.data.turtle.records.push({id: id, turtle_foo: 'turtle' + (id-99)});
this.data.partner.records[0].turtles.push(id);
}
var form = createView({
View: FormView,
model: 'partner',
data: this.data,
arch:'<form string="Partners">' +
'<sheet>' +
'<field name="turtles">' +
'<tree editable="top">' +
'<field name="turtle_foo"/>' +
'</tree>' +
'</field>' +
'</sheet>' +
'</form>',
res_id: 1,
mockRPC: function (route, args) {
assert.step(args.method);
if (args.method === 'write') {
assert.deepEqual(args.args[1].turtles[0], [0, false, {turtle_foo: 'rainbow dash'}])
}
return this._super.apply(this, arguments);
},
});
form.$buttons.find('.o_form_button_edit').click();
form.$('.o_pager_next').click();
assert.strictEqual(form.$('tr.o_data_row').length, 2,
"should have 2 data rows on the current page");
form.$('.o_field_x2many_list_row_add a').click();
assert.strictEqual(form.$('tr.o_data_row').length, 3,
"should have 3 data rows on the current page");
assert.ok(form.$('tr.o_data_row').first().hasClass('o_selected_row'),
"first row should be selected");
assert.strictEqual(form.$('tr.o_data_row input').val(), 'default foo',
"selected input should have correct string");
form.$('.o_data_row input[name="turtle_foo"]').val('rainbow dash').trigger('input');
form.$buttons.find('.o_form_button_save').click();
assert.strictEqual(form.$('tr.o_data_row').length, 40,
"should have 40 data rows on the current page");
assert.verifySteps(['read', 'read', 'read', 'default_get', 'write', 'read', 'read']);
form.destroy();
});
QUnit.module('FieldMany2Many');