From 5aa38ef929223353ca26fb6b881798c91e46348d Mon Sep 17 00:00:00 2001 From: simcha90 <56388545+simcha90@users.noreply.github.com> Date: Tue, 16 Feb 2021 16:12:35 +0200 Subject: [PATCH] feat(native-filters): enable filter indicator and make datasource optional (#13148) * refactor: continue decouple native filters * refactor(native-filters): decouple native filter parms from config modal * lint: fix TS errors * lint: fix TS errors * refactor: update filter badge * refactor: rename ant filters * test: fix tests * test: fix tests * refactor: remove 18 from dataset --- .../spec/fixtures/mockDashboardInfo.js | 11 +- .../spec/fixtures/mockNativeFilters.ts | 11 +- .../nativeFilters/NativeFiltersModal_spec.tsx | 17 +- .../dashboard/fixtures/mockNativeFilters.js | 6 +- .../components/FiltersBadge/selectors.ts | 57 ++--- .../nativeFilters/FilterBar/FilterValue.tsx | 14 +- .../FilterConfigModal/FilterConfigForm.tsx | 194 ++++++++++-------- .../FilterConfigModal/FilterConfigModal.tsx | 18 +- .../FilterConfigModal/FiltersList.tsx | 67 ------ .../nativeFilters/FilterConfigModal/state.ts | 7 +- .../nativeFilters/FilterConfigModal/types.ts | 4 +- .../nativeFilters/FilterConfigModal/utils.ts | 10 +- .../components/nativeFilters/types.ts | 10 +- .../components/nativeFilters/utils.ts | 48 +++-- .../src/dashboard/containers/FiltersBadge.tsx | 9 +- ...dRangeFilter.tsx => RangeFilterPlugin.tsx} | 8 +- .../src/filters/components/Range/index.ts | 6 +- .../src/filters/components/Range/types.ts | 10 +- ...ies.tsx => SelectFilterPlugin.stories.tsx} | 4 +- ...electFilter.tsx => SelectFilterPlugin.tsx} | 10 +- .../src/filters/components/Select/index.ts | 6 +- .../src/filters/components/Select/types.ts | 16 +- ...ntdTimeFilter.tsx => TimeFilterPlugin.tsx} | 8 +- .../src/filters/components/Time/index.ts | 5 +- .../src/filters/components/Time/types.ts | 10 +- .../src/filters/components/index.ts | 4 +- .../src/filters/components/types.ts | 2 +- .../src/visualizations/presets/MainPreset.js | 8 +- 28 files changed, 292 insertions(+), 288 deletions(-) delete mode 100644 superset-frontend/src/dashboard/components/nativeFilters/FilterConfigModal/FiltersList.tsx rename superset-frontend/src/filters/components/Range/{AntdRangeFilter.tsx => RangeFilterPlugin.tsx} (87%) rename superset-frontend/src/filters/components/Select/{AntdSelectFilter.stories.tsx => SelectFilterPlugin.stories.tsx} (94%) rename superset-frontend/src/filters/components/Select/{AntdSelectFilter.tsx => SelectFilterPlugin.tsx} (92%) rename superset-frontend/src/filters/components/Time/{AntdTimeFilter.tsx => TimeFilterPlugin.tsx} (89%) diff --git a/superset-frontend/spec/fixtures/mockDashboardInfo.js b/superset-frontend/spec/fixtures/mockDashboardInfo.js index 228c72473f..68f397440c 100644 --- a/superset-frontend/spec/fixtures/mockDashboardInfo.js +++ b/superset-frontend/spec/fixtures/mockDashboardInfo.js @@ -19,7 +19,16 @@ export default { id: 1234, slug: 'dashboardSlug', - metadata: {}, + metadata: { + filter_configuration: [ + { + id: 'DefaultsID', + filterType: 'filter_select', + targets: [{}], + cascadeParentIds: [], + }, + ], + }, userId: 'mock_user_id', dash_edit_perm: true, dash_save_perm: true, diff --git a/superset-frontend/spec/fixtures/mockNativeFilters.ts b/superset-frontend/spec/fixtures/mockNativeFilters.ts index 42404fcbb9..3cc5a9f169 100644 --- a/superset-frontend/spec/fixtures/mockNativeFilters.ts +++ b/superset-frontend/spec/fixtures/mockNativeFilters.ts @@ -16,7 +16,6 @@ * specific language governing permissions and limitations * under the License. */ -import { FilterType } from 'src/dashboard/components/nativeFilters/types'; import { NativeFiltersState } from 'src/dashboard/reducers/types'; export const nativeFilters: NativeFiltersState = { @@ -24,7 +23,7 @@ export const nativeFilters: NativeFiltersState = { 'NATIVE_FILTER-e7Q8zKixx': { id: 'NATIVE_FILTER-e7Q8zKixx', name: 'region', - filterType: FilterType.filter_select, + filterType: 'filter_select', targets: [ { datasetId: 2, @@ -49,7 +48,7 @@ export const nativeFilters: NativeFiltersState = { 'NATIVE_FILTER-x9QPw0so1': { id: 'NATIVE_FILTER-x9QPw0so1', name: 'country_code', - filterType: FilterType.filter_select, + filterType: 'filter_select', targets: [ { datasetId: 2, @@ -75,6 +74,9 @@ export const nativeFilters: NativeFiltersState = { filtersState: { 'NATIVE_FILTER-e7Q8zKixx': { id: 'NATIVE_FILTER-e7Q8zKixx', + currentState: { + value: ['East Asia & Pacific'], + }, extraFormData: { append_form_data: { filters: [ @@ -128,6 +130,9 @@ export const singleNativeFiltersState = { [NATIVE_FILTER_ID]: { id: NATIVE_FILTER_ID, extraFormData, + currentState: { + value: ['No, not an ethnic minority'], + }, }, }, }; diff --git a/superset-frontend/spec/javascripts/dashboard/components/nativeFilters/NativeFiltersModal_spec.tsx b/superset-frontend/spec/javascripts/dashboard/components/nativeFilters/NativeFiltersModal_spec.tsx index 8432288c22..69082c5558 100644 --- a/superset-frontend/spec/javascripts/dashboard/components/nativeFilters/NativeFiltersModal_spec.tsx +++ b/superset-frontend/spec/javascripts/dashboard/components/nativeFilters/NativeFiltersModal_spec.tsx @@ -40,10 +40,25 @@ Object.defineProperty(window, 'matchMedia', { })), }); +jest.mock('@superset-ui/core', () => ({ + // @ts-ignore + ...jest.requireActual('@superset-ui/core'), + getChartMetadataRegistry: () => ({ + items: { + filter_select: { + value: { + datasourceCount: 1, + behaviors: ['NATIVE_FILTER'], + }, + }, + }, + }), +})); + describe('FiltersConfigModal', () => { const mockedProps = { isOpen: true, - initialFilterId: 'DefaultFilterId', + initialFilterId: 'DefaultsID', createNewOnOpen: true, onCancel: jest.fn(), save: jest.fn(), diff --git a/superset-frontend/spec/javascripts/dashboard/fixtures/mockNativeFilters.js b/superset-frontend/spec/javascripts/dashboard/fixtures/mockNativeFilters.js index 0025faa3e2..8ed2bb7ad8 100644 --- a/superset-frontend/spec/javascripts/dashboard/fixtures/mockNativeFilters.js +++ b/superset-frontend/spec/javascripts/dashboard/fixtures/mockNativeFilters.js @@ -18,10 +18,10 @@ */ export const nativeFiltersInfo = { filters: { - DefaultID1: { - id: 'DefaultID1', + DefaultsID: { + id: 'DefaultsID', name: 'test', - type: 'text', + type: 'filter_select', targets: [ { datasetId: 0, diff --git a/superset-frontend/src/dashboard/components/FiltersBadge/selectors.ts b/superset-frontend/src/dashboard/components/FiltersBadge/selectors.ts index 29a8aae2cd..97bf3b699b 100644 --- a/superset-frontend/src/dashboard/components/FiltersBadge/selectors.ts +++ b/superset-frontend/src/dashboard/components/FiltersBadge/selectors.ts @@ -18,10 +18,9 @@ */ import { TIME_FILTER_MAP } from 'src/explore/constants'; import { getChartIdsInFilterScope } from 'src/dashboard/util/activeDashboardFilters'; -import { - NativeFiltersState, - NativeFilterState, -} from 'src/dashboard/reducers/types'; +import { NativeFiltersState } from 'src/dashboard/reducers/types'; +import { getTreeCheckedItems } from '../nativeFilters/FilterConfigModal/utils'; +import { Layout } from '../../types'; export enum IndicatorStatus { Unset = 'UNSET', @@ -130,7 +129,7 @@ const getRejectedColumns = (chart: any): Set => ); export type Indicator = { - column: string; + column?: string; name: string; value: string[]; status: IndicatorStatus; @@ -172,50 +171,52 @@ export const selectIndicatorsForChart = ( return indicators; }; -const selectNativeIndicatorValue = ( - filterState: NativeFilterState, -): string[] => { - const filters = filterState?.extraFormData?.append_form_data?.filters; - if (filters?.length) { - const filter = filters[0]; - if ('val' in filter) { - const val = filter.val as string | string[]; - if (Array.isArray(val)) { - return val; - } - return [val]; - } - } - return []; -}; - export const selectNativeIndicatorsForChart = ( nativeFilters: NativeFiltersState, chartId: number, charts: any, + dashboardLayout: Layout, ): Indicator[] => { const chart = charts[chartId]; const appliedColumns = getAppliedColumns(chart); const rejectedColumns = getRejectedColumns(chart); - const getStatus = (column: string, value: string[]): IndicatorStatus => { - if (rejectedColumns.has(column)) return IndicatorStatus.Incompatible; - if (appliedColumns.has(column) && value.length > 0) { + const getStatus = ( + value: string[], + isAffectedByScope: boolean, + column?: string, + ): IndicatorStatus => { + if (!column && isAffectedByScope) { + // Filter without datasource + return IndicatorStatus.Applied; + } + if (column && rejectedColumns.has(column)) + return IndicatorStatus.Incompatible; + if (column && appliedColumns.has(column) && value.length > 0) { return IndicatorStatus.Applied; } return IndicatorStatus.Unset; }; const indicators = Object.values(nativeFilters.filters).map(nativeFilter => { - const column = nativeFilter.targets[0].column.name; + const isAffectedByScope = getTreeCheckedItems( + nativeFilter.scope, + dashboardLayout, + ).some( + layoutItem => dashboardLayout[layoutItem]?.meta?.chartId === chartId, + ); + const column = nativeFilter.targets[0]?.column?.name; const filterState = nativeFilters.filtersState[nativeFilter.id]; - const value = selectNativeIndicatorValue(filterState); + let value = filterState?.currentState?.value ?? []; + if (!Array.isArray(value)) { + value = [value]; + } return { column, name: nativeFilter.name, path: [nativeFilter.id], - status: getStatus(column, value), + status: getStatus(value, isAffectedByScope, column), value, }; }); diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterValue.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterValue.tsx index d0e202481b..509f80bb33 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterValue.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterValue.tsx @@ -51,15 +51,19 @@ const FilterValue: React.FC = ({ const { id, targets, filterType } = filter; const cascadingFilters = useCascadingFilters(id); const filterState = useFilterState(id); - const [loading, setLoading] = useState(true); const [state, setState] = useState([]); const [error, setError] = useState(false); const [formData, setFormData] = useState>({}); const inputRef = useRef(null); const [target] = targets; - const { datasetId = 18, column } = target; + const { + datasetId, + column = {}, + }: Partial<{ datasetId: number; column: { name?: string } }> = target; const { name: groupby } = column; const currentValue = filterState.currentState?.value; + const hasDataSource = !!(datasetId && groupby); + const [loading, setLoading] = useState(hasDataSource); useEffect(() => { const newFormData = getFormData({ datasetId, @@ -71,6 +75,9 @@ const FilterValue: React.FC = ({ }); if (!areObjectsEqual(formData || {}, newFormData)) { setFormData(newFormData); + if (!hasDataSource) { + return; + } getChartDataRequest({ formData: newFormData, force: false, @@ -131,7 +138,8 @@ const FilterValue: React.FC = ({ height={20} width={220} formData={formData} - queriesData={state} + // For charts that don't have datasource we need workaround for empty placeholder + queriesData={hasDataSource ? state : [{ data: [null] }]} chartType={filterType} // @ts-ignore (update superset-ui) hooks={{ setExtraFormData }} diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterConfigModal/FilterConfigForm.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterConfigModal/FilterConfigForm.tsx index 44e0683b8d..b46d91b22f 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterConfigModal/FilterConfigForm.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterConfigModal/FilterConfigForm.tsx @@ -21,6 +21,8 @@ import { SuperChart, t, getChartControlPanelRegistry, + getChartMetadataRegistry, + Behavior, } from '@superset-ui/core'; import { FormInstance } from 'antd/lib/form'; import React, { useCallback } from 'react'; @@ -39,15 +41,10 @@ import { CustomControlItem } from '@superset-ui/chart-controls'; import { ColumnSelect } from './ColumnSelect'; import { NativeFiltersForm } from './types'; import FilterScope from './FilterScope'; -import { - FilterTypeNames, - getControlItems, - setFilterFieldValues, - useForceUpdate, -} from './utils'; +import { getControlItems, setFilterFieldValues, useForceUpdate } from './utils'; import { useBackendFormUpdate } from './state'; import { getFormData } from '../utils'; -import { Filter, FilterType } from '../types'; +import { Filter } from '../types'; type DatasetSelectValue = { value: number; @@ -121,9 +118,25 @@ export const FilterConfigForm: React.FC = ({ const controlItems = getControlItems( controlPanelRegistry.get(formFilter?.filterType), ); - useBackendFormUpdate(form, filterId, filterToEdit); - const initDatasetId = filterToEdit?.targets[0].datasetId; + const nativeFilterItems = getChartMetadataRegistry().items; + const nativeFilterVizTypes = Object.entries(nativeFilterItems) + // @ts-ignore + .filter(([, { value }]) => + value.behaviors?.includes(Behavior.NATIVE_FILTER), + ) + .map(([key]) => key); + + // @ts-ignore + const hasDatasource = !!nativeFilterItems[formFilter?.filterType]?.value + ?.datasourceCount; + + const hasFilledDatasource = + (formFilter?.dataset && formFilter?.column) || !hasDatasource; + + useBackendFormUpdate(form, filterId, filterToEdit, hasDatasource); + + const initDatasetId = filterToEdit?.targets[0]?.datasetId; const initColumn = filterToEdit?.targets[0]?.column?.name; const newFormData = getFormData({ datasetId: formFilter?.dataset?.value, @@ -179,69 +192,76 @@ export const FilterConfigForm: React.FC = ({ {t('Datasource')}} - rules={[{ required: !removed, message: t('Datasource is required') }]} - data-test="datasource-input" + name={['filters', filterId, 'filterType']} + rules={[{ required: !removed, message: t('Name is required') }]} + initialValue={filterToEdit?.filterType || 'filter_select'} + label={{t('Filter Type')}} > - { - // We need reset column when dataset changed - const datasetId = formFilter?.dataset?.value; - if (datasetId && e?.value !== datasetId) { - setFilterFieldValues(form, filterId, { - column: null, - }); - } + ({ - value: filterType, - label: FilterTypeNames[filterType], - }))} - onChange={({ value }: { value: FilterType }) => { - setFilterFieldValues(form, filterId, { - filterType: value, - defaultValue: null, - }); - forceUpdate(); - }} - /> - - {formFilter?.dataset && formFilter?.column && ( + {hasDatasource && ( + <> + {t('Datasource')}} + rules={[ + { required: !removed, message: t('Datasource is required') }, + ]} + data-test="datasource-input" + > + { + // We need reset column when dataset changed + const datasetId = formFilter?.dataset?.value; + if (datasetId && e?.value !== datasetId) { + setFilterFieldValues(form, filterId, { + column: null, + }); + } + forceUpdate(); + }} + /> + + {t('Field')}} + rules={[{ required: !removed, message: t('Field is required') }]} + data-test="field-input" + > + + + + )} + {hasFilledDatasource && (