From 8bcb17b67415e4e28fbb1fc6b5feae767f0b8b1e Mon Sep 17 00:00:00 2001
From: Priyanka Kakadiya <pka@odoo.com>
Date: Wed, 10 Apr 2019 09:44:41 +0000
Subject: [PATCH] [FIX] web: grouped Kanban: traceback on click during reload
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Have a grouped Kanban view with an x2many field. Interact with the
searchview, and then click anywhere.

The fetch of the x2many relational data is batched for the whole view
a single RPC for all groups) since e10fd346.

If the click occurs before all search_read are done (i.e. before the
fetch of relational data is started), there is a crash.

This is due to a call to get({raw: true}) to obtain the current model's
state, which is currently being reloaded and in an intermediate state.
Ideally, this shouldn't happen, but with the sync API of get, we can't
change that easily. So at most, we can ensure that it doesn't crash.
Moreover, sensitive functions requesting the state for the data can do
that in the mutex to ensure that the state isn't currently being reloaded.

Related to Issue: 1937149

closes odoo/odoo#32587

Signed-off-by: Géry Debongnie (ged) <ged@openerp.com>


Co-authored-by: Mohammed Shekha <msh@openerp.com>
Co-authored-by: Aaron Bohy <aab@odoo.com>
---
 .../static/src/js/views/basic/basic_model.js  | 13 +++-
 .../static/tests/views/kanban_model_tests.js  | 64 +++++++++++++++++++
 2 files changed, 74 insertions(+), 3 deletions(-)

diff --git a/addons/web/static/src/js/views/basic/basic_model.js b/addons/web/static/src/js/views/basic/basic_model.js
index 0314eed5a2f..95003e2226f 100644
--- a/addons/web/static/src/js/views/basic/basic_model.js
+++ b/addons/web/static/src/js/views/basic/basic_model.js
@@ -563,9 +563,16 @@ var BasicModel = AbstractModel.extend({
                     }
                 } else if (field.type === 'one2many' || field.type === 'many2many') {
                     if (options.raw) {
-                        relDataPoint = this.localData[data[fieldName]];
-                        relDataPoint = this._applyX2ManyOperations(relDataPoint);
-                        data[fieldName] = relDataPoint.res_ids;
+                        if (typeof data[fieldName] === 'string') {
+                            relDataPoint = this.localData[data[fieldName]];
+                            relDataPoint = this._applyX2ManyOperations(relDataPoint);
+                            data[fieldName] = relDataPoint.res_ids;
+                        } else {
+                            // no datapoint has been created yet (because the loading of relational
+                            // data has been batched, and hasn't started yet), so the value is still
+                            // the list of ids in the relation
+                            data[fieldName] = data[fieldName] || [];
+                        }
                     } else {
                         data[fieldName] = this.get(data[fieldName]) || [];
                     }
diff --git a/addons/web/static/tests/views/kanban_model_tests.js b/addons/web/static/tests/views/kanban_model_tests.js
index 20eb2918d04..2bc75f93d39 100644
--- a/addons/web/static/tests/views/kanban_model_tests.js
+++ b/addons/web/static/tests/views/kanban_model_tests.js
@@ -293,6 +293,70 @@ QUnit.module('Views', {
 
         model.destroy();
     });
+
+    QUnit.test('call get (raw: true) before loading x2many data', function (assert) {
+        // Sometimes, get can be called on a datapoint that is currently being
+        // reloaded, and thus in a partially updated state (e.g. in a kanban
+        // view, the user interacts with the searchview, and before the view is
+        // fully reloaded, it clicks on CREATE). Ideally, this shouldn't happen,
+        // but with the sync API of get, we can't change that easily. So at most,
+        // we can ensure that it doesn't crash. Moreover, sensitive functions
+        // requesting the state for more precise information that, e.g., the
+        // count, can do that in the mutex to ensure that the state isn't
+        // currently being reloaded.
+        // In this test, we have a grouped kanban view with a one2many, whose
+        // relational data is loaded in batch, once for all groups. We call get
+        // when the search_read for the first group has returned, but not the
+        // second (and thus, the read of the one2many hasn't started yet).
+        // Note: this test can be removed as soon as search_reads are performed
+        // alongside read_group.
+        assert.expect(2);
+
+        this.data.partner.records[1].product_ids = [37, 41];
+        this.params.fieldsInfo = {
+            kanban: {
+                product_ids: {
+                    fieldsInfo: {
+                        default: { display_name: {}, color: {} },
+                    },
+                    relatedFields: this.data.product.fields,
+                    viewType: 'default',
+                },
+            },
+        };
+        this.params.viewType = 'kanban';
+        this.params.groupedBy = ['foo'];
+
+        var block;
+        var def = $.Deferred();
+        var model = createModel({
+            Model: KanbanModel,
+            data: this.data,
+            mockRPC: function (route) {
+                var result = this._super.apply(this, arguments);
+                if (route === '/web/dataset/search_read' && block) {
+                    block = false;
+                    return $.when(def).then(_.constant(result));
+                }
+                return result;
+            },
+        });
+
+        model.load(this.params).then(function (handle) {
+            block = true;
+            model.reload(handle, {});
+
+            var state = model.get(handle, {raw: true});
+            assert.strictEqual(state.count, 2);
+
+            def.resolve();
+
+            state = model.get(handle, {raw: true});
+            assert.strictEqual(state.count, 2);
+        });
+
+        model.destroy();
+    });
 });
 
 });