From 9e4b53297931219fe643ef0413a5660df0380fc0 Mon Sep 17 00:00:00 2001 From: Josh Hunt Date: Thu, 18 May 2023 16:14:59 +0100 Subject: [PATCH] Chore: Refactor Search out-of-order fix (#68445) --- .../actions/dashboardActions.ts | 15 ++++++++------- .../search/state/SearchStateManager.ts | 19 ++++++++++++------- 2 files changed, 20 insertions(+), 14 deletions(-) diff --git a/public/app/features/commandPalette/actions/dashboardActions.ts b/public/app/features/commandPalette/actions/dashboardActions.ts index a46c4443859..44d6ff4ebf6 100644 --- a/public/app/features/commandPalette/actions/dashboardActions.ts +++ b/public/app/features/commandPalette/actions/dashboardActions.ts @@ -83,7 +83,7 @@ export async function getSearchResultActions(searchQuery: string): Promise([]); const [isFetchingSearchResults, setIsFetchingSearchResults] = useState(false); - const lastRequestTimestamp = useRef(); + const lastSearchTimestamp = useRef(0); // Hit dashboards API useEffect(() => { @@ -91,19 +91,20 @@ export function useSearchResults(searchQuery: string, isShowing: boolean) { if (isShowing && searchQuery.length > 0) { setIsFetchingSearchResults(true); debouncedSearch(searchQuery).then((resultActions) => { - // Only update the state if this is the most recent request - // We don't need to worry about clearing the isFetching state either - // If there's a later request in progress, this will clear it for us - if (!lastRequestTimestamp.current || timestamp > lastRequestTimestamp.current) { + // Only keep the results if it's was issued after the most recently resolved search. + // This prevents results showing out of order if first request is slower than later ones. + // We don't need to worry about clearing the isFetching state either - if there's a later + // request in progress, this will clear it for us + if (timestamp > lastSearchTimestamp.current) { setSearchResults(resultActions); setIsFetchingSearchResults(false); - lastRequestTimestamp.current = timestamp; + lastSearchTimestamp.current = timestamp; } }); } else { setSearchResults([]); setIsFetchingSearchResults(false); - lastRequestTimestamp.current = timestamp; + lastSearchTimestamp.current = timestamp; } }, [isShowing, searchQuery]); diff --git a/public/app/features/search/state/SearchStateManager.ts b/public/app/features/search/state/SearchStateManager.ts index 570181818c6..5b3dacfe85a 100644 --- a/public/app/features/search/state/SearchStateManager.ts +++ b/public/app/features/search/state/SearchStateManager.ts @@ -39,7 +39,8 @@ export class SearchStateManager extends StateManagerBase { updateLocation = debounce((query) => locationService.partial(query, true), 300); doSearchWithDebounce = debounce(() => this.doSearch(), 300); lastQuery?: SearchQuery; - lastSearchPromise: Promise = Promise.resolve(); + + lastSearchTimestamp = 0; initStateFromUrl(folderUid?: string, doInitialSearch = true) { const stateFromUrl = parseRouteParams(locationService.getSearchObject()); @@ -227,20 +228,24 @@ export class SearchStateManager extends StateManagerBase { const searcher = getGrafanaSearcher(); - // Issue this search now, but wait until previous searches have resolved to update it in the state + const searchTimestamp = Date.now(); const searchPromise = this.state.starred ? searcher.starred(this.lastQuery) : searcher.search(this.lastQuery); - this.lastSearchPromise = this.lastSearchPromise - .then(() => searchPromise) - .then((result) => this.setState({ result, loading: false })) + searchPromise + .then((result) => { + // Only keep the results if it's was issued after the most recently resolved search. + // This prevents results showing out of order if first request is slower than later ones + if (searchTimestamp > this.lastSearchTimestamp) { + this.setState({ result, loading: false }); + this.lastSearchTimestamp = searchTimestamp; + } + }) .catch((error) => { reportSearchFailedQueryInteraction(this.state.eventTrackingNamespace, { ...trackingInfo, error: error?.message, }); this.setState({ loading: false }); - - return Promise.resolve(); // make sure this.lastSearchPromise is always resolved }); }