From e7fab234201e357da52c3576ad2e8fc9ceee9359 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=A9ry=20Debongnie?= Date: Wed, 7 Mar 2018 14:02:14 +0100 Subject: [PATCH] [FIX] web: better experience for one2many edition (paged) A big problem with the o2m edition happens when the user has a full page (so, in general, 40 records) and click on add. If the o2m is in editable="bottom" mode (which is actually a really common usecase), then the next record will be added at the end. However, since we have a limit of 40, it will immediately go to the next page. The effect is even weirder when there is a required field in the tree view. In that case, the new record appears correctly, the user can edit it, then, as soon as the required fields are set, it will immediately disappear from the current screen. In this commit, we have a really simple solution: we simply increase the limit in this case. An interesting thing is that it is actually temporary: when the user goes to the next page, the limit is back to its original value (due to the pager click: it resets the value to what the pager knows). --- .../static/src/js/views/basic/basic_model.js | 8 +- .../tests/fields/relational_fields_tests.js | 113 +++++++++++++++++- 2 files changed, 114 insertions(+), 7 deletions(-) diff --git a/addons/web/static/src/js/views/basic/basic_model.js b/addons/web/static/src/js/views/basic/basic_model.js index c8bd4ae030f..f081d0a6caa 100644 --- a/addons/web/static/src/js/views/basic/basic_model.js +++ b/addons/web/static/src/js/views/basic/basic_model.js @@ -809,6 +809,7 @@ var BasicModel = AbstractModel.extend({ this.discardChanges(id, {rollback: false}); } } else if (element._changes) { + delete element.tempLimitIncrement; _.each(element._changes, function (change) { delete change.isNew; }); @@ -1683,6 +1684,10 @@ var BasicModel = AbstractModel.extend({ case 'CREATE': var options = {position: command.position}; def = this._addX2ManyDefaultRecord(list, options).then(function (id) { + if (command.position === 'bottom' && list.orderedResIDs.length >= list.limit) { + list.tempLimitIncrement = (list.tempLimitIncrement || 0) + 1; + list.limit += 1; + } // FIXME: hack for lunch widget, which does useless default_get and onchange if (command.data) { return self._applyChange(id, command.data); @@ -3819,7 +3824,8 @@ var BasicModel = AbstractModel.extend({ // generate the current count and res_ids list by applying the changes var currentCount = list.count; var currentResIDs = list.res_ids; - var upperBound = list.limit ? Math.min(list.offset + list.limit, currentCount) : currentCount; + var effectiveLimit = (list.limit || 0) - (list.tempLimitIncrement || 0); + var upperBound = effectiveLimit ? Math.min(list.offset + effectiveLimit, currentCount) : currentCount; var fieldNames = list.getFieldNames(); for (var i = list.offset; i < upperBound; i++) { var resId = currentResIDs[i]; diff --git a/addons/web/static/tests/fields/relational_fields_tests.js b/addons/web/static/tests/fields/relational_fields_tests.js index fa22f5d4561..dfafb58abd5 100644 --- a/addons/web/static/tests/fields/relational_fields_tests.js +++ b/addons/web/static/tests/fields/relational_fields_tests.js @@ -2783,7 +2783,7 @@ QUnit.module('relational_fields', { }); QUnit.test('onchange followed by edition on the second page', function (assert) { - assert.expect(20); + assert.expect(12); var ids = []; for (var i=1; i<85; i++) { @@ -2853,10 +2853,32 @@ QUnit.module('relational_fields', { assert.strictEqual(form.$('.o_data_row:has(.o_data_cell:contains(#39))').index(), 2, "should display '#39' at the third line"); form.destroy(); + }); + + QUnit.test('onchange followed by edition on the second page (part 2)', function (assert) { + assert.expect(8); + + var ids = []; + for (var i=1; i<85; i++) { + var id = 10 + i; + ids.push(id); + this.data.turtle.records.push({ + id: id, + turtle_int: id/3|0, + turtle_foo: "#" + i, + }); + } + ids.splice(41, 0, 1, 2, 3); + this.data.partner.records[0].turtles = ids; + this.data.partner.onchanges = { + turtles: function (obj) { + obj.turtles = [[5]].concat(obj.turtles); + }, + }; // bottom order - form = createView({ + var form = createView({ View: FormView, model: 'partner', data: this.data, @@ -2888,16 +2910,16 @@ QUnit.module('relational_fields', { form.$('.o_field_x2many_list_row_add a').click(); - assert.strictEqual(form.$('.o_data_row').length, 40, "should display 39 records and the create line"); + assert.strictEqual(form.$('.o_data_row').length, 41, "should display 41 records and the create line"); assert.strictEqual(form.$('.o_data_row:has(.o_data_cell:contains(#76))').index(), 38, "should display '#76' at the penultimate line"); - assert.strictEqual(form.$('.o_data_row:has(.o_field_char)').index(), 39, "should display the create line at the last position"); + assert.strictEqual(form.$('.o_data_row:has(.o_field_char)').index(), 40, "should display the create line at the last position"); form.$('.o_data_row input:first').val('value 3').trigger('input'); form.$('.o_field_x2many_list_row_add a').click(); - assert.strictEqual(form.$('.o_data_row').length, 40, "should display 39 records and the create line"); + assert.strictEqual(form.$('.o_data_row').length, 42, "should display 42 records and the create line"); assert.strictEqual(form.$('.o_data_row:has(.o_data_cell:contains(#76))').index(), 38, "should display '#76' at the penultimate line"); - assert.strictEqual(form.$('.o_data_row:has(.o_field_char)').index(), 39, "should display the create line at the last position"); + assert.strictEqual(form.$('.o_data_row:has(.o_field_char)').index(), 41, "should display the create line at the last position"); form.destroy(); }); @@ -7595,6 +7617,42 @@ QUnit.module('relational_fields', { form.destroy(); }); + QUnit.test('editing tabbed one2many (editable=bottom), again...', function (assert) { + assert.expect(1); + + this.data.partner.records[0].turtles = []; + for (var i = 0; i < 9; 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:'
' + + '' + + '' + + '' + + '' + + '' + + '
', + res_id: 1, + }); + + + form.$buttons.find('.o_form_button_edit').click(); + form.$('.o_field_x2many_list_row_add a').click(); + form.$('.o_data_row input[name="turtle_foo"]').val('rainbow dash').trigger('input'); + form.$('.o_x2m_control_panel .o_pager_next').click(); + form.$('.o_x2m_control_panel .o_pager_next').click(); + + assert.strictEqual(form.$('tr.o_data_row').length, 3, + "should have 3 data rows on the current page"); + form.destroy(); + }); + QUnit.test('editing tabbed one2many (editable=top)', function (assert) { assert.expect(15); @@ -8210,6 +8268,49 @@ QUnit.module('relational_fields', { form.destroy(); }); + QUnit.test('add a new line after limit is reached should behave nicely', function (assert) { + assert.expect(2); + + this.data.partner.records[0].turtles = [1,2,3]; + + this.data.partner.onchanges = { + turtles: function (obj) { + obj.turtles = [ + [5], + [1, 1, {turtle_foo: "yop"}], + [1, 2, {turtle_foo: "blip"}], + [1, 3, {turtle_foo: "kawa"}], + [0, obj.turtles[3][2], {turtle_foo: "abc"}], + ]; + }, + }; + + var form = createView({ + View: FormView, + model: 'partner', + data: this.data, + arch:'
' + + '' + + '' + + '' + + '' + + '' + + '
', + res_id: 1, + viewOptions: { + mode: 'edit', + }, + }); + + form.$('.o_field_x2many_list_row_add a').click(); + assert.strictEqual(form.$('.o_data_row').length, 4, 'should have 4 data rows'); + form.$('.o_input[name="turtle_foo"]').val('a').trigger('input'); + assert.strictEqual(form.$('.o_data_row').length, 4, + 'should still have 4 data rows (the limit is increased to 4)'); + + form.destroy(); + }); + QUnit.module('FieldMany2Many'); QUnit.test('many2many kanban: edition', function (assert) {