From 04ba755f82c237f825cfdf331969e398a148b83b Mon Sep 17 00:00:00 2001
From: Aaron Bohy <aab@odoo.com>
Date: Thu, 24 May 2018 16:30:04 +0200
Subject: [PATCH] [FIX] web: AbstractController: handle concurrent reloads

Let's assume the following scenario in a Kanban view with a default
filter. The user removes the filter, and quickly adds a new one.
On a slow network, it may happen that the second reload request
returns before the first one. On odoo.com, this is easy to
reproduce on the tasks Kanban view.

Before this rev., when this occured, the result of the first
request was displayed, whereas it should have been ignored as
another request was done later on.
---
 .../src/js/views/abstract_controller.js       |  8 +++-
 addons/web/static/tests/views/list_tests.js   | 41 ++++++++++++++++++-
 2 files changed, 46 insertions(+), 3 deletions(-)

diff --git a/addons/web/static/src/js/views/abstract_controller.js b/addons/web/static/src/js/views/abstract_controller.js
index 6a3f9a3bbaa..2391fb1fa8c 100644
--- a/addons/web/static/src/js/views/abstract_controller.js
+++ b/addons/web/static/src/js/views/abstract_controller.js
@@ -12,6 +12,7 @@ odoo.define('web.AbstractController', function (require) {
  * reading localstorage, ...) has to go through the controller.
  */
 
+var concurrency = require('web.concurrency');
 var Widget = require('web.Widget');
 
 
@@ -41,6 +42,9 @@ var AbstractController = Widget.extend({
         this.handle = params.handle;
         this.activeActions = params.activeActions;
         this.initialState = params.initialState;
+
+        // use a DropPrevious to correctly handle concurrent updates
+        this.dp = new concurrency.DropPrevious();
     },
     /**
      * Simply renders and updates the url.
@@ -206,11 +210,11 @@ var AbstractController = Widget.extend({
         var self = this;
         var shouldReload = (options && 'reload' in options) ? options.reload : true;
         var def = shouldReload ? this.model.reload(this.handle, params) : $.when();
-        return def.then(function (handle) {
+        return this.dp.add(def).then(function (handle) {
             self.handle = handle || self.handle; // update handle if we reloaded
             var state = self.model.get(self.handle);
             var localState = self.renderer.getLocalState();
-            return self.renderer.updateState(state, params).then(function () {
+            return self.dp.add(self.renderer.updateState(state, params)).then(function () {
                 self.renderer.setLocalState(localState);
                 self._update(state);
             });
diff --git a/addons/web/static/tests/views/list_tests.js b/addons/web/static/tests/views/list_tests.js
index f1c36aac0a7..f655151f995 100644
--- a/addons/web/static/tests/views/list_tests.js
+++ b/addons/web/static/tests/views/list_tests.js
@@ -3310,7 +3310,6 @@ QUnit.module('Views', {
         list.destroy();
     });
 
-
     QUnit.test('basic support for widgets', function (assert) {
         assert.expect(1);
 
@@ -3416,6 +3415,46 @@ QUnit.module('Views', {
         testUtils.unpatch(mixins.ParentedMixin);
     });
 
+    QUnit.test('concurrent reloads finishing in inverse order', function (assert) {
+        assert.expect(3);
+
+        var blockSearchRead = false;
+        var def = $.Deferred();
+        var list = createView({
+            View: ListView,
+            model: 'foo',
+            data: this.data,
+            arch: '<tree><field name="foo"/></tree>',
+            mockRPC: function (route) {
+                var result = this._super.apply(this, arguments);
+                if (route === '/web/dataset/search_read' && blockSearchRead) {
+                    return $.when(def).then(_.constant(result));
+                }
+                return result;
+            },
+        });
+
+        assert.strictEqual(list.$('.o_list_view .o_data_row').length, 4,
+            "list view should contain 4 records");
+
+        // reload with a domain (this request is blocked)
+        blockSearchRead = true;
+        list.reload({domain: [['foo', '=', 'yop']]});
+
+        assert.strictEqual(list.$('.o_list_view .o_data_row').length, 4,
+            "list view should still contain 4 records (search_read being blocked)");
+
+        // reload without the domain
+        blockSearchRead = false;
+        list.reload({domain: []});
+
+        // unblock the RPC
+        def.resolve();
+        assert.strictEqual(list.$('.o_list_view .o_data_row').length, 4,
+            "list view should still contain 4 records");
+
+        list.destroy();
+    });
 });
 
 });