From 51e9aba2cb4c9006ca9906562d6b0057cc1f1e85 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?G=C3=A9ry=20Debongnie?= <ged@odoo.com>
Date: Wed, 26 Apr 2017 21:06:18 +0200
Subject: [PATCH] [FIX] web: do_action should properly resolves when complete

We changed the do_action method recently: before, it was calling
recursively its parent, now it simply triggers up an event.  We also
moved it to the 'services' mixin.  However, we did not properly connect
the 'on_success' option: it is not an option for the view manager, but
for the abstract web client.  As such, it should not be in the options
parameter, but it should simply be a key in the event payload.

This commit was found by solving the two following issues:
- open sale application, click on 'Inbox' icon to go to discuss,
  Discuss is opened, but the navbar stays with sales application
- after installing module website_crm_partner_assign in kanban view for apps,
  it does not reload...

Both of these were caused by the need to do something after a do_action,
and the deferred was not resolved at all.
---
 addons/web/static/src/js/core/mixins.js      | 24 ++++++++------
 addons/web/static/tests/core/mixins_tests.js | 34 ++++++++++++++++++++
 addons/web/views/webclient_templates.xml     |  1 +
 3 files changed, 49 insertions(+), 10 deletions(-)
 create mode 100644 addons/web/static/tests/core/mixins_tests.js

diff --git a/addons/web/static/src/js/core/mixins.js b/addons/web/static/src/js/core/mixins.js
index fff2a013bb5..9c643597f04 100644
--- a/addons/web/static/src/js/core/mixins.js
+++ b/addons/web/static/src/js/core/mixins.js
@@ -495,19 +495,23 @@ var ServicesMixin = {
         return session;
     },
     /**
-     * Informs the action manager to do an action. This supposes that
-     * the action manager can be found amongst the ancestors of the current widget.
-     * If that's not the case this method will simply return `false`.
+     * Informs the action manager to do an action. This supposes that the action
+     * manager can be found amongst the ancestors of the current widget.
+     * If that's not the case this method will simply return an unresolved
+     * deferred.
+     *
+     * @param {any} action
+     * @param {any} options
+     * @returns {Deferred}
      */
     do_action: function (action, options) {
         var def = $.Deferred();
-        options = options || {};
-        var oldOnSuccess = options.on_success;
-        options.on_success = function (result) {
-            oldOnSuccess(result);
-            def.resolve(result);
-        };
-        this.trigger_up('do_action', {'action': action, options: options});
+
+        this.trigger_up('do_action', {
+            action: action,
+            options: options,
+            on_success: function (result) { def.resolve(result); },
+        });
         return def;
     },
     do_notify: function (title, message, sticky, className) {
diff --git a/addons/web/static/tests/core/mixins_tests.js b/addons/web/static/tests/core/mixins_tests.js
new file mode 100644
index 00000000000..c49c33ec648
--- /dev/null
+++ b/addons/web/static/tests/core/mixins_tests.js
@@ -0,0 +1,34 @@
+odoo.define('web.mixins_tests', function (require) {
+"use strict";
+
+var testUtils = require('web.test_utils');
+var Widget = require('web.Widget');
+
+QUnit.module('core', {}, function () {
+
+    QUnit.module('mixins');
+
+    QUnit.test('perform a do_action properly', function (assert) {
+        assert.expect(3);
+
+        var widget = new Widget();
+
+        testUtils.intercept(widget, 'do_action', function (event) {
+            assert.strictEqual(event.data.action, 'test.some_action_id',
+                "should have sent proper action name");
+            assert.deepEqual(event.data.options, {clear_breadcrumbs: true},
+                "should have sent proper options");
+            event.data.on_success();
+        });
+
+        widget.do_action('test.some_action_id', {clear_breadcrumbs: true}).then(function () {
+            assert.step('deferred should have been resolved');
+        });
+        widget.destroy();
+    });
+
+
+});
+
+});
+
diff --git a/addons/web/views/webclient_templates.xml b/addons/web/views/webclient_templates.xml
index 27831759b4b..e5789e9e7aa 100644
--- a/addons/web/views/webclient_templates.xml
+++ b/addons/web/views/webclient_templates.xml
@@ -450,6 +450,7 @@
                 <script type="text/javascript" src="/web/static/tests/core/class_tests.js"></script>
                 <script type="text/javascript" src="/web/static/tests/core/rpc_tests.js"></script>
                 <script type="text/javascript" src="/web/static/tests/core/domain_tests.js"></script>
+                <script type="text/javascript" src="/web/static/tests/core/mixins_tests.js"></script>
                 <script type="text/javascript" src="/web/static/tests/core/time_tests.js"></script>
                 <script type="text/javascript" src="/web/static/tests/core/concurrency_tests.js"></script>
                 <script type="text/javascript" src="/web/static/tests/core/util_tests.js"></script>