From 77a59f8d6a23ca16bb3df40ce20de61010f8a645 Mon Sep 17 00:00:00 2001
From: "Lucas Perais (lpe)" <lpe@odoo.com>
Date: Tue, 22 Jun 2021 09:19:13 +0000
Subject: [PATCH] [FIX] web: action_service: do not commit a dialog that has
 failed

Open a action in target new that you'll know will fail to open.
Make sure you put an onClose callback when executing doAction.

Open another action in target new, that will succeed.

Before this commit, the onClose of the first dialog is executed, while it should not.
Indeed, if the dialog fails, the whole business flow is invalid altogether
and no more business operation (in the onClose) should be executed

After this commit, the onClose callback of the failing dialog is not executed.
---
 .../src/webclient/actions/action_service.js   |  4 +-
 .../tests/webclient/actions/target_tests.js   | 91 ++++++++++++++++++-
 2 files changed, 93 insertions(+), 2 deletions(-)

diff --git a/addons/web/static/src/webclient/actions/action_service.js b/addons/web/static/src/webclient/actions/action_service.js
index 8d4b4cf57e7..f7c3d540ecf 100644
--- a/addons/web/static/src/webclient/actions/action_service.js
+++ b/addons/web/static/src/webclient/actions/action_service.js
@@ -472,6 +472,7 @@ function makeActionManager(env) {
                     }).then(() => {
                         dialogCloseProm = undefined;
                     });
+                    dialog = nextDialog;
                 } else {
                     // LEGACY CODE COMPATIBILITY: remove when controllers will be written in owl
                     // we determine here which actions no longer occur in the nextStack,
@@ -529,6 +530,7 @@ function makeActionManager(env) {
 
         ControllerComponent.Component = controller.Component;
 
+        let nextDialog = {};
         if (action.target === "new") {
             cleanDomFromBootstrap();
             const actionDialogProps = {
@@ -556,7 +558,7 @@ function makeActionManager(env) {
                     }
                 },
             });
-            dialog = {
+            nextDialog = {
                 remove: removeDialog,
                 onClose: onClose || options.onClose,
             };
diff --git a/addons/web/static/tests/webclient/actions/target_tests.js b/addons/web/static/tests/webclient/actions/target_tests.js
index 78aa943518e..cb9cc612784 100644
--- a/addons/web/static/tests/webclient/actions/target_tests.js
+++ b/addons/web/static/tests/webclient/actions/target_tests.js
@@ -4,8 +4,17 @@ import testUtils from "web.test_utils";
 import core from "web.core";
 import AbstractAction from "web.AbstractAction";
 import { registry } from "@web/core/registry";
-import { legacyExtraNextTick } from "../../helpers/utils";
+import {
+    click,
+    legacyExtraNextTick,
+    patchWithCleanup,
+    makeDeferred,
+} from "../../helpers/utils";
 import { createWebClient, doAction, getActionManagerServerData } from "./../helpers";
+import { registerCleanup } from "../../helpers/cleanup";
+import { errorService } from "@web/core/errors/error_service";
+import { useService } from "@web/core/service_hook";
+import { ClientErrorDialog } from "@web/core/errors/error_dialogs";
 
 let serverData;
 
@@ -322,6 +331,86 @@ QUnit.module("ActionManager", (hooks) => {
         assert.verifySteps([]);
     });
 
+    QUnit.test("do not commit a dialog in error", async (assert) => {
+        assert.expect(6);
+
+        const handler = (ev) => {
+            // need to preventDefault to remove error from console (so python test pass)
+            ev.preventDefault();
+        };
+        window.addEventListener("unhandledrejection", handler);
+        registerCleanup(() => window.removeEventListener("unhandledrejection", handler));
+
+        patchWithCleanup(QUnit, {
+            onUnhandledRejection: () => {},
+        });
+
+        class ErrorClientAction extends owl.Component {
+            setup() {
+                throw new Error("my error");
+            }
+        }
+        ErrorClientAction.template = owl.tags.xml`<div/>`;
+        registry.category("actions").add("failing", ErrorClientAction);
+
+        class ClientActionTargetNew extends owl.Component {}
+        ClientActionTargetNew.template = owl.tags.xml`<div class="my_action_new" />`;
+        registry.category("actions").add("clientActionNew", ClientActionTargetNew);
+
+        class ClientAction extends owl.Component {
+            setup() {
+                this.action = useService("action");
+            }
+            async onClick() {
+                try {
+                    await this.action.doAction(
+                        { type: "ir.actions.client", tag: "failing", target: "new" },
+                        { onClose: () => assert.step("failing dialog closed") }
+                    );
+                } catch (e) {
+                    assert.strictEqual(e.message, "my error");
+                }
+            }
+        }
+        ClientAction.template = owl.tags.xml`<div class="my_action" t-on-click="onClick" />`;
+        registry.category("actions").add("clientAction", ClientAction);
+
+        const errorDialogOpened = makeDeferred();
+        patchWithCleanup(ClientErrorDialog.prototype, {
+            mounted() {
+                this._super(...arguments);
+                errorDialogOpened.resolve();
+            },
+        });
+
+        registry.category("services").add("error", errorService);
+        const webClient = await createWebClient({});
+
+        await doAction(webClient, { type: "ir.actions.client", tag: "clientAction" });
+        await click(webClient.el, ".my_action");
+        await errorDialogOpened;
+
+        assert.containsOnce(webClient, ".modal");
+        await click(webClient.el, ".modal-body button.btn-link");
+        assert.ok(
+            webClient.el
+                .querySelector(".modal-body .o_error_detail")
+                .textContent.includes("my error")
+        );
+
+        await click(webClient.el, ".modal-footer button");
+        assert.containsNone(webClient, ".modal");
+
+        await doAction(webClient, {
+            type: "ir.actions.client",
+            tag: "clientActionNew",
+            target: "new",
+        });
+        assert.containsOnce(webClient, ".modal .my_action_new");
+
+        assert.verifySteps([]);
+    });
+
     QUnit.module('Actions in target="inline"');
     QUnit.test(
         'form views for actions in target="inline" open in edit mode',