From 2e44285172580f50813646cfef6c08b05fb42458 Mon Sep 17 00:00:00 2001 From: Diego Pucci Date: Tue, 15 Oct 2024 16:47:53 +0300 Subject: [PATCH 1/5] refactor(Dashboard): Native filters form update endpoint --- .../src/dashboard/actions/dashboardInfo.ts | 6 + .../src/dashboard/actions/dashboardState.js | 4 +- .../src/dashboard/actions/nativeFilters.ts | 108 +++++----- .../FilterConfigurationLink/index.tsx | 8 +- .../FilterBar/FilterControls/FilterValue.tsx | 1 - .../FilterScope/FilterScope.test.tsx | 1 + .../FiltersConfigForm/FiltersConfigForm.tsx | 61 ++++-- .../getControlItemsMap.test.tsx | 1 + .../FiltersConfigForm/getControlItemsMap.tsx | 4 + .../FiltersConfigModal.test.tsx | 196 +++++++++++++++--- .../FiltersConfigModal/FiltersConfigModal.tsx | 147 ++++++++++--- .../nativeFilters/FiltersConfigModal/types.ts | 11 + .../nativeFilters/FiltersConfigModal/utils.ts | 115 +++++----- .../src/dashboard/reducers/dashboardInfo.js | 37 ++++ .../src/dashboard/reducers/nativeFilters.ts | 45 +++- superset-frontend/src/dataMask/actions.ts | 35 ++-- superset-frontend/src/dataMask/reducer.ts | 41 +++- superset/commands/dashboard/exceptions.py | 4 + superset/commands/dashboard/update.py | 17 +- superset/constants.py | 1 + superset/daos/dashboard.py | 64 ++++++ superset/dashboards/api.py | 97 ++++++++- superset/dashboards/schemas.py | 6 + .../integration_tests/dashboards/api_tests.py | 175 ++++++++++++++++ 24 files changed, 960 insertions(+), 225 deletions(-) diff --git a/superset-frontend/src/dashboard/actions/dashboardInfo.ts b/superset-frontend/src/dashboard/actions/dashboardInfo.ts index 14659b9fc33eb..ea25e8c715c89 100644 --- a/superset-frontend/src/dashboard/actions/dashboardInfo.ts +++ b/superset-frontend/src/dashboard/actions/dashboardInfo.ts @@ -29,11 +29,17 @@ import { import { onSave } from './dashboardState'; export const DASHBOARD_INFO_UPDATED = 'DASHBOARD_INFO_UPDATED'; +export const DASHBOARD_INFO_FILTERS_CHANGED = 'DASHBOARD_INFO_FILTERS_CHANGED'; // updates partially changed dashboard info export function dashboardInfoChanged(newInfo: { metadata: any }) { return { type: DASHBOARD_INFO_UPDATED, newInfo }; } + +export function nativeFiltersConfigChanged(newInfo: Record) { + return { type: DASHBOARD_INFO_FILTERS_CHANGED, newInfo }; +} + export const SAVE_CHART_CONFIG_BEGIN = 'SAVE_CHART_CONFIG_BEGIN'; export const SAVE_CHART_CONFIG_COMPLETE = 'SAVE_CHART_CONFIG_COMPLETE'; export const SAVE_CHART_CONFIG_FAIL = 'SAVE_CHART_CONFIG_FAIL'; diff --git a/superset-frontend/src/dashboard/actions/dashboardState.js b/superset-frontend/src/dashboard/actions/dashboardState.js index 9b0c1818212a9..443db787e0df3 100644 --- a/superset-frontend/src/dashboard/actions/dashboardState.js +++ b/superset-frontend/src/dashboard/actions/dashboardState.js @@ -63,7 +63,7 @@ import { } from './dashboardInfo'; import { fetchDatasourceMetadata } from './datasources'; import { updateDirectPathToFilter } from './dashboardFilters'; -import { SET_FILTER_CONFIG_COMPLETE } from './nativeFilters'; +import { SET_IN_SCOPE_STATUS_OF_FILTERS } from './nativeFilters'; import getOverwriteItems from '../util/getOverwriteItems'; import { applyColors, @@ -337,7 +337,7 @@ export function saveDashboardRequest(data, id, saveType) { } if (metadata.native_filter_configuration) { dispatch({ - type: SET_FILTER_CONFIG_COMPLETE, + type: SET_IN_SCOPE_STATUS_OF_FILTERS, filterConfig: metadata.native_filter_configuration, }); } diff --git a/superset-frontend/src/dashboard/actions/nativeFilters.ts b/superset-frontend/src/dashboard/actions/nativeFilters.ts index 968805dbb7605..f594080b704fa 100644 --- a/superset-frontend/src/dashboard/actions/nativeFilters.ts +++ b/superset-frontend/src/dashboard/actions/nativeFilters.ts @@ -19,28 +19,31 @@ import { FilterConfiguration, Filters, makeApi } from '@superset-ui/core'; import { Dispatch } from 'redux'; import { cloneDeep } from 'lodash'; -import { - SET_DATA_MASK_FOR_FILTER_CONFIG_FAIL, - setDataMaskForFilterConfigComplete, -} from 'src/dataMask/actions'; +import { setDataMaskForFilterChangesComplete } from 'src/dataMask/actions'; import { HYDRATE_DASHBOARD } from './hydrate'; -import { dashboardInfoChanged } from './dashboardInfo'; -import { DashboardInfo } from '../types'; +import { + dashboardInfoChanged, + nativeFiltersConfigChanged, +} from './dashboardInfo'; +import { SaveFilterChangesType } from '../components/nativeFilters/FiltersConfigModal/types'; -export const SET_FILTER_CONFIG_BEGIN = 'SET_FILTER_CONFIG_BEGIN'; -export interface SetFilterConfigBegin { - type: typeof SET_FILTER_CONFIG_BEGIN; +export const SET_NATIVE_FILTERS_CONFIG_BEGIN = + 'SET_NATIVE_FILTERS_CONFIG_BEGIN'; +export interface SetNativeFiltersConfigBegin { + type: typeof SET_NATIVE_FILTERS_CONFIG_BEGIN; filterConfig: FilterConfiguration; } -export const SET_FILTER_CONFIG_COMPLETE = 'SET_FILTER_CONFIG_COMPLETE'; -export interface SetFilterConfigComplete { - type: typeof SET_FILTER_CONFIG_COMPLETE; - filterConfig: FilterConfiguration; +export const SET_NATIVE_FILTERS_CONFIG_COMPLETE = + 'SET_NATIVE_FILTERS_CONFIG_COMPLETE'; +export interface SetNativeFiltersConfigComplete { + type: typeof SET_NATIVE_FILTERS_CONFIG_COMPLETE; + filterChanges: SaveFilterChangesType; } -export const SET_FILTER_CONFIG_FAIL = 'SET_FILTER_CONFIG_FAIL'; -export interface SetFilterConfigFail { - type: typeof SET_FILTER_CONFIG_FAIL; + +export const SET_NATIVE_FILTERS_CONFIG_FAIL = 'SET_NATIVE_FILTERS_CONFIG_FAIL'; +export interface SetNativeFiltersConfigFail { + type: typeof SET_NATIVE_FILTERS_CONFIG_FAIL; filterConfig: FilterConfiguration; } export const SET_IN_SCOPE_STATUS_OF_FILTERS = 'SET_IN_SCOPE_STATUS_OF_FILTERS'; @@ -49,60 +52,45 @@ export interface SetInScopeStatusOfFilters { filterConfig: FilterConfiguration; } +const isFilterChangesEmpty = (filterChanges: SaveFilterChangesType) => + Object.values(filterChanges).every( + array => Array.isArray(array) && !array.length, + ); + export const setFilterConfiguration = - (filterConfig: FilterConfiguration) => + (filterChanges: SaveFilterChangesType) => async (dispatch: Dispatch, getState: () => any) => { + if (isFilterChangesEmpty(filterChanges)) { + return; + } + + const { id } = getState().dashboardInfo; + const oldFilters = getState().nativeFilters?.filters; + dispatch({ - type: SET_FILTER_CONFIG_BEGIN, - filterConfig, + type: SET_NATIVE_FILTERS_CONFIG_BEGIN, + filterChanges, }); - const { id, metadata } = getState().dashboardInfo; - const oldFilters = getState().nativeFilters?.filters; - // TODO extract this out when makeApi supports url parameters - const updateDashboard = makeApi< - Partial, - { result: DashboardInfo } + const updateFilters = makeApi< + SaveFilterChangesType, + { result: SaveFilterChangesType } >({ method: 'PUT', - endpoint: `/api/v1/dashboard/${id}`, + endpoint: `/api/v1/dashboard/${id}/filters`, }); - - const mergedFilterConfig = filterConfig.map(filter => { - const oldFilter = oldFilters[filter.id]; - if (!oldFilter) { - return filter; - } - return { ...oldFilter, ...filter }; - }); - try { - const response = await updateDashboard({ - json_metadata: JSON.stringify({ - ...metadata, - native_filter_configuration: mergedFilterConfig, - }), - }); - dispatch( - dashboardInfoChanged({ - metadata: JSON.parse(response.result.json_metadata), - }), - ); + const response = await updateFilters(filterChanges); + dispatch(nativeFiltersConfigChanged(response.result)); dispatch({ - type: SET_FILTER_CONFIG_COMPLETE, - filterConfig: mergedFilterConfig, + type: SET_NATIVE_FILTERS_CONFIG_COMPLETE, + filterChanges, }); - dispatch( - setDataMaskForFilterConfigComplete(mergedFilterConfig, oldFilters), - ); + dispatch(setDataMaskForFilterChangesComplete(filterChanges, oldFilters)); } catch (err) { dispatch({ - type: SET_FILTER_CONFIG_FAIL, - filterConfig: mergedFilterConfig, - }); - dispatch({ - type: SET_DATA_MASK_FOR_FILTER_CONFIG_FAIL, - filterConfig: mergedFilterConfig, + type: SET_NATIVE_FILTERS_CONFIG_FAIL, + filterConfig: filterChanges, }); } }; @@ -221,9 +209,9 @@ export function updateCascadeParentIds( } export type AnyFilterAction = - | SetFilterConfigBegin - | SetFilterConfigComplete - | SetFilterConfigFail + | SetNativeFiltersConfigBegin + | SetNativeFiltersConfigComplete + | SetNativeFiltersConfigFail | SetInScopeStatusOfFilters | SetBootstrapData | SetFocusedNativeFilter diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterConfigurationLink/index.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterConfigurationLink/index.tsx index d7e8564eb5105..7be1e7814c085 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterConfigurationLink/index.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterConfigurationLink/index.tsx @@ -21,9 +21,10 @@ import { ReactNode, FC, useCallback, useState, memo } from 'react'; import { useDispatch } from 'react-redux'; import { setFilterConfiguration } from 'src/dashboard/actions/nativeFilters'; import Button from 'src/components/Button'; -import { FilterConfiguration, styled } from '@superset-ui/core'; +import { styled } from '@superset-ui/core'; import FiltersConfigModal from 'src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigModal'; import { getFilterBarTestId } from '../utils'; +import { SaveFilterChangesType } from '../../FiltersConfigModal/types'; export interface FCBProps { createNewOnOpen?: boolean; @@ -46,14 +47,13 @@ export const FilterConfigurationLink: FC = ({ }) => { const dispatch = useDispatch(); const [isOpen, setOpen] = useState(false); - const close = useCallback(() => { setOpen(false); }, [setOpen]); const submit = useCallback( - async (filterConfig: FilterConfiguration) => { - dispatch(await setFilterConfiguration(filterConfig)); + async (filterChanges: SaveFilterChangesType) => { + dispatch(await setFilterConfiguration(filterChanges)); close(); }, [dispatch, close], diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterValue.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterValue.tsx index 2d85f413c355a..45ccd4dd4119a 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterValue.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterValue.tsx @@ -182,7 +182,6 @@ const FilterValue: FC = ({ if (isFeatureEnabled(FeatureFlag.GlobalAsyncQueries)) { // deal with getChartDataRequest transforming the response data const result = 'result' in json ? json.result[0] : json; - if (response.status === 200) { setState([result]); handleFilterLoadFinish(); diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/FilterScope/FilterScope.test.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/FilterScope/FilterScope.test.tsx index 93f3dd6ec1c83..aece899a16078 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/FilterScope/FilterScope.test.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/FilterScope/FilterScope.test.tsx @@ -47,6 +47,7 @@ describe('FilterScope', () => { activeFilterPanelKeys: `DefaultFilterId-${FilterPanels.configuration.key}`, isActive: true, validateDependencies: jest.fn(), + onModifyFilter: jest.fn(), }; const MockModal = ({ scope }: { scope?: object }) => { 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 77a423df76954..b7c66412fa94e 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/FiltersConfigForm.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/FiltersConfigForm.tsx @@ -39,8 +39,9 @@ import { t, ClientErrorObject, getClientErrorObject, + SLOW_DEBOUNCE, } from '@superset-ui/core'; -import { isEqual } from 'lodash'; +import { debounce, isEqual } from 'lodash'; import { forwardRef, useCallback, @@ -306,6 +307,7 @@ export interface FiltersConfigFormProps { filterToEdit?: Filter; removedFilters: Record; restoreFilter: (filterId: string) => void; + onModifyFilter: (filterId: string) => void; form: FormInstance; getAvailableFilters: ( filterId: string, @@ -346,6 +348,7 @@ const FiltersConfigForm = ( restoreFilter, handleActiveFilterPanelChange, setErroredFilters, + onModifyFilter, validateDependencies, getDependencySuggestion, isActive, @@ -372,6 +375,12 @@ const FiltersConfigForm = ( const formValues = filters?.[filterId]; const formFilter = formValues || undoFormValues || defaultFormFilter; + const handleModifyFilter = useCallback(() => { + if (onModifyFilter) { + onModifyFilter(filterId); + } + }, [onModifyFilter, filterId]); + const dependencies: string[] = formFilter?.dependencies || filterToEdit?.cascadeParentIds || []; @@ -412,12 +421,28 @@ const FiltersConfigForm = ( filterToEdit?.targets[0]?.datasetId ?? mostUsedDataset(loadedDatasets, charts); + const formChanged = useCallback(() => { + form.setFields([ + { + name: 'changed', + value: true, + }, + ]); + handleModifyFilter(); + }, [form, handleModifyFilter]); + + const debouncedFormChanged = useCallback( + debounce(formChanged, SLOW_DEBOUNCE), + [], + ); + const { controlItems = {}, mainControlItems = {} } = formFilter ? getControlItemsMap({ expanded, datasetId, disabled: false, forceUpdate, + formChanged, form, filterId, filterType: formFilter?.filterType, @@ -488,7 +513,6 @@ const FiltersConfigForm = ( groupby: formFilter?.column, ...formFilter, }); - formData.extra_form_data = dependenciesDefaultValues; setNativeFilterFieldValuesWrapper({ @@ -549,6 +573,7 @@ const FiltersConfigForm = ( groupby: hasColumn ? formFilter?.column : undefined, ...formFilter, }); + newFormData.extra_form_data = dependenciesDefaultValues; const [hasDefaultValue, isRequired, defaultValueTooltip, setHasDefaultValue] = @@ -557,15 +582,6 @@ const FiltersConfigForm = ( const showDataset = !datasetId || datasetDetails || formFilter?.dataset?.label; - const formChanged = useCallback(() => { - form.setFields([ - { - name: 'changed', - value: true, - }, - ]); - }, [form]); - const updateFormValues = useCallback( (values: any) => { setNativeFilterFieldValues(form, filterId, values); @@ -794,6 +810,7 @@ const FiltersConfigForm = ( granularity_sqla: column, }); forceUpdate(); + formChanged(); }} /> @@ -817,7 +834,7 @@ const FiltersConfigForm = ( hidden initialValue={NativeFilterType.NativeFilter} > - + - + @@ -920,6 +941,7 @@ const FiltersConfigForm = ( }); } forceUpdate(); + formChanged(); }} /> @@ -1018,6 +1040,7 @@ const FiltersConfigForm = ( adhoc_filters: filters, }); forceUpdate(); + formChanged(); validatePreFilter(); }} label={ @@ -1050,6 +1073,7 @@ const FiltersConfigForm = ( time_range: timeRange, }); forceUpdate(); + formChanged(); validatePreFilter(); }} /> @@ -1085,6 +1109,7 @@ const FiltersConfigForm = ( { onSortChanged(value.target.value); + formChanged(); }} > {t('Sort ascending')} @@ -1124,6 +1149,7 @@ const FiltersConfigForm = ( }); forceUpdate(); } + formChanged(); }} /> @@ -1156,9 +1182,10 @@ const FiltersConfigForm = ( } > - onEnableSingleValueChanged(value.target.value) - } + onChange={value => { + onEnableSingleValueChanged(value.target.value); + formChanged(); + }} > {t('Minimum')} @@ -1187,7 +1214,7 @@ const FiltersConfigForm = ( initialValue={filterToEdit?.description} label={{t('Description')}} > -