From a715e63b88d8d4a3e897c7bba8f62066d55f10e7 Mon Sep 17 00:00:00 2001
From: Aaron Bohy <aab@odoo.com>
Date: Mon, 13 Jun 2022 08:32:12 +0000
Subject: [PATCH] [IMP] mail,web: tests: crash if triggerEvent called on
 invisible el

Before this commit, the test utils that trigger an event (e.g.
click) didn't check whether the target of the event was visible.
As a consequence, when writing a test, one might trigger an event
on an invisible and undesirable target, and don't understand why
it doesn't work due to the absence of feedback. This commit
improves that situation by throwing an error in those sitations.

Obviously, some tests relying on the former behavior needed to
be slightly adapted.

closes odoo/odoo#93549

Related: odoo/enterprise#28360
Signed-off-by: Aaron Bohy (aab) <aab@odoo.com>
---
 .../mail/static/tests/helpers/webclient_setup.js | 15 +++++++++++++--
 addons/web/static/tests/helpers/utils.js         | 14 +++++++++++++-
 .../static/tests/search/search_panel_tests.js    |  6 +++---
 addons/web/static/tests/views/view_tests.js      | 16 ++++++++++------
 .../tests/webclient/actions/legacy_tests.js      |  5 ++++-
 addons/web/static/tests/webclient/helpers.js     |  2 ++
 .../static/tests/webclient/webclient_tests.js    |  2 +-
 7 files changed, 46 insertions(+), 14 deletions(-)

diff --git a/addons/mail/static/tests/helpers/webclient_setup.js b/addons/mail/static/tests/helpers/webclient_setup.js
index 3c39d81f7c3..bc66018298b 100644
--- a/addons/mail/static/tests/helpers/webclient_setup.js
+++ b/addons/mail/static/tests/helpers/webclient_setup.js
@@ -12,7 +12,7 @@ import { messagingService } from '@mail/services/messaging_service';
 import { makeMessagingToLegacyEnv } from '@mail/utils/make_messaging_to_legacy_env';
 
 import { registry } from '@web/core/registry';
-import { patchWithCleanup } from "@web/../tests/helpers/utils";
+import { getFixture, patchWithCleanup } from "@web/../tests/helpers/utils";
 import { createWebClient } from "@web/../tests/webclient/helpers";
 
 import AbstractStorageService from 'web.AbstractStorageService';
@@ -166,7 +166,18 @@ function setupMessagingServiceRegistries({
         serviceRegistry: legacyServiceRegistry,
         ...webClientParameters.legacyParams,
     };
-    return createWebClient(webClientParameters);
+    const target = getFixture();
+    // FIXME: the o_web_client className is automatically added by createWebClient,
+    // to ensure that the stylesheet is correctly applied. However, it causes issues
+    // with some mail test, so we remove it, except if it has been already added by
+    // the test itself. Ideally, the tests requiring this hack should be fixed and
+    // this logic should be removed.
+    const shouldRemoveClass = !target.classList.contains("o_web_client");
+    const webClient = await createWebClient(webClientParameters);
+    if (shouldRemoveClass) {
+        target.classList.remove("o_web_client");
+    }
+    return webClient;
 }
 
 export {
diff --git a/addons/web/static/tests/helpers/utils.js b/addons/web/static/tests/helpers/utils.js
index e0bd951fb83..8354eaafe65 100644
--- a/addons/web/static/tests/helpers/utils.js
+++ b/addons/web/static/tests/helpers/utils.js
@@ -265,14 +265,26 @@ export async function triggerEvent(el, selector, eventType, eventAttrs = {}) {
         event = new Event(eventType, Object.assign({}, eventAttrs, { bubbles: true }));
     }
     const target = findElement(el, selector);
+    if (!target) {
+        throw new Error(`Can't find a target to trigger ${eventType} event`);
+    }
+    const isVisible =
+        target === document ||
+        target === window ||
+        target.offsetWidth > 0 ||
+        target.offsetHeight > 0;
+    if (!isVisible) {
+        throw new Error(`Called triggerEvent ${eventType} on invisible target`);
+    }
     target.dispatchEvent(event);
     await nextTick();
 }
 
 export async function triggerEvents(el, querySelector, events) {
     for (let e = 0; e < events.length; e++) {
-        await triggerEvent(el, querySelector, events[e]);
+        triggerEvent(el, querySelector, events[e]);
     }
+    await nextTick();
 }
 
 /**
diff --git a/addons/web/static/tests/search/search_panel_tests.js b/addons/web/static/tests/search/search_panel_tests.js
index b354e7f3c81..66a537068ad 100644
--- a/addons/web/static/tests/search/search_panel_tests.js
+++ b/addons/web/static/tests/search/search_panel_tests.js
@@ -20,6 +20,7 @@ import { registry } from "@web/core/registry";
 import { FilterMenu } from "@web/search/filter_menu/filter_menu";
 import { GroupByMenu } from "@web/search/group_by_menu/group_by_menu";
 import { SearchPanel } from "@web/search/search_panel/search_panel";
+import testUtils from "web.test_utils";
 
 const { Component, xml } = owl;
 
@@ -2328,9 +2329,8 @@ QUnit.module("Search", (hooks) => {
 
         await doAction(webclient, 1, { viewType: "form" });
 
-        await click(target, "[name=company_id] .o_input");
-        await click(target, "[name=company_id] .o_input");
-        await click(document, ".o_m2o_dropdown_option");
+        await testUtils.fields.many2one.clickOpenDropdown("company_id");
+        await testUtils.fields.many2one.clickItem("company_id", "Search");
         await legacyExtraNextTick();
 
         assert.containsOnce(document.body, ".modal .o_list_view");
diff --git a/addons/web/static/tests/views/view_tests.js b/addons/web/static/tests/views/view_tests.js
index 5a694a025c1..f57160aede2 100644
--- a/addons/web/static/tests/views/view_tests.js
+++ b/addons/web/static/tests/views/view_tests.js
@@ -745,7 +745,7 @@ QUnit.module("Views", (hooks) => {
 
         serverData.views["animal,1,toy"] = `
             <toy>
-                <a type="action" data-method="setTheControl" data-model="animal" />
+                <a type="action" data-method="setTheControl" data-model="animal">link</a>
             </toy>`;
 
         const mockRPC = (route) => {
@@ -800,7 +800,9 @@ QUnit.module("Views", (hooks) => {
 
         serverData.views["animal,1,toy"] = `
             <toy>
-                <a type="action" name="myLittleAction" data-context="{ &quot;somekey&quot;: &quot;somevalue&quot; }"/>
+                <a type="action" name="myLittleAction" data-context="{ &quot;somekey&quot;: &quot;somevalue&quot; }">
+                    link
+                </a>
             </toy>`;
 
         const config = {
@@ -853,7 +855,9 @@ QUnit.module("Views", (hooks) => {
 
         serverData.views["animal,1,toy"] = `
             <toy>
-                <a type="action" title="myTitle" data-model="animal" data-resId="66" data-views="[[55, 'toy']]" data-domain="[['field', '=', 'val']]" data-context="{ &quot;somekey&quot;: &quot;somevalue&quot; }"/>
+                <a type="action" title="myTitle" data-model="animal" data-resId="66" data-views="[[55, 'toy']]" data-domain="[['field', '=', 'val']]" data-context="{ &quot;somekey&quot;: &quot;somevalue&quot; }">
+                    link
+                </a>
             </toy>`;
 
         const config = {
@@ -973,7 +977,7 @@ QUnit.module("Views", (hooks) => {
 
         const banners = [
             `<div class="banner1">
-                <a type="action" data-method="setTheControl" data-model="animal" data-reload-on-close="true" />
+                <a type="action" data-method="setTheControl" data-model="animal" data-reload-on-close="true">link</a>
             </div>`,
             `<div class="banner2">
                 MyBanner
@@ -1085,13 +1089,13 @@ QUnit.module("Views", (hooks) => {
         serverData.views["animal,1,toy"] = `
             <toy banner_route="/banner_route">
                 <Banner t-if="env.config.bannerRoute" />
-                <a type="action" data-method="setTheControl" data-model="animal" />
+                <a type="action" data-method="setTheControl" data-model="animal">link</a>
             </toy>`;
 
         const mockRPC = async (route) => {
             if (route.includes("banner_route")) {
                 return {
-                    html: `<div><a type="action" data-method="heartOfTheSun" data-model="animal" /></div>`,
+                    html: `<div><a type="action" data-method="heartOfTheSun" data-model="animal">link</a></div>`,
                 };
             }
             if (route.includes("setTheControl")) {
diff --git a/addons/web/static/tests/webclient/actions/legacy_tests.js b/addons/web/static/tests/webclient/actions/legacy_tests.js
index 06a2b951e34..294d2035be3 100644
--- a/addons/web/static/tests/webclient/actions/legacy_tests.js
+++ b/addons/web/static/tests/webclient/actions/legacy_tests.js
@@ -461,6 +461,7 @@ QUnit.module("ActionManager", (hooks) => {
             async start() {
                 const res = await this._super(...arguments);
                 const link = document.createElement("a");
+                link.innerText = "some link";
                 link.setAttribute("id", `client_${this.ID}`);
                 link.addEventListener("click", () => {
                     this.do_action("testClientAction", {
@@ -481,7 +482,9 @@ QUnit.module("ActionManager", (hooks) => {
         assert.verifySteps(['id: 0 props: {"className":"o_action","breadcrumbs":[]}']);
 
         await click(document.getElementById("client_0"));
-        assert.verifySteps(['id: 1 props: {"chain":"never break","className":"o_action","breadcrumbs":[]}']);
+        assert.verifySteps([
+            'id: 1 props: {"chain":"never break","className":"o_action","breadcrumbs":[]}',
+        ]);
     });
 
     QUnit.test("breadcrumbs are correct in stacked legacy client actions", async function (assert) {
diff --git a/addons/web/static/tests/webclient/helpers.js b/addons/web/static/tests/webclient/helpers.js
index 266b93726f8..717346ba2a7 100644
--- a/addons/web/static/tests/webclient/helpers.js
+++ b/addons/web/static/tests/webclient/helpers.js
@@ -286,7 +286,9 @@ export async function createWebClient(params) {
     const WebClientClass = params.WebClientClass || WebClient;
     const target = params && params.target ? params.target : getFixture();
     const wc = await mount(WebClientClass, target, { env });
+    target.classList.add("o_web_client"); // necessary for the stylesheet
     registerCleanup(() => {
+        target.classList.remove("o_web_client");
         for (const controller of controllers) {
             if (!controller.isDestroyed()) {
                 controller.destroy();
diff --git a/addons/web/static/tests/webclient/webclient_tests.js b/addons/web/static/tests/webclient/webclient_tests.js
index 764e0b37960..aff44b66e71 100644
--- a/addons/web/static/tests/webclient/webclient_tests.js
+++ b/addons/web/static/tests/webclient/webclient_tests.js
@@ -82,7 +82,7 @@ QUnit.test("control-click propagation stopped on <a href/>", async (assert) => {
             ev.preventDefault();
         }
     }
-    MyComponent.template = xml`<a href="#" class="MyComponent" t-on-click="onclick" />`;
+    MyComponent.template = xml`<a href="#" class="MyComponent" t-on-click="onclick">Some link</a>`;
     let env = await makeTestEnv(baseConfig);
 
     // Mount the component as standalone and control-click the <a href/>