From 365fe390fdc814bc079419f0bd940b8edd652618 Mon Sep 17 00:00:00 2001 From: FrancoisGe Date: Tue, 14 Feb 2023 09:06:31 +0000 Subject: [PATCH] [REF] web: remove redirect in router The redirect function in the router exists only for the wait option. This option is only used in one case (client action home). We have therefore decided to remove the redirect function and to call browser.location.assign(...) directly. We will also remove the "wait" param for the "reload" and "home" client actions. Because no call to "reload" needs it (1) and all calls to "home" want it wait=True. So we will move the code that was executed if wait=true to the "home" action client. (1) In the POS, wait=true is used for a "reload" but this has no impact. Wait=true was intended to wait for the server to restart before reloading the page. In the case of the POS, there is no restart of the server, so wait=True is useless. closes odoo/odoo#112621 Signed-off-by: Aaron Bohy (aab) --- .../wizard/base_module_install_request.py | 1 - addons/point_of_sale/models/pos_config.py | 1 - .../static/src/core/browser/router_service.js | 18 --------- .../src/webclient/actions/action_service.js | 5 ++- .../src/webclient/actions/client_actions.js | 25 +++++++----- .../static/tests/core/router_service_tests.js | 32 --------------- .../web/static/tests/helpers/mock_services.js | 9 ----- .../webclient/actions/client_action_tests.js | 8 ++-- .../webclient/actions/url_action_tests.js | 39 ++++++++----------- .../website_preview/website_preview.js | 3 +- 10 files changed, 40 insertions(+), 101 deletions(-) diff --git a/addons/base_install_request/wizard/base_module_install_request.py b/addons/base_install_request/wizard/base_module_install_request.py index 54e206ddb98..3b7b38402e7 100644 --- a/addons/base_install_request/wizard/base_module_install_request.py +++ b/addons/base_install_request/wizard/base_module_install_request.py @@ -84,5 +84,4 @@ class BaseModuleInstallReview(models.TransientModel): return { 'type': 'ir.actions.client', 'tag': 'home', - 'params': {'wait': True}, } diff --git a/addons/point_of_sale/models/pos_config.py b/addons/point_of_sale/models/pos_config.py index 5dfc98a66c0..d36d470c871 100644 --- a/addons/point_of_sale/models/pos_config.py +++ b/addons/point_of_sale/models/pos_config.py @@ -478,7 +478,6 @@ class PosConfig(models.Model): return { 'type': 'ir.actions.client', 'tag': 'reload', - 'params': {'wait': True} } def _force_http(self): diff --git a/addons/web/static/src/core/browser/router_service.js b/addons/web/static/src/core/browser/router_service.js index b36f5f2c4fb..3469af73be4 100644 --- a/addons/web/static/src/core/browser/router_service.js +++ b/addons/web/static/src/core/browser/router_service.js @@ -107,23 +107,6 @@ export function routeToUrl(route) { return route.pathname + (search ? "?" + search : "") + (hash ? "#" + hash : ""); } -async function redirect(env, url, wait = false) { - if (wait) { - await new Promise((resolve) => { - const waitForServer = (delay) => { - browser.setTimeout(async () => { - env.services - .rpc("/web/webclient/version_info", {}) - .then(resolve) - .catch(() => waitForServer(250)); - }, delay); - }; - waitForServer(1000); - }); - } - browser.location.assign(url); -} - function getRoute(urlObj) { const { pathname, search, hash } = urlObj; const searchQuery = parseSearchQuery(search); @@ -190,7 +173,6 @@ function makeRouter(env) { }, pushState: makeDebouncedPush("push"), replaceState: makeDebouncedPush("replace"), - redirect: (url, wait) => redirect(env, url, wait), cancelPushes: () => browser.clearTimeout(pushTimeout), }; } diff --git a/addons/web/static/src/webclient/actions/action_service.js b/addons/web/static/src/webclient/actions/action_service.js index 3da012f55ab..fe42a7070a7 100644 --- a/addons/web/static/src/webclient/actions/action_service.js +++ b/addons/web/static/src/webclient/actions/action_service.js @@ -86,7 +86,8 @@ export class InvalidButtonParamsError extends Error {} // ----------------------------------------------------------------------------- // regex that matches context keys not to forward from an action to another -const CTX_KEY_REGEX = /^(?:(?:default_|search_default_|show_).+|.+_view_ref|group_by|group_by_no_leaf|active_id|active_ids|orderedBy)$/; +const CTX_KEY_REGEX = + /^(?:(?:default_|search_default_|show_).+|.+_view_ref|group_by|group_by_no_leaf|active_id|active_ids|orderedBy)$/; // only register this template once for all dynamic classes ControllerComponent const ControllerComponentTemplate = xml``; @@ -839,7 +840,7 @@ function makeActionManager(env) { }; browser.addEventListener("beforeunload", onUnload); env.services.ui.block(); - env.services.router.redirect(action.url); + browser.location.assign(action.url); browser.removeEventListener("beforeunload", onUnload); if (!willUnload) { env.services.ui.unblock(); diff --git a/addons/web/static/src/webclient/actions/client_actions.js b/addons/web/static/src/webclient/actions/client_actions.js index df5232314d5..684b80e288f 100644 --- a/addons/web/static/src/webclient/actions/client_actions.js +++ b/addons/web/static/src/webclient/actions/client_actions.js @@ -48,10 +48,9 @@ registry.category("actions").add("invalid_action", InvalidAction); * Client action to reload the whole interface. * If action.params.menu_id, it opens the given menu entry. * If action.params.action_id, it opens the given action. - * If action.params.wait, reload will wait the server to be reachable before reloading */ function reload(env, action) { - const { menu_id, action_id, wait } = action.params || {}; + const { menu_id, action_id } = action.params || {}; const { router } = env.services; const route = { ...router.current }; @@ -65,7 +64,7 @@ function reload(env, action) { } } - // We want to force location.assign(...) (in router.redirect(...)) to do a page reload. + // We want to force location.assign(...) to do a page reload. // To do this, we need to make sure that the url is different. route.search = { ...route.search }; if ("reload" in route.search) { @@ -76,20 +75,28 @@ function reload(env, action) { const url = browser.location.origin + routeToUrl(route); env.bus.trigger("CLEAR-CACHES"); - router.redirect(url, wait); + browser.location.assign(url); } registry.category("actions").add("reload", reload); /** * Client action to go back home. - * If action.params.wait, reload will wait the server to be reachable before reloading */ -function home(env, action) { - const { wait } = action.params || {}; +async function home(env) { + await new Promise((resolve) => { + const waitForServer = (delay) => { + browser.setTimeout(async () => { + env.services + .rpc("/web/webclient/version_info", {}) + .then(resolve) + .catch(() => waitForServer(250)); + }, delay); + }; + waitForServer(1000); + }); const url = "/" + (browser.location.search || ""); - env.services.router.redirect(url, wait); - browser.location.reload(url); + browser.location.assign(url); } registry.category("actions").add("home", home); diff --git a/addons/web/static/tests/core/router_service_tests.js b/addons/web/static/tests/core/router_service_tests.js index d4d535e30e4..a79f4c1adfd 100644 --- a/addons/web/static/tests/core/router_service_tests.js +++ b/addons/web/static/tests/core/router_service_tests.js @@ -2,7 +2,6 @@ import { browser } from "@web/core/browser/browser"; import { parseHash, parseSearchQuery, routeToUrl } from "@web/core/browser/router_service"; -import { makeTestEnv } from "../helpers/mock_env"; import { makeFakeRouterService } from "../helpers/mock_services"; import { nextTick, patchWithCleanup } from "../helpers/utils"; @@ -101,37 +100,6 @@ QUnit.test("routeToUrl encodes URI compatible strings", (assert) => { assert.strictEqual(routeToUrl(route), "/asf?a=11&g=summer%20wine#b=2&c=&e=kloug%2Cgloubi"); }); -QUnit.test("can redirect an URL", async (assert) => { - patchWithCleanup(browser, { - setTimeout(handler, delay) { - handler(); - assert.step(`timeout: ${delay}`); - }, - }); - let firstCheckServer = true; - const env = await makeTestEnv({ - async mockRPC(route) { - if (route === "/web/webclient/version_info") { - if (firstCheckServer) { - firstCheckServer = false; - } else { - return true; - } - } - }, - }); - const onRedirect = (url) => assert.step(url); - const router = await createRouter({ env, onRedirect }); - - router.redirect("/my/test/url"); - await nextTick(); - assert.verifySteps(["/my/test/url"]); - - router.redirect("/my/test/url/2", true); - await nextTick(); - assert.verifySteps(["timeout: 1000", "timeout: 250", "/my/test/url/2"]); -}); - QUnit.module("Router: Push state"); QUnit.test("can push in same timeout", async (assert) => { diff --git a/addons/web/static/tests/helpers/mock_services.js b/addons/web/static/tests/helpers/mock_services.js index e3a890f55e8..f3b86ac953e 100644 --- a/addons/web/static/tests/helpers/mock_services.js +++ b/addons/web/static/tests/helpers/mock_services.js @@ -161,7 +161,6 @@ export function makeMockFetch(mockRPC) { /** * @param {Object} [params={}] - * @param {Object} [params.onRedirect] hook on the "redirect" method * @returns {typeof routerService} */ export function makeFakeRouterService(params = {}) { @@ -173,14 +172,6 @@ export function makeFakeRouterService(params = {}) { browser.location.hash = objectToUrlEncodedString(hash); }); registerCleanup(router.cancelPushes); - patchWithCleanup(router, { - async redirect() { - await this._super(...arguments); - if (params.onRedirect) { - params.onRedirect(...arguments); - } - }, - }); return router; }, }; diff --git a/addons/web/static/tests/webclient/actions/client_action_tests.js b/addons/web/static/tests/webclient/actions/client_action_tests.js index 32599003645..07c2ef939b6 100644 --- a/addons/web/static/tests/webclient/actions/client_action_tests.js +++ b/addons/web/static/tests/webclient/actions/client_action_tests.js @@ -613,16 +613,14 @@ QUnit.module("ActionManager", (hooks) => { QUnit.test("test reload client action", async function (assert) { patchWithCleanup(browser.location, { + assign: (url) => { + assert.step(url); + }, origin: "", hash: "#test=42", }); const webClient = await createWebClient({ serverData }); - patchWithCleanup(webClient.env.services.router, { - redirect: (url) => { - assert.step(url); - }, - }); await doAction(webClient, { type: "ir.actions.client", diff --git a/addons/web/static/tests/webclient/actions/url_action_tests.js b/addons/web/static/tests/webclient/actions/url_action_tests.js index c2d793cbc68..77586ab626b 100644 --- a/addons/web/static/tests/webclient/actions/url_action_tests.js +++ b/addons/web/static/tests/webclient/actions/url_action_tests.js @@ -2,7 +2,6 @@ import { registry } from "@web/core/registry"; import { makeTestEnv } from "../../helpers/mock_env"; -import { makeFakeRouterService } from "../../helpers/mock_services"; import { setupWebClientRegistries, doAction, getActionManagerServerData } from "./../helpers"; import { patchWithCleanup } from "@web/../tests/helpers/utils"; import { browser } from "@web/core/browser/browser"; @@ -18,14 +17,11 @@ QUnit.module("ActionManager", (hooks) => { QUnit.module("URL actions"); QUnit.test("execute an 'ir.actions.act_url' action with target 'self'", async (assert) => { - serviceRegistry.add( - "router", - makeFakeRouterService({ - onRedirect(url) { - assert.step(url); - }, - }) - ); + patchWithCleanup(browser.location, { + assign: (url) => { + assert.step(url); + }, + }); setupWebClientRegistries(); const env = await makeTestEnv({ serverData }); await doAction(env, { @@ -37,20 +33,17 @@ QUnit.module("ActionManager", (hooks) => { }); QUnit.test("an 'ir.actions.act_url' with target 'self' blocks the ui", async (assert) => { - serviceRegistry.add( - "ui", - { - start() { - return { - block: () => assert.step("block"), - // we can't simulate a page unload in the tests, so in this scenario the - // ui will be unblocked directly (and we thus need to define the unblock - // function) - unblock: () => {}, - }; - }, - } - ); + serviceRegistry.add("ui", { + start() { + return { + block: () => assert.step("block"), + // we can't simulate a page unload in the tests, so in this scenario the + // ui will be unblocked directly (and we thus need to define the unblock + // function) + unblock: () => {}, + }; + }, + }); setupWebClientRegistries(); const env = await makeTestEnv({ serverData }); await doAction(env, { diff --git a/addons/website/static/src/client_actions/website_preview/website_preview.js b/addons/website/static/src/client_actions/website_preview/website_preview.js index 3f320234755..ef1d92ed99c 100644 --- a/addons/website/static/src/client_actions/website_preview/website_preview.js +++ b/addons/website/static/src/client_actions/website_preview/website_preview.js @@ -1,5 +1,6 @@ /** @odoo-module **/ +import { browser } from '@web/core/browser/browser'; import { registry } from '@web/core/registry'; import { useService, useBus } from '@web/core/utils/hooks'; import core from 'web.core'; @@ -375,7 +376,7 @@ export class WebsitePreview extends Component { }); } else if (href && target !== '_blank' && !isEditing && this._isTopWindowURL(linkEl)) { ev.preventDefault(); - this.router.redirect(href); + browser.location.assign(href); } else if (this.iframe.el.contentWindow.location.pathname !== new URL(href).pathname) { this.websiteService.websiteRootInstance = undefined; }