[FIX] web: don't close dialog on doAction with target 'new'

Rev. ee44d23 fixed a usability issue: when a doAction is performed
from a dialog, it automatically closes the dialog (as the action is
rendered in the background, behind the dialog).

However, it also closes dialogs when the action has target='new',
i.e. when it also opens in a dialog, which is not strictly
necessary in this case.

This is problematic in mobile for the product configurator widget:
in the sale.order form view, the order_line field is displayed as a
kanban, and in its form view, the product_(template_)id field has
widget product_configurator. When a value is set, it performs a
doAction (target="new") to configure the product. The widget listens
to the dialog's 'closed' event to update itself. However, with what
is done in ee44d23, the x2many form dialog is closed by the DoAction,
so the flow doesn't work.

This rev. reverts the attempt done in ee44d23, and does something a
bit more brutal: as soon as an action is pushed in target 'current'
(in the main part of the interface), all dialogs are automatically
closed.

closes odoo/odoo#34678

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

addons/web/static/src/js/chrome/action_manager.js

@@ -12,7 +12,6 @@ odoo.define('web.ActionManager', function (require) {
 var AbstractAction = require('web.AbstractAction');
 var concurrency = require('web.concurrency');
 var Context = require('web.Context');
-var config = require('web.config');
 var core = require('web.core');
 var Dialog = require('web.Dialog');
 var dom = require('web.dom');
@@ -717,6 +716,9 @@ var ActionManager = Widget.extend({
             controller: controller,
         });
 
+        // close all dialogs when the current controller changes
+        core.bus.trigger('close_dialogs');
+
         // toggle the fullscreen mode for actions in target='fullscreen'
         this._toggleFullscreen();
     },

addons/web/static/src/js/core/dialog.js

@@ -22,8 +22,8 @@ var Dialog = Widget.extend({
     custom_events: _.extend({}, Widget.prototype.custom_events, {
         focus_control_button: '_onFocusControlButton',
     }),
-    events: _.extend({} , Widget.prototype.events, {
-        'keydown .modal-footer button':'_onFooterButtonKeyDown',
+    events: _.extend({}, Widget.prototype.events, {
+        'keydown .modal-footer button': '_onFooterButtonKeyDown',
     }),
     /**
      * @param {Widget} parent
@@ -92,6 +92,8 @@ var Dialog = Widget.extend({
         this.backdrop = options.backdrop;
         this.renderHeader = options.renderHeader;
         this.renderFooter = options.renderFooter;
+
+        core.bus.on('close_dialogs', this, this.destroy.bind(this));
     },
     /**
      * Wait for XML dependencies and instantiate the modal structure (except

addons/web/static/src/js/views/form/form_controller.js

@@ -14,7 +14,6 @@ var FormController = BasicController.extend({
     custom_events: _.extend({}, BasicController.prototype.custom_events, {
         bounce_edit: '_onBounceEdit',
         button_clicked: '_onButtonClicked',
-        do_action: '_onDoAction',
         edited_list: '_onEditedList',
         open_one2many_record: '_onOpenOne2ManyRecord',
         open_record: '_onOpenRecord',
@@ -543,26 +542,6 @@ var FormController = BasicController.extend({
     _onDiscard: function () {
         this._discardChanges();
     },
-    /**
-     * Destroy subdialog widgets after an action is finished.
-     *
-     * @param {OdooEvent} ev
-     * @private
-     */
-    _onDoAction: function (ev) {
-        var self = this;
-        // A priori, different widgets could write on the "on_success" key.
-        // Below we ensure that all the actions required by those widgets
-        // are executed in a suitable order before every cycle of destruction.
-        var callback = ev.data.on_success || function () {};
-        ev.data.on_success = function () {
-            callback();
-            function isDialog (widget) {
-                return (widget instanceof Dialog);
-            }
-            _.invoke(self.getChildren().filter(isDialog), 'destroy');
-        };
-    },
     /**
      * Called when the user clicks on 'Duplicate Record' in the sidebar
      *

addons/web/static/tests/chrome/action_manager_tests.js

@@ -23,13 +23,14 @@ QUnit.module('ActionManager', {
                 fields: {
                     foo: {string: "Foo", type: "char"},
                     bar: {string: "Bar", type: "many2one", relation: 'partner'},
+                    o2m: {string: "One2Many", type: "one2many", relation: 'partner', relation_field: 'bar'},
                 },
                 records: [
-                    {id: 1, display_name: "First record", foo: "yop", bar: 2},
-                    {id: 2, display_name: "Second record", foo: "blip", bar: 1},
-                    {id: 3, display_name: "Third record", foo: "gnap", bar: 1},
-                    {id: 4, display_name: "Fourth record", foo: "plop", bar: 2},
-                    {id: 5, display_name: "Fifth record", foo: "zoup", bar: 2},
+                    {id: 1, display_name: "First record", foo: "yop", bar: 2, o2m: [2, 3]},
+                    {id: 2, display_name: "Second record", foo: "blip", bar: 1, o2m: [1, 4, 5]},
+                    {id: 3, display_name: "Third record", foo: "gnap", bar: 1, o2m: []},
+                    {id: 4, display_name: "Fourth record", foo: "plop", bar: 2, o2m: []},
+                    {id: 5, display_name: "Fifth record", foo: "zoup", bar: 2, o2m: []},
                 ],
             },
             pony: {
@@ -512,6 +513,68 @@ QUnit.module('ActionManager', {
         actionManager.destroy();
     });
 
+    QUnit.test('executing an action with target != "new" closes all dialogs', async function (assert) {
+        assert.expect(4);
+
+        this.archs['partner,false,form'] = '<form>' +
+                '<field name="o2m">' +
+                    '<tree><field name="foo"/></tree>' +
+                    '<form><field name="foo"/></form>' +
+                '</field>' +
+            '</form>';
+
+        var actionManager = await createActionManager({
+            actions: this.actions,
+            archs: this.archs,
+            data: this.data,
+        });
+
+        await actionManager.doAction(3);
+        assert.containsOnce(actionManager, '.o_list_view');
+
+        await testUtils.dom.click(actionManager.$('.o_list_view .o_data_row:first'));
+        assert.containsOnce(actionManager, '.o_form_view');
+
+        await testUtils.dom.click(actionManager.$('.o_form_view .o_data_row:first'));
+        assert.containsOnce(document.body, '.modal .o_form_view');
+
+        await actionManager.doAction(1); // target != 'new'
+        assert.containsNone(document.body, '.modal');
+
+        actionManager.destroy();
+    });
+
+    QUnit.test('executing an action with target "new" does not close dialogs', async function (assert) {
+        assert.expect(4);
+
+        this.archs['partner,false,form'] = '<form>' +
+                '<field name="o2m">' +
+                    '<tree><field name="foo"/></tree>' +
+                    '<form><field name="foo"/></form>' +
+                '</field>' +
+            '</form>';
+
+        var actionManager = await createActionManager({
+            actions: this.actions,
+            archs: this.archs,
+            data: this.data,
+        });
+
+        await actionManager.doAction(3);
+        assert.containsOnce(actionManager, '.o_list_view');
+
+        await testUtils.dom.click(actionManager.$('.o_list_view .o_data_row:first'));
+        assert.containsOnce(actionManager, '.o_form_view');
+
+        await testUtils.dom.click(actionManager.$('.o_form_view .o_data_row:first'));
+        assert.containsOnce(document.body, '.modal .o_form_view');
+
+        await actionManager.doAction(5); // target 'new'
+        assert.containsN(document.body, '.modal .o_form_view', 2);
+
+        actionManager.destroy();
+    });
+
     QUnit.module('Push State');
 
     QUnit.test('properly push state', async function (assert) {
@@ -3849,7 +3912,6 @@ QUnit.module('ActionManager', {
         delete core.action_registry.map.slowAction;
     });
 
-
     QUnit.test('abstract action does not crash on navigation_moves', async function (assert) {
         assert.expect(1);
         var ClientAction = AbstractAction.extend({

addons/web/static/tests/views/form_tests.js

@@ -7375,99 +7375,6 @@ QUnit.module('Views', {
         form.destroy();
     });
 
-    QUnit.test('a popup window should automatically close after a do_action event', async function (assert) {
-        // Having clicked on a one2many in a form view and clicked on a many2one
-        // field in the resulting popup window that popup window should automatically close.
-
-        assert.expect(2);
-
-        this.data.partner.records[0].product_ids = [37];
-        this.data.product.records[0].partner_type_id = 12;
-
-        var form = await createView({
-            View: FormView,
-            model: 'partner',
-            data: this.data,
-            arch:'<form>' +
-                    '<field name="product_ids">' +
-                        '<tree><field name="partner_type_id"/></tree>' +
-                        '<form><field name="partner_type_id"/></form>' +
-                    '</field>' +
-                '</form>',
-            res_id: 1,
-            mockRPC: function (route, args) {
-                if (args.method === 'get_formview_action' && args.model === 'partner_type') {
-                    return Promise.resolve();
-                }
-                return this._super(route, args);
-            },
-            intercepts: {
-                do_action: function (event) {
-                    event.data.on_success();
-                }
-            },
-        });
-        // Open one2many
-        await testUtils.dom.click(form.$('.o_data_row'));
-        assert.strictEqual($('.modal-content').length, 1, "a popup window should have opened");
-        // Click on many2one and trigger do_action
-        await testUtils.dom.click($('.modal-content a[name="partner_type_id"]'));
-        assert.strictEqual($('.modal-content').length, 0, "the popup window should have closed");
-
-        form.destroy();
-    });
-
-    QUnit.test('all popup windows should automatically close after a do_action event', async function (assert) {
-
-        // Having clicked successively on two different one2many in form views
-        // and clicked on a many2one in the last popup window all popup
-        // windows should automatically close.
-
-        assert.expect(2);
-
-        this.data.partner.records[0].p = [2];
-        this.data.partner.records[1].product_ids = [37];
-        this.data.product.records[0].partner_type_id = 12;
-
-        var form = await createView({
-            View: FormView,
-            model: 'partner',
-            data: this.data,
-            arch:'<form>' +
-                        '<field name="p">' +
-                            '<tree><field name="display_name"/></tree>' +
-                        '</field>' +
-                '</form>',
-            archs: {
-                'partner,false,form': '<form><field name="product_ids">' +
-                                            '<tree><field name="partner_type_id"/></tree>' +
-                                            '<form><field name="partner_type_id"/></form>' +
-                                        '</field></form>',
-            },
-            res_id: 1,
-            mockRPC: function (route, args) {
-                if (args.method === 'get_formview_action' && args.model === 'partner_type') {
-                    return Promise.resolve();
-                }
-                return this._super(route, args);
-            },
-            intercepts: {
-                do_action: function (event) {
-                    event.data.on_success();
-                }
-            },
-        });
-        // Open two one2manys
-        await testUtils.dom.click(form.$('.o_data_row'));
-        await testUtils.dom.click($('.modal-content .o_data_row'));
-        assert.strictEqual($('.modal-content').length, 2, "Two popup windows should have opened.");
-        // Click on many2one and trigger do_action
-        await testUtils.dom.click($('.modal-content a[name="partner_type_id"]'));
-        assert.strictEqual($('.modal-content').length, 0, "All popup windows should have closed.");
-
-        form.destroy();
-    });
-
     QUnit.test('keep editing after call_button fail', async function (assert) {
         assert.expect(4);