From ee84815f2b57ed7ba096d3390aa8c6509e3f7845 Mon Sep 17 00:00:00 2001
From: "Anh Thao Pham (pta)" <pta@odoo.com>
Date: Thu, 19 Aug 2021 10:05:35 +0000
Subject: [PATCH] [FIX] web: fix data in view graph for concurrent groupby
- Go to any graph view (i.e. in CRM)
- Change "Group By" 2 times very very quickly
(In Chrome, you can set Throttling to "Slow 3G" in Network tab of devtools)
Data from first groupby will be mixed with data from the second one.
A graph reload is performing the following steps:
* clean previous data
* fetch data
* set data
The issue happens when a second reload is performed while the previous one
has not set its data yet and the cleaning step of the second one is performed
before the setting step of the first one.
The solution will be to ignore the setting step of all previous reloads if
there is a more recent one.
opw-2460145
closes odoo/odoo#76269
X-original-commit: 1cda59cf236e00ca6463d7212582ded050cc5207
Signed-off-by: Aaron Bohy (aab) <aab@odoo.com>
Signed-off-by: Anh Thao PHAM <kitan191@users.noreply.github.com>
---
.../static/src/js/views/graph/graph_model.js | 13 +++++-----
addons/web/static/tests/views/graph_tests.js | 25 +++++++++++++++++++
2 files changed, 32 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 b1bcddb47c2a..112d474378ff 100644
--- a/addons/web/static/src/js/views/graph/graph_model.js
+++ b/addons/web/static/src/js/views/graph/graph_model.js
@@ -187,6 +187,8 @@ return AbstractModel.extend({
var fields = _.map(groupBy, function (groupBy) {
return groupBy.split(':')[0];
});
+ const loadId = this.loadId ? ++this.loadId : 1;
+ this.loadId = loadId;
if (this.chart.measure !== '__count__') {
if (this.fields[this.chart.measure].type === 'many2one') {
@@ -209,7 +211,7 @@ return AbstractModel.extend({
fields: fields,
groupBy: groupBy,
lazy: false,
- }).then(self._processData.bind(self, originIndex)));
+ }).then(self._processData.bind(self, originIndex, loadId)));
});
return Promise.all(proms);
},
@@ -218,15 +220,14 @@ return AbstractModel.extend({
* depending of some input, we have to normalize the result.
* Each group coming from the read_group produces a dataPoint
*
- * @todo This is not good for race conditions. The processing should get
- * the object this.chart in argument, or an array or something. We want to
- * avoid writing on a this.chart object modified by a subsequent read_group
- *
* @private
* @param {number} originIndex
* @param {any} rawData result from the read_group
*/
- _processData: function (originIndex, rawData) {
+ _processData: function (originIndex, loadId, rawData) {
+ if (loadId < this.loadId) {
+ return;
+ }
var self = this;
var isCount = this.chart.measure === '__count__';
var labels;
diff --git a/addons/web/static/tests/views/graph_tests.js b/addons/web/static/tests/views/graph_tests.js
index fda4c5cb1901..0c5995b8531b 100644
--- a/addons/web/static/tests/views/graph_tests.js
+++ b/addons/web/static/tests/views/graph_tests.js
@@ -681,6 +681,31 @@ QUnit.module('Views', {
graph.destroy();
});
+ QUnit.test('only process most recent data for concurrent groupby', async function (assert) {
+ assert.expect(4);
+
+ const graph = await createView({
+ View: GraphView,
+ model: 'foo',
+ data: this.data,
+ arch: `
+ <graph>
+ <field name="product_id" type="row"/>
+ <field name="foo" type="measure"/>
+ </graph>`,
+ });
+
+ assert.checkLabels(graph, [['xphone'], ['xpad']]);
+ assert.checkDatasets(graph, 'data', {data: [82, 157]});
+
+ testUtils.graph.reload(graph, {groupBy: ['color_id']});
+ await testUtils.graph.reload(graph, {groupBy: ['date:month']});
+ assert.checkLabels(graph, [['January 2016'], ['March 2016'], ['May 2016'], ['Undefined'], ['April 2016']]);
+ assert.checkDatasets(graph, 'data', {data: [56, 26, 4, 105, 48]});
+
+ graph.destroy();
+ });
+
QUnit.test('use a many2one as a measure should work (without groupBy)', async function (assert) {
assert.expect(4);
--
GitLab