From 8166c1ac53ba305fcfed1a11ea42abe1fefd421a Mon Sep 17 00:00:00 2001 From: "Michael S. Molina" Date: Thu, 11 Apr 2024 15:56:26 -0300 Subject: [PATCH] fix: Incorrect onChange value when an unloaded value is pasted into AsyncSelect --- .../components/Select/AsyncSelect.test.tsx | 42 +++++++++++++++---- .../src/components/Select/AsyncSelect.tsx | 23 ++++++---- 2 files changed, 49 insertions(+), 16 deletions(-) diff --git a/superset-frontend/src/components/Select/AsyncSelect.test.tsx b/superset-frontend/src/components/Select/AsyncSelect.test.tsx index 8e8e151b6641f..b8626026384ac 100644 --- a/superset-frontend/src/components/Select/AsyncSelect.test.tsx +++ b/superset-frontend/src/components/Select/AsyncSelect.test.tsx @@ -384,12 +384,14 @@ test('removes duplicated values', async () => { }, }); fireEvent(input, paste); - const values = await findAllSelectValues(); - expect(values.length).toBe(4); - expect(values[0]).toHaveTextContent('a'); - expect(values[1]).toHaveTextContent('b'); - expect(values[2]).toHaveTextContent('c'); - expect(values[3]).toHaveTextContent('d'); + await waitFor(async () => { + const values = await findAllSelectValues(); + expect(values.length).toBe(4); + expect(values[0]).toHaveTextContent('a'); + expect(values[1]).toHaveTextContent('b'); + expect(values[2]).toHaveTextContent('c'); + expect(values[3]).toHaveTextContent('d'); + }); }); test('renders a custom label', async () => { @@ -879,7 +881,7 @@ test('fires onChange when pasting a selection', async () => { }, }); fireEvent(input, paste); - expect(onChange).toHaveBeenCalledTimes(1); + await waitFor(() => expect(onChange).toHaveBeenCalledTimes(1)); }); test('does not duplicate options when using numeric values', async () => { @@ -935,8 +937,30 @@ test('pasting an existing option does not duplicate it in multiple mode', async }, }); fireEvent(input, paste); - // Only Peter should be added - expect(await findAllSelectOptions()).toHaveLength(4); + await waitFor(async () => + // Only Peter should be added + expect(await findAllSelectOptions()).toHaveLength(4), + ); +}); + +test('onChange is called with the value property when pasting an option that was not loaded yet', async () => { + const onChange = jest.fn(); + render(); + await open(); + const input = getElementByClassName('.ant-select-selection-search-input'); + const lastOption = OPTIONS[OPTIONS.length - 1]; + const paste = createEvent.paste(input, { + clipboardData: { + getData: () => lastOption.label, + }, + }); + fireEvent(input, paste); + await waitFor(() => + expect(onChange).toHaveBeenCalledWith( + expect.objectContaining({ value: lastOption.value }), + expect.anything(), + ), + ); }); test('does not fire onChange if the same value is selected in single mode', async () => { diff --git a/superset-frontend/src/components/Select/AsyncSelect.tsx b/superset-frontend/src/components/Select/AsyncSelect.tsx index 4f7a69784d9a6..5e6a2a1d9299a 100644 --- a/superset-frontend/src/components/Select/AsyncSelect.tsx +++ b/superset-frontend/src/components/Select/AsyncSelect.tsx @@ -538,8 +538,15 @@ const AsyncSelect = forwardRef( ); const getPastedTextValue = useCallback( - (text: string) => { - const option = getOption(text, fullSelectOptions, true); + async (text: string) => { + let option = getOption(text, fullSelectOptions, true); + if (!option && !allValuesLoaded) { + const fetchOptions = options as SelectOptionsPagePromise; + option = await fetchOptions(text, 0, pageSize).then( + ({ data }: SelectOptionsTypePage) => + data.find(item => item.label === text), + ); + } const value: AntdLabeledValue = { label: text, value: text, @@ -550,20 +557,22 @@ const AsyncSelect = forwardRef( } return value; }, - [fullSelectOptions], + [allValuesLoaded, fullSelectOptions, options, pageSize], ); - const onPaste = (e: ClipboardEvent) => { + const onPaste = async (e: ClipboardEvent) => { const pastedText = e.clipboardData.getData('text'); if (isSingleMode) { - setSelectValue(getPastedTextValue(pastedText)); + setSelectValue(await getPastedTextValue(pastedText)); } 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 = await Promise.all( + array.map(item => getPastedTextValue(item)), + ); setSelectValue(previous => [ ...((previous || []) as AntdLabeledValue[]), - ...values, + ...values.filter(value => !hasOption(value.value, previous)), ]); } fireOnChange();