From e5db9d87437cb88df4abf18ec4cfd1df18be4eaa Mon Sep 17 00:00:00 2001 From: Aaron Bohy Date: Wed, 23 Mar 2022 13:57:07 +0000 Subject: [PATCH] [FIX] web: action service: concurrency issue There is a concurrency issue in the action service: 1) be in an action with several views 2) execute another action (e.g. by clicking on a menu) -> [calls the route "/web/action/load", and it takes a while] 3) meanwhile, click on another view in the view switcher If the call to "/web/action/load" is long enough, it may happen that the view requested in step 3 is briefly shown before the action associated with the menu clicked in step 2. In this scenario, the action of step 2 should never be displayed, because the user requested something else afterwards (in this case a switch view). One can produce a similar issue when restoring a previous action of the breadcrumbs instead of switching view. This commit fixes those two issues by registering a resolved promise in the keepLast (concurrency utils), s.t. it "cancels" the potential current operation (in this case, the call to "/web/load/action"). closes odoo/odoo#87204 X-original-commit: fb9c8cd6fee476d2506b8c1338bf88c4520ea7b6 Signed-off-by: Mathieu Duckerts-Antoine Signed-off-by: Aaron Bohy (aab) --- .../src/webclient/actions/action_service.js | 2 + .../webclient/actions/concurrency_tests.js | 78 +++++++++++++++++++ 2 files changed, 80 insertions(+) diff --git a/addons/web/static/src/webclient/actions/action_service.js b/addons/web/static/src/webclient/actions/action_service.js index 8396063e903..ab5cf6b5f77 100644 --- a/addons/web/static/src/webclient/actions/action_service.js +++ b/addons/web/static/src/webclient/actions/action_service.js @@ -1237,6 +1237,7 @@ function makeActionManager(env) { * @returns {Promise} */ async function switchView(viewType, props = {}) { + await keepLast.add(Promise.resolve()); const controller = controllerStack[controllerStack.length - 1]; const view = _getView(viewType); if (!view) { @@ -1296,6 +1297,7 @@ function makeActionManager(env) { * @param {string} jsId */ async function restore(jsId) { + await keepLast.add(Promise.resolve()); let index; if (!jsId) { index = controllerStack.length - 2; diff --git a/addons/web/static/tests/webclient/actions/concurrency_tests.js b/addons/web/static/tests/webclient/actions/concurrency_tests.js index f54d76fcb00..6fdaf48b963 100644 --- a/addons/web/static/tests/webclient/actions/concurrency_tests.js +++ b/addons/web/static/tests/webclient/actions/concurrency_tests.js @@ -466,6 +466,84 @@ QUnit.module("ActionManager", (hooks) => { } ); + QUnit.test("restoring a controller when doing an action -- load_action slow", async function (assert) { + assert.expect(14); + let def; + const mockRPC = async (route, args) => { + assert.step((args && args.method) || route); + if (route === "/web/action/load") { + return Promise.resolve(def); + } + }; + const webClient = await createWebClient({ serverData, mockRPC }); + await doAction(webClient, 3); + assert.containsOnce(target, ".o_list_view"); + await click(target.querySelector(".o_list_view .o_data_cell")); + await legacyExtraNextTick(); + assert.containsOnce(target, ".o_form_view"); + def = makeDeferred(); + doAction(webClient, 4, { clearBreadcrumbs: true }); + await nextTick(); + await legacyExtraNextTick(); + assert.containsOnce(target, ".o_form_view", "should still contain the form view"); + await click(target.querySelector(".o_control_panel .breadcrumb-item a")); + def.resolve(); + await nextTick(); + await legacyExtraNextTick(); + assert.containsOnce(target, ".o_list_view"); + assert.strictEqual( + target.querySelector(".o_control_panel .breadcrumb-item").textContent, + "Partners" + ); + assert.containsNone(target, ".o_form_view"); + assert.verifySteps([ + "/web/webclient/load_menus", + "/web/action/load", + "load_views", + "/web/dataset/search_read", + "read", + "/web/action/load", + "/web/dataset/search_read", + ]); + }); + + QUnit.test("switching when doing an action -- load_action slow", async function (assert) { + assert.expect(12); + let def; + const mockRPC = async (route, args) => { + assert.step((args && args.method) || route); + if (route === "/web/action/load") { + return Promise.resolve(def); + } + }; + const webClient = await createWebClient({ serverData, mockRPC }); + await doAction(webClient, 3); + assert.containsOnce(target, ".o_list_view"); + def = makeDeferred(); + doAction(webClient, 4, { clearBreadcrumbs: true }); + await nextTick(); + await legacyExtraNextTick(); + assert.containsOnce(target, ".o_list_view", "should still contain the list view"); + await switchView(target, "kanban"); + def.resolve(); + await nextTick(); + await legacyExtraNextTick(); + assert.containsOnce(target, ".o_kanban_view"); + assert.strictEqual( + target.querySelector(".o_control_panel .breadcrumb-item").textContent, + "Partners" + ); + assert.containsNone(target, ".o_list_view"); + assert.verifySteps([ + "/web/webclient/load_menus", + "/web/action/load", + "load_views", + "/web/dataset/search_read", + "/web/action/load", + "/web/dataset/search_read", + ]); + }); + QUnit.test("switching when doing an action -- load_views slow", async function (assert) { assert.expect(13); let def;