[FIX] web: concurrency issue in command palette

Before this commit, if the user searched the command palette and pressed
the enter key without waiting for the rendering, then the command executed was incorrect.

The "inputPromise" promise used to know if the new commands were loaded correctly.
This one did not wait for the resolution "setCommands" called by "onSearchInput".
So we modified the "debouncePromise" utility to make sure that it waits for
the resolution of the function given to it before resolving its promise.

closes odoo/odoo#76613

X-original-commit: 53b905d6993b4e25fec574d92429f2ff91501dfa
Signed-off-by: Aaron Bohy (aab) <aab@odoo.com>

addons/web/static/src/core/utils/timing.js

@@ -3,12 +3,12 @@
 import { browser } from "../browser/browser";
 
 /**
- * Returns a function, that, as long as it continues to be invoked, will not
- * be triggered. The function will be called after it stops being called for
- * N milliseconds. If `immediate` is passed, trigger the function on the
- * leading edge, instead of the trailing.
- *
- * Inspired by https://davidwalsh.name/javascript-debounce-function
+ * Creates and returns a new debounced version of the passed function (func)
+ * which will postpone its execution until after wait milliseconds
+ * have elapsed since the last time it was invoked. The debounced function
+ * will return a Promise that will be resolved when the function (func)
+ * has been fully executed. If `immediate` is passed, trigger the function
+ * on the leading edge, instead of the trailing.
  * @param {Function} func
  * @param {number} wait
  * @param {boolean} [immediate=false]
@@ -20,46 +20,22 @@ export function debounce(func, wait, immediate = false) {
     return {
         [funcName](...args) {
             const context = this;
+            let resolve;
+            const prom = new Promise((r) => {
+                resolve = r;
+            });
             function later() {
                 if (!immediate) {
-                    func.apply(context, args);
+                    Promise.resolve(func.apply(context, args)).then(resolve);
                 }
             }
             const callNow = immediate && !timeout;
             browser.clearTimeout(timeout);
             timeout = browser.setTimeout(later, wait);
             if (callNow) {
-                func.apply(context, args);
+                Promise.resolve(func.apply(context, args)).then(resolve);
             }
-        },
-    }[funcName];
-}
-
-/**
- * Returns a function which returns a Promise, that, as long as it continues
- * to be invoked, will not be triggered. The function (func) will be called
- * after it stops being called for N milliseconds and the Promise will be resolved.
- *
- * @param {Function} func
- * @param {number} wait
- * @returns {Function}
- */
-export function debouncePromise(func, wait) {
-    let resolver;
-    const funcName = func.name ? func.name + " (promise)" : "promise";
-    const obj = {
-        [funcName]: (...args) => {
-            func.apply(this, args);
-            resolver();
-        },
-    };
-    const debouncedFunc = debounce(obj[funcName], wait);
-    return {
-        [funcName](...args) {
-            return new Promise((resolve) => {
-                resolver = resolve;
-                debouncedFunc.apply(this, args);
-            });
+            return prom;
         },
     }[funcName];
 }

addons/web/static/src/webclient/commands/command_palette.js

@@ -5,7 +5,7 @@ import { useHotkey } from "@web/core/hotkeys/hotkey_hook";
 import { KeepLast } from "@web/core/utils/concurrency";
 import { scrollTo } from "@web/core/utils/scrolling";
 import { fuzzyLookup } from "@web/core/utils/search";
-import { debouncePromise } from "@web/core/utils/timing";
+import { debounce } from "@web/core/utils/timing";
 import { _lt } from "@web/core/l10n/translation";
 
 const { Component, hooks } = owl;
@@ -69,7 +69,7 @@ export class CommandPalette extends Component {
         this.keepLast = new KeepLast();
         this.DefaultCommandItem = DefaultCommandItem;
         this.activeElement = useService("ui").activeElement;
-        const onDebouncedSearchInput = debouncePromise.apply(this, [this.onSearchInput, 250]);
+        const onDebouncedSearchInput = debounce.apply(this, [this.onSearchInput, 200]);
         this.onDebouncedSearchInput = (...args) => {
             this.inputPromise = onDebouncedSearchInput.apply(this, args).catch(() => {
                 this.inputPromise = null;
@@ -258,7 +258,7 @@ export class CommandPalette extends Component {
             namespace = searchValue[0];
             searchValue = searchValue.slice(1);
         }
-        this.setCommands(namespace, {
+        await this.setCommands(namespace, {
             searchValue,
             activeElement: this.activeElement,
         });

addons/web/static/tests/core/utils/timing_tests.js

@@ -1,19 +1,86 @@
 /** @odoo-module **/
 
-import { debouncePromise } from "@web/core/utils/timing";
+import { browser } from "@web/core/browser/browser";
+import { debounce } from "@web/core/utils/timing";
+import { makeDeferred, patchWithCleanup } from "../../helpers/utils";
 
 QUnit.module("utils", () => {
     QUnit.module("timing");
 
-    QUnit.test("debouncePromise", async function (assert) {
+    QUnit.test("debounce on an async function", async function (assert) {
+        let callback;
+        patchWithCleanup(browser, {
+            setTimeout: (later) => {
+                callback = later;
+            },
+        });
+        const imSearchDef = makeDeferred();
         const myFunc = () => {
-            assert.step("exec");
+            assert.step("myFunc");
+            return imSearchDef;
         };
-        const myDebouncedFunc = debouncePromise(myFunc, 10);
-        let toAwait = myDebouncedFunc();
+        const myDebouncedFunc = debounce(myFunc, 3000);
+        myDebouncedFunc().then(() => {
+            throw new Error("Should never be resolved");
+        });
+        myDebouncedFunc().then((x) => {
+            assert.step("resolved " + x);
+        });
         assert.verifySteps([]);
-        toAwait = myDebouncedFunc();
-        await toAwait;
-        assert.verifySteps(["exec"]);
+        callback();
+        assert.verifySteps(["myFunc"]);
+        imSearchDef.resolve(42);
+        await Promise.resolve(); // wait for promise returned by myFunc
+        await Promise.resolve(); // wait for promise returned by debounce
+
+        assert.verifySteps(["resolved 42"]);
+    });
+
+    QUnit.test("debounce on a sync function", async function (assert) {
+        let callback;
+        patchWithCleanup(browser, {
+            setTimeout: (later) => {
+                callback = later;
+            },
+        });
+        const myFunc = () => {
+            assert.step("myFunc");
+            return 42;
+        };
+        const myDebouncedFunc = debounce(myFunc, 3000);
+        myDebouncedFunc().then(() => {
+            throw new Error("Should never be resolved");
+        });
+        myDebouncedFunc().then((x) => {
+            assert.step("resolved " + x);
+        });
+        assert.verifySteps([]);
+        callback();
+        assert.verifySteps(["myFunc"]);
+        await Promise.resolve(); // wait for promise returned by myFunc
+        await Promise.resolve(); // wait for promise returned by debounce
+
+        assert.verifySteps(["resolved 42"]);
+    });
+
+    QUnit.test("debounce with immediate", async function (assert) {
+        patchWithCleanup(browser, {
+            setTimeout: (later) => {
+                later();
+            },
+        });
+        const myFunc = () => {
+            assert.step("myFunc");
+            return 42;
+        };
+        const myDebouncedFunc = debounce(myFunc, 3000, true);
+        myDebouncedFunc().then((x) => {
+            assert.step("resolved " + x);
+        });
+        assert.verifySteps(["myFunc"]);
+        await Promise.resolve(); // wait for promise returned by myFunc
+        await Promise.resolve(); // wait for promise returned by debounce
+
+        assert.verifySteps(["resolved 42"]);
     });
 });

addons/web/static/tests/webclient/commands/command_palette_tests.js

@@ -1,5 +1,6 @@
 /** @odoo-module **/
 
+import { makeDeferred } from "@mail/utils/deferred/deferred";
 import { browser } from "@web/core/browser/browser";
 import { dialogService } from "@web/core/dialog/dialog_service";
 import { hotkeyService } from "@web/core/hotkeys/hotkey_service";
@@ -366,6 +367,49 @@ QUnit.test("multi provider with the same namespace", async (assert) => {
     );
 });
 
+QUnit.test("check the concurrency during a research", async (assert) => {
+    testComponent = await mount(TestComponent, { env, target });
+    const imSearchDef = makeDeferred();
+    const provide = async (env, options) => {
+        if (options.searchValue) {
+            await imSearchDef;
+        }
+        return [
+            {
+                name: "a",
+                action: () => {
+                    assert.step("a");
+                },
+            },
+            {
+                name: "b",
+                action: () => {
+                    assert.step("b");
+                },
+            },
+        ];
+    };
+    const providers = [{ namespace: "default", provide }];
+    const config = {
+        providers,
+    };
+    env.services.dialog.add(CommandPaletteDialog, {
+        config,
+    });
+    await nextTick();
+    assert.containsOnce(target, ".o_command_palette");
+    assert.containsN(target, ".o_command", 2);
+
+    await editSearchBar("b");
+    triggerHotkey("enter");
+    await nextTick();
+    assert.verifySteps([]);
+
+    imSearchDef.resolve();
+    await nextTick();
+    assert.verifySteps(["b"]);
+});
+
 QUnit.test("open the command palette with a namespace already in the searchbar", async (assert) => {
     testComponent = await mount(TestComponent, { env, target });
     const action = () => {};