From ab2e71d9fc2486f248fd6518237ac4b54cf45bd5 Mon Sep 17 00:00:00 2001 From: Aaron Bohy Date: Mon, 14 Aug 2017 15:58:48 +0200 Subject: [PATCH] [FIX] web: Kanban: notify environment of ids changes In grouped Kanban views, the list of res_ids of the displayed records may change for several reasons: 1) 'Load more' is clicked for a column 2) a record is moved from a column to another 3) a record is moved inside a column (resequence) 4) a column is moved 5) a column is (un)archived 6) a column is (un)folded 7) a record is created/destroyed directly from the Kanban. When this happens, the environment must be notified of the new list of res_ids (the order matters). Before this rev., this wasn't the case. As a consequence, when a record was opened in a Form view after one of the 5 operations above, the pager could display unexpected values. Note that before the new views, cases 2, 3 and 4 weren't handled, and case 7 already worked before this commit. --- .../static/src/js/views/basic/basic_model.js | 1 + .../src/js/views/kanban/kanban_controller.js | 13 +++- .../src/js/views/kanban/kanban_model.js | 14 ++++ addons/web/static/tests/views/kanban_tests.js | 78 ++++++++++++++++++- 4 files changed, 100 insertions(+), 6 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 e8409487abc..d55109bde01 100644 --- a/addons/web/static/src/js/views/basic/basic_model.js +++ b/addons/web/static/src/js/views/basic/basic_model.js @@ -3360,6 +3360,7 @@ var BasicModel = AbstractModel.extend({ list.data = data; list.res_ids = ids; } + self._updateParentResIDs(list); return list; }); }, diff --git a/addons/web/static/src/js/views/kanban/kanban_controller.js b/addons/web/static/src/js/views/kanban/kanban_controller.js index 6870dbbbe24..91ada3a2629 100644 --- a/addons/web/static/src/js/views/kanban/kanban_controller.js +++ b/addons/web/static/src/js/views/kanban/kanban_controller.js @@ -107,7 +107,10 @@ var KanbanController = BasicController.extend({ * @returns {Deferred} */ _resequenceRecords: function (column_id, ids) { - return this.model.resequence(this.modelName, ids, column_id); + var self = this; + return this.model.resequence(this.modelName, ids, column_id).then(function () { + self._updateEnv(); + }); }, /** * In grouped mode, set 'Create' button as btn-default if there is no column @@ -181,6 +184,7 @@ var KanbanController = BasicController.extend({ .then(function (db_id) { var data = self.model.get(db_id); self.renderer.updateColumn(db_id, data); + self._updateEnv(); }); } }, @@ -218,6 +222,7 @@ var KanbanController = BasicController.extend({ this.model.loadMore(column.db_id).then(function (db_id) { var data = self.model.get(db_id); self.renderer.updateColumn(db_id, data); + self._updateEnv(); }); }, /** @@ -325,9 +330,12 @@ var KanbanController = BasicController.extend({ * @param {OdooEvent} event */ _onResequenceColumn: function (event) { + var self = this; var state = this.model.get(this.handle, {raw: true}); var model = state.fields[state.groupedBy[0]].relation; - this.model.resequence(model, event.data.ids, this.handle); + this.model.resequence(model, event.data.ids, this.handle).then(function () { + self._updateEnv(); + }); }, /** * @param {OdooEvent} event @@ -338,6 +346,7 @@ var KanbanController = BasicController.extend({ this.model.toggleGroup(column.db_id).then(function (db_id) { var data = self.model.get(db_id); self.renderer.updateColumn(db_id, data); + self._updateEnv(); }); }, /** diff --git a/addons/web/static/src/js/views/kanban/kanban_model.js b/addons/web/static/src/js/views/kanban/kanban_model.js index 91e4801d241..e21a1376e51 100644 --- a/addons/web/static/src/js/views/kanban/kanban_model.js +++ b/addons/web/static/src/js/views/kanban/kanban_model.js @@ -147,6 +147,7 @@ var KanbanModel = BasicModel.extend({ return this.notifyChanges(recordID, changes).then(function () { return self.save(recordID); }).then(function () { + var resID = self.localData[recordID].res_id; // Remove record from its current group var old_group; for (var i = 0; i < parent.data.length; i++) { @@ -155,11 +156,14 @@ var KanbanModel = BasicModel.extend({ if (index >= 0) { old_group.data.splice(index, 1); old_group.count--; + old_group.res_ids = _.without(old_group.res_ids, resID); + self._updateParentResIDs(old_group); break; } } // Add record to its new group new_group.data.push(recordID); + new_group.res_ids.push(resID); new_group.count++; return [old_group.id, new_group.id]; }); @@ -197,6 +201,16 @@ var KanbanModel = BasicModel.extend({ 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; }); }, diff --git a/addons/web/static/tests/views/kanban_tests.js b/addons/web/static/tests/views/kanban_tests.js index 712cce02067..c1549e9daeb 100644 --- a/addons/web/static/tests/views/kanban_tests.js +++ b/addons/web/static/tests/views/kanban_tests.js @@ -136,11 +136,12 @@ QUnit.module('Views', { }); QUnit.test('basic grouped rendering with active field', function (assert) { - assert.expect(4); + assert.expect(6); // add active field on partner model and make all records active this.data.partner.fields.active = {string: 'Active', type: 'char', default: true}; + var envIDs = [1, 2, 3, 4]; // the ids that should be in the environment during this test var kanban = createView({ View: KanbanView, model: 'partner', @@ -152,6 +153,12 @@ QUnit.module('Views', { '
' + '', groupBy: ['bar'], + intercepts: { + env_updated: function (event) { + assert.deepEqual(event.data.ids, envIDs, + "should notify the environment with the correct ids"); + }, + }, }); // check available actions in kanban header's config dropdown @@ -163,6 +170,7 @@ QUnit.module('Views', { // archive the records of the first column assert.strictEqual(kanban.$('.o_kanban_group:last .o_kanban_record').length, 3, "last column should contain 3 records"); + envIDs = [4]; kanban.$('.o_kanban_group:last .o_column_archive').click(); // click on 'Archive' assert.strictEqual(kanban.$('.o_kanban_group:last .o_kanban_record').length, 0, "last column should contain no record"); @@ -481,8 +489,9 @@ QUnit.module('Views', { }); QUnit.test('can drag and drop a record from one column to the next', function (assert) { - assert.expect(7); + assert.expect(9); + var envIDs = [1, 3, 2, 4]; // the ids that should be in the environment during this test this.data.partner.fields.sequence = {type: 'number', string: "Sequence"}; var kanban = createView({ View: KanbanView, @@ -504,6 +513,12 @@ QUnit.module('Views', { } return this._super(route, args); }, + intercepts: { + env_updated: function (event) { + assert.deepEqual(event.data.ids, envIDs, + "should notify the environment with the correct ids"); + }, + }, }); assert.strictEqual(kanban.$('.o_kanban_group:nth-child(1) .o_kanban_record').length, 2, "column should contain 2 record(s)"); @@ -513,6 +528,7 @@ QUnit.module('Views', { assert.strictEqual(kanban.$('.thisiseditable').length, 4, "all records should be editable"); var $record = kanban.$('.o_kanban_group:nth-child(1) .o_kanban_record:first'); var $group = kanban.$('.o_kanban_group:nth-child(2)'); + envIDs = [3, 2, 4, 1]; // first record of first column moved to the bottom of second column testUtils.dragAndDrop($record, $group); assert.strictEqual(kanban.$('.o_kanban_group:nth-child(1) .o_kanban_record').length, 1, @@ -611,6 +627,42 @@ QUnit.module('Views', { kanban.destroy(); }); + QUnit.test('environment is updated when (un)folding groups', function (assert) { + assert.expect(3); + + var envIDs = [1, 3, 2, 4]; // the ids that should be in the environment during this test + var kanban = createView({ + View: KanbanView, + model: 'partner', + data: this.data, + arch: '' + + '' + + '' + + '
' + + '' + + '', + groupBy: ['product_id'], + intercepts: { + env_updated: function (event) { + assert.deepEqual(envIDs, event.data.ids, + "should notify the environment with the correct ids"); + }, + }, + }); + + // fold the second group and check that the res_ids it contains are no + // longer in the environment + envIDs = [1, 3]; + kanban.$('.o_kanban_group:last .o_kanban_toggle_fold').click(); + + // re-open the second group and check that the res_ids it contains are + // back in the environment + envIDs = [1, 3, 2, 4]; + kanban.$('.o_kanban_group:last .o_kanban_toggle_fold').click(); + + kanban.destroy(); + }); + QUnit.test('create a column in grouped on m2o', function (assert) { assert.expect(13); @@ -1217,8 +1269,9 @@ QUnit.module('Views', { }); QUnit.test('resequence columns in grouped by m2o', function (assert) { - assert.expect(4); + assert.expect(7); + var envIDs = [1, 3, 2, 4]; // the ids that should be in the environment during this test var kanban = createView({ View: KanbanView, model: 'partner', @@ -1236,6 +1289,12 @@ QUnit.module('Views', { } return this._super.apply(this, arguments); }, + intercepts: { + env_updated: function (event) { + assert.deepEqual(event.data.ids, envIDs, + "should notify the environment with the correct ids"); + }, + }, }); assert.ok(kanban.$('.o_kanban_view').hasClass('ui-sortable'), @@ -1248,6 +1307,7 @@ QUnit.module('Views', { // there is a 100ms delay on the d&d feature (jquery sortable) for // kanban columns, making it hard to test. So we rather bypass the d&d // for this test, and directly call the event handler + envIDs = [2, 4, 1, 3]; // the columns will be inverted kanban._onResequenceColumn({data: {ids: [5, 3]}}); kanban.update({}, {reload: false}); // re-render without reloading @@ -1376,8 +1436,9 @@ QUnit.module('Views', { }); QUnit.test('load more records in column', function (assert) { - assert.expect(9); + assert.expect(12); + var envIDs = [1, 2, 4]; // the ids that should be in the environment during this test var kanban = createView({ View: KanbanView, model: 'partner', @@ -1397,12 +1458,19 @@ QUnit.module('Views', { } return this._super.apply(this, arguments); }, + intercepts:{ + env_updated: function (event) { + assert.deepEqual(event.data.ids, envIDs, + "should notify the environment with the correct ids"); + }, + }, }); assert.strictEqual(kanban.$('.o_kanban_group:eq(1) .o_kanban_record').length, 2, "there should be 2 records in the column"); // load more + envIDs = [1, 2, 3, 4]; // id 3 will be loaded kanban.$('.o_kanban_group:eq(1)').find('.o_kanban_load_more').click(); assert.strictEqual(kanban.$('.o_kanban_group:eq(1) .o_kanban_record').length, 3, @@ -1411,6 +1479,8 @@ QUnit.module('Views', { assert.verifySteps([[2, undefined], [2, undefined], [2, 2]], "the records should be correctly fetched"); + // reload + envIDs = [1, 2, 4]; // first group is limited again to 2 records kanban.reload(); assert.strictEqual(kanban.$('.o_kanban_group:eq(1) .o_kanban_record').length, 2, "there should be 2 records in the column after reload");