Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Select is accepting unknown pasted values when allowNewOptions is false #28017

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 27 additions & 1 deletion superset-frontend/src/components/Select/AsyncSelect.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -928,7 +928,14 @@ test('pasting an existing option does not duplicate it in multiple mode', async
],
totalCount: 3,
}));
render(<AsyncSelect {...defaultProps} options={options} mode="multiple" />);
render(
<AsyncSelect
{...defaultProps}
options={options}
mode="multiple"
allowNewOptions
/>,
);
await open();
const input = getElementByClassName('.ant-select-selection-search-input');
const paste = createEvent.paste(input, {
Expand All @@ -943,6 +950,25 @@ test('pasting an existing option does not duplicate it in multiple mode', async
);
});

test('pasting an non-existent option should not add it if allowNewOptions is false', async () => {
render(
<AsyncSelect
{...defaultProps}
allowNewOptions={false}
options={async () => ({ data: [], totalCount: 0 })}
/>,
);
await open();
const input = getElementByClassName('.ant-select-selection-search-input');
const paste = createEvent.paste(input, {
clipboardData: {
getData: () => 'John',
},
});
await waitFor(() => fireEvent(input, paste));
expect(await findAllSelectOptions()).toHaveLength(0);
});

test('onChange is called with the value property when pasting an option that was not loaded yet', async () => {
const onChange = jest.fn();
render(<AsyncSelect {...defaultProps} onChange={onChange} />);
Expand Down
16 changes: 11 additions & 5 deletions superset-frontend/src/components/Select/AsyncSelect.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -547,6 +547,9 @@ const AsyncSelect = forwardRef(
data.find(item => item.label === text),
);
}
if (!option && !allowNewOptions) {
return undefined;
}
const value: AntdLabeledValue = {
label: text,
value: text,
Expand All @@ -557,19 +560,22 @@ const AsyncSelect = forwardRef(
}
return value;
},
[allValuesLoaded, fullSelectOptions, options, pageSize],
[allValuesLoaded, allowNewOptions, fullSelectOptions, options, pageSize],
);

const onPaste = async (e: ClipboardEvent<HTMLInputElement>) => {
const pastedText = e.clipboardData.getData('text');
if (isSingleMode) {
setSelectValue(await getPastedTextValue(pastedText));
const value = await getPastedTextValue(pastedText);
if (value) {
setSelectValue(value);
}
} else {
const token = tokenSeparators.find(token => pastedText.includes(token));
const array = token ? uniq(pastedText.split(token)) : [pastedText];
const values = await Promise.all(
array.map(item => getPastedTextValue(item)),
);
const values = (
await Promise.all(array.map(item => getPastedTextValue(item)))
).filter(item => item !== undefined) as AntdLabeledValue[];
setSelectValue(previous => [
...((previous || []) as AntdLabeledValue[]),
...values.filter(value => !hasOption(value.value, previous)),
Expand Down
14 changes: 14 additions & 0 deletions superset-frontend/src/components/Select/Select.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -1039,6 +1039,7 @@ test('pasting an existing option does not duplicate it in multiple mode', async
options={options}
mode="multiple"
allowSelectAll={false}
allowNewOptions
/>,
);
await open();
Expand All @@ -1053,6 +1054,19 @@ test('pasting an existing option does not duplicate it in multiple mode', async
expect(await findAllSelectOptions()).toHaveLength(4);
});

test('pasting an non-existent option should not add it if allowNewOptions is false', async () => {
render(<Select {...defaultProps} options={[]} allowNewOptions={false} />);
await open();
const input = getElementByClassName('.ant-select-selection-search-input');
const paste = createEvent.paste(input, {
clipboardData: {
getData: () => 'John',
},
});
fireEvent(input, paste);
expect(await findAllSelectOptions()).toHaveLength(0);
});

test('does not fire onChange if the same value is selected in single mode', async () => {
const onChange = jest.fn();
render(<Select {...defaultProps} onChange={onChange} />);
Expand Down
14 changes: 11 additions & 3 deletions superset-frontend/src/components/Select/Select.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -543,6 +543,9 @@ const Select = forwardRef(
const getPastedTextValue = useCallback(
(text: string) => {
const option = getOption(text, fullSelectOptions, true);
if (!option && !allowNewOptions) {
return undefined;
}
if (labelInValue) {
const value: AntdLabeledValue = {
label: text,
Expand All @@ -556,17 +559,22 @@ const Select = forwardRef(
}
return option ? (isObject(option) ? option.value! : option) : text;
},
[fullSelectOptions, labelInValue],
[allowNewOptions, fullSelectOptions, labelInValue],
);

const onPaste = (e: ClipboardEvent<HTMLInputElement>) => {
const pastedText = e.clipboardData.getData('text');
if (isSingleMode) {
setSelectValue(getPastedTextValue(pastedText));
const value = getPastedTextValue(pastedText);
if (value) {
setSelectValue(value);
}
} else {
const token = tokenSeparators.find(token => pastedText.includes(token));
const array = token ? uniq(pastedText.split(token)) : [pastedText];
const values = array.map(item => getPastedTextValue(item));
const values = array
.map(item => getPastedTextValue(item))
.filter(item => item !== undefined);
if (labelInValue) {
setSelectValue(previous => [
...((previous || []) as AntdLabeledValue[]),
Expand Down
Loading