From 4f4e686fb275ad657e40205d27b89df86b764ee5 Mon Sep 17 00:00:00 2001 From: Aaron Bohy Date: Thu, 21 Dec 2017 20:26:11 +0100 Subject: [PATCH] [FIX] web: graph: don't lose initial groupby at reload Before this rev., when a graph view was reloaded (e.g. by clicking on the graph view switcher), the inital groupby was lost if it didn't come from the searchview (i.e. if it came from the context or from the arch). For instance, go to Sales > Reporting > Sales (the view is grouped by 'date', and there are two groups), reload by clicking on the graph view switcher: there is only one group left. The reload RPC was actually fine. What was wrong is the 'state' object given to the renderer after the reload, as it didn't contain the correct groupby, so the view wasn't correctly re-rendered. --- .../static/src/js/views/graph/graph_model.js | 5 +-- addons/web/static/tests/views/graph_tests.js | 40 +++++++++++++++++-- 2 files changed, 39 insertions(+), 6 deletions(-) diff --git a/addons/web/static/src/js/views/graph/graph_model.js b/addons/web/static/src/js/views/graph/graph_model.js index a6d2b057d0e..0518e279637 100644 --- a/addons/web/static/src/js/views/graph/graph_model.js +++ b/addons/web/static/src/js/views/graph/graph_model.js @@ -67,7 +67,6 @@ return AbstractModel.extend({ domain: params.domain, context: params.context, }; - this.defaultGroupedBy = params.groupedBy; return this._loadGraph(); }, /** @@ -96,7 +95,7 @@ return AbstractModel.extend({ this.chart.domain = params.domain; } if ('groupBy' in params) { - this.chart.groupedBy = params.groupBy.length ? params.groupBy : this.defaultGroupedBy; + this.chart.groupedBy = params.groupBy.length ? params.groupBy : this.initialGroupBys; } if ('measure' in params) { this.chart.measure = params.measure; @@ -121,7 +120,7 @@ return AbstractModel.extend({ * @returns {Deferred} */ _loadGraph: function () { - var groupedBy = this.chart.groupedBy.length ? this.chart.groupedBy : this.initialGroupBys; + var groupedBy = this.chart.groupedBy; var fields = _.map(groupedBy, function (groupBy) { return groupBy.split(':')[0]; }); diff --git a/addons/web/static/tests/views/graph_tests.js b/addons/web/static/tests/views/graph_tests.js index 07f7d060347..f7d22f27a64 100644 --- a/addons/web/static/tests/views/graph_tests.js +++ b/addons/web/static/tests/views/graph_tests.js @@ -90,7 +90,6 @@ QUnit.module('Views', { }); }); - QUnit.test('default type attribute', function (assert) { assert.expect(1); @@ -106,7 +105,6 @@ QUnit.module('Views', { graph.destroy(); }); - QUnit.test('switching mode', function (assert) { assert.expect(6); @@ -191,7 +189,6 @@ QUnit.module('Views', { }); }); - QUnit.test('no content helper', function (assert) { var done = assert.async(); assert.expect(4); @@ -442,6 +439,43 @@ QUnit.module('Views', { graph.destroy(); }); + + QUnit.test('initial groupby is kept when reloading', function (assert) { + var done = assert.async(); + assert.expect(4); + + var graph = createView({ + View: GraphView, + model: 'foo', + data: this.data, + arch: '' + + '' + + '' + + '', + mockRPC: function (route, args) { + if (args.method === 'read_group') { + assert.deepEqual(args.kwargs.groupby, ['product_id'], + "should group by the correct field"); + } + return this._super.apply(this, arguments); + }, + }); + + + return concurrency.delay(0).then(function () { + assert.strictEqual(graph.$('.nv-groups rect').length, 2, + "should display two groups"); + + graph.reload({groupBy: []}); + return concurrency.delay(0).then(function () { + assert.strictEqual(graph.$('.nv-groups rect').length, 2, + "should still display two groups"); + + graph.destroy(); + done(); + }); + }); + }); }); });