feat(native-filters): apply cascading without instant filtering (apac…

…he#14966)
CybercentreCanada · Dec 17, 2021 · 8b53752 · 8b53752
1 parent ac3a423
commit 8b53752
Show file tree

Hide file tree

Showing 8 changed files with 35 additions and 36 deletions.
diff --git a/...ashboard/components/nativeFilters/FilterBar/CascadeFilters/CascadeFilterControl/index.tsx b/...ashboard/components/nativeFilters/FilterBar/CascadeFilters/CascadeFilterControl/index.tsx
@@ -22,8 +22,10 @@ import Icon from 'src/components/Icon';
 import FilterControl from 'src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterControl';
 import { CascadeFilter } from 'src/dashboard/components/nativeFilters/FilterBar/CascadeFilters/types';
 import { Filter } from 'src/dashboard/components/nativeFilters/types';
+import { DataMaskStateWithId } from 'src/dataMask/types';
 
 export interface CascadeFilterControlProps {
+  dataMaskSelected?: DataMaskStateWithId;
   filter: CascadeFilter;
   directPathToChild?: string[];
   onFilterSelectionChange: (filter: Filter, dataMask: DataMask) => void;
@@ -45,6 +47,7 @@ const StyledCaretIcon = styled(Icon)`
 `;
 
 const CascadeFilterControl: React.FC<CascadeFilterControlProps> = ({
+  dataMaskSelected,
   filter,
   directPathToChild,
   onFilterSelectionChange,
@@ -53,6 +56,7 @@ const CascadeFilterControl: React.FC<CascadeFilterControlProps> = ({
     <StyledFilterControlBox>
       <StyledCaretIcon name="caret-down" />
       <FilterControl
+        dataMaskSelected={dataMaskSelected}
         filter={filter}
         directPathToChild={directPathToChild}
         onFilterSelectionChange={onFilterSelectionChange}
@@ -63,6 +67,7 @@ const CascadeFilterControl: React.FC<CascadeFilterControlProps> = ({
       {filter.cascadeChildren?.map(childFilter => (
         <li key={childFilter.id}>
           <CascadeFilterControl
+            dataMaskSelected={dataMaskSelected}
             filter={childFilter}
             directPathToChild={directPathToChild}
             onFilterSelectionChange={onFilterSelectionChange}

diff --git a/.../src/dashboard/components/nativeFilters/FilterBar/CascadeFilters/CascadePopover/index.tsx b/.../src/dashboard/components/nativeFilters/FilterBar/CascadeFilters/CascadePopover/index.tsx
@@ -21,16 +21,14 @@ import { styled, t, DataMask } from '@superset-ui/core';
 import Popover from 'src/components/Popover';
 import Icon from 'src/components/Icon';
 import { Pill } from 'src/dashboard/components/FiltersBadge/Styles';
-import { useSelector } from 'react-redux';
-import { getInitialDataMask } from 'src/dataMask/reducer';
-import { DataMaskWithId } from 'src/dataMask/types';
+import { DataMaskStateWithId } from 'src/dataMask/types';
 import FilterControl from 'src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterControl';
 import CascadeFilterControl from 'src/dashboard/components/nativeFilters/FilterBar/CascadeFilters/CascadeFilterControl';
 import { CascadeFilter } from 'src/dashboard/components/nativeFilters/FilterBar/CascadeFilters/types';
 import { Filter } from 'src/dashboard/components/nativeFilters/types';
-import { RootState } from 'src/dashboard/types';
 
 interface CascadePopoverProps {
+  dataMaskSelected: DataMaskStateWithId;
   filter: CascadeFilter;
   visible: boolean;
   directPathToChild?: string[];
@@ -76,16 +74,15 @@ const StyledPill = styled(Pill)`
 `;
 
 const CascadePopover: React.FC<CascadePopoverProps> = ({
+  dataMaskSelected,
   filter,
   visible,
   onVisibleChange,
   onFilterSelectionChange,
   directPathToChild,
 }) => {
   const [currentPathToChild, setCurrentPathToChild] = useState<string[]>();
-  const dataMask = useSelector<RootState, DataMaskWithId>(
-    state => state.dataMask[filter.id] ?? getInitialDataMask(filter.id),
-  );
+  const dataMask = dataMaskSelected[filter.id];
 
   useEffect(() => {
     setCurrentPathToChild(directPathToChild);
@@ -98,7 +95,7 @@ const CascadePopover: React.FC<CascadePopoverProps> = ({
   const getActiveChildren = useCallback(
     (filter: CascadeFilter): CascadeFilter[] | null => {
       const children = filter.cascadeChildren || [];
-      const currentValue = dataMask.filterState?.value;
+      const currentValue = dataMask?.filterState?.value;
 
       const activeChildren = children.flatMap(
         childFilter => getActiveChildren(childFilter) || [],
@@ -147,6 +144,7 @@ const CascadePopover: React.FC<CascadePopoverProps> = ({
   if (!filter.cascadeChildren?.length) {
     return (
       <FilterControl
+        dataMaskSelected={dataMaskSelected}
         filter={filter}
         directPathToChild={directPathToChild}
         onFilterSelectionChange={onFilterSelectionChange}
@@ -166,6 +164,7 @@ const CascadePopover: React.FC<CascadePopoverProps> = ({
 
   const content = (
     <CascadeFilterControl
+      dataMaskSelected={dataMaskSelected}
       data-test="cascade-filters-control"
       key={filter.id}
       filter={filter}
@@ -188,6 +187,7 @@ const CascadePopover: React.FC<CascadePopoverProps> = ({
       <div>
         {activeFilters.map(activeFilter => (
           <FilterControl
+            dataMaskSelected={dataMaskSelected}
             key={activeFilter.id}
             filter={activeFilter}
             onFilterSelectionChange={onFilterSelectionChange}

diff --git a/...rontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterControl.tsx b/...rontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterControl.tsx
@@ -42,6 +42,7 @@ const StyledFilterControlContainer = styled.div`
 `;
 
 const FilterControl: React.FC<FilterProps> = ({
+  dataMaskSelected,
   filter,
   icon,
   onFilterSelectionChange,
@@ -57,6 +58,7 @@ const FilterControl: React.FC<FilterProps> = ({
         <div data-test="filter-icon">{icon}</div>
       </StyledFilterControlTitleBox>
       <FilterValue
+        dataMaskSelected={dataMaskSelected}
         filter={filter}
         directPathToChild={directPathToChild}
         onFilterSelectionChange={onFilterSelectionChange}

diff --git a/...ontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterControls.tsx b/...ontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterControls.tsx
@@ -21,7 +21,7 @@ import { DataMask, styled, t } from '@superset-ui/core';
 import { css } from '@emotion/react';
 import { useSelector } from 'react-redux';
 import * as portals from 'react-reverse-portal';
-import { DataMaskState } from 'src/dataMask/types';
+import { DataMaskStateWithId } from 'src/dataMask/types';
 import { Collapse } from 'src/common/components';
 import { TAB_TYPE } from 'src/dashboard/util/componentTypes';
 import { RootState } from 'src/dashboard/types';
@@ -41,7 +41,7 @@ const Wrapper = styled.div`
 
 type FilterControlsProps = {
   directPathToChild?: string[];
-  dataMaskSelected: DataMaskState;
+  dataMaskSelected: DataMaskStateWithId;
   onFilterSelectionChange: (filter: Filter, dataMask: DataMask) => void;
 };
 
@@ -101,6 +101,7 @@ const FilterControls: FC<FilterControlsProps> = ({
             <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)

diff --git a/...-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterValue.tsx b/...-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterValue.tsx
@@ -53,13 +53,14 @@ const FilterItem = styled.div`
 `;
 
 const FilterValue: React.FC<FilterProps> = ({
+  dataMaskSelected,
   filter,
   directPathToChild,
   onFilterSelectionChange,
 }) => {
   const { id, targets, filterType, adhoc_filters, time_range } = filter;
   const metadata = getChartMetadataRegistry().get(filterType);
-  const cascadingFilters = useCascadingFilters(id);
+  const cascadingFilters = useCascadingFilters(id, dataMaskSelected);
   const [state, setState] = useState<ChartDataResponseResult[]>([]);
   const [error, setError] = useState<string>('');
   const [formData, setFormData] = useState<Partial<QueryFormData>>({});

diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/state.ts b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/state.ts
@@ -18,22 +18,27 @@
  */
 import { useSelector } from 'react-redux';
 import { NativeFiltersState } from 'src/dashboard/reducers/types';
+import { DataMaskStateWithId } from 'src/dataMask/types';
+import { ExtraFormData } from '@superset-ui/core';
 import { mergeExtraFormData } from '../../utils';
-import { useNativeFiltersDataMask } from '../state';
 
 // eslint-disable-next-line import/prefer-default-export
-export function useCascadingFilters(id: string) {
+export function useCascadingFilters(
+  id: string,
+  dataMaskSelected?: DataMaskStateWithId,
+): ExtraFormData {
   const { filters } = useSelector<any, NativeFiltersState>(
     state => state.nativeFilters,
   );
   const filter = filters[id];
   const cascadeParentIds: string[] = filter?.cascadeParentIds ?? [];
   let cascadedFilters = {};
-  const nativeFiltersDataMask = useNativeFiltersDataMask();
   cascadeParentIds.forEach(parentId => {
-    const parentState = nativeFiltersDataMask[parentId] || {};
-    const { extraFormData: parentExtra = {} } = parentState;
-    cascadedFilters = mergeExtraFormData(cascadedFilters, parentExtra);
+    const parentState = dataMaskSelected?.[parentId];
+    cascadedFilters = mergeExtraFormData(
+      cascadedFilters,
+      parentState?.extraFormData,
+    );
   });
   return cascadedFilters;
 }
diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/types.ts b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/types.ts
@@ -18,9 +18,11 @@
  */
 import React from 'react';
 import { DataMask } from '@superset-ui/core';
+import { DataMaskStateWithId } from 'src/dataMask/types';
 import { Filter } from '../../types';
 
 export interface FilterProps {
+  dataMaskSelected?: DataMaskStateWithId;
   filter: Filter & {
     dataMask?: DataMask;
   };

diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/index.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/index.tsx
@@ -19,7 +19,7 @@
 
 /* eslint-disable no-param-reassign */
 import { HandlerFunction, styled, t } from '@superset-ui/core';
-import React, { useEffect, useMemo, useState } from 'react';
+import React, { useEffect, useState } from 'react';
 import { useDispatch } from 'react-redux';
 import cx from 'classnames';
 import Icon from 'src/components/Icon';
@@ -37,11 +37,7 @@ import { testWithId } from 'src/utils/testUtils';
 import { Filter } from 'src/dashboard/components/nativeFilters/types';
 import Loading from 'src/components/Loading';
 import { getInitialDataMask } from 'src/dataMask/reducer';
-import {
-  getOnlyExtraFormData,
-  mapParentFiltersToChildren,
-  TabIds,
-} from './utils';
+import { getOnlyExtraFormData, TabIds } from './utils';
 import FilterSets from './FilterSets';
 import {
   useNativeFiltersDataMask,
@@ -175,10 +171,6 @@ const FilterBar: React.FC<FiltersBarProps> = ({
   const filterValues = Object.values<Filter>(filters);
   const dataMaskApplied: DataMaskStateWithId = useNativeFiltersDataMask();
   const [isFilterSetChanged, setIsFilterSetChanged] = useState(false);
-  const cascadeChildren = useMemo(
-    () => mapParentFiltersToChildren(filterValues),
-    [filterValues],
-  );
 
   useEffect(() => {
     setDataMaskSelected(() => dataMaskApplied);
@@ -190,15 +182,6 @@ const FilterBar: React.FC<FiltersBarProps> = ({
   ) => {
     setIsFilterSetChanged(tab !== TabIds.AllFilters);
     setDataMaskSelected(draft => {
-      const children = cascadeChildren[filter.id] || [];
-      // force instant updating on initialization or for parent filters when dataMaskSelected has filter
-      if (
-        dataMaskSelected[filter.id] &&
-        (filter.isInstant || children.length > 0)
-      ) {
-        dispatch(updateDataMask(filter.id, dataMask));
-      }
-
       draft[filter.id] = {
         ...(getInitialDataMask(filter.id) as DataMaskWithId),
         ...dataMask,