From 1113272d56bf94661569bbbe355ae30ff0fe14b2 Mon Sep 17 00:00:00 2001 From: "Didier (did)" Date: Mon, 31 Jul 2023 10:06:58 +0000 Subject: [PATCH] [FIX] mail: discuss search results should match input MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Before this PR, RPC made by input inside the discuss sidebar could resolve in the wrong order, witch display outdated results. This PR ensure that only the most recent RPC is sent to the server, cancelling irrelevant RPC. Note: This PR also remove the support for Promise inside the NavigableList component since it's not compatible with useSequential. closes odoo/odoo#130767 X-original-commit: ba2d029d876f9acd7c869cc3d345cc4ed7cab00a Signed-off-by: Alexandre Kühn (aku) --- .../static/src/core/common/navigable_list.js | 39 +++------ .../static/src/core/common/navigable_list.xml | 2 +- .../static/src/core/common/suggestion_hook.js | 21 +---- .../src/discuss/core/web/channel_selector.js | 86 +++++++++++++------ .../src/discuss/core/web/channel_selector.xml | 2 +- addons/mail/static/src/utils/common/hooks.js | 28 ++++++ .../static/tests/discuss_app/discuss_tests.js | 51 ++++++++++- 7 files changed, 155 insertions(+), 74 deletions(-) diff --git a/addons/mail/static/src/core/common/navigable_list.js b/addons/mail/static/src/core/common/navigable_list.js index b8c5187357a..62f53b8bbe1 100644 --- a/addons/mail/static/src/core/common/navigable_list.js +++ b/addons/mail/static/src/core/common/navigable_list.js @@ -14,21 +14,21 @@ export class NavigableList extends Component { static components = { ImStatus }; static template = "mail.NavigableList"; static props = { - anchorRef: {}, + anchorRef: { optional: true }, class: { type: String, optional: true }, onSelect: { type: Function }, - options: { type: [Array, Promise] }, + options: { type: Array }, optionTemplate: { type: String, optional: true }, placeholder: { type: String, optional: true }, position: { type: String, optional: true }, + isLoading: { type: Boolean, optional: true }, }; - static defaultProps = { position: "bottom" }; + static defaultProps = { position: "bottom", isLoading: false }; setup() { this.rootRef = useRef("root"); this.state = useState({ activeOption: null, - isLoading: false, open: false, options: [], }); @@ -61,14 +61,11 @@ export class NavigableList extends Component { } get show() { - return Boolean(this.state.open && (this.state.isLoading || this.state.options.length)); + return Boolean(this.state.open && (this.props.isLoading || this.state.options.length)); } - async open() { - if (this.state.isLoading) { - return; - } - await this.load(); + open() { + this.load(); this.state.open = true; this.navigate("first"); } @@ -78,24 +75,12 @@ export class NavigableList extends Component { this.state.activeOption = null; } - async load() { + load() { this.state.options = []; - if (this.props.options instanceof Promise) { - this.state.isLoading = true; - const options = await this.props.options; - this.state.options = options.map((option, index) => ({ - ...option, - id: index, - })); - this.state.isLoading = false; - return; - } - if (this.props.options instanceof Array) { - this.state.options = this.props.options.map((option, index) => ({ - ...option, - id: index, - })); - } + this.state.options = this.props.options.map((option, index) => ({ + ...option, + id: index, + })); } isActiveOption(option) { diff --git a/addons/mail/static/src/core/common/navigable_list.xml b/addons/mail/static/src/core/common/navigable_list.xml index 4f40ae12f75..3a59c31b750 100644 --- a/addons/mail/static/src/core/common/navigable_list.xml +++ b/addons/mail/static/src/core/common/navigable_list.xml @@ -4,7 +4,7 @@
-
+
diff --git a/addons/mail/static/src/core/common/suggestion_hook.js b/addons/mail/static/src/core/common/suggestion_hook.js index 8ba27fc18f3..1c18ca48ef5 100644 --- a/addons/mail/static/src/core/common/suggestion_hook.js +++ b/addons/mail/static/src/core/common/suggestion_hook.js @@ -1,11 +1,13 @@ /* @odoo-module */ +import { useSequential } from "@mail/utils/common/hooks"; import { useComponent, useEffect, useState } from "@odoo/owl"; import { useService } from "@web/core/utils/hooks"; export function useSuggestion() { const comp = useComponent(); + const sequential = useSequential(); /** @type {import("@mail/core/common/suggestion_service").SuggestionService} */ const suggestionService = useService("mail.suggestion"); const self = { @@ -74,10 +76,6 @@ export function useSuggestion() { get thread() { return comp.props.composer.thread || comp.props.composer.message.originThread; }, - fetch: { - inProgress: false, - rpcFunction: undefined, - }, insert(option) { const cursorPosition = comp.props.composer.selection.start; const content = comp.props.composer.textInputContent; @@ -103,19 +101,6 @@ export function useSuggestion() { comp.props.composer.selection.end = textLeft.length + recordReplacement.length + 1; comp.props.composer.forceCursorMove = true; }, - async process(func) { - if (self.fetch.inProgress) { - self.fetch.rpcFunction = func; - return; - } - self.fetch.inProgress = true; - self.fetch.rpcFunction = undefined; - await func(); - self.fetch.inProgress = false; - if (self.fetch.rpcFunction) { - self.process(self.fetch.rpcFunction); - } - }, search: { delimiter: undefined, position: undefined, @@ -153,7 +138,7 @@ export function useSuggestion() { useEffect( () => { self.update(); - self.process(async () => { + sequential(async () => { if (self.search.position === undefined || !self.search.delimiter) { return; // ignore obsolete call } diff --git a/addons/mail/static/src/discuss/core/web/channel_selector.js b/addons/mail/static/src/discuss/core/web/channel_selector.js index 4a55651273d..1786949369a 100644 --- a/addons/mail/static/src/discuss/core/web/channel_selector.js +++ b/addons/mail/static/src/discuss/core/web/channel_selector.js @@ -6,13 +6,14 @@ import { useDiscussCoreCommon } from "@mail/discuss/core/common/discuss_core_com import { cleanTerm } from "@mail/utils/common/format"; import { createLocalId } from "@mail/utils/common/misc"; -import { Component, onMounted, useRef, useState } from "@odoo/owl"; +import { Component, onMounted, useEffect, useRef, useState } from "@odoo/owl"; import { getActiveHotkey } from "@web/core/hotkeys/hotkey_service"; import { _t } from "@web/core/l10n/translation"; import { TagsList } from "@web/core/tags_list/tags_list"; import { useService } from "@web/core/utils/hooks"; import { isEventHandled, markEventHandled } from "@web/core/utils/misc"; +import { useSequential } from "@mail/utils/common/hooks"; export class ChannelSelector extends Component { static components = { TagsList, NavigableList }; @@ -30,9 +31,22 @@ export class ChannelSelector extends Component { /** @type {import("@mail/core/common/suggestion_service").SuggestionService} */ this.suggestionService = useService("mail.suggestion"); this.orm = useService("orm"); + this.sequential = useSequential(); this.state = useState({ value: "", selectedPartners: [], + navigableListProps: { + anchorRef: undefined, + position: "bottom-fit", + onSelect: (ev, option) => this.onSelect(option), + placeholder: _t("Loading"), + optionTemplate: + this.props.category.id === "channels" + ? "discuss.ChannelSelector.channel" + : "discuss.ChannelSelector.chat", + options: [], + isLoading: false, + }, }); this.inputRef = useRef("input"); this.rootRef = useRef("root"); @@ -40,6 +54,25 @@ export class ChannelSelector extends Component { onMounted(() => this.inputRef.el.focus()); } this.markEventHandled = markEventHandled; + useEffect( + () => { + this.state.navigableListProps.anchorRef = this.rootRef?.el; + this.state.navigableListProps.optionTemplate = + this.props.category.id === "channels" + ? "discuss.ChannelSelector.channel" + : "discuss.ChannelSelector.chat"; + }, + () => [this.rootRef, this.props.category] + ); + useEffect( + () => { + this.state.navigableListProps.isLoading = true; + this.fetchSuggestions().then( + () => (this.state.navigableListProps.isLoading = false) + ); + }, + () => [this.state.value] + ); } async fetchSuggestions() { @@ -51,9 +84,15 @@ export class ChannelSelector extends Component { ["name", "ilike", cleanedTerm], ]; const fields = ["name"]; - const results = await this.orm.searchRead("discuss.channel", domain, fields, { - limit: 10, - }); + const results = await this.sequential(() => + this.orm.searchRead("discuss.channel", domain, fields, { + limit: 10, + }) + ); + if (!results) { + this.state.navigableListProps.options = []; + return; + } const choices = results.map((channel) => { return { channelId: channel.id, @@ -66,14 +105,21 @@ export class ChannelSelector extends Component { classList: "o-discuss-ChannelSelector-suggestion", label: cleanedTerm, }); - return choices; + this.state.navigableListProps.options = choices; + return; } if (this.props.category.id === "chats") { - const results = await this.orm.call("res.partner", "im_search", [ - cleanedTerm, - 10, - this.state.selectedPartners, - ]); + const results = await this.sequential(() => + this.orm.call("res.partner", "im_search", [ + cleanedTerm, + 10, + this.state.selectedPartners, + ]) + ); + if (!results) { + this.state.navigableListProps.options = []; + return; + } const suggestions = this.suggestionService .sortPartnerSuggestions(results, cleanedTerm) .map((data) => { @@ -98,10 +144,12 @@ export class ChannelSelector extends Component { unselectable: true, }); } - return suggestions; + this.state.navigableListProps.options = suggestions; + return; } } - return []; + this.state.navigableListProps.options = []; + return; } onSelect(option) { @@ -197,18 +245,4 @@ export class ChannelSelector extends Component { } return res; } - - get navigableListProps() { - return { - anchorRef: this.rootRef.el, - position: "bottom-fit", - onSelect: (ev, option) => this.onSelect(option), - placeholder: _t("Loading"), - optionTemplate: - this.props.category.id === "channels" - ? "discuss.ChannelSelector.channel" - : "discuss.ChannelSelector.chat", - options: this.fetchSuggestions(), - }; - } } diff --git a/addons/mail/static/src/discuss/core/web/channel_selector.xml b/addons/mail/static/src/discuss/core/web/channel_selector.xml index d9ac36c0a96..caa33f1ccfd 100644 --- a/addons/mail/static/src/discuss/core/web/channel_selector.xml +++ b/addons/mail/static/src/discuss/core/web/channel_selector.xml @@ -15,7 +15,7 @@ maxlength="100" />
- + diff --git a/addons/mail/static/src/utils/common/hooks.js b/addons/mail/static/src/utils/common/hooks.js index 8b45ae7bdef..a7966d0a3ff 100644 --- a/addons/mail/static/src/utils/common/hooks.js +++ b/addons/mail/static/src/utils/common/hooks.js @@ -428,3 +428,31 @@ export function useMessageToReplyTo() { }, }); } + +export function useSequential() { + let inProgress = false; + let nextFunction; + let nextResolve; + async function call() { + const resolve = nextResolve; + const func = nextFunction; + nextResolve = undefined; + nextFunction = undefined; + inProgress = true; + const data = await func(); + inProgress = false; + resolve(data); + if (nextFunction && nextResolve) { + call(); + } + } + return (func) => { + nextResolve?.(); + const prom = new Promise((resolve) => (nextResolve = resolve)); + nextFunction = func; + if (!inProgress) { + call(); + } + return prom; + }; +} diff --git a/addons/mail/static/tests/discuss_app/discuss_tests.js b/addons/mail/static/tests/discuss_app/discuss_tests.js index 9ff396dfc35..179a00446d5 100644 --- a/addons/mail/static/tests/discuss_app/discuss_tests.js +++ b/addons/mail/static/tests/discuss_app/discuss_tests.js @@ -17,7 +17,13 @@ import { } from "@mail/../tests/helpers/test_utils"; import { makeFakeNotificationService } from "@web/../tests/helpers/mock_services"; -import { editInput, nextTick, triggerEvent, triggerHotkey } from "@web/../tests/helpers/utils"; +import { + editInput, + makeDeferred, + nextTick, + triggerEvent, + triggerHotkey, +} from "@web/../tests/helpers/utils"; QUnit.module("discuss"); @@ -1899,3 +1905,46 @@ QUnit.test( ); } ); + +QUnit.test( + "Chats input should wait until the previous RPC is done before starting a new one", + async (assert) => { + const pyEnv = await startServer(); + const [partnerId1, partnerId2] = pyEnv["res.partner"].create([ + { name: "Mario" }, + { name: "Mama" }, + ]); + pyEnv["res.users"].create([{ partner_id: partnerId1 }, { partner_id: partnerId2 }]); + const deferred1 = makeDeferred(); + const deferred2 = makeDeferred(); + const { openDiscuss } = await start({ + async mockRPC(route, params) { + if (route === "/web/dataset/call_kw/res.partner/im_search") { + const { args } = params; + if (args[0] === "m") { + assert.step("First RPC"); + await deferred1; + } + if (args[0] === "mar") { + assert.step("Second RPC"); + await deferred2; + } + } + }, + }); + await openDiscuss(); + await click(".o-mail-DiscussSidebarCategory-add[title='Start a conversation']"); + await insertText(".o-discuss-ChannelSelector input", "m"); + await waitUntil(".o-mail-NavigableList-item:contains(Loading)"); + await insertText(".o-discuss-ChannelSelector input", "a"); + await insertText(".o-discuss-ChannelSelector input", "r"); + deferred1.resolve(); + assert.verifySteps(["First RPC"]); + await waitUntil(".o-discuss-ChannelSelector-suggestion:contains(Mario)"); + await waitUntil(".o-discuss-ChannelSelector-suggestion:contains(Mama)"); + deferred2.resolve(); + assert.verifySteps(["Second RPC"]); + await waitUntil(".o-discuss-ChannelSelector-suggestion:contains(Mama)", 0); + await waitUntil(".o-discuss-ChannelSelector-suggestion:contains(Mario)"); + } +);