From 41230db0ead66ba728b0d8cf1eec968a1eb88be6 Mon Sep 17 00:00:00 2001 From: "Michael S. Molina" <70410625+michael-s-molina@users.noreply.github.com> Date: Mon, 15 Apr 2024 09:33:17 -0300 Subject: [PATCH] fix: Select is accepting unknown pasted values when `allowNewOptions` is false (#28017) (cherry picked from commit caad29b5b36bdb817897b78c53c412f07916907f) --- .../components/Select/AsyncSelect.test.tsx | 28 ++++++++++++++++++- .../src/components/Select/AsyncSelect.tsx | 16 +++++++---- .../src/components/Select/Select.test.tsx | 14 ++++++++++ .../src/components/Select/Select.tsx | 14 ++++++++-- 4 files changed, 63 insertions(+), 9 deletions(-) diff --git a/superset-frontend/src/components/Select/AsyncSelect.test.tsx b/superset-frontend/src/components/Select/AsyncSelect.test.tsx index b8626026384ac..652b1f0ea25ab 100644 --- a/superset-frontend/src/components/Select/AsyncSelect.test.tsx +++ b/superset-frontend/src/components/Select/AsyncSelect.test.tsx @@ -928,7 +928,14 @@ test('pasting an existing option does not duplicate it in multiple mode', async ], totalCount: 3, })); - render(); + render( + , + ); await open(); const input = getElementByClassName('.ant-select-selection-search-input'); const paste = createEvent.paste(input, { @@ -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( + ({ 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(); diff --git a/superset-frontend/src/components/Select/AsyncSelect.tsx b/superset-frontend/src/components/Select/AsyncSelect.tsx index 16cd1b26b817e..5468528893a89 100644 --- a/superset-frontend/src/components/Select/AsyncSelect.tsx +++ b/superset-frontend/src/components/Select/AsyncSelect.tsx @@ -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, @@ -553,19 +556,22 @@ const AsyncSelect = forwardRef( } return value; }, - [allValuesLoaded, fullSelectOptions, options, pageSize], + [allValuesLoaded, allowNewOptions, fullSelectOptions, options, pageSize], ); const onPaste = async (e: ClipboardEvent) => { 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)), diff --git a/superset-frontend/src/components/Select/Select.test.tsx b/superset-frontend/src/components/Select/Select.test.tsx index 1daff06d4de77..bbe09a27f4f86 100644 --- a/superset-frontend/src/components/Select/Select.test.tsx +++ b/superset-frontend/src/components/Select/Select.test.tsx @@ -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(); @@ -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(); diff --git a/superset-frontend/src/components/Select/Select.tsx b/superset-frontend/src/components/Select/Select.tsx index 3db455cbe2714..d92b86318d0a7 100644 --- a/superset-frontend/src/components/Select/Select.tsx +++ b/superset-frontend/src/components/Select/Select.tsx @@ -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, @@ -556,17 +559,22 @@ const Select = forwardRef( } return option ? (isObject(option) ? option.value! : option) : text; }, - [fullSelectOptions, labelInValue], + [allowNewOptions, fullSelectOptions, labelInValue], ); const onPaste = (e: ClipboardEvent) => { 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[]),