From 7b6744ecb78106bca3daa23d4ded490c43479703 Mon Sep 17 00:00:00 2001
From: Nicolas Lempereur <nle@odoo.com>
Date: Tue, 24 Apr 2018 15:03:27 +0200
Subject: [PATCH] [FIX] web: pivot race condition load data

When doing several grouping quickly (or if the system is slow) on the
pivot view, we may try to display data that now doesn't corresponds to
the current selected groupBys.

This commit ensures that when we load data of the pivot view, we drop
the previous data loading that are currently being carried out.

Without the fix, the added test failed with:

  TypeError: Cannot read property 'length' of undefined

because the test does:

- select one level of group by
- select two levels of group by
- resolve the first level of group by
- resolve the second level of group by

Without the fix, we would try to display the data for one level of group
by when the pivot view had two levels selected.

With the fix, displaying the first level of group by was discarded once
a second one was being requested.

opw-1825892
closes #24396
---
 .../static/src/js/views/pivot/pivot_model.js  |  6 ++--
 addons/web/static/tests/views/pivot_tests.js  | 33 +++++++++++++++++++
 2 files changed, 37 insertions(+), 2 deletions(-)

diff --git a/addons/web/static/src/js/views/pivot/pivot_model.js b/addons/web/static/src/js/views/pivot/pivot_model.js
index 6eddd44ffe7..76e22c8b4e5 100644
--- a/addons/web/static/src/js/views/pivot/pivot_model.js
+++ b/addons/web/static/src/js/views/pivot/pivot_model.js
@@ -17,6 +17,7 @@ odoo.define('web.PivotModel', function (require) {
  */
 
 var AbstractModel = require('web.AbstractModel');
+var concurrency = require('web.concurrency');
 var core = require('web.core');
 var session = require('web.session');
 var utils = require('web.utils');
@@ -32,6 +33,7 @@ var PivotModel = AbstractModel.extend({
         this._super.apply(this, arguments);
         this.numbering = {};
         this.data = null;
+        this._loadDataDropPrevious = new concurrency.DropPrevious();
     },
 
     //--------------------------------------------------------------------------
@@ -532,7 +534,7 @@ var PivotModel = AbstractModel.extend({
             }
         }
 
-        return $.when.apply(null, groupBys.map(function (groupBy) {
+        return this._loadDataDropPrevious.add($.when.apply(null, groupBys.map(function (groupBy) {
             return self._rpc({
                     model: self.modelName,
                     method: 'read_group',
@@ -542,7 +544,7 @@ var PivotModel = AbstractModel.extend({
                     groupBy: groupBy,
                     lazy: false,
                 });
-        })).then(function () {
+        }))).then(function () {
             var data = Array.prototype.slice.call(arguments);
             if (data[0][0].__count === 0) {
                 self.data.has_data = false;
diff --git a/addons/web/static/tests/views/pivot_tests.js b/addons/web/static/tests/views/pivot_tests.js
index 198fd0189ae..8508c7e7fed 100644
--- a/addons/web/static/tests/views/pivot_tests.js
+++ b/addons/web/static/tests/views/pivot_tests.js
@@ -1001,4 +1001,37 @@ QUnit.module('Views', {
 
         pivot.destroy();
     });
+
+    QUnit.test('parallel data loading should discard all but the last one', function (assert) {
+        assert.expect(2);
+
+        var def;
+
+        var pivot = createView({
+            View: PivotView,
+            model: "partner",
+            data: this.data,
+            arch: '<pivot>' +
+                      '<field name="foo" type="measure"/>' +
+                  '</pivot>',
+            mockRPC: function (route, args) {
+                var result = this._super.apply(this, arguments);
+                if (args.method === 'read_group') {
+                    return $.when(def).then(_.constant(result));
+                }
+                return result;
+            },
+        });
+
+        def = $.Deferred();
+        pivot.update({groupBy: ['product_id']});
+        pivot.update({groupBy: ['product_id', 'customer']});
+        def.resolve();
+
+        assert.strictEqual(pivot.$('.o_pivot_cell_value').length, 6,
+            "should have 6 cells");
+        assert.strictEqual(pivot.$('tbody tr').length, 6,
+            "should have 6 rows");
+        pivot.destroy();
+    });
 });});