From 580533ecfb32a77700689551c5fd68b8bfbc173d Mon Sep 17 00:00:00 2001 From: "Michael Mattiello (mcm)" Date: Wed, 9 Jun 2021 13:38:59 +0000 Subject: [PATCH] [REF] web, *: change dialog service api MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * iap, web_tour This commit changes the API of the dialog service. Before, the API gave 2 functions: "open" and "close". Now, there is only one function "add" which is the same as "open" but returning a callback that does the close. closes odoo-dev/odoo#940 Related: odoo-dev/enterprise#171 Signed-off-by: Géry Debongnie (ged) --- .../js/insufficient_credit_error_handler.js | 2 +- .../confirmation_dialog.js | 1 + addons/web/static/src/core/dialog/dialog.js | 4 +- .../src/core/dialog/dialog_container.js | 36 +++++------ .../static/src/core/dialog/dialog_service.js | 64 ++++++++++++------- .../static/src/core/errors/error_handlers.js | 10 +-- .../web/static/src/legacy/action_adapters.js | 2 +- addons/web/static/src/legacy/debug_manager.js | 11 ++-- .../src/legacy/legacy_rpc_error_handler.js | 2 +- .../src/webclient/actions/action_dialog.js | 1 + .../src/webclient/actions/action_service.js | 20 +++--- .../commands/command_palette_dialog.js | 1 + .../src/webclient/commands/command_service.js | 4 +- .../webclient/user_menu/user_menu_items.js | 2 +- .../tests/core/debug/debug_manager_tests.js | 9 ++- .../static/tests/core/dialog_service_tests.js | 26 ++++---- addons/web/static/tests/core/dialog_tests.js | 53 +++++---------- .../tests/core/errors/error_dialogs_tests.js | 19 +++--- .../tests/core/errors/error_service_tests.js | 20 +++--- .../web/static/tests/helpers/mock_services.js | 8 +-- .../tests/legacy/core/owl_dialog_tests.js | 32 ++++------ .../static/src/debug/debug_manager.js | 2 +- 22 files changed, 159 insertions(+), 170 deletions(-) diff --git a/addons/iap/static/src/js/insufficient_credit_error_handler.js b/addons/iap/static/src/js/insufficient_credit_error_handler.js index 0303638265b..e450d413aae 100644 --- a/addons/iap/static/src/js/insufficient_credit_error_handler.js +++ b/addons/iap/static/src/js/insufficient_credit_error_handler.js @@ -41,7 +41,7 @@ function insufficientCreditHandler(env, error, originalError) { } const { data } = originalError; if (data && data.name === "odoo.addons.iap.tools.iap_tools.InsufficientCreditError") { - env.services.dialog.open(InsufficientCreditDialog, { + env.services.dialog.add(InsufficientCreditDialog, { errorData: JSON.parse(data.message), }); return true; diff --git a/addons/web/static/src/core/confirmation_dialog/confirmation_dialog.js b/addons/web/static/src/core/confirmation_dialog/confirmation_dialog.js index 9522ee88f49..b879f17a8a9 100644 --- a/addons/web/static/src/core/confirmation_dialog/confirmation_dialog.js +++ b/addons/web/static/src/core/confirmation_dialog/confirmation_dialog.js @@ -24,6 +24,7 @@ ConfirmationDialog.props = { body: String, confirm: Function, cancel: Function, + close: Function, }; ConfirmationDialog.bodyTemplate = "web.ConfirmationDialogBody"; diff --git a/addons/web/static/src/core/dialog/dialog.js b/addons/web/static/src/core/dialog/dialog.js index 997132e577c..0766058f92a 100644 --- a/addons/web/static/src/core/dialog/dialog.js +++ b/addons/web/static/src/core/dialog/dialog.js @@ -15,7 +15,6 @@ export class Dialog extends Component { ); } this.modalRef = useRef("modal"); - this.dialogService = useService("dialog"); useActiveElement("modal"); useHotkey( "escape", @@ -33,7 +32,6 @@ export class Dialog extends Component { this.size = this.constructor.size; this.technical = this.constructor.technical; this.title = this.constructor.title; - this.__id = null; } /** @@ -41,7 +39,7 @@ export class Dialog extends Component { * @private */ close() { - this.dialogService.close(this.__id); + this.props.close(); } } diff --git a/addons/web/static/src/core/dialog/dialog_container.js b/addons/web/static/src/core/dialog/dialog_container.js index 6862c78b51f..7c01c4a466c 100644 --- a/addons/web/static/src/core/dialog/dialog_container.js +++ b/addons/web/static/src/core/dialog/dialog_container.js @@ -16,37 +16,31 @@ ErrorHandler.template = tags.xml` { - this.dialogs[dialog.id] = dialog; - this.render(); - }); - this.props.bus.on("CLOSE", this, (id) => { - this.closeDialog(id); - }); + this.props.bus.on("ADD", this, this.add); + this.props.bus.on("REMOVE", this, this.remove); + } + __destroy() { + this.props.bus.off("ADD", this); + this.props.bus.off("REMOVE", this); + super.__destroy(); } - onDialogClosed(id) { - this.closeDialog(id); + add(params) { + this.dialogs[params.id] = params; + this.render(); } - - closeDialog(id) { + remove(id) { if (this.dialogs[id]) { - if (this.dialogs[id].options && this.dialogs[id].options.onCloseCallback) { - this.dialogs[id].options.onCloseCallback(); - } delete this.dialogs[id]; this.render(); } } - errorCallBack(id) { - return () => this.closeDialog(id); + onDialogClosed(id) { + this.remove(id); } - - __destroy() { - this.props.bus.off("ADD", this); - this.props.bus.off("CLOSE", this); - super.__destroy(); + errorCallBack(id) { + return () => this.remove(id); } } DialogContainer.components = { ErrorHandler }; diff --git a/addons/web/static/src/core/dialog/dialog_service.js b/addons/web/static/src/core/dialog/dialog_service.js index b3696f43137..b05489d5fb4 100644 --- a/addons/web/static/src/core/dialog/dialog_service.js +++ b/addons/web/static/src/core/dialog/dialog_service.js @@ -7,40 +7,56 @@ import { DialogContainer } from "./dialog_container"; const { core } = owl; const { EventBus } = core; +/** + * @typedef {{ + * onClose?(): void; + * }} DialogServiceInterfaceAddOptions + */ +/** + * @typedef {{ + * add( + * Component: any, + * props: {}, + * options?: DialogServiceInterfaceAddOptions + * ): () => void; + * }} DialogServiceInterface + */ + export const dialogService = { + /** @returns {DialogServiceInterface} */ start() { const bus = new EventBus(); let dialogId = 0; - function open(dialogClass, props, options) { - if (!(dialogClass.prototype instanceof Dialog)) { - throw new Error(dialogClass.name + " must be a subclass of Dialog"); - } - const id = ++dialogId; - class dialogController extends dialogClass { - setup() { - super.setup(); - this.__id = id; - } - } - const dialog = { - id, - class: dialogController, - props, - options, - }; - bus.trigger("ADD", dialog); - return id; - } - function close(id) { - bus.trigger("CLOSE", id); - } registry.category("main_components").add("DialogContainer", { Component: DialogContainer, props: { bus }, }); - return { open, close }; + function add(dialogClass, props, options = {}) { + if (!(dialogClass.prototype instanceof Dialog)) { + throw new Error(`${dialogClass.name} must be a subclass of Dialog`); + } + + const id = ++dialogId; + function close() { + if (options.onClose) { + options.onClose(); + } + bus.trigger("REMOVE", id); + } + + const dialog = { + id, + class: dialogClass, + props: { ...props, close }, + }; + bus.trigger("ADD", dialog); + + return close; + } + + return { add }; }, }; diff --git a/addons/web/static/src/core/errors/error_handlers.js b/addons/web/static/src/core/errors/error_handlers.js index fdbd4b11fbe..1de0938a911 100644 --- a/addons/web/static/src/core/errors/error_handlers.js +++ b/addons/web/static/src/core/errors/error_handlers.js @@ -30,7 +30,7 @@ const errorDialogRegistry = registry.category("error_dialogs"); */ function corsErrorHandler(env, error) { if (error instanceof UncaughtCorsError) { - env.services.dialog.open(NetworkErrorDialog, { + env.services.dialog.add(NetworkErrorDialog, { traceback: error.traceback, message: error.message, name: error.name, @@ -51,7 +51,7 @@ errorHandlerRegistry.add("corsErrorHandler", corsErrorHandler, { sequence: 95 }) */ function clientErrorHandler(env, error) { if (error instanceof UncaughtClientError) { - env.services.dialog.open(ClientErrorDialog, { + env.services.dialog.add(ClientErrorDialog, { traceback: error.traceback, message: error.message, name: error.name, @@ -90,7 +90,7 @@ function rpcErrorHandler(env, error, originalError) { ErrorComponent = errorDialogRegistry.get(exceptionName); } - env.services.dialog.open(ErrorComponent || RPCErrorDialog, { + env.services.dialog.add(ErrorComponent || RPCErrorDialog, { traceback: error.traceback, message: originalError.message, name: originalError.name, @@ -170,7 +170,7 @@ function emptyRejectionErrorHandler(env, error) { if (!(error instanceof UncaughtPromiseError)) { return false; } - env.services.dialog.open(ClientErrorDialog, { + env.services.dialog.add(ClientErrorDialog, { traceback: error.traceback, message: error.message, name: error.name, @@ -191,7 +191,7 @@ errorHandlerRegistry.add("emptyRejectionErrorHandler", emptyRejectionErrorHandle * @returns {boolean} */ function defaultHandler(env, error) { - env.services.dialog.open(ErrorDialog, { + env.services.dialog.add(ErrorDialog, { traceback: error.traceback, message: error.message, name: error.name, diff --git a/addons/web/static/src/legacy/action_adapters.js b/addons/web/static/src/legacy/action_adapters.js index d0c234ed4a1..6eac07841af 100644 --- a/addons/web/static/src/legacy/action_adapters.js +++ b/addons/web/static/src/legacy/action_adapters.js @@ -103,7 +103,7 @@ class ActionAdapter extends ComponentAdapter { } } WarningDialog.bodyTemplate = tags.xml``; - this.dialogs.open(WarningDialog, { + this.dialogs.add(WarningDialog, { title: payload.title, message: payload.message, }); diff --git a/addons/web/static/src/legacy/debug_manager.js b/addons/web/static/src/legacy/debug_manager.js index c1fc6282cc6..a99f679ef82 100644 --- a/addons/web/static/src/legacy/debug_manager.js +++ b/addons/web/static/src/legacy/debug_manager.js @@ -190,7 +190,10 @@ function viewRecordRules({ accessRights, action, env }) { } class FieldViewGetDialog extends Dialog {} -FieldViewGetDialog.props = Object.assign({}, Dialog.props, { arch: { type: String } }); +FieldViewGetDialog.props = Object.assign({}, Dialog.props, { + arch: { type: String }, + close: Function, +}); FieldViewGetDialog.bodyTemplate = tags.xml`
`;
 FieldViewGetDialog.title = _lt("Fields View Get");
 
@@ -390,7 +393,7 @@ function fieldsViewGet({ component, env }) {
             const props = {
                 arch: component.props.viewInfo.arch,
             };
-            env.services.dialog.open(FieldViewGetDialog, props);
+            env.services.dialog.add(FieldViewGetDialog, props);
         },
         sequence: 340,
     };
@@ -435,7 +438,7 @@ function setDefaults({ action, component, env }) {
         type: "item",
         description: env._t("Set Defaults"),
         callback: () => {
-            env.services.dialog.open(SetDefaultDialog, {
+            env.services.dialog.add(SetDefaultDialog, {
                 res_model: action.res_model,
                 component,
             });
@@ -453,7 +456,7 @@ function viewMetadata({ action, component, env }) {
         type: "item",
         description: env._t("View Metadata"),
         callback: () => {
-            env.services.dialog.open(GetMetadataDialog, {
+            env.services.dialog.add(GetMetadataDialog, {
                 res_model: action.res_model,
                 selectedIds,
             });
diff --git a/addons/web/static/src/legacy/legacy_rpc_error_handler.js b/addons/web/static/src/legacy/legacy_rpc_error_handler.js
index 65ab97be354..67a36606f88 100644
--- a/addons/web/static/src/legacy/legacy_rpc_error_handler.js
+++ b/addons/web/static/src/legacy/legacy_rpc_error_handler.js
@@ -43,7 +43,7 @@ function legacyRPCErrorHandler(env, error, originalError) {
             ErrorComponent = errorDialogRegistry.get(exceptionName);
         }
 
-        env.services.dialog.open(ErrorComponent || RPCErrorDialog, {
+        env.services.dialog.add(ErrorComponent || RPCErrorDialog, {
             traceback: originalError.traceback || originalError.stack,
             message: originalError.message,
             name: originalError.name,
diff --git a/addons/web/static/src/webclient/actions/action_dialog.js b/addons/web/static/src/webclient/actions/action_dialog.js
index 2938f69cd6f..2cb83464dfc 100644
--- a/addons/web/static/src/webclient/actions/action_dialog.js
+++ b/addons/web/static/src/webclient/actions/action_dialog.js
@@ -32,6 +32,7 @@ ActionDialog.props = {
     ActionComponent: { optional: true },
     actionProps: { optional: true },
     title: { optional: true },
+    close: Function,
 };
 
 /**
diff --git a/addons/web/static/src/webclient/actions/action_service.js b/addons/web/static/src/webclient/actions/action_service.js
index 23b24263389..9e9dea88eb7 100644
--- a/addons/web/static/src/webclient/actions/action_service.js
+++ b/addons/web/static/src/webclient/actions/action_service.js
@@ -550,14 +550,14 @@ function makeActionManager(env) {
             }
 
             const { onClose } = dialog;
-            const oldDialogId = dialog.id;
-            if (oldDialogId) {
+            const removeOldDialog = dialog.remove;
+            if (removeOldDialog) {
                 dialog = {};
-                env.services.dialog.close(oldDialogId);
+                removeOldDialog();
             }
-            const dialogId = env.services.dialog.open(ActionDialog, actionDialogProps, {
-                onCloseCallback: () => {
-                    if (dialog.id) {
+            const removeDialog = env.services.dialog.add(ActionDialog, actionDialogProps, {
+                onClose: () => {
+                    if (dialog.remove) {
                         if (dialog.onClose) {
                             dialog.onClose();
                         }
@@ -566,7 +566,7 @@ function makeActionManager(env) {
                 },
             });
             dialog = {
-                id: dialogId,
+                remove: removeDialog,
                 onClose: onClose || options.onClose,
             };
             return currentActionProm;
@@ -923,11 +923,11 @@ function makeActionManager(env) {
     async function _executeCloseAction(params = {}) {
         cleanDomFromBootstrap();
         let onClose;
-        const dialogId = dialog.id;
-        if (dialogId) {
+        const removeDialog = dialog.remove;
+        if (removeDialog) {
             onClose = dialog.onClose;
             dialog = {};
-            env.services.dialog.close(dialogId);
+            removeDialog();
         } else {
             onClose = params.onClose;
         }
diff --git a/addons/web/static/src/webclient/commands/command_palette_dialog.js b/addons/web/static/src/webclient/commands/command_palette_dialog.js
index 3c032438203..50362739c66 100644
--- a/addons/web/static/src/webclient/commands/command_palette_dialog.js
+++ b/addons/web/static/src/webclient/commands/command_palette_dialog.js
@@ -34,4 +34,5 @@ CommandPaletteDialog.bodyTemplate = "web.CommandPaletteDialogBody";
 CommandPaletteDialog.components = Object.assign({}, Dialog.components, { CommandPalette });
 CommandPaletteDialog.props = {
     commands: { type: Array, element: { type: Object } },
+    close: Function,
 };
diff --git a/addons/web/static/src/webclient/commands/command_service.js b/addons/web/static/src/webclient/commands/command_service.js
index 19b906ec6c9..692b16cf468 100644
--- a/addons/web/static/src/webclient/commands/command_service.js
+++ b/addons/web/static/src/webclient/commands/command_service.js
@@ -67,11 +67,11 @@ export const commandService = {
 
             // Open palette dialog
             isPaletteOpened = true;
-            dialog.open(
+            dialog.add(
                 CommandPaletteDialog,
                 { commands },
                 {
-                    onCloseCallback: () => {
+                    onClose: () => {
                         isPaletteOpened = false;
                     },
                 }
diff --git a/addons/web/static/src/webclient/user_menu/user_menu_items.js b/addons/web/static/src/webclient/user_menu/user_menu_items.js
index 8b00ba11586..826ce6d11fa 100644
--- a/addons/web/static/src/webclient/user_menu/user_menu_items.js
+++ b/addons/web/static/src/webclient/user_menu/user_menu_items.js
@@ -44,7 +44,7 @@ function shortCutsItem(env) {
         hide: env.isSmall,
         description: env._t("Shortcuts"),
         callback: () => {
-            env.services.dialog.open(ShortCutsDialog);
+            env.services.dialog.add(ShortCutsDialog);
         },
         sequence: 30,
     };
diff --git a/addons/web/static/tests/core/debug/debug_manager_tests.js b/addons/web/static/tests/core/debug/debug_manager_tests.js
index 4a111e14e55..640f3551a20 100644
--- a/addons/web/static/tests/core/debug/debug_manager_tests.js
+++ b/addons/web/static/tests/core/debug/debug_manager_tests.js
@@ -123,7 +123,11 @@ QUnit.module("DebugMenu", (hooks) => {
 
     QUnit.test("Don't display the DebugMenu if debug mode is disabled", async (assert) => {
         const env = await makeTestEnv(testConfig);
-        const actionDialog = await mount(ActionDialog, { env, target, props: {} });
+        const actionDialog = await mount(ActionDialog, {
+            env,
+            target,
+            props: { close: () => {} },
+        });
         registerCleanup(() => {
             actionDialog.destroy();
         });
@@ -173,9 +177,10 @@ QUnit.module("DebugMenu", (hooks) => {
                     useSubEnv({ inDialog: true });
                     useDebugMenu("custom", { customKey: "abc" });
                 }
+                close() {}
             }
             Parent.components = { ActionDialog };
-            Parent.template = tags.xml``;
+            Parent.template = tags.xml``;
             patchWithCleanup(odoo, { debug: "1" });
             const env = await makeTestEnv(testConfig);
             const actionDialog = await mount(Parent, { env, target });
diff --git a/addons/web/static/tests/core/dialog_service_tests.js b/addons/web/static/tests/core/dialog_service_tests.js
index caa917a0905..ac292b6375b 100644
--- a/addons/web/static/tests/core/dialog_service_tests.js
+++ b/addons/web/static/tests/core/dialog_service_tests.js
@@ -57,7 +57,7 @@ QUnit.test("Simple rendering with a single dialog", async (assert) => {
     CustomDialog.title = "Welcome";
     pseudoWebClient = await mount(PseudoWebClient, { env, target });
     assert.containsNone(target, ".o_dialog_container .o_dialog");
-    env.services.dialog.open(CustomDialog);
+    env.services.dialog.add(CustomDialog);
     await nextTick();
     assert.containsOnce(target, ".o_dialog_container .o_dialog");
     assert.strictEqual(target.querySelector("header .modal-title").textContent, "Welcome");
@@ -74,18 +74,18 @@ QUnit.test("Simple rendering and close a single dialog", async (assert) => {
     pseudoWebClient = await mount(PseudoWebClient, { env, target });
     assert.containsNone(target, ".o_dialog_container .o_dialog");
 
-    const dialogId = env.services.dialog.open(CustomDialog);
+    const removeDialog = env.services.dialog.add(CustomDialog);
     await nextTick();
     assert.containsOnce(target, ".o_dialog_container .o_dialog");
     assert.strictEqual(target.querySelector("header .modal-title").textContent, "Welcome");
 
-    env.services.dialog.close(dialogId);
+    removeDialog();
     await nextTick();
     assert.containsNone(target, ".o_dialog_container .o_dialog");
 
     // Call a second time, the close on the dialog.
     // As the dialog is already close, this call is just ignored. No error should be raised.
-    env.services.dialog.close(dialogId);
+    removeDialog();
     await nextTick();
 });
 
@@ -99,11 +99,11 @@ QUnit.test("rendering with two dialogs", async (assert) => {
     }
     pseudoWebClient = await mount(PseudoWebClient, { env, target });
     assert.containsNone(target, ".o_dialog_container .o_dialog");
-    env.services.dialog.open(CustomDialog, { title: "Hello" });
+    env.services.dialog.add(CustomDialog, { title: "Hello" });
     await nextTick();
     assert.containsOnce(target, ".o_dialog_container .o_dialog");
     assert.strictEqual(target.querySelector("header .modal-title").textContent, "Hello");
-    env.services.dialog.open(CustomDialog, { title: "Sauron" });
+    env.services.dialog.add(CustomDialog, { title: "Sauron" });
     await nextTick();
     assert.containsN(target, ".o_dialog_container .o_dialog", 2);
     assert.deepEqual(
@@ -125,7 +125,7 @@ QUnit.test("multiple dialogs can become the UI active element", async (assert) =
     }
     pseudoWebClient = await mount(PseudoWebClient, { env, target });
 
-    env.services.dialog.open(CustomDialog, { title: "Hello" });
+    env.services.dialog.add(CustomDialog, { title: "Hello" });
     await nextTick();
     let dialogModal = target.querySelector(
         ".o_dialog_container .o_dialog .modal:not(.o_inactive_modal)"
@@ -133,7 +133,7 @@ QUnit.test("multiple dialogs can become the UI active element", async (assert) =
 
     assert.strictEqual(dialogModal, env.services.ui.activeElement);
 
-    env.services.dialog.open(CustomDialog, { title: "Sauron" });
+    env.services.dialog.add(CustomDialog, { title: "Sauron" });
     await nextTick();
     dialogModal = target.querySelector(
         ".o_dialog_container .o_dialog .modal:not(.o_inactive_modal)"
@@ -141,7 +141,7 @@ QUnit.test("multiple dialogs can become the UI active element", async (assert) =
 
     assert.strictEqual(dialogModal, env.services.ui.activeElement);
 
-    env.services.dialog.open(CustomDialog, { title: "Rafiki" });
+    env.services.dialog.add(CustomDialog, { title: "Rafiki" });
     await nextTick();
     dialogModal = target.querySelector(
         ".o_dialog_container .o_dialog .modal:not(.o_inactive_modal)"
@@ -170,11 +170,11 @@ QUnit.test("Interactions between multiple dialogs", async (assert) => {
     }
     pseudoWebClient = await mount(PseudoWebClient, { env, target });
 
-    env.services.dialog.open(CustomDialog, { title: "Hello" });
+    env.services.dialog.add(CustomDialog, { title: "Hello" });
     await nextTick();
-    env.services.dialog.open(CustomDialog, { title: "Sauron" });
+    env.services.dialog.add(CustomDialog, { title: "Sauron" });
     await nextTick();
-    env.services.dialog.open(CustomDialog, { title: "Rafiki" });
+    env.services.dialog.add(CustomDialog, { title: "Rafiki" });
     await nextTick();
 
     let modals = document.querySelectorAll(".modal");
@@ -248,7 +248,7 @@ QUnit.test("dialog component crashes", async (assert) => {
 
     pseudoWebClient = await mount(PseudoWebClient, { env, target });
 
-    env.services.dialog.open(FailingDialog);
+    env.services.dialog.add(FailingDialog);
     await prom;
     assert.verifySteps(["error"]);
     assert.containsOnce(pseudoWebClient, ".modal");
diff --git a/addons/web/static/tests/core/dialog_tests.js b/addons/web/static/tests/core/dialog_tests.js
index 4196912ebb0..8165bd9f096 100644
--- a/addons/web/static/tests/core/dialog_tests.js
+++ b/addons/web/static/tests/core/dialog_tests.js
@@ -110,67 +110,48 @@ QUnit.module("Components", (hooks) => {
 
     QUnit.test("click on the button x triggers the service close", async function (assert) {
         assert.expect(3);
-        serviceRegistry.add(
-            "dialog",
-            makeFakeDialogService({
-                close: (id) => {
-                    assert.step("close with id: " + id);
-                },
-            }),
-            { force: true }
-        );
         const env = await makeTestEnv();
-        class MyDialog extends SimpleDialog {
-            setup() {
-                super.setup();
-                this.__id = 666;
+        class MyDialog extends SimpleDialog {}
+        class Parent extends owl.Component {
+            close() {
+                assert.step("close");
             }
         }
-        class Parent extends owl.Component {}
+
         Parent.template = owl.tags.xml`
-                
-                    Hello!
-                
-          `;
+            
+                Hello!
+            
+        `;
         Parent.components = { MyDialog };
         parent = await mount(Parent, { env, target });
         assert.containsOnce(target, ".o_dialog");
         await click(target, ".o_dialog header button.close");
-        assert.verifySteps(["close with id: 666"]);
+        assert.verifySteps(["close"]);
     });
 
     QUnit.test(
         "click on the default footer button triggers the service close",
         async function (assert) {
-            serviceRegistry.add(
-                "dialog",
-                makeFakeDialogService({
-                    close: (id) => {
-                        assert.step("close with id: " + id);
-                    },
-                }),
-                { force: true }
-            );
             const env = await makeTestEnv();
             assert.expect(3);
-            class MyDialog extends SimpleDialog {
-                setup() {
-                    super.setup();
-                    this.__id = 777;
+            class MyDialog extends SimpleDialog {}
+            class Parent extends owl.Component {
+                close() {
+                    assert.step("close");
                 }
             }
-            class Parent extends owl.Component {}
 
             Parent.template = owl.tags.xml`
-                
+                
                     Hello!
                 
-          `;
+            `;
             Parent.components = { MyDialog };
             parent = await mount(Parent, { env, target });
             assert.containsOnce(target, ".o_dialog");
             await click(target, ".o_dialog footer button");
-            assert.verifySteps(["close with id: 777"]);
+            assert.verifySteps(["close"]);
         }
     );
 
diff --git a/addons/web/static/tests/core/errors/error_dialogs_tests.js b/addons/web/static/tests/core/errors/error_dialogs_tests.js
index 8a5d2bbeec0..44630047d90 100644
--- a/addons/web/static/tests/core/errors/error_dialogs_tests.js
+++ b/addons/web/static/tests/core/errors/error_dialogs_tests.js
@@ -201,16 +201,10 @@ QUnit.test("WarningDialog", async (assert) => {
 });
 
 QUnit.test("RedirectWarningDialog", async (assert) => {
-    serviceRegistry.add(
-        "dialog",
-        makeFakeDialogService({
-            close: () => {
-                assert.step("dialog-closed");
-            },
-        }),
-        { force: true }
-    );
     assert.expect(10);
+
+    class CloseRedirectWarningDialog extends RedirectWarningDialog {}
+
     class Parent extends Component {
         constructor() {
             super(...arguments);
@@ -222,9 +216,12 @@ QUnit.test("RedirectWarningDialog", async (assert) => {
                 ],
             };
         }
+        close() {
+            assert.step("dialog-closed");
+        }
     }
-    Parent.components = { RedirectWarningDialog };
-    Parent.template = tags.xml``;
+    Parent.components = { RedirectWarningDialog: CloseRedirectWarningDialog };
+    Parent.template = tags.xml``;
     const faceActionService = {
         name: "action",
         start() {
diff --git a/addons/web/static/tests/core/errors/error_service_tests.js b/addons/web/static/tests/core/errors/error_service_tests.js
index 0a5ade8c3d5..e8f13fb206b 100644
--- a/addons/web/static/tests/core/errors/error_service_tests.js
+++ b/addons/web/static/tests/core/errors/error_service_tests.js
@@ -58,7 +58,7 @@ QUnit.test("handle RPC_ERROR of type='server' and no associated dialog class", a
     error.message = "Some strange error occured";
     error.data = { debug: "somewhere" };
     error.subType = "strange_error";
-    function open(dialogClass, props) {
+    function addDialog(dialogClass, props) {
         assert.strictEqual(dialogClass, RPCErrorDialog);
         assert.deepEqual(props, {
             name: "RPC_ERROR",
@@ -73,7 +73,7 @@ QUnit.test("handle RPC_ERROR of type='server' and no associated dialog class", a
             traceback: error.stack,
         });
     }
-    serviceRegistry.add("dialog", makeFakeDialogService({ open: open }), { force: true });
+    serviceRegistry.add("dialog", makeFakeDialogService(addDialog), { force: true });
     await makeTestEnv();
     const errorEvent = new PromiseRejectionEvent("error", { reason: error, promise: null });
     await unhandledRejectionCb(errorEvent);
@@ -90,7 +90,7 @@ QUnit.test(
         error.code = 701;
         error.message = "Some strange error occured";
         error.exceptionName = "strange_error";
-        function open(dialogClass, props) {
+        function addDialog(dialogClass, props) {
             assert.strictEqual(dialogClass, CustomDialog);
             assert.deepEqual(props, {
                 name: "RPC_ERROR",
@@ -103,7 +103,7 @@ QUnit.test(
                 traceback: error.stack,
             });
         }
-        serviceRegistry.add("dialog", makeFakeDialogService({ open: open }), { force: true });
+        serviceRegistry.add("dialog", makeFakeDialogService(addDialog), { force: true });
         await makeTestEnv();
         errorDialogRegistry.add("strange_error", CustomDialog);
         const errorEvent = new PromiseRejectionEvent("error", { reason: error, promise: null });
@@ -176,7 +176,7 @@ QUnit.test("handle uncaught promise errors", async (assert) => {
     error.message = "This is an error test";
     error.name = "TestError";
 
-    function open(dialogClass, props) {
+    function addDialog(dialogClass, props) {
         assert.strictEqual(dialogClass, ClientErrorDialog);
         assert.deepEqual(props, {
             name: "UncaughtPromiseError > TestError",
@@ -184,7 +184,7 @@ QUnit.test("handle uncaught promise errors", async (assert) => {
             traceback: error.stack,
         });
     }
-    serviceRegistry.add("dialog", makeFakeDialogService({ open: open }), { force: true });
+    serviceRegistry.add("dialog", makeFakeDialogService(addDialog), { force: true });
     await makeTestEnv();
 
     const errorEvent = new PromiseRejectionEvent("error", { reason: error, promise: null });
@@ -197,12 +197,12 @@ QUnit.test("handle uncaught client errors", async (assert) => {
     error.message = "This is an error test";
     error.name = "TestError";
 
-    function open(dialogClass, props) {
+    function addDialog(dialogClass, props) {
         assert.strictEqual(dialogClass, ClientErrorDialog);
         assert.strictEqual(props.name, "UncaughtClientError > TestError");
         assert.strictEqual(props.message, "Uncaught Javascript Error > This is an error test");
     }
-    serviceRegistry.add("dialog", makeFakeDialogService({ open: open }), { force: true });
+    serviceRegistry.add("dialog", makeFakeDialogService(addDialog), { force: true });
     await makeTestEnv();
 
     const errorEvent = new ErrorEvent("error", {
@@ -220,11 +220,11 @@ QUnit.test("handle uncaught CORS errors", async (assert) => {
     error.message = "This is a cors error";
     error.name = "CORS error";
 
-    function open(dialogClass, props) {
+    function addDialog(dialogClass, props) {
         assert.strictEqual(dialogClass, NetworkErrorDialog);
         assert.strictEqual(props.message, "Uncaught CORS Error");
     }
-    serviceRegistry.add("dialog", makeFakeDialogService({ open: open }), { force: true });
+    serviceRegistry.add("dialog", makeFakeDialogService(addDialog), { force: true });
     await makeTestEnv();
 
     // CORS error event has no colno, no lineno and no filename
diff --git a/addons/web/static/tests/helpers/mock_services.js b/addons/web/static/tests/helpers/mock_services.js
index 8b88e006a24..db5a9562f56 100644
--- a/addons/web/static/tests/helpers/mock_services.js
+++ b/addons/web/static/tests/helpers/mock_services.js
@@ -203,15 +203,11 @@ export function makeFakeNotificationService(mock) {
     };
 }
 
-export function makeFakeDialogService(values = {}) {
-    const open = values.open || (() => {});
-    const close = values.close || (() => {});
+export function makeFakeDialogService(addDialog) {
     return {
-        name: "dialog",
         start() {
             return {
-                open,
-                close,
+                add: addDialog || (() => () => {}),
             };
         },
     };
diff --git a/addons/web/static/tests/legacy/core/owl_dialog_tests.js b/addons/web/static/tests/legacy/core/owl_dialog_tests.js
index 568a46b8b0e..be0c9468b10 100644
--- a/addons/web/static/tests/legacy/core/owl_dialog_tests.js
+++ b/addons/web/static/tests/legacy/core/owl_dialog_tests.js
@@ -278,30 +278,23 @@ odoo.define('web.owl_dialog_tests', function (require) {
 
         QUnit.test("Interactions between legacy owl dialogs and new owl dialogs", async function (assert) {
             assert.expect(7);
-
-            const serviceRegistry = registry.category("services");
-            serviceRegistry.add("dialog", makeFakeDialogService({
-                    close: (id) => {
-                        assert.step(`dialog_${id}_closed`);
-                        parent.dialogs.splice(parent.dialogs.findIndex(d => d.id === id), 1);
-                    },
-                })
-            );
             const { legacyEnv, env } = await makeLegacyDialogMappingTestEnv();
 
+            let id = 1;
             // OwlDialog env
-            patchWithCleanup(Dialog.prototype, {
+            class OwlDialogWrapper extends owl.Component {
                 setup() {
                     this.env = legacyEnv;
-                    this._super();
                 }
-            });
-            let id = 1;
+            }
+            OwlDialogWrapper.template = xml`
+                
+            `;
+            OwlDialogWrapper.components = { Dialog };
             class WowlDialogSubClass extends WowlDialog{
                 setup(){
                     super.setup();
                     this.contentClass = this.props.contentClass;
-                    this.__id = id;
                 }
             }
             class Parent extends Component {
@@ -311,8 +304,10 @@ odoo.define('web.owl_dialog_tests', function (require) {
                 }
                 // Handlers
                 _onDialogClosed(id) {
-                    assert.step(`dialog_${id}_closed`);
-                    this.dialogs.splice(this.dialogs.findIndex(d => d.id === id), 1);
+                    return () => {
+                        assert.step(`dialog_${id}_closed`);
+                        this.dialogs.splice(this.dialogs.findIndex(d => d.id === id), 1);
+                    };
                 }
             }
             Parent.template = xml`
@@ -320,14 +315,15 @@ odoo.define('web.owl_dialog_tests', function (require) {
                     

                     
+                        close="_onDialogClosed(dialog.id)"
+                    />
                 
`;
             const parent = await mount(Parent, { env, target: getFixture() });
 
             parent.dialogs.push({ id: 1, class: WowlDialogSubClass });
             await nextTick();
             id ++;
-            parent.dialogs.push({ id: 2, class: Dialog });
+            parent.dialogs.push({ id: 2, class: OwlDialogWrapper });
             await nextTick();
             id ++;
             parent.dialogs.push({ id: 3, class: WowlDialogSubClass });
diff --git a/addons/web_tour/static/src/debug/debug_manager.js b/addons/web_tour/static/src/debug/debug_manager.js
index c50a20ca442..892e4b0a819 100644
--- a/addons/web_tour/static/src/debug/debug_manager.js
+++ b/addons/web_tour/static/src/debug/debug_manager.js
@@ -35,7 +35,7 @@ function startTour({ env }) {
         type: "item",
         description: env._t("Start Tour"),
         callback: async () => {
-            env.services.dialog.open(ToursDialog);
+            env.services.dialog.add(ToursDialog);
         },
         sequence: 60,
     };