From e660de69364c5f40b646fa54cf16f493f8ccf1e8 Mon Sep 17 00:00:00 2001 From: "Michael S. Molina" <70410625+michael-s-molina@users.noreply.github.com> Date: Thu, 22 Jul 2021 15:17:31 -0300 Subject: [PATCH] chore: Adds lazy loading to the Select component (#15799) --- .../src/components/Select/Select.stories.tsx | 19 ++- .../src/components/Select/Select.tsx | 150 ++++++++++++++---- .../FiltersConfigForm/DatasetSelect.tsx | 24 +-- .../FiltersConfigForm/DefaultValue.tsx | 12 +- .../FiltersConfigForm/FiltersConfigForm.tsx | 68 ++++---- .../getControlItemsMap.test.tsx | 1 + .../FiltersConfigForm/getControlItemsMap.tsx | 3 +- .../FiltersConfigModal.test.tsx | 16 +- 8 files changed, 195 insertions(+), 98 deletions(-) diff --git a/superset-frontend/src/components/Select/Select.stories.tsx b/superset-frontend/src/components/Select/Select.stories.tsx index 5955176f536c..e6dd867d972c 100644 --- a/superset-frontend/src/components/Select/Select.stories.tsx +++ b/superset-frontend/src/components/Select/Select.stories.tsx @@ -144,6 +144,11 @@ InteractiveSelect.argTypes = { disable: true, }, }, + fetchOnlyOnSearch: { + table: { + disable: true, + }, + }, }; InteractiveSelect.story = { @@ -296,10 +301,12 @@ const USERS = [ export const AsyncSelect = ({ withError, + withInitialValue, responseTime, ...rest }: SelectProps & { withError: boolean; + withInitialValue: boolean; responseTime: number; }) => { const [requests, setRequests] = useState([]); @@ -375,6 +382,11 @@ export const AsyncSelect = ({ diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/DefaultValue.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/DefaultValue.tsx index b56f2c221b49..e876006ee9cc 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/DefaultValue.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/DefaultValue.tsx @@ -16,7 +16,7 @@ * specific language governing permissions and limitations * under the License. */ -import React, { FC, useEffect, useState } from 'react'; +import React, { FC } from 'react'; import { Behavior, SetDataMaskHook, @@ -48,17 +48,9 @@ const DefaultValue: FC = ({ formData, enableNoResults, }) => { - const [loading, setLoading] = useState(hasDataset); const formFilter = (form.getFieldValue('filters') || {})[filterId]; const queriesData = formFilter?.defaultValueQueriesData; - - useEffect(() => { - if (!hasDataset || queriesData !== null) { - setLoading(false); - } else { - setLoading(true); - } - }, [hasDataset, queriesData]); + const loading = hasDataset && queriesData === null; const value = formFilter.defaultDataMask?.filterState.value; const isMissingRequiredValue = hasDefaultValue && (value === null || value === undefined); diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/FiltersConfigForm.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/FiltersConfigForm.tsx index ae27d16e0970..c7300c626e42 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/FiltersConfigForm.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/FiltersConfigForm.tsx @@ -355,8 +355,14 @@ const FiltersConfigForm = ( const hasDataset = !!nativeFilterItems[formFilter?.filterType]?.value ?.datasourceCount; + const datasetId = + formFilter?.dataset?.value ?? + filterToEdit?.targets[0]?.datasetId ?? + mostUsedDataset(loadedDatasets, charts); + const { controlItems = {}, mainControlItems = {} } = formFilter ? getControlItemsMap({ + datasetId, disabled: false, forceUpdate, form, @@ -372,10 +378,9 @@ const FiltersConfigForm = ( const nativeFilterItem = nativeFilterItems[formFilter?.filterType] ?? {}; // @ts-ignore const enableNoResults = !!nativeFilterItem.value?.enableNoResults; - const datasetId = formFilter?.dataset?.value; useEffect(() => { - if (datasetId && hasColumn) { + if (datasetId) { cachedSupersetGet({ endpoint: `/api/v1/dataset/${datasetId}`, }) @@ -391,7 +396,7 @@ const FiltersConfigForm = ( addDangerToast(response.message); }); } - }, [datasetId, hasColumn]); + }, [datasetId]); useImperativeHandle(ref, () => ({ changeTab(tab: 'configuration' | 'scoping') { @@ -491,10 +496,6 @@ const FiltersConfigForm = ( [filterId, forceUpdate, form, formFilter, hasDataset], ); - const initialDatasetId = - filterToEdit?.targets[0]?.datasetId ?? - mostUsedDataset(loadedDatasets, charts); - const newFormData = getFormData({ datasetId, groupby: hasColumn ? formFilter?.column : undefined, @@ -508,6 +509,8 @@ const FiltersConfigForm = ( setHasDefaultValue, ] = useDefaultValue(formFilter, filterToEdit); + const showDataset = !datasetId || datasetDetails; + useEffect(() => { if (hasDataset && hasFilledDataset && hasDefaultValue && isDataDirty) { refreshHandler(); @@ -519,6 +522,7 @@ const FiltersConfigForm = ( formFilter, isDataDirty, refreshHandler, + showDataset, ]); const updateFormValues = useCallback( @@ -713,24 +717,29 @@ const FiltersConfigForm = ( {hasDataset && ( - {t('Dataset')}} - rules={[ - { required: !removed, message: t('Dataset is required') }, - ]} - {...getFiltersConfigModalTestId('datasource-input')} - > - {!datasetId || !hasColumn || datasetDetails ? ( + {showDataset ? ( + {t('Dataset')}} + initialValue={ + datasetDetails + ? { + label: datasetDetails.table_name, + value: datasetDetails.id, + } + : undefined + } + rules={[ + { required: !removed, message: t('Dataset is required') }, + ]} + {...getFiltersConfigModalTestId('datasource-input')} + > { + onChange={(value: { label: string; value: number }) => { // We need to reset the column when the dataset has changed - if (value !== datasetId) { + if (value.value !== datasetId) { setNativeFilterFieldValues(form, filterId, { - dataset: { value }, + dataset: value, defaultDataMask: null, column: null, }); @@ -738,10 +747,12 @@ const FiltersConfigForm = ( forceUpdate(); }} /> - ) : ( + + ) : ( + {t('Dataset')}}> - )} - + + )} {hasDataset && Object.keys(mainControlItems).map( key => mainControlItems[key].element, @@ -784,11 +795,8 @@ const FiltersConfigForm = ( required={hasDefaultValue} rules={[ { - validator: (rule, value) => { - const hasValue = - value?.filterState?.value !== null && - value?.filterState?.value !== undefined; - if (hasValue) { + validator: () => { + if (formFilter?.defaultDataMask?.filterState?.value) { return Promise.resolve(); } return Promise.reject( diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/getControlItemsMap.test.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/getControlItemsMap.test.tsx index 7c33f394496f..f2f62983f360 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/getControlItemsMap.test.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/getControlItemsMap.test.tsx @@ -63,6 +63,7 @@ const filterMock: Filter = { }; const createProps: () => ControlItemsProps = () => ({ + datasetId: 1, disabled: false, forceUpdate: jest.fn(), form: formMock, diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/getControlItemsMap.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/getControlItemsMap.tsx index 24a938cb5f63..65ac4fbca259 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/getControlItemsMap.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/getControlItemsMap.tsx @@ -41,6 +41,7 @@ import { Filter } from '../../types'; import { ColumnSelect } from './ColumnSelect'; export interface ControlItemsProps { + datasetId: number; disabled: boolean; forceUpdate: Function; form: FormInstance; @@ -56,6 +57,7 @@ const CleanFormItem = styled(FormItem)` `; export default function getControlItemsMap({ + datasetId, disabled, forceUpdate, form, @@ -87,7 +89,6 @@ export default function getControlItemsMap({ filterToEdit?.controlValues?.[mainControlItem.name] ?? mainControlItem?.config?.default; const initColumn = filterToEdit?.targets[0]?.column?.name; - const datasetId = formFilter?.dataset?.value; const element = ( <> diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigModal.test.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigModal.test.tsx index e3b04c254f0f..a5497d00d8cb 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigModal.test.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigModal.test.tsx @@ -28,7 +28,7 @@ import { TimeGrainFilterPlugin, } from 'src/filters/components'; import { render, screen, waitFor } from 'spec/helpers/testing-library'; -import mockDatasource, { datasourceId } from 'spec/fixtures/mockDatasource'; +import mockDatasource, { id, datasourceId } from 'spec/fixtures/mockDatasource'; import chartQueries from 'spec/fixtures/mockChartQueries'; import { FiltersConfigModal, @@ -71,9 +71,9 @@ const noTemporalColumnsState = () => { }; }; -fetchMock.get('glob:*/api/v1/dataset/1', { +const datasetResult = (id: number) => ({ description_columns: {}, - id: 1, + id, label_columns: { columns: 'Columns', table_name: 'Table Name', @@ -87,11 +87,14 @@ fetchMock.get('glob:*/api/v1/dataset/1', { }, ], table_name: 'birth_names', - id: 1, + id, }, show_columns: ['id', 'table_name'], }); +fetchMock.get('glob:*/api/v1/dataset/1', datasetResult(1)); +fetchMock.get(`glob:*/api/v1/dataset/${id}`, datasetResult(id)); + fetchMock.post('glob:*/api/v1/chart/data', { result: [ { @@ -320,6 +323,11 @@ test('validates the pre-filter value', async () => { test("doesn't render time range pre-filter if there are no temporal columns in datasource", async () => { defaultRender(noTemporalColumnsState()); + userEvent.click(screen.getByText(DATASET_REGEX)); + await waitFor(() => { + expect(screen.queryByLabelText('Loading')).not.toBeInTheDocument(); + userEvent.click(screen.getByText('birth_names')); + }); userEvent.click(screen.getByText(ADVANCED_REGEX)); userEvent.click(getCheckbox(PRE_FILTER_REGEX)); await waitFor(() =>