From fa00ff97be5429f2d3fb43a28a8d4eb38e25f4c5 Mon Sep 17 00:00:00 2001 From: Priyanka Kakadiya Date: Tue, 21 Apr 2020 12:27:53 +0000 Subject: [PATCH] [IMP] web: graph: add bar/line chart sorting PURPOSE Currently, reporting views such as the bar and line charts have their x-axis sorted either alphabetically or according to a sequence. When reporting, the user would be interested in sorting the x-axis values by their measure. SPECIFICATIONS Add 'ascending' and 'descending' options in graph view for bar and line charts Task 2070103 closes odoo/odoo#49970 Signed-off-by: Aaron Bohy (aab) Co-authored-by: Mohammed Shekha --- .../src/js/views/graph/graph_controller.js | 9 + .../static/src/js/views/graph/graph_model.js | 5 + .../src/js/views/graph/graph_renderer.js | 33 ++++ .../static/src/js/views/graph/graph_view.js | 1 + addons/web/static/src/xml/base.xml | 4 + addons/web/static/tests/views/graph_tests.js | 161 +++++++++++++++++- doc/reference/views.rst | 4 + odoo/addons/base/rng/graph_view.rng | 1 + 8 files changed, 217 insertions(+), 1 deletion(-) diff --git a/addons/web/static/src/js/views/graph/graph_controller.js b/addons/web/static/src/js/views/graph/graph_controller.js index eb5fd107081..f33db981667 100644 --- a/addons/web/static/src/js/views/graph/graph_controller.js +++ b/addons/web/static/src/js/views/graph/graph_controller.js @@ -184,6 +184,11 @@ var GraphController = AbstractController.extend({ .data('stacked', state.stacked) .toggleClass('active', state.stacked) .toggleClass('o_hidden', state.mode !== 'bar'); + this.$buttons + .find('.o_graph_button[data-order]') + .toggleClass('o_hidden', state.mode === 'pie' || !!Object.keys(state.timeRanges).length) + .filter('.o_graph_button[data-order="' + state.orderBy + '"]') + .toggleClass('active', !!state.orderBy); }, //-------------------------------------------------------------------------- @@ -258,6 +263,10 @@ var GraphController = AbstractController.extend({ this.update({ mode: $target.data('mode') }); } else if ($target.data('mode') === 'stack') { this.update({ stacked: !$target.data('stacked') }); + } else if (['asc', 'desc'].includes($target.data('order'))) { + const order = $target.data('order'); + const state = this.model.get(); + this.update({ orderBy: state.orderBy === order ? false : order }); } } }, 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 e1411324c77..6ee4d06651e 100644 --- a/addons/web/static/src/js/views/graph/graph_model.js +++ b/addons/web/static/src/js/views/graph/graph_model.js @@ -72,6 +72,7 @@ return AbstractModel.extend({ origins: [], stacked: params.stacked, timeRanges: params.timeRanges, + orderBy: params.orderBy }); this._computeDerivedParams(); @@ -126,6 +127,10 @@ return AbstractModel.extend({ this.chart.stacked = params.stacked; return Promise.resolve(); } + if ('orderBy' in params) { + this.chart.orderBy = params.orderBy; + return Promise.resolve(); + } return this._loadGraph(); }, diff --git a/addons/web/static/src/js/views/graph/graph_renderer.js b/addons/web/static/src/js/views/graph/graph_renderer.js index 35e78690528..c179a08216e 100644 --- a/addons/web/static/src/js/views/graph/graph_renderer.js +++ b/addons/web/static/src/js/views/graph/graph_renderer.js @@ -769,6 +769,7 @@ return AbstractRenderer.extend({ }); } var dataPoints = this._filterDataPoints(); + dataPoints = this._sortDataPoints(dataPoints); if (!dataPoints.length && this.state.mode !== 'pie') { this.$el.append(qweb.render('View.NoContentHelper')); } else if (this.isInDOM) { @@ -1006,6 +1007,38 @@ return AbstractRenderer.extend({ } return shortLabel; }, + /** + * Sort datapoints according to the current order (ASC or DESC). + * + * Note: this should be moved to the model at some point. + * + * @private + * @param {Object[]} dataPoints + * @returns {Object[]} sorted dataPoints if orderby set on state + */ + _sortDataPoints(dataPoints) { + if (!Object.keys(this.state.timeRanges).length && this.state.orderBy && + ['bar', 'line'].includes(this.state.mode) && this.state.groupBy.length) { + // group data by their x-axis value, and then sort datapoints + // based on the sum of values by group in ascending/descending order + const groupByFieldName = this.state.groupBy[0].split(':')[0]; + const groupedByMany2One = this.fields[groupByFieldName].type === 'many2one'; + const groupedDataPoints = {}; + dataPoints.forEach(function (dataPoint) { + const key = groupedByMany2One ? dataPoint.resId : dataPoint.labels[0]; + groupedDataPoints[key] = groupedDataPoints[key] || []; + groupedDataPoints[key].push(dataPoint); + }); + dataPoints = _.sortBy(groupedDataPoints, function (group) { + return group.reduce((sum, dataPoint) => sum + dataPoint.value, 0); + }); + dataPoints = dataPoints.flat(); + if (this.state.orderBy === 'desc') { + dataPoints = dataPoints.reverse('value'); + } + } + return dataPoints; + }, //-------------------------------------------------------------------------- // Handlers diff --git a/addons/web/static/src/js/views/graph/graph_view.js b/addons/web/static/src/js/views/graph/graph_view.js index 5ae120343c6..dbfd9f0e211 100644 --- a/addons/web/static/src/js/views/graph/graph_view.js +++ b/addons/web/static/src/js/views/graph/graph_view.js @@ -134,6 +134,7 @@ var GraphView = AbstractView.extend({ this.rendererParams.disableLinking = !!JSON.parse(this.arch.attrs.disable_linking || '0'); this.loadParams.mode = this.arch.attrs.type || 'bar'; + this.loadParams.orderBy = this.arch.attrs.order; this.loadParams.measure = measure || '__count__'; this.loadParams.groupBys = groupBys; this.loadParams.fields = this.fields; diff --git a/addons/web/static/src/xml/base.xml b/addons/web/static/src/xml/base.xml index 77d6f264ff2..aabc2b9f528 100644 --- a/addons/web/static/src/xml/base.xml +++ b/addons/web/static/src/xml/base.xml @@ -1046,6 +1046,10 @@ +
diff --git a/addons/web/static/tests/views/graph_tests.js b/addons/web/static/tests/views/graph_tests.js index a8baa45d8d2..294f0d4935b 100644 --- a/addons/web/static/tests/views/graph_tests.js +++ b/addons/web/static/tests/views/graph_tests.js @@ -1119,6 +1119,162 @@ QUnit.module('Views', { graph.destroy(); }); + QUnit.test('graph view sort by measure', async function (assert) { + assert.expect(18); + + // change first record from foo as there are 4 records count for each product + this.data.product.records.push({ id: 38, display_name: "zphone"}); + this.data.foo.records[7].product_id = 38; + + const graph = await createView({ + View: GraphView, + model: "foo", + data: this.data, + arch: ` + + `, + }); + + assert.containsN(graph, 'button[data-order]', 2, + "there should be two order buttons for sorting axis labels in bar mode"); + assert.checkLegend(graph, 'Count', 'measure should be by count'); + assert.hasClass(graph.$('button[data-order="desc"]'), 'active', + 'sorting should be applie on descending order by default when sorting="desc"'); + assert.checkDatasets(graph, 'data', {data: [4, 3, 1]}); + + await testUtils.dom.click(graph.$buttons.find('button[data-order="asc"]')); + assert.hasClass(graph.$('button[data-order="asc"]'), 'active', + "ascending order should be applied"); + assert.checkDatasets(graph, 'data', {data: [1, 3, 4]}); + + await testUtils.dom.click(graph.$buttons.find('button[data-order="desc"]')); + assert.hasClass(graph.$('button[data-order="desc"]'), 'active', + "descending order button should be active"); + assert.checkDatasets(graph, 'data', { data: [4, 3, 1] }); + + // again click on descending button to deactivate order button + await testUtils.dom.click(graph.$buttons.find('button[data-order="desc"]')); + assert.doesNotHaveClass(graph.$('button[data-order="desc"]'), 'active', + "descending order button should not be active"); + assert.checkDatasets(graph, 'data', {data: [4, 3, 1]}); + + // set line mode + await testUtils.dom.click(graph.$buttons.find('button[data-mode="line"]')); + assert.containsN(graph, 'button[data-order]', 2, + "there should be two order buttons for sorting axis labels in line mode"); + assert.checkLegend(graph, 'Count', 'measure should be by count'); + assert.doesNotHaveClass(graph.$('button[data-order="desc"]'), 'active', + "descending order should be applied"); + assert.checkDatasets(graph, 'data', {data: [4, 3, 1]}); + + await testUtils.dom.click(graph.$buttons.find('button[data-order="asc"]')); + assert.hasClass(graph.$('button[data-order="asc"]'), 'active', + "ascending order button should be active"); + assert.checkDatasets(graph, 'data', { data: [1, 3, 4] }); + + await testUtils.dom.click(graph.$buttons.find('button[data-order="desc"]')); + assert.hasClass(graph.$('button[data-order="desc"]'), 'active', + "descending order button should be active"); + assert.checkDatasets(graph, 'data', { data: [4, 3, 1] }); + + graph.destroy(); + + }); + + QUnit.test('graph view sort by measure for grouped data', async function (assert) { + assert.expect(9); + + // change first record from foo as there are 4 records count for each product + this.data.product.records.push({ id: 38, display_name: "zphone", }); + this.data.foo.records[7].product_id = 38; + + const graph = await createView({ + View: GraphView, + model: "foo", + data: this.data, + arch: ` + + + `, + }); + + assert.checkLegend(graph, ["true","false"], 'measure should be by count'); + assert.containsN(graph, 'button[data-order]', 2, + "there should be two order buttons for sorting axis labels"); + assert.checkDatasets(graph, 'data', [{data: [3, 0, 0]}, {data: [1, 3, 1]}]); + + await testUtils.dom.click(graph.$buttons.find('button[data-order="asc"]')); + assert.hasClass(graph.$('button[data-order="asc"]'), 'active', + "ascending order should be applied by default"); + assert.checkDatasets(graph, 'data', [{ data: [1, 3, 1] }, { data: [0, 0, 3] }]); + + await testUtils.dom.click(graph.$buttons.find('button[data-order="desc"]')); + assert.hasClass(graph.$('button[data-order="desc"]'), 'active', + "ascending order button should be active"); + assert.checkDatasets(graph, 'data', [{data: [1, 3, 1]}, {data: [3, 0, 0]}]); + + // again click on descending button to deactivate order button + await testUtils.dom.click(graph.$buttons.find('button[data-order="desc"]')); + assert.doesNotHaveClass(graph.$('button[data-order="desc"]'), 'active', + "descending order button should not be active"); + assert.checkDatasets(graph, 'data', [{ data: [3, 0, 0] }, { data: [1, 3, 1] }]); + + graph.destroy(); + + }); + + QUnit.test('graph view sort by measure for multiple grouped data', async function (assert) { + assert.expect(9); + + // change first record from foo as there are 4 records count for each product + this.data.product.records.push({ id: 38, display_name: "zphone" }); + this.data.foo.records[7].product_id = 38; + + // add few more records to data to have grouped data date wise + const data = [ + {id: 9, foo: 48, bar: false, product_id: 41, date: "2016-04-01"}, + {id: 10, foo: 49, bar: false, product_id: 41, date: "2016-04-01"}, + {id: 11, foo: 50, bar: true, product_id: 37, date: "2016-01-03"}, + {id: 12, foo: 50, bar: true, product_id: 41, date: "2016-01-03"}, + ]; + + Object.assign(this.data.foo.records, data); + + const graph = await createView({ + View: GraphView, + model: "foo", + data: this.data, + arch: ` + + + `, + groupBy: ['date', 'product_id'] + }); + + assert.checkLegend(graph, ["xpad","xphone","zphone"], 'measure should be by count'); + assert.containsN(graph, 'button[data-order]', 2, + "there should be two order buttons for sorting axis labels"); + assert.checkDatasets(graph, 'data', [{data: [2, 1, 1, 2]}, {data: [0, 1, 0, 0]}, {data: [1, 0, 0, 0]}]); + + await testUtils.dom.click(graph.$buttons.find('button[data-order="asc"]')); + assert.hasClass(graph.$('button[data-order="asc"]'), 'active', + "ascending order should be applied by default"); + assert.checkDatasets(graph, 'data', [{ data: [1, 1, 2, 2] }, { data: [0, 1, 0, 0] }, { data: [0, 0, 0, 1] }]); + + await testUtils.dom.click(graph.$buttons.find('button[data-order="desc"]')); + assert.hasClass(graph.$('button[data-order="desc"]'), 'active', + "descending order button should be active"); + assert.checkDatasets(graph, 'data', [{data: [1, 0, 0, 0]}, {data: [2, 2, 1, 1]}, {data: [0, 0, 1, 0]}]); + + // again click on descending button to deactivate order button + await testUtils.dom.click(graph.$buttons.find('button[data-order="desc"]')); + assert.doesNotHaveClass(graph.$('button[data-order="desc"]'), 'active', + "descending order button should not be active"); + assert.checkDatasets(graph, 'data', [{ data: [2, 1, 1, 2] }, { data: [0, 1, 0, 0] }, { data: [1, 0, 0, 0] }]); + + graph.destroy(); + }); + QUnit.module('GraphView: comparison mode', { beforeEach: async function () { this.data.foo.records[0].date = '2016-12-15'; @@ -1287,7 +1443,7 @@ QUnit.module('Views', { }, }, function () { QUnit.test('comparison with one groupby equal to comparison date field', async function (assert) { - assert.expect(10); + assert.expect(11); this.combinationsToCheck = { 'last_30_days,previous_period,day': { @@ -1335,6 +1491,9 @@ QUnit.module('Views', { await this.setMode('pie'); await this.testCombinations(combinations, assert); + // isNotVisible can not have two elements so checking visibility of first element + assert.isNotVisible(this.actionManager.$('button[data-order]:first'), + "there should not be order button in comparison mode") assert.ok(true, "No combination causes a crash"); }); diff --git a/doc/reference/views.rst b/doc/reference/views.rst index 488234a9cfd..d3c9b7fb60c 100644 --- a/doc/reference/views.rst +++ b/doc/reference/views.rst @@ -1077,6 +1077,10 @@ attributes: within a group ``disable_linking`` set to ``True`` to prevent from redirecting clicks on graph to list view +``order`` + if set, x-axis values will be sorted by default according their measure with + respect to the given order (``asc`` or ``desc``). Only used for ``bar`` and + ``pie`` charts. The only allowed element within a graph view is ``field`` which can have the following attributes: diff --git a/odoo/addons/base/rng/graph_view.rng b/odoo/addons/base/rng/graph_view.rng index c0884f424a2..32639b30d9c 100644 --- a/odoo/addons/base/rng/graph_view.rng +++ b/odoo/addons/base/rng/graph_view.rng @@ -21,6 +21,7 @@ +