feat(dashboard): Add divider component in native filters (#17410)

* Tests are working, type errors are fixed * Fix filterset * add license header to the new file * fix test * PR comments * Linting * test fix * small fix
apache · Nov 24, 2021 · 9576478 · 9576478
1 parent c216565
commit 9576478
Show file tree

Hide file tree

Showing 19 changed files with 472 additions and 200 deletions.
diff --git a/...-frontend/spec/javascripts/dashboard/components/nativeFilters/NativeFiltersModal_spec.tsx b/...-frontend/spec/javascripts/dashboard/components/nativeFilters/NativeFiltersModal_spec.tsx
@@ -25,6 +25,7 @@ import { Provider } from 'react-redux';
 import { mockStore } from 'spec/fixtures/mockStore';
 import { styledMount as mount } from 'spec/helpers/theming';
 import waitForComponentToPaint from 'spec/helpers/waitForComponentToPaint';
+import { Dropdown, Menu } from 'src/common/components';
 import Alert from 'src/components/Alert';
 import { FiltersConfigModal } from 'src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigModal';
 
@@ -60,7 +61,7 @@ jest.mock('@superset-ui/core', () => ({
 describe('FiltersConfigModal', () => {
   const mockedProps = {
     isOpen: true,
-    initialFilterId: 'DefaultsID',
+    initialFilterId: 'NATIVE_FILTER-1',
     createNewOnOpen: true,
     onCancel: jest.fn(),
     onSave: jest.fn(),
@@ -112,9 +113,13 @@ describe('FiltersConfigModal', () => {
       await waitForComponentToPaint(wrapper);
     }
 
-    function addFilter() {
+    async function addFilter() {
       act(() => {
-        wrapper.find('[aria-label="Add filter"]').at(0).simulate('click');
+        wrapper.find(Dropdown).at(0).simulate('mouseEnter');
+      });
+      await waitForComponentToPaint(wrapper, 300);
+      act(() => {
+        wrapper.find(Menu.Item).at(0).simulate('click');
       });
     }
 
@@ -124,7 +129,7 @@ describe('FiltersConfigModal', () => {
     });
 
     it('shows correct alert message for unsaved filters', async () => {
-      addFilter();
+      await addFilter();
       await clickCancel();
       expect(onCancel.mock.calls).toHaveLength(0);
       expect(wrapper.find(Alert).text()).toContain(

diff --git a/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardContainer.tsx b/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardContainer.tsx
@@ -36,6 +36,7 @@ import { getChartIdsInFilterScope } from '../../util/activeDashboardFilters';
 import findTabIndexByComponentId from '../../util/findTabIndexByComponentId';
 import { findTabsWithChartsInScope } from '../nativeFilters/utils';
 import { setInScopeStatusOfFilters } from '../../actions/nativeFilters';
+import { NATIVE_FILTER_DIVIDER_PREFIX } from '../nativeFilters/FiltersConfigModal/utils';
 
 type DashboardContainerProps = {
   topLevelTabs?: LayoutItem;
@@ -71,6 +72,7 @@ const DashboardContainer: FC<DashboardContainerProps> = ({ topLevelTabs }) => {
   const filterScopes = Object.values(nativeFilters ?? {}).map(filter => ({
     id: filter.id,
     scope: filter.scope,
+    type: filter.type,
   }));
   useEffect(() => {
     if (
@@ -80,6 +82,13 @@ const DashboardContainer: FC<DashboardContainerProps> = ({ topLevelTabs }) => {
       return;
     }
     const scopes = filterScopes.map(filterScope => {
+      if (filterScope.id.startsWith(NATIVE_FILTER_DIVIDER_PREFIX)) {
+        return {
+          filterId: filterScope.id,
+          tabsInScope: [],
+          chartsInScope: [],
+        };
+      }
       const { scope } = filterScope;
       const chartsInScope: number[] = getChartIdsInFilterScope({
         filterScope: {

diff --git a/superset-frontend/src/dashboard/components/FiltersBadge/selectors.ts b/superset-frontend/src/dashboard/components/FiltersBadge/selectors.ts
@@ -29,6 +29,7 @@ import { DataMaskStateWithId, DataMaskType } from 'src/dataMask/types';
 import { areObjectsEqual } from 'src/reduxUtils';
 import { Layout } from '../../types';
 import { getTreeCheckedItems } from '../nativeFilters/FiltersConfigModal/FiltersConfigForm/FilterScope/utils';
+import { NativeFilterType } from '../nativeFilters/types';
 
 export enum IndicatorStatus {
   Unset = 'UNSET',
@@ -268,11 +269,13 @@ export const selectNativeIndicatorsForChart = (
     nativeFilterIndicators =
       nativeFilters &&
       Object.values(nativeFilters)
-        .filter(nativeFilter =>
-          getTreeCheckedItems(nativeFilter.scope, dashboardLayout).some(
-            layoutItem =>
-              dashboardLayout[layoutItem]?.meta?.chartId === chartId,
-          ),
+        .filter(
+          nativeFilter =>
+            nativeFilter.type === NativeFilterType.NATIVE_FILTER &&
+            getTreeCheckedItems(nativeFilter.scope, dashboardLayout).some(
+              layoutItem =>
+                dashboardLayout[layoutItem]?.meta?.chartId === chartId,
+            ),
         )
         .map(nativeFilter => {
           const column = nativeFilter.targets[0]?.column?.name;

diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterBar.test.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterBar.test.tsx
@@ -88,12 +88,12 @@ const addFilterFlow = async () => {
   userEvent.click(screen.getByText('Time range'));
   userEvent.type(screen.getByTestId(getModalTestId('name-input')), FILTER_NAME);
   userEvent.click(screen.getByText('Save'));
-  await screen.findByText('All Filters (1)');
+  await screen.findByText('All filters (1)');
 };
 
 const addFilterSetFlow = async () => {
   // add filter set
-  userEvent.click(screen.getByText('Filter Sets (0)'));
+  userEvent.click(screen.getByText('Filter sets (0)'));
 
   // check description
   expect(screen.getByText('Filters (1)')).toBeInTheDocument();
@@ -301,6 +301,40 @@ describe('FilterBar', () => {
     expect(screen.getByTestId(getTestId('apply-button'))).toBeDisabled();
   });
 
+  it('renders dividers', async () => {
+    const divider = {
+      id: 'NATIVE_FILTER_DIVIDER-1',
+      type: 'DIVIDER',
+      scope: {
+        rootPath: ['ROOT_ID'],
+        excluded: [],
+      },
+      title: 'Select time range',
+      description: 'Select year/month etc..',
+      chartsInScope: [],
+      tabsInScope: [],
+    };
+    const stateWithDivider = {
+      ...stateWithoutNativeFilters,
+      nativeFilters: {
+        filters: {
+          'NATIVE_FILTER_DIVIDER-1': divider,
+        },
+      },
+    };
+
+    renderWrapper(openedBarProps, stateWithDivider);
+
+    const title = await screen.findByText('Select time range');
+    const description = await screen.findByText('Select year/month etc..');
+
+    expect(title.tagName).toBe('H3');
+    expect(description.tagName).toBe('P');
+    // Do not enable buttons if there are not filters
+    expect(screen.getByTestId(getTestId('clear-button'))).toBeDisabled();
+    expect(screen.getByTestId(getTestId('apply-button'))).toBeDisabled();
+  });
+
   it('create filter and apply it flow', async () => {
     // @ts-ignore
     global.featureFlags = {
@@ -332,7 +366,7 @@ describe('FilterBar', () => {
 
     // change filter
     expect(screen.getByTestId(getTestId('apply-button'))).toBeDisabled();
-    userEvent.click(await screen.findByText('All Filters (1)'));
+    userEvent.click(await screen.findByText('All filters (1)'));
     await changeFilterValue();
     await waitFor(() => expect(screen.getAllByText('Last day').length).toBe(2));
 

diff --git a/...ontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterControls.tsx b/...ontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterControls.tsx
@@ -30,7 +30,10 @@ import {
   useDashboardHasTabs,
   useSelectFiltersInScope,
 } from 'src/dashboard/components/nativeFilters/state';
-import { Filter } from 'src/dashboard/components/nativeFilters/types';
+import {
+  Filter,
+  NativeFilterType,
+} from 'src/dashboard/components/nativeFilters/types';
 import CascadePopover from '../CascadeFilters/CascadePopover';
 import { useFilters } from '../state';
 import { buildCascadeFiltersTree } from './utils';
@@ -79,21 +82,32 @@ const FilterControls: FC<FilterControlsProps> = ({
   const showCollapsePanel = dashboardHasTabs && cascadeFilters.length > 0;
 
   const cascadePopoverFactory = useCallback(
-    index => (
-      <CascadePopover
-        data-test="cascade-filters-control"
-        key={cascadeFilters[index].id}
-        dataMaskSelected={dataMaskSelected}
-        visible={visiblePopoverId === cascadeFilters[index].id}
-        onVisibleChange={visible =>
-          setVisiblePopoverId(visible ? cascadeFilters[index].id : null)
-        }
-        filter={cascadeFilters[index]}
-        onFilterSelectionChange={onFilterSelectionChange}
-        directPathToChild={directPathToChild}
-        inView={false}
-      />
-    ),
+    index => {
+      const filter = cascadeFilters[index];
+      if (filter.type === NativeFilterType.DIVIDER) {
+        return (
+          <div>
+            <h3>{filter.title}</h3>
+            <p>{filter.description}</p>
+          </div>
+        );
+      }
+      return (
+        <CascadePopover
+          data-test="cascade-filters-control"
+          key={filter.id}
+          dataMaskSelected={dataMaskSelected}
+          visible={visiblePopoverId === filter.id}
+          onVisibleChange={visible =>
+            setVisiblePopoverId(visible ? filter.id : null)
+          }
+          filter={filter}
+          onFilterSelectionChange={onFilterSelectionChange}
+          directPathToChild={directPathToChild}
+          inView={false}
+        />
+      );
+    },
     [
       cascadeFilters,
       JSON.stringify(dataMaskSelected),

diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/utils.ts b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/utils.ts
@@ -22,12 +22,14 @@ import {
   setFocusedNativeFilter,
   unsetFocusedNativeFilter,
 } from 'src/dashboard/actions/nativeFilters';
-import { Filter } from '../../types';
+import { Filter, NativeFilterType, Divider } from '../../types';
 import { CascadeFilter } from '../CascadeFilters/types';
 import { mapParentFiltersToChildren } from '../utils';
 
 // eslint-disable-next-line import/prefer-default-export
-export function buildCascadeFiltersTree(filters: Filter[]): CascadeFilter[] {
+export function buildCascadeFiltersTree(
+  filters: Array<Divider | Filter>,
+): Array<CascadeFilter | Divider> {
   const cascadeChildren = mapParentFiltersToChildren(filters);
 
   const getCascadeFilter = (filter: Filter): CascadeFilter => {
@@ -39,7 +41,11 @@ export function buildCascadeFiltersTree(filters: Filter[]): CascadeFilter[] {
   };
 
   return filters
-    .filter(filter => !filter.cascadeParentIds?.length)
+    .filter(
+      filter =>
+        filter.type === NativeFilterType.DIVIDER ||
+        !(filter as Filter).cascadeParentIds?.length,
+    )
     .map(getCascadeFilter);
 }
 

diff --git a/...et-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterSets/FiltersHeader.tsx b/...et-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterSets/FiltersHeader.tsx
@@ -26,6 +26,7 @@ import { FilterSet } from 'src/dashboard/reducers/types';
 import { getFilterValueForDisplay } from './utils';
 import { useFilters } from '../state';
 import { getFilterBarTestId } from '../index';
+import { NativeFilterType } from '../../types';
 
 const FilterHeader = styled.div`
   display: flex;
@@ -68,7 +69,9 @@ export type FiltersHeaderProps = {
 const FiltersHeader: FC<FiltersHeaderProps> = ({ dataMask, filterSet }) => {
   const theme = useTheme();
   const filters = useFilters();
-  const filterValues = Object.values(filters);
+  const filterValues = Object.values(filters).filter(
+    nativeFilter => nativeFilter.type === NativeFilterType.NATIVE_FILTER,
+  );
 
   let resultFilters = filterValues ?? [];
   if (filterSet?.nativeFilters) {

diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/index.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/index.tsx
@@ -33,7 +33,10 @@ import { DataMaskStateWithId, DataMaskWithId } from 'src/dataMask/types';
 import { useImmer } from 'use-immer';
 import { isEmpty, isEqual } from 'lodash';
 import { testWithId } from 'src/utils/testUtils';
-import { Filter } from 'src/dashboard/components/nativeFilters/types';
+import {
+  Filter,
+  NativeFilterType,
+} from 'src/dashboard/components/nativeFilters/types';
 import Loading from 'src/components/Loading';
 import { getInitialDataMask } from 'src/dataMask/reducer';
 import { URL_PARAMS } from 'src/constants';
@@ -82,7 +85,6 @@ const Bar = styled.div<{ width: number }>`
   border-bottom: 1px solid ${({ theme }) => theme.colors.grayscale.light2};
   min-height: 100%;
   display: none;
-
   &.open {
     display: flex;
   }
@@ -97,14 +99,12 @@ const CollapsedBar = styled.div<{ offset: number }>`
   padding-top: ${({ theme }) => theme.gridUnit * 2}px;
   display: none;
   text-align: center;
-
   &.open {
     display: flex;
     flex-direction: column;
     align-items: center;
     padding: ${({ theme }) => theme.gridUnit * 2}px;
   }
-
   svg {
     cursor: pointer;
   }
@@ -278,9 +278,12 @@ const FilterBar: React.FC<FiltersBarProps> = ({
     filterValues,
   );
   const isInitialized = useInitialization();
-
   const tabPaneStyle = useMemo(() => ({ overflow: 'auto', height }), [height]);
 
+  const numberOfFilters = filterValues.filter(
+    filterValue => filterValue.type === NativeFilterType.NATIVE_FILTER,
+  ).length;
+
   return (
     <BarWrapper
       {...getFilterBarTestId()}
@@ -320,8 +323,8 @@ const FilterBar: React.FC<FiltersBarProps> = ({
             activeKey={editFilterSetId ? TabIds.AllFilters : undefined}
           >
             <Tabs.TabPane
-              tab={t('All Filters (%(filterCount)d)', {
-                filterCount: filterValues.length,
+              tab={t('All filters (%(filterCount)d)', {
+                filterCount: numberOfFilters,
               })}
               key={TabIds.AllFilters}
               css={tabPaneStyle}
@@ -342,7 +345,7 @@ const FilterBar: React.FC<FiltersBarProps> = ({
             </Tabs.TabPane>
             <Tabs.TabPane
               disabled={!!editFilterSetId}
-              tab={t('Filter Sets (%(filterSetCount)d)', {
+              tab={t('Filter sets (%(filterSetCount)d)', {
                 filterSetCount: filterSetFilterValues.length,
               })}
               key={TabIds.FilterSets}

diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/utils.ts b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/utils.ts
@@ -20,19 +20,20 @@
 import { DataMaskStateWithId } from 'src/dataMask/types';
 import { areObjectsEqual } from 'src/reduxUtils';
 import { FilterState } from '@superset-ui/core';
-import { Filter } from '../types';
+import { Filter, Divider } from '../types';
 
 export enum TabIds {
   AllFilters = 'allFilters',
   FilterSets = 'filterSets',
 }
 
-export function mapParentFiltersToChildren(filters: Filter[]): {
+export function mapParentFiltersToChildren(filters: Array<Filter | Divider>): {
   [id: string]: Filter[];
 } {
   const cascadeChildren = {};
   filters.forEach(filter => {
-    const [parentId] = filter.cascadeParentIds || [];
+    const [parentId] =
+      ('cascadeParentIds' in filter && filter.cascadeParentIds) || [];
     if (parentId) {
       if (!cascadeChildren[parentId]) {
         cascadeChildren[parentId] = [];