From 33ba61bfdbdc1d9d986070c677f6bf18cbacf6b2 Mon Sep 17 00:00:00 2001
From: Nicolas Martinelli <nim@odoo.com>
Date: Tue, 10 Apr 2018 14:54:33 +0200
Subject: [PATCH] [FIX] web: domain evaluation

To reproduce the error, go to Accounting > Import Statement, then
Cancel. An error occurs because `belongs_to_company` is not in the data.
Indeed, it is directly set on the action
`open_account_journal_dashboard_kanban`.

Commit 9961bbf4eb87f re-evaluates if a record should be displayed in the
Kanban view after applying an action (in particular, archive a record).
However, the domain evaluated might contain values which are not in the
data. This is for example the case if there is a domain on an action.

Fixes #24142
opw-1835216
opw-1835123
---
 .../src/js/views/kanban/kanban_controller.js  | 43 ++++++++++++++++---
 addons/web/static/tests/views/kanban_tests.js | 31 +++++++++++++
 2 files changed, 69 insertions(+), 5 deletions(-)

diff --git a/addons/web/static/src/js/views/kanban/kanban_controller.js b/addons/web/static/src/js/views/kanban/kanban_controller.js
index 20a8e11cd52..11d17a78fa9 100644
--- a/addons/web/static/src/js/views/kanban/kanban_controller.js
+++ b/addons/web/static/src/js/views/kanban/kanban_controller.js
@@ -238,12 +238,45 @@ var KanbanController = BasicController.extend({
                     var kanban_record = event.target;
                     kanban_record.update(data);
 
-                    // Check if we still need to display the record
-                    var domain = parent ? parent.domain : group.domain;
-                    if ('active' in data.data && _.pluck(domain, 0).indexOf('active') === -1) {
-                        domain = [['active', '=', true]].concat(domain);
+                    // Check if we still need to display the record. Some fields of the domain are
+                    // not guaranteed to be in data. This is for example the case if the action
+                    // contains a domain on a field which is not in the Kanban view. Therefore,
+                    // we need to handle multiple cases based on 3 variables:
+                    // domInData: all domain fields are in the data
+                    // activeInDomain: 'active' is already in the domain
+                    // activeInData: 'active' is available in the data
+                    
+                    var domain = (parent ? parent.domain : group.domain) || [];
+                    var domInData = _.every(domain, function (d) {
+                        return d[0] in data.data;
+                    });
+                    var activeInDomain = _.pluck(domain, 0).indexOf('active') !== -1;
+                    var activeInData = 'active' in data.data;
+
+                    // Case # | domInData | activeInDomain | activeInData
+                    //   1    |   true    |      true      |      true     => no domain change
+                    //   2    |   true    |      true      |      false    => not possible
+                    //   3    |   true    |      false     |      true     => add active in domain
+                    //   4    |   true    |      false     |      false    => no domain change
+                    //   5    |   false   |      true      |      true     => no evaluation
+                    //   6    |   false   |      true      |      false    => no evaluation
+                    //   7    |   false   |      false     |      true     => replace domain
+                    //   8    |   false   |      false     |      false    => no evaluation
+
+                    // There are 3 cases which cannot be evaluated since we don't have all the
+                    // necessary information. The complete solution would be to perform a RPC in
+                    // these cases, but this is out of scope. A simpler one is to do a try / catch.
+
+                    if (domInData && !activeInDomain && activeInData) {
+                        domain = domain.concat([['active', '=', true]]);
+                    } else if (!domInData && !activeInDomain && activeInData) {
+                        domain = [['active', '=', true]];
+                    }
+                    try {
+                        var visible = new Domain(domain).compute(data.evalContext);
+                    } catch (e) {
+                        return;
                     }
-                    var visible = new Domain(domain).compute(data.evalContext);
                     if (!visible) {
                         kanban_record.destroy();
                     }
diff --git a/addons/web/static/tests/views/kanban_tests.js b/addons/web/static/tests/views/kanban_tests.js
index aa40af6fa7a..a820eeb4876 100644
--- a/addons/web/static/tests/views/kanban_tests.js
+++ b/addons/web/static/tests/views/kanban_tests.js
@@ -1752,6 +1752,37 @@ QUnit.module('Views', {
         kanban.destroy();
     });
 
+    QUnit.test('button executes action with domain field not in view', function (assert) {
+        assert.expect(1);
+
+        var kanban = createView({
+            View: KanbanView,
+            model: "partner",
+            data: this.data,
+            domain: [['bar', '=', true]],
+            arch:
+                '<kanban>' +
+                    '<templates><div t-name="kanban-box">' +
+                        '<field name="foo"/>' +
+                        '<button type="object" name="a1" />' +
+                        '<button type="object" name="toggle_action" />' +
+                    '</div></templates>' +
+                '</kanban>',
+        });
+
+        testUtils.intercept(kanban, 'execute_action', function (event) {
+            event.data.on_closed();
+        });
+
+        try {
+            kanban.$('.o_kanban_record:contains(yop) button[data-name="toggle_action"]').click();
+            assert.strictEqual(true, true, 'Everything went fine');
+        } catch (e) {
+            assert.strictEqual(true, false, 'Error triggered at action execution');
+        }
+        kanban.destroy();
+    });
+
     QUnit.test('rendering date and datetime', function (assert) {
         assert.expect(2);