From 1cda59cf236e00ca6463d7212582ded050cc5207 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#75307
Signed-off-by: Aaron Bohy (aab) <aab@odoo.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 dd9a9e1a9bfc..bcca914a36f9 100644
--- a/addons/web/static/src/js/views/graph/graph_model.js
+++ b/addons/web/static/src/js/views/graph/graph_model.js
@@ -194,6 +194,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') {
@@ -216,7 +218,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(defs);
},
@@ -225,15 +227,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 d0cbe9d7a2e0..077d48991939 100644
--- a/addons/web/static/tests/views/graph_tests.js
+++ b/addons/web/static/tests/views/graph_tests.js
@@ -629,6 +629,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