diff --git a/superset-frontend/packages/superset-ui-core/src/components/Select/AsyncSelect.test.tsx b/superset-frontend/packages/superset-ui-core/src/components/Select/AsyncSelect.test.tsx index 323e02fb14b..d7346dbe47c 100644 --- a/superset-frontend/packages/superset-ui-core/src/components/Select/AsyncSelect.test.tsx +++ b/superset-frontend/packages/superset-ui-core/src/components/Select/AsyncSelect.test.tsx @@ -1019,6 +1019,166 @@ test('restores base options when search is cleared', async () => { expect(screen.queryByText('Search Match')).not.toBeInTheDocument(); }); +test('replaces results when switching between two searches', async () => { + const page0Data = Array.from({ length: 10 }, (_, i) => ({ + label: `Option ${i}`, + value: i, + })); + const loadOptions = jest.fn(async (search: string) => { + if (search === '') { + return { data: page0Data, totalCount: 100 }; + } + return { + data: [{ label: `Match-${search}`, value: `v-${search}` }], + totalCount: 1, + }; + }); + + render(); + await open(); + await waitFor(() => expect(loadOptions).toHaveBeenCalledTimes(1)); + + await type('alpha'); + await waitFor(async () => { + const options = await findAllSelectOptions(); + expect(options).toHaveLength(1); + expect(options[0]).toHaveTextContent('Match-alpha'); + }); + + await type('beta'); + await waitFor(async () => { + const options = await findAllSelectOptions(); + expect(options).toHaveLength(1); + expect(options[0]).toHaveTextContent('Match-beta'); + }); + expect(screen.queryByText('Match-alpha')).not.toBeInTheDocument(); +}); + +test('refetches a dropped search response when the same search is repeated', async () => { + type OptionRow = { label: string; value: string | number }; + type PageResponse = { data: OptionRow[]; totalCount: number }; + // Resolves the in-flight loadOptions promise of the calling test. + let resolveAlpha: ((value: PageResponse) => void) | null = null; + const page0Data: OptionRow[] = Array.from({ length: 10 }, (_, i) => ({ + label: `Option ${i}`, + value: i, + })); + const alphaData: OptionRow[] = [{ label: 'Match-alpha', value: 'va' }]; + const betaData: OptionRow[] = [{ label: 'Match-beta', value: 'vb' }]; + + const loadOptions = jest.fn((search: string) => { + if (search === '') { + return Promise.resolve({ + data: page0Data, + totalCount: 100, + }); + } + if (search === 'alpha') { + // First call: hold the promise so it resolves only after beta returns. + // Second call (after beta): resolve immediately so the cache MUST allow + // a refetch. + if (!resolveAlpha) { + return new Promise(resolve => { + resolveAlpha = resolve; + }); + } + return Promise.resolve({ data: alphaData, totalCount: 1 }); + } + return Promise.resolve({ data: betaData, totalCount: 1 }); + }); + + render(); + await open(); + await waitFor(() => expect(loadOptions).toHaveBeenCalledWith('', 0, 10)); + + await type('alpha'); + await waitFor(() => expect(loadOptions).toHaveBeenCalledWith('alpha', 0, 10)); + // alpha's promise is held; switch to beta which resolves first. + await type('beta'); + await waitFor(async () => { + const options = await findAllSelectOptions(); + expect(options).toHaveLength(1); + expect(options[0]).toHaveTextContent('Match-beta'); + }); + + // Release the stale alpha response. It must be dropped — its key must not + // be cached, or returning to "alpha" later would short-circuit the fetch. + resolveAlpha!({ data: alphaData, totalCount: 1 }); + await waitFor(async () => { + // Beta is still showing because alpha's response was dropped. + const options = await findAllSelectOptions(); + expect(options[0]).toHaveTextContent('Match-beta'); + }); + + // Returning to "alpha" must re-trigger the fetch (cache wasn't poisoned). + const callsBeforeAlphaReturn = loadOptions.mock.calls.filter( + args => args[0] === 'alpha', + ).length; + await type('alpha'); + await waitFor(() => { + const callsAfter = loadOptions.mock.calls.filter( + args => args[0] === 'alpha', + ).length; + expect(callsAfter).toBeGreaterThan(callsBeforeAlphaReturn); + }); + await waitFor(async () => { + const options = await findAllSelectOptions(); + expect(options[0]).toHaveTextContent('Match-alpha'); + }); +}); + +test('re-shows search results when the same search term is repeated after a clear', async () => { + // Regression: a prior fix cached search responses' totalCount in + // fetchedQueries. After restore-on-clear had replaced selectOptions with + // the base list, re-typing a previously-resolved term would hit the cache + // short-circuit and leave selectOptions stale (empty / base-only). + const page0Data = Array.from({ length: 10 }, (_, i) => ({ + label: `Option ${i}`, + value: i, + })); + const alphaData = [{ label: 'Match-alpha', value: 'va' }]; + const loadOptions = jest.fn(async (search: string) => { + if (search === '') { + // totalCount > data.length so allValuesLoaded stays false and the + // search path is not bypassed by the "all loaded" short-circuit. + return { data: page0Data, totalCount: 100 }; + } + return { data: alphaData, totalCount: 1 }; + }); + + render(); + await open(); + await waitFor(() => expect(loadOptions).toHaveBeenCalledWith('', 0, 10)); + + await type('alpha'); + await waitFor(async () => { + const options = await findAllSelectOptions(); + expect(options).toHaveLength(1); + expect(options[0]).toHaveTextContent('Match-alpha'); + }); + + await type(''); + await waitFor(() => + expect(screen.queryByText('Match-alpha')).not.toBeInTheDocument(), + ); + + const callsBefore = loadOptions.mock.calls.filter( + args => args[0] === 'alpha', + ).length; + await type('alpha'); + await waitFor(() => { + const callsAfter = loadOptions.mock.calls.filter( + args => args[0] === 'alpha', + ).length; + expect(callsAfter).toBeGreaterThan(callsBefore); + }); + await waitFor(async () => { + const options = await findAllSelectOptions(); + expect(options).toHaveLength(1); + expect(options[0]).toHaveTextContent('Match-alpha'); + }); +}); + test('does not duplicate options when using numeric values', async () => { render( ()); const initialOptionsRef = useRef(EMPTY_OPTIONS); + const inputValueRef = useRef(''); const mappedMode = isSingleMode ? undefined : 'multiple'; const allowFetch = !fetchOnlyOnSearch || inputValue; const [maxTagCount, setMaxTagCount] = useState( @@ -184,6 +185,10 @@ const AsyncSelect = forwardRef( selectValueRef.current = selectValue; }, [selectValue]); + useEffect(() => { + inputValueRef.current = inputValue; + }, [inputValue]); + const sortSelectedFirst = useCallback( (a: AntdLabeledValue, b: AntdLabeledValue) => sortSelectedFirstHelper(a, b, selectValueRef.current), @@ -336,7 +341,43 @@ const AsyncSelect = forwardRef( const fetchOptions = options as SelectOptionsPagePromise; fetchOptions(search, page, pageSize) .then(({ data, totalCount }: SelectOptionsTypePage) => { - if (search && page === 0) { + // Drop responses whose search arg no longer matches the user's + // current input — otherwise a slow base fetch can land after a + // search fetch (or a stale debounced search after a clear) and + // re-pollute the dropdown via mergeData / search-replace. Search + // responses are never cached in fetchedQueries: the cache stores + // only totalCount, so a cache hit would short-circuit the fetch + // and leave selectOptions stale (e.g. after restore-on-clear). + // Re-issuing the search is cheap and correct. + const matchesCurrentSearch = inputValueRef.current === search; + if (search && !matchesCurrentSearch) { + return; + } + if (!search) { + // Accumulate base pages in a ref independent of selectOptions + // (during an active search, selectOptions holds search results + // and is not a safe accumulator). The accumulator is kept up + // to date even when this response landed during a search, so + // restore-on-clear has a complete snapshot. + const dataValues = new Set(data.map(opt => opt.value)); + const accumulated = initialOptionsRef.current + .filter(opt => !dataValues.has(opt.value)) + .concat(data) + .sort(sortComparatorForNoSearch); + initialOptionsRef.current = accumulated; + if (!fetchOnlyOnSearch && accumulated.length >= totalCount) { + setAllValuesLoaded(true); + } + fetchedQueries.current.set(key, totalCount); + if (matchesCurrentSearch) { + // No active search — push to live selectOptions and update + // totalCount. When matchesCurrentSearch is false, the user + // is mid-search; leave the search's totalCount in place so + // pagination math stays correct. + mergeData(data); + setTotalCount(totalCount); + } + } else if (page === 0) { // Replace cached options with server results; preserve // optimistic isNewOption entries inserted by handleOnSearch // so allowNewOptions users can still click the value they @@ -350,17 +391,12 @@ const AsyncSelect = forwardRef( .concat(data) .sort(sortComparatorForNoSearch); }); + setTotalCount(totalCount); } else { - const mergedData = mergeData(data); - if (!search) { - initialOptionsRef.current = mergedData; - if (!fetchOnlyOnSearch && mergedData.length >= totalCount) { - setAllValuesLoaded(true); - } - } + // page > 0 during an active search — append normally. + mergeData(data); + setTotalCount(totalCount); } - fetchedQueries.current.set(key, totalCount); - setTotalCount(totalCount); }) .catch(internalOnError) .finally(() => { @@ -539,6 +575,9 @@ const AsyncSelect = forwardRef( if (inputValue) { debouncedFetchPage(inputValue, 0); } else { + // Cancel any pending debounced search fetch so it can't fire after + // we've already restored the base list. + debouncedFetchPage.cancel(); // On returning to empty input after a search, restore the cached // base options so the dropdown shows the original page-0 list // instead of the stale search results.