From b2058b99429862df6b5a103ac41dc6ba31fe3cda Mon Sep 17 00:00:00 2001 From: FrancoisGe Date: Tue, 1 Mar 2022 09:52:44 +0000 Subject: [PATCH] [FIX] web: CP concurence issue with custom delay Commit 62ebc97bd887a6358e02cd131590083fdd80d12f introduced the possibility to customise the debounce delay associated with a command palette search based on namespace. This introduced a concurrency bug when changing namespace. Problem: Let's say we have 2 namespaces with very different debounce delays. For example, # has 2000 and @ has 0. 1. A user enter "#" 2. He changes his mind before the end of the search. He deletes "#" and enter "@". 3. The search associated with "@" is resolved before the search for "#". The associated results are displayed. 4. The search associated with "#" is resolved. The associated results are displayed. 5. The user does not get the results related to his last search which are those of "@". Solution: When we change the namespace, we will cancel the current search function. Part-of: odoo/odoo#82773 --- .../src/core/commands/command_palette.js | 22 +++--- .../core/commands/command_palette_tests.js | 72 +++++++++++++++++++ 2 files changed, 82 insertions(+), 12 deletions(-) diff --git a/addons/web/static/src/core/commands/command_palette.js b/addons/web/static/src/core/commands/command_palette.js index 4592bc3b5ba..85ed664ca35 100644 --- a/addons/web/static/src/core/commands/command_palette.js +++ b/addons/web/static/src/core/commands/command_palette.js @@ -271,20 +271,18 @@ export class CommandPalette extends Component { debounceSearch(value) { const { namespace } = this.processSearchValue(value); - if (!this.debounceSearchByNamespace[namespace]) { - const namespaceConfig = this.configByNamespace[namespace] || {}; - if (namespaceConfig.debounceDelay) { - this.debounceSearchByNamespace[namespace] = debounce.apply(this, [ - this.search, - namespaceConfig.debounceDelay, - ]); - } else { - this.debounceSearchByNamespace[namespace] = this.defaultDebounceSearch; + const namespaceConfig = this.configByNamespace[namespace] || {}; + if (this.namespace !== namespace) { + if (this.lastDebounceSearch) { + this.lastDebounceSearch.cancel(); } + this.lastDebounceSearch = debounce( + (value) => this.search(value), + namespaceConfig.debounceDelay || 0 + ); + this.namespace = namespace; } - const debounceSearch = - this.debounceSearchByNamespace[namespace] || this.defaultDebounceSearch; - this.searchValuePromise = debounceSearch.apply(this, [value]).catch(() => { + this.searchValuePromise = this.lastDebounceSearch(value).catch(() => { this.searchValuePromise = null; }); } diff --git a/addons/web/static/tests/core/commands/command_palette_tests.js b/addons/web/static/tests/core/commands/command_palette_tests.js index c610577978a..8f3d0836744 100644 --- a/addons/web/static/tests/core/commands/command_palette_tests.js +++ b/addons/web/static/tests/core/commands/command_palette_tests.js @@ -181,6 +181,78 @@ QUnit.test("custom debounce delay", async (assert) => { assert.verifySteps(["0", "200", "100", "0"]); }); +QUnit.test("concurrency with custom debounce delay", async (assert) => { + const def = makeDeferred(); + patchWithCleanup(browser, { + clearTimeout: () => {}, + setTimeout: async (later, delay) => { + if (delay === 200) { + await def; + } + later(); + }, + }); + + await mount(TestComponent, target, { env }); + const configByNamespace = { + "@": { + debounceDelay: 200, + }, + "#": { + debounceDelay: 100, + }, + }; + const action = () => {}; + const providers = [ + { + namespace: "@", + provide: () => [ + { + name: "Command@", + action, + }, + ], + }, + { + namespace: "#", + provide: () => [ + { + name: "Command#", + action, + }, + ], + }, + ]; + const config = { + configByNamespace, + providers, + }; + env.services.dialog.add(CommandPaletteDialog, { + config, + }); + await nextTick(); + assert.containsOnce(target, ".o_command_palette"); + assert.containsNone(target, ".o_command"); + await editSearchBar("@"); + await nextTick(); + assert.deepEqual( + [...target.querySelectorAll(".o_command")].map((el) => el.textContent), + [] + ); + await editSearchBar("#"); + await nextTick(); + assert.deepEqual( + [...target.querySelectorAll(".o_command")].map((el) => el.textContent), + ["Command#"] + ); + def.resolve(); + await nextTick(); + assert.deepEqual( + [...target.querySelectorAll(".o_command")].map((el) => el.textContent), + ["Command#"] + ); +}); + QUnit.test("custom placeholder", async (assert) => { await mount(TestComponent, target, { env }); const config = {