From 52d779007d0b63fb6c2ae94de095bfe83c5019c1 Mon Sep 17 00:00:00 2001 From: len-odoo Date: Tue, 11 Sep 2018 08:33:33 +0000 Subject: [PATCH] [FIX] web: use read to update record values after resequence Commit: https://github.com/odoo/odoo/commit/818c18e55d0718286ff5bf332186a10f4d7a58ef Updated the resequence logic in a way that was almost falser than before. The added test however did work by coincidence, as index values were equal to the sequence field values. To be sure that we synchronize with what happens on the server, we do a read after the resequence. Additionnally, we take into account the result of the server resequence return; if it is false, it means no resequencing happened, so we should not do a read. opw 1867049 closes odoo/odoo#27184 --- .../static/src/js/views/basic/basic_model.js | 64 +++++++++++++------ .../web/static/tests/helpers/mock_server.js | 4 ++ .../static/tests/views/kanban_model_tests.js | 7 +- addons/web/static/tests/views/kanban_tests.js | 11 +--- addons/web/static/tests/views/list_tests.js | 24 ++++--- 5 files changed, 69 insertions(+), 41 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 db78c789da3..116bd73fbdb 100644 --- a/addons/web/static/src/js/views/basic/basic_model.js +++ b/addons/web/static/src/js/views/basic/basic_model.js @@ -893,27 +893,51 @@ var BasicModel = AbstractModel.extend({ route: '/web/dataset/resequence', params: params, }) - .then(function () { - var offset = options.offset ? options.offset : 0; - var old_data = data.data.slice(); - data.data = _.sortBy(data.data, function (d) { - if (_.contains(resIDs, self.localData[d].res_id)) { - return _.indexOf(resIDs, self.localData[d].res_id) + offset; - } else { - return _.indexOf(old_data, d); + .then(function (wasResequenced) { + if (!wasResequenced) { + // the field on which the resequence was triggered does not + // exist, so no resequence happened server-side + return $.when(); + } + var field = params.field ? params.field : 'sequence'; + + return self._rpc({ + model: modelName, + method: 'read', + args: [resIDs, [field]], + }).then(function (records) { + if (data.data.length) { + var dataType = self.localData[data.data[0]].type; + if (dataType === 'record') { + _.each(data.data, function (dataPoint) { + var recordData = self.localData[dataPoint].data; + var inRecords = _.findWhere(records, {id: recordData.id}); + if (inRecords) { + recordData[field] = inRecords[field]; + } + }); + data.data = _.sortBy(data.data, function (d) { + return self.localData[d].data[field]; + }); + } + if (dataType === 'list') { + data.data = _.sortBy(data.data, function (d) { + return _.indexOf(resIDs, self.localData[d].res_id) + }); + } } - }); - data.res_ids = []; - _.each(data.data, function (d) { - var dataPoint = self.localData[d]; - if (dataPoint.type === 'record') { - data.res_ids.push(dataPoint.res_id); - } else { - data.res_ids = data.res_ids.concat(dataPoint.res_ids); - } - }); - self._updateParentResIDs(data); - return parentID; + data.res_ids = []; + _.each(data.data, function (d) { + var dataPoint = self.localData[d]; + if (dataPoint.type === 'record') { + data.res_ids.push(dataPoint.res_id); + } else { + data.res_ids = data.res_ids.concat(dataPoint.res_ids); + } + }); + self._updateParentResIDs(data); + return parentID; + }) }); }, /** diff --git a/addons/web/static/tests/helpers/mock_server.js b/addons/web/static/tests/helpers/mock_server.js index 92c051c84bd..a26d62b436f 100644 --- a/addons/web/static/tests/helpers/mock_server.js +++ b/addons/web/static/tests/helpers/mock_server.js @@ -775,10 +775,14 @@ var MockServer = Class.extend({ var offset = args.offset ? Number(args.offset) : 0; var field = args.field ? args.field : 'sequence'; var records = this.data[args.model].records; + if (!(field in this.data[args.model].fields)) { + return false; + } for (var i in args.ids) { var record = _.findWhere(records, {id: args.ids[i]}); record[field] = Number(i) + offset; } + return true; }, /** * Simulate a 'search_count' operation diff --git a/addons/web/static/tests/views/kanban_model_tests.js b/addons/web/static/tests/views/kanban_model_tests.js index 1a3391c720d..20eb2918d04 100644 --- a/addons/web/static/tests/views/kanban_model_tests.js +++ b/addons/web/static/tests/views/kanban_model_tests.js @@ -192,6 +192,8 @@ QUnit.module('Views', { var done = assert.async(); assert.expect(8); + this.data.product.fields.sequence = {string: "Sequence", type: "integer"}; + this.data.partner.fields.sequence = {string: "Sequence", type: "integer"}; this.data.partner.records.push({id: 3, foo: 'aaa', product_id: 37}); var nbReseq = 0; @@ -204,7 +206,7 @@ QUnit.module('Views', { if (nbReseq === 1) { // resequencing columns assert.deepEqual(args.ids, [41, 37], "ids should be correct"); - assert.strictEqual(args.model, 'product_id', + assert.strictEqual(args.model, 'product', "model should be correct"); } else if (nbReseq === 2) { // resequencing records assert.deepEqual(args.ids, [3, 1], @@ -212,7 +214,6 @@ QUnit.module('Views', { assert.strictEqual(args.model, 'partner', "model should be correct"); } - return $.when(); } return this._super.apply(this, arguments); }, @@ -229,7 +230,7 @@ QUnit.module('Views', { "first group should be res_id 37"); // resequence columns - return model.resequence('product_id', [41, 37], stateID); + return model.resequence('product', [41, 37], stateID); }) .then(function (stateID) { var state = model.get(stateID); diff --git a/addons/web/static/tests/views/kanban_tests.js b/addons/web/static/tests/views/kanban_tests.js index 921cf3655da..1a424c92428 100644 --- a/addons/web/static/tests/views/kanban_tests.js +++ b/addons/web/static/tests/views/kanban_tests.js @@ -1241,7 +1241,7 @@ QUnit.module('Views', { }); QUnit.test('delete a column in grouped on m2o', function (assert) { - assert.expect(33); + assert.expect(36); testUtils.patch(KanbanRenderer, { _renderGrouped: function () { @@ -1858,6 +1858,7 @@ QUnit.module('Views', { QUnit.test('resequence columns in grouped by m2o', function (assert) { assert.expect(7); + this.data.product.fields.sequence = {string: "Sequence", type: "integer"}; var envIDs = [1, 3, 2, 4]; // the ids that should be in the environment during this test var kanban = createView({ @@ -1871,12 +1872,6 @@ QUnit.module('Views', { '' + '', groupBy: ['product_id'], - mockRPC: function (route) { - if (route === '/web/dataset/resequence') { - return $.when(); - } - return this._super.apply(this, arguments); - }, intercepts: { env_updated: function (event) { assert.deepEqual(event.data.ids, envIDs, @@ -1900,7 +1895,7 @@ QUnit.module('Views', { kanban.update({}, {reload: false}); // re-render without reloading assert.strictEqual(kanban.$('.o_kanban_group:first').data('id'), 5, - "first column should be id 5 before resequencing"); + "first column should be id 5 after resequencing"); kanban.destroy(); }); diff --git a/addons/web/static/tests/views/list_tests.js b/addons/web/static/tests/views/list_tests.js index 9203cad8b99..bd4c8f8e339 100644 --- a/addons/web/static/tests/views/list_tests.js +++ b/addons/web/static/tests/views/list_tests.js @@ -3025,10 +3025,10 @@ QUnit.module('Views', { 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}, + {id: 1, int_field: 11}, + {id: 2, int_field: 12}, + {id: 3, int_field: 13}, + {id: 4, int_field: 14}, ] } }; @@ -3047,14 +3047,15 @@ QUnit.module('Views', { assert.deepEqual(args, { model: "foo", ids: [4, 3], - offset: 2, + offset: 13, field: "int_field", }); } if (moves === 1) { assert.deepEqual(args, { model: "foo", - ids: [1, 4, 2, 3], + ids: [4, 2], + offset: 12, field: "int_field", }); } @@ -3062,14 +3063,15 @@ QUnit.module('Views', { assert.deepEqual(args, { model: "foo", ids: [2, 4], - offset: 1, + offset: 12, field: "int_field", }); } if (moves === 3) { assert.deepEqual(args, { model: "foo", - ids: [1, 4, 2, 3], + ids: [4, 2], + offset: 12, field: "int_field", }); } @@ -3136,7 +3138,6 @@ QUnit.module('Views', { "should write the right field as sequence"); assert.deepEqual(args.ids, [4, 2, 3], "should write the sequence in correct order"); - return $.when(); } return this._super.apply(this, arguments); }, @@ -3266,13 +3267,16 @@ QUnit.module('Views', { '', mockRPC: function (route, args) { if (route === '/web/dataset/resequence') { + var _super = this._super.bind(this); 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"); assert.deepEqual(args.ids, [4, 2, 3], "should write the sequence in correct order"); - return $.when(def); + return $.when(def).then(function () { + return _super(route, args); + }); } return this._super.apply(this, arguments); },