From 7919830c1b2326d8bbedb0d06fb3d6828bd316c7 Mon Sep 17 00:00:00 2001 From: Cody Leff Date: Thu, 1 Dec 2022 15:58:03 -0700 Subject: [PATCH 1/6] Add new isNativeFilterWithDataMask type guard. --- .../superset-ui-core/src/query/types/Dashboard.ts | 11 ++++++++++- .../FilterBar/FilterControls/FilterControls.tsx | 6 +++--- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/superset-frontend/packages/superset-ui-core/src/query/types/Dashboard.ts b/superset-frontend/packages/superset-ui-core/src/query/types/Dashboard.ts index b7ac546993783..d93caec0dd06a 100644 --- a/superset-frontend/packages/superset-ui-core/src/query/types/Dashboard.ts +++ b/superset-frontend/packages/superset-ui-core/src/query/types/Dashboard.ts @@ -91,7 +91,7 @@ export type Filter = { description: string; }; -export type FilterWithDataMask = Filter & { dataMask?: DataMaskWithId }; +export type FilterWithDataMask = Filter & { dataMask: DataMaskWithId }; export type Divider = Partial> & { id: string; @@ -106,6 +106,15 @@ export function isNativeFilter( return filterElement.type === NativeFilterType.NATIVE_FILTER; } +export function isNativeFilterWithDataMask( + filterElement: Filter | Divider, +): filterElement is FilterWithDataMask { + return ( + isNativeFilter(filterElement) && + (filterElement as FilterWithDataMask).dataMask?.filterState?.value + ); +} + export function isFilterDivider( filterElement: Filter | Divider, ): filterElement is Divider { diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterControls.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterControls.tsx index 71b810685119b..e8a3a76b70a04 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterControls.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterControls.tsx @@ -25,9 +25,9 @@ import { css, SupersetTheme, t, - isNativeFilter, isFeatureEnabled, FeatureFlag, + isNativeFilterWithDataMask, } from '@superset-ui/core'; import { createHtmlPortalNode, @@ -140,8 +140,8 @@ const FilterControls: FC = ({ const activeOverflowedFiltersInScope = useMemo( () => - overflowedFiltersInScope.filter( - filter => isNativeFilter(filter) && filter.dataMask?.filterState?.value, + overflowedFiltersInScope.filter(filter => + isNativeFilterWithDataMask(filter), ).length, [overflowedFiltersInScope], ); From 70f1f305d5e31d12e4cb547039a52c5db283f7e1 Mon Sep 17 00:00:00 2001 From: Cody Leff Date: Thu, 1 Dec 2022 16:05:10 -0700 Subject: [PATCH 2/6] Use whole number for grid unit multiplier. --- .../nativeFilters/FilterBar/FilterControls/FilterDivider.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterDivider.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterDivider.tsx index cc5e9e70f8ca2..5ea38b134566d 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterDivider.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterDivider.tsx @@ -130,7 +130,7 @@ const HorizontalOverflowDivider = ({ display: block; font-size: ${theme.typography.sizes.s}px; color: ${theme.colors.grayscale.base}; - margin: ${theme.gridUnit * 2.5}px 0 0 0; + margin: ${theme.gridUnit * 2}px 0 0 0; line-height: 1; `} > From 8c3199cb1c3274b36e765fb32937b21817a7a243 Mon Sep 17 00:00:00 2001 From: Cody Leff Date: Thu, 1 Dec 2022 16:24:27 -0700 Subject: [PATCH 3/6] Add extra feature flag guard. --- superset-frontend/src/dashboard/actions/hydrate.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/superset-frontend/src/dashboard/actions/hydrate.js b/superset-frontend/src/dashboard/actions/hydrate.js index ad0b38f4e7964..917492bd7b209 100644 --- a/superset-frontend/src/dashboard/actions/hydrate.js +++ b/superset-frontend/src/dashboard/actions/hydrate.js @@ -430,7 +430,9 @@ export const hydrateDashboard = conf: common?.conf, }, filterBarOrientation: - metadata.filter_bar_orientation ?? FilterBarOrientation.VERTICAL, + (isFeatureEnabled(FeatureFlag.HORIZONTAL_FILTER_BAR) && + metadata.filter_bar_orientation) || + FilterBarOrientation.VERTICAL, }, dataMask, dashboardFilters, From 37f5e24967f4d8b15c23ff4e6eab3bb908e6843d Mon Sep 17 00:00:00 2001 From: Cody Leff Date: Thu, 1 Dec 2022 16:31:54 -0700 Subject: [PATCH 4/6] Remove now-unnecessary uses of isNativeFilterWithDataMask. --- .../nativeFilters/FilterBar/useFilterControlFactory.tsx | 3 +-- .../src/dashboard/components/nativeFilters/state.ts | 9 +++------ 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/useFilterControlFactory.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/useFilterControlFactory.tsx index 6893e629cb49c..45561ffd49fb0 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/useFilterControlFactory.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/useFilterControlFactory.tsx @@ -23,7 +23,6 @@ import { DataMaskStateWithId, Divider, Filter, - FilterWithDataMask, isFilterDivider, } from '@superset-ui/core'; import { FilterBarOrientation } from 'src/dashboard/types'; @@ -38,7 +37,7 @@ export const useFilterControlFactory = ( ) => { const filters = useFilters(); const filterValues = useMemo(() => Object.values(filters), [filters]); - const filtersWithValues: (FilterWithDataMask | Divider)[] = useMemo( + const filtersWithValues: (Filter | Divider)[] = useMemo( () => filterValues.map(filter => ({ ...filter, diff --git a/superset-frontend/src/dashboard/components/nativeFilters/state.ts b/superset-frontend/src/dashboard/components/nativeFilters/state.ts index 51d987a577904..030b65859b3ea 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/state.ts +++ b/superset-frontend/src/dashboard/components/nativeFilters/state.ts @@ -23,7 +23,6 @@ import { FilterConfiguration, Divider, isFilterDivider, - FilterWithDataMask, } from '@superset-ui/core'; import { ActiveTabs, DashboardLayout, RootState } from '../../types'; import { TAB_TYPE } from '../../util/componentTypes'; @@ -110,15 +109,13 @@ function useIsFilterInScope() { })); } -export function useSelectFiltersInScope( - filters: (FilterWithDataMask | Divider)[], -) { +export function useSelectFiltersInScope(filters: (Filter | Divider)[]) { const dashboardHasTabs = useDashboardHasTabs(); const isFilterInScope = useIsFilterInScope(); return useMemo(() => { - let filtersInScope: (FilterWithDataMask | Divider)[] = []; - const filtersOutOfScope: (FilterWithDataMask | Divider)[] = []; + let filtersInScope: (Filter | Divider)[] = []; + const filtersOutOfScope: (Filter | Divider)[] = []; // we check native filters scopes only on dashboards with tabs if (!dashboardHasTabs) { From 1a4daa13fd624ed2f07d2292937f774b981fd9b6 Mon Sep 17 00:00:00 2001 From: Cody Leff Date: Fri, 2 Dec 2022 08:08:23 -0700 Subject: [PATCH 5/6] Update dashboard type tests, trigger CI. --- .../test/query/types/Dashboard.test.ts | 65 +++++++++++++------ 1 file changed, 44 insertions(+), 21 deletions(-) diff --git a/superset-frontend/packages/superset-ui-core/test/query/types/Dashboard.test.ts b/superset-frontend/packages/superset-ui-core/test/query/types/Dashboard.test.ts index ea6236338c765..2c9cde43a59a4 100644 --- a/superset-frontend/packages/superset-ui-core/test/query/types/Dashboard.test.ts +++ b/superset-frontend/packages/superset-ui-core/test/query/types/Dashboard.test.ts @@ -21,27 +21,50 @@ import { isFilterDivider, Filter, NativeFilterType, + FilterWithDataMask, + Divider, + isNativeFilterWithDataMask, } from '@superset-ui/core'; -test('should do native filter type guard', () => { - const dummyFilter: Filter = { - cascadeParentIds: [], - defaultDataMask: {}, - id: 'dummyID', - name: 'dummyName', - scope: { rootPath: [], excluded: [] }, - filterType: 'dummyType', - targets: [{}], - controlValues: {}, - type: NativeFilterType.NATIVE_FILTER, - description: 'dummyDesc', - }; - expect(isNativeFilter(dummyFilter)).toBeTruthy(); - expect( - isFilterDivider({ - ...dummyFilter, - type: NativeFilterType.DIVIDER, - title: 'dummyTitle', - }), - ).toBeTruthy(); +const filter: Filter = { + cascadeParentIds: [], + defaultDataMask: {}, + id: 'filter_id', + name: 'Filter Name', + scope: { rootPath: [], excluded: [] }, + filterType: 'filter_type', + targets: [{}], + controlValues: {}, + type: NativeFilterType.NATIVE_FILTER, + description: 'Filter description.', +}; + +const filterWithDataMask: FilterWithDataMask = { + ...filter, + dataMask: { id: 'data_mask_id', filterState: { value: 'Filter value' } }, +}; + +const filterDivider: Divider = { + id: 'divider_id', + type: NativeFilterType.DIVIDER, + title: 'Divider title', + description: 'Divider description.', +}; + +test('filter type guard', () => { + expect(isNativeFilter(filter)).toBeTruthy(); + expect(isNativeFilter(filterWithDataMask)).toBeTruthy(); + expect(isNativeFilter(filterDivider)).toBeFalsy(); +}); + +test('filter with dataMask type guard', () => { + expect(isNativeFilterWithDataMask(filter)).toBeFalsy(); + expect(isNativeFilterWithDataMask(filterWithDataMask)).toBeTruthy(); + expect(isNativeFilterWithDataMask(filterDivider)).toBeFalsy(); +}); + +test('filter divider with dataMask type guard', () => { + expect(isFilterDivider(filter)).toBeFalsy(); + expect(isFilterDivider(filterWithDataMask)).toBeFalsy(); + expect(isFilterDivider(filterDivider)).toBeTruthy(); }); From e46208c6e469a6a7571e729d0afcae382f3e2e0c Mon Sep 17 00:00:00 2001 From: Cody Leff Date: Fri, 2 Dec 2022 10:55:22 -0700 Subject: [PATCH 6/6] Fix mislabeled test. --- .../superset-ui-core/test/query/types/Dashboard.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/superset-frontend/packages/superset-ui-core/test/query/types/Dashboard.test.ts b/superset-frontend/packages/superset-ui-core/test/query/types/Dashboard.test.ts index 2c9cde43a59a4..f72bff490f116 100644 --- a/superset-frontend/packages/superset-ui-core/test/query/types/Dashboard.test.ts +++ b/superset-frontend/packages/superset-ui-core/test/query/types/Dashboard.test.ts @@ -63,7 +63,7 @@ test('filter with dataMask type guard', () => { expect(isNativeFilterWithDataMask(filterDivider)).toBeFalsy(); }); -test('filter divider with dataMask type guard', () => { +test('filter divider type guard', () => { expect(isFilterDivider(filter)).toBeFalsy(); expect(isFilterDivider(filterWithDataMask)).toBeFalsy(); expect(isFilterDivider(filterDivider)).toBeTruthy();