From 818c18e55d0718286ff5bf332186a10f4d7a58ef Mon Sep 17 00:00:00 2001 From: len-odoo Date: Tue, 24 Jul 2018 15:26:41 +0200 Subject: [PATCH] [FIX] web: fix js data during consecutive sequence of resequence moves Let x_1, ..., x_a, x_{a+1}, ..., x_{a+k}, x_{a+k+1}, ..., x_{n} be a record sequence ordered by its 'sequence' field. Suppose the reordering occurs between elements x_{a+k} and x_{a+1}. If the subsequence x_1, ..., x_a is ordered, then it does not change. This is also true of the subsequence x_{a+k+1}, ..., x_{n}. The resquence function makes exactly this assumption, and only sends the server the subsequence x_{a+1}, ..., x_{a+k} to update its 'sequence' field. The update of x is done as follows: it is index + offset, where index is the position of x in the subsequence, and offset is the 'sequence' of the first element of the sequence (i.e. x_{a+1}.sequence). It is easy to see that we need another hypothesis: that the sequence numbers are unique. To show it, suppose that the sequence of x_1, ..., x_a is 0, then all x_{a+1}, ..., x_{a+k} have sequence 1, and lastly all x_{a+k}, x_{a+k+1}, ..., x_{n} have sequence 2. Then after the call to the server resequence, the subsequence x_{a+1}, ..., x_{a+k} would have sequence numbers 1, 2,..., k. Since the x_{a+k+1}, ..., x_{n} subsequence have 'sequence' value 1, this would obviously be incorrect. We fix the reordering of the data in the js frontend to be consistent with what is done by the server. Note that this is only pertinent for sequences of reordering moves, since otherwise the data comes directly from the server. opw 1867049 --- .../static/src/js/views/basic/basic_model.js | 8 +- .../web/static/tests/helpers/mock_server.js | 19 ++- addons/web/static/tests/views/list_tests.js | 110 +++++++++++++++++- 3 files changed, 132 insertions(+), 5 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 7621fe35446..16f77b7954c 100644 --- a/addons/web/static/src/js/views/basic/basic_model.js +++ b/addons/web/static/src/js/views/basic/basic_model.js @@ -894,8 +894,14 @@ var BasicModel = AbstractModel.extend({ params: params, }) .then(function () { + var offset = options.offset ? options.offset : 0; + var old_data = data.data.slice(); data.data = _.sortBy(data.data, function (d) { - return _.indexOf(resIDs, self.localData[d].res_id); + if (_.contains(resIDs, self.localData[d].res_id)) { + return _.indexOf(resIDs, self.localData[d].res_id) + offset; + } else { + return _.indexOf(old_data, d); + } }); data.res_ids = []; _.each(data.data, function (d) { diff --git a/addons/web/static/tests/helpers/mock_server.js b/addons/web/static/tests/helpers/mock_server.js index 4ae38143225..ede17917cca 100644 --- a/addons/web/static/tests/helpers/mock_server.js +++ b/addons/web/static/tests/helpers/mock_server.js @@ -763,6 +763,23 @@ var MockServer = Class.extend({ return data; }, + /** + * Simulates a 'resequence' operation + * + * @private + * @param {string} model + * @param {string} field + * @param {Array} ids + */ + _mockResequence: function (args) { + var offset = args.offset ? Number(args.offset) : 0; + var field = args.field ? args.field : 'sequence'; + var records = this.data[args.model].records; + for (var i in args.ids) { + var record = _.findWhere(this.data[args.model].records, {id: args.ids[i]}); + record[field] = Number(i) + offset; + } + }, /** * Simulate a 'search_count' operation * @@ -922,7 +939,7 @@ var MockServer = Class.extend({ return $.when(this._mockSearchReadController(args)); case '/web/dataset/resequence': - return $.when(); + return $.when(this._mockResequence(args)); } if (route.indexOf('/web/image') >= 0 || _.contains(['.png', '.jpg'], route.substr(route.length - 4))) { return $.when(); diff --git a/addons/web/static/tests/views/list_tests.js b/addons/web/static/tests/views/list_tests.js index 1330f84c32c..eebf09c9365 100644 --- a/addons/web/static/tests/views/list_tests.js +++ b/addons/web/static/tests/views/list_tests.js @@ -3019,20 +3019,118 @@ QUnit.module('Views', { list.destroy(); }); + QUnit.test('result of consecutive resequences is correctly sorted', function (assert) { + assert.expect(9); + this.data = { // we want the data to be minimal to have a minimal test + foo: { + fields: {int_field: {string: "int_field", type: "integer", sortable: true}}, + records: [ + {id: 1, int_field: 0}, + {id: 2, int_field: 1}, + {id: 3, int_field: 2}, + {id: 4, int_field: 3}, + ] + } + }; + var moves = 0; + var list = createView({ + View: ListView, + model: 'foo', + data: this.data, + arch: '' + + '' + + '' + + '', + mockRPC: function (route, args) { + if (route === '/web/dataset/resequence') { + if (moves === 0) { + assert.deepEqual(args, { + model: "foo", + ids: [4, 3], + offset: 2, + field: "int_field", + }); + } + if (moves === 1) { + assert.deepEqual(args, { + model: "foo", + ids: [1, 4, 2, 3], + field: "int_field", + }); + } + if (moves === 2) { + assert.deepEqual(args, { + model: "foo", + ids: [2, 4], + offset: 1, + field: "int_field", + }); + } + if (moves === 3) { + assert.deepEqual(args, { + model: "foo", + ids: [1, 4, 2, 3], + field: "int_field", + }); + } + moves += 1; + } + return this._super.apply(this, arguments); + }, + }); + assert.strictEqual(list.$('tbody tr td').text(), '1234', + "default should be sorted by id"); + testUtils.dragAndDrop( + list.$('.ui-sortable-handle').eq(3), + list.$('tbody tr').eq(2), + {position: 'top'} + ); + assert.strictEqual(list.$('tbody tr td').text(), '1243', + "the int_field (sequence) should have been correctly updated"); + testUtils.dragAndDrop( + list.$('.ui-sortable-handle').eq(2), + list.$('tbody tr').eq(1), + {position: 'top'} + ); + assert.deepEqual(list.$('tbody tr td').text(), '1423', + "the int_field (sequence) should have been correctly updated"); + testUtils.dragAndDrop( + list.$('.ui-sortable-handle').eq(1), + list.$('tbody tr').eq(3), + {position: 'top'} + ); + assert.deepEqual(list.$('tbody tr td').text(), '1243', + "the int_field (sequence) should have been correctly updated"); + testUtils.dragAndDrop( + list.$('.ui-sortable-handle').eq(2), + list.$('tbody tr').eq(1), + {position: 'top'} + ); + assert.deepEqual(list.$('tbody tr td').text(), '1423', + "the int_field (sequence) should have been correctly updated"); + list.destroy(); + }); + QUnit.test('editable list with handle widget', function (assert) { assert.expect(12); + // resequence makes sense on a sequence field, not on arbitrary fields + this.data.foo.records[0].int_field = 0; + this.data.foo.records[1].int_field = 1; + this.data.foo.records[2].int_field = 2; + this.data.foo.records[3].int_field = 3; + var list = createView({ View: ListView, model: 'foo', data: this.data, - arch: '' + + arch: '' + '' + '' + '', mockRPC: function (route, args) { if (route === '/web/dataset/resequence') { - assert.strictEqual(args.offset, -4, + assert.strictEqual(args.offset, 1, "should write the sequence starting from the lowest current one"); assert.strictEqual(args.field, 'int_field', "should write the right field as sequence"); @@ -3080,6 +3178,12 @@ QUnit.module('Views', { QUnit.test('editable list with handle widget with slow network', function (assert) { assert.expect(15); + // resequence makes sense on a sequence field, not on arbitrary fields + this.data.foo.records[0].int_field = 0; + this.data.foo.records[1].int_field = 1; + this.data.foo.records[2].int_field = 2; + this.data.foo.records[3].int_field = 3; + var def = $.Deferred(); var list = createView({ @@ -3092,7 +3196,7 @@ QUnit.module('Views', { '', mockRPC: function (route, args) { if (route === '/web/dataset/resequence') { - assert.strictEqual(args.offset, -4, + assert.strictEqual(args.offset, 1, "should write the sequence starting from the lowest current one"); assert.strictEqual(args.field, 'int_field', "should write the right field as sequence");