diff --git a/superset-frontend/packages/superset-ui-core/src/components/Select/AsyncSelect.stories.tsx b/superset-frontend/packages/superset-ui-core/src/components/Select/AsyncSelect.stories.tsx index 862319a2a1d9..9c80d9d7cc7f 100644 --- a/superset-frontend/packages/superset-ui-core/src/components/Select/AsyncSelect.stories.tsx +++ b/superset-frontend/packages/superset-ui-core/src/components/Select/AsyncSelect.stories.tsx @@ -318,3 +318,51 @@ AsynchronousSelect.argTypes = { }, }, }; + +const createMockUserFetcher = + (responseTime = 0.3) => + ( + search: string, + page: number, + pageSize: number, + ): Promise => { + const username = search.trim().toLowerCase(); + const all = USERS.filter( + u => !username || u.toLowerCase().includes(username), + ); + const totalCount = all.length; + const start = page * pageSize; + const deleteCount = + start + pageSize < totalCount ? pageSize : totalCount - start; + const pageData = all.slice(start, start + deleteCount).map(u => ({ + label: u, + value: u, + })); + return new Promise(resolve => { + setTimeout(() => { + resolve({ data: pageData, totalCount }); + }, responseTime * 1000); + }); + }; + +/** + * Demonstrates allowNewOptions in AsyncSelect. Optimistic custom options appear + * with a [new] badge and are automatically dropped if the server returns a matching + * option upon search completion. + */ +export const WithAllowNewOptions = (args: Omit) => ( +
+ +
+); + +WithAllowNewOptions.args = { + ...AsynchronousSelect.args, + allowNewOptions: true, + placeholder: 'Type a name or create a new one...', +}; 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 657eedd5d8f0..d30d0faa0825 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 @@ -592,6 +592,43 @@ test('clear all the values', async () => { expect(values.length).toBe(0); }); +test('cleans up temporary new option when clearAll is called in allowNewOptions mode', async () => { + const onClear = jest.fn(); + render( + , + ); + await clearAll(); + expect(onClear).toHaveBeenCalled(); + const values = await findAllSelectValues(); + expect(values.length).toBe(0); +}); + +test('deduplicates pasted values against loaded options when allowNewOptions is enabled', async () => { + render(); + await open(); + const input = getElementByClassName('.ant-select-input'); + const paste = createEvent.paste(input, { + clipboardData: { + getData: () => `${OPTIONS[0].label},${OPTIONS[0].label},BrandNewItem`, + }, + }); + fireEvent(input, paste); + await waitFor(async () => { + const values = await findAllSelectValues(); + expect(values.length).toBe(2); + expect(values[0]).toHaveTextContent(OPTIONS[0].label); + expect(values[1]).toHaveTextContent('BrandNewItem'); + }); +}); + test('does not add a new option if allowNewOptions is false', async () => { render(); await open(); @@ -1285,6 +1322,110 @@ test('preserves new option entry across search fetch when allowNewOptions is on' expect(screen.queryByText('Option 0')).not.toBeInTheDocument(); }); +test('drops new option entry when fetched search options contain a matching label (regression for #44873)', async () => { + const page0Data = Array.from({ length: 10 }, (_, i) => ({ + label: `Option ${i}`, + value: i, + })); + const searchData = [{ label: 'Matching Option', value: 42 }]; + const loadOptions = jest.fn(async (search: string) => { + if (search === '') { + return { data: page0Data, totalCount: 100 }; + } + return { data: searchData, totalCount: 1 }; + }); + + render( + , + ); + await open(); + await waitFor(() => expect(loadOptions).toHaveBeenCalledTimes(1)); + + await type('Matching Option'); + await waitFor(() => expect(loadOptions).toHaveBeenCalledTimes(2)); + + const options = await findAllSelectOptions(); + // Drops optimistic new option entry when server returns a match for the label + expect(options).toHaveLength(1); + expect(options[0]).toHaveTextContent('Matching Option'); + expect(screen.queryByText('Option 0')).not.toBeInTheDocument(); +}); + +test('drops optimistic new option when paginated mergeData contains matching label', async () => { + const initialData = [{ label: 'Existing 1', value: 1 }]; + const nextPageData = [{ label: 'Created Later', value: 99 }]; + const loadOptions = jest.fn(async (_search: string, page: number) => { + if (page === 0) { + return { data: initialData, totalCount: 20 }; + } + return { data: nextPageData, totalCount: 20 }; + }); + + render( + , + ); + await open(); + await waitFor(() => expect(loadOptions).toHaveBeenCalledTimes(1)); + + // Type a term that will match next page's label + await type('Created Later'); + await waitFor(() => expect(loadOptions).toHaveBeenCalledTimes(2)); + + // Optimistic option is present with "new" badge before pagination + const optionsBefore = await findAllSelectOptions(); + expect( + Array.from(optionsBefore).some(opt => + opt.textContent?.includes('Created Later'), + ), + ).toBe(true); + expect(screen.getByTestId('new-option-badge')).toBeInTheDocument(); + + // Wait for loading to finish so handlePagination's `!isLoading` gate is open + await waitFor(() => + expect(document.querySelector('.ant-select-suffix .ant-spin')).toBeNull(), + ); + + // Trigger pagination by dispatching a scroll event on the virtual-list container + const holder = document.querySelector( + '.ant-select-dropdown-list-holder', + ) as HTMLElement | null; + if (!holder) throw new Error('virtual-list holder not rendered'); + Object.defineProperty(holder, 'scrollHeight', { + configurable: true, + get: () => 1000, + }); + Object.defineProperty(holder, 'offsetHeight', { + configurable: true, + get: () => 200, + }); + Object.defineProperty(holder, 'clientHeight', { + configurable: true, + get: () => 200, + }); + Object.defineProperty(holder, 'scrollTop', { + configurable: true, + get: () => 900, + set: () => {}, + }); + fireEvent.scroll(holder); + + await waitFor(() => + expect(loadOptions).toHaveBeenCalledWith( + 'Created Later', + 1, + expect.anything(), + ), + ); + + // After next page loads and mergeData fires, duplicate optimistic + // option is dropped and server option (without "new" badge) remains. + await waitFor(async () => { + const matchingOptions = screen.getAllByText('Created Later'); + expect(matchingOptions).toHaveLength(1); + expect(screen.queryByTestId('new-option-badge')).not.toBeInTheDocument(); + }); +}); + test('restores base options when search is cleared', async () => { const page0Data = Array.from({ length: 10 }, (_, i) => ({ label: `Option ${i}`, diff --git a/superset-frontend/packages/superset-ui-core/src/components/Select/AsyncSelect.tsx b/superset-frontend/packages/superset-ui-core/src/components/Select/AsyncSelect.tsx index bed4187a37d7..2ae8843fe49b 100644 --- a/superset-frontend/packages/superset-ui-core/src/components/Select/AsyncSelect.tsx +++ b/superset-frontend/packages/superset-ui-core/src/components/Select/AsyncSelect.tsx @@ -52,6 +52,7 @@ import { getValue, hasOption, isLabeledValue, + isNewOption, sortSelectedFirstHelper, sortComparatorWithSearchHelper, sortComparatorForNoSearchHelper, @@ -81,6 +82,7 @@ import { StyledError, StyledErrorMessage, StyledHeader, + StyledNewOptionBadge, StyledSelect, StyledStopOutlined, } from './styles'; @@ -359,10 +361,12 @@ const AsyncSelect = forwardRef( setSelectValue(array.filter(element => element !== value)); } // removes new option - if (option.isNewOption) { + if (isNewOption(option)) { + const targetVal = getValue(value); setSelectOptions( fullSelectOptions.filter( - option => getValue(option.value) !== getValue(value), + opt => + getValue(opt.value) !== targetVal && opt.label !== targetVal, ), ); } @@ -386,6 +390,30 @@ const AsyncSelect = forwardRef( fullSelectOptionsRef.current = fullSelectOptions; + const isRedundantNewOption = ( + opt: DefaultOptionType | AntdLabeledValue, + incomingData: SelectOptionsType, + ) => { + if (!isNewOption(opt)) { + return false; + } + const label = + typeof opt.label === 'string' || typeof opt.label === 'number' + ? opt.label + : undefined; + return ( + label !== undefined && + incomingData.some(item => { + if (isObject(item) && Array.isArray((item as any).options)) { + return (item as any).options.some((child: any) => + utilsIsEqual(child, label, 'label'), + ); + } + return utilsIsEqual(item, label, 'label'); + }) + ); + }; + const mergeData = useCallback( (data: SelectOptionsType) => { let mergedData: SelectOptionsType = []; @@ -395,7 +423,15 @@ const AsyncSelect = forwardRef( // merges with existing and creates unique options setSelectOptions(prevOptions => { mergedData = prevOptions - .filter(previousOption => !dataValues.has(previousOption.value)) + .filter(previousOption => { + if (dataValues.has(previousOption.value)) { + return false; + } + if (isRedundantNewOption(previousOption, data)) { + return false; + } + return true; + }) .concat(data) // Forward-compat: TS 6.0 infers stricter antd option types; widen // the comparator to accept the broader DefaultOptionType shape. @@ -505,7 +541,10 @@ const AsyncSelect = forwardRef( setSelectOptions(prevOptions => { const dataValues = new Set(data.map(opt => opt.value)); const preservedNew = prevOptions.filter( - opt => opt.isNewOption && !dataValues.has(opt.value), + opt => + isNewOption(opt) && + !dataValues.has(opt.value) && + !isRedundantNewOption(opt, data), ); return preservedNew .concat(data) @@ -606,7 +645,7 @@ const AsyncSelect = forwardRef( isNewOption: true, }; const cleanSelectOptions = fullSelectOptions.filter( - opt => !opt.isNewOption || hasOption(opt.value, selectValue), + opt => !isNewOption(opt) || hasOption(opt.value, selectValue), ); const newOptions = newOption ? [newOption, ...cleanSelectOptions] @@ -952,7 +991,18 @@ const AsyncSelect = forwardRef( // paste path, that lets an option with a React label match on the // plain text its caller put in `title`. optionFilterProp="title" - optionRender={option => option.label || option.value} + optionRender={option => + isNewOption(option.data) || isNewOption(option) ? ( + <> + {option.label || option.value} + + {t('New')} + + + ) : ( + option.label || option.value + ) + } placeholder={placeholder} showSearch={shouldShowSearch} tokenSeparators={quoteAwareTokenSeparators} diff --git a/superset-frontend/packages/superset-ui-core/src/components/Select/Select.stories.tsx b/superset-frontend/packages/superset-ui-core/src/components/Select/Select.stories.tsx index 238f9c5edeee..b19eaeefc204 100644 --- a/superset-frontend/packages/superset-ui-core/src/components/Select/Select.stories.tsx +++ b/superset-frontend/packages/superset-ui-core/src/components/Select/Select.stories.tsx @@ -468,3 +468,26 @@ AdvancedPlayground.argTypes = { control: { type: 'number' }, }, }; + +/** + * Demonstrates the allowNewOptions feature, allowing users to type and create + * custom options that are rendered with a subtle [new] visual badge, while + * ensuring existing options are deduplicated against typed terms. + */ +export const WithAllowNewOptions = (args: SelectProps) => ( +
+ , + ); + await clearAll(); + expect(onClear).toHaveBeenCalled(); + const values = await findAllSelectValues(); + expect(values.length).toBe(0); +}); + test('does not add a new option if allowNewOptions is false', async () => { render(