Skip to content

Commit

Permalink
fix: Select is accepting unknown pasted values when allowNewOptions
Browse files Browse the repository at this point in the history
… is false (#28017)

(cherry picked from commit caad29b)
  • Loading branch information
michael-s-molina committed Apr 16, 2024
1 parent ffd8baa commit bf6cd56
Show file tree
Hide file tree
Showing 4 changed files with 63 additions and 9 deletions.
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 @@ -543,6 +543,9 @@ const AsyncSelect = forwardRef(
data.find(item => item.label === text),
);
}
if (!option && !allowNewOptions) {
return undefined;
}
const value: AntdLabeledValue = {
label: text,
value: text,
Expand All @@ -553,19 +556,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

0 comments on commit bf6cd56

Please sign in to comment.