From ea2207afeab8c4d8da6b010aebd99f896e04359b Mon Sep 17 00:00:00 2001 From: Aaron Bohy Date: Thu, 4 Jul 2019 13:34:51 +0000 Subject: [PATCH] [FIX] web: don't close dialog on doAction with target 'new' MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- .../static/src/js/chrome/action_manager.js | 4 +- addons/web/static/src/js/core/dialog.js | 6 +- .../src/js/views/form/form_controller.js | 21 ----- .../tests/chrome/action_manager_tests.js | 74 +++++++++++++-- addons/web/static/tests/views/form_tests.js | 93 ------------------- 5 files changed, 75 insertions(+), 123 deletions(-) diff --git a/addons/web/static/src/js/chrome/action_manager.js b/addons/web/static/src/js/chrome/action_manager.js index 014aa918e54..3d2dc7a4076 100644 --- a/addons/web/static/src/js/chrome/action_manager.js +++ b/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(); }, diff --git a/addons/web/static/src/js/core/dialog.js b/addons/web/static/src/js/core/dialog.js index 8f128b657d9..e3101e52bde 100644 --- a/addons/web/static/src/js/core/dialog.js +++ b/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 diff --git a/addons/web/static/src/js/views/form/form_controller.js b/addons/web/static/src/js/views/form/form_controller.js index 827afbb99ed..89477f9c974 100644 --- a/addons/web/static/src/js/views/form/form_controller.js +++ b/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 * diff --git a/addons/web/static/tests/chrome/action_manager_tests.js b/addons/web/static/tests/chrome/action_manager_tests.js index 578fcfbd660..8a6675ecc58 100644 --- a/addons/web/static/tests/chrome/action_manager_tests.js +++ b/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'] = '
' + + '' + + '' + + '' + + '' + + ''; + + 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'] = '
' + + '' + + '' + + '' + + '' + + ''; + + 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({ diff --git a/addons/web/static/tests/views/form_tests.js b/addons/web/static/tests/views/form_tests.js index 5ce5718178b..5a59928a711 100644 --- a/addons/web/static/tests/views/form_tests.js +++ b/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:'
' + - '' + - '' + - '' + - '' + - '', - 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:'
' + - '' + - '' + - '' + - '
', - archs: { - 'partner,false,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);