perf: Optimize native filters and cross filters (#31243)

apache · Dec 2, 2024 · ce0e06a · ce0e06a
1 parent 5006f97
commit ce0e06a
Show file tree

Hide file tree

Showing 23 changed files with 376 additions and 432 deletions.
diff --git a/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardContainer.tsx b/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardContainer.tsx
@@ -60,6 +60,7 @@ import {
   ensureSyncedSharedLabelsColors,
   ensureSyncedLabelsColorMap,
 } from 'src/dashboard/actions/dashboardState';
+import { CHART_TYPE } from 'src/dashboard/util/componentTypes';
 import { getColorNamespace, resetColors } from 'src/utils/colorScheme';
 import { NATIVE_FILTER_DIVIDER_PREFIX } from '../nativeFilters/FiltersConfigModal/utils';
 import { findTabsWithChartsInScope } from '../nativeFilters/utils';
@@ -160,14 +161,18 @@ const DashboardContainer: FC<DashboardContainerProps> = ({ topLevelTabs }) => {
         };
       }
 
+      const chartLayoutItems = Object.values(dashboardLayout).filter(
+        item => item?.type === CHART_TYPE,
+      );
+
       const chartsInScope: number[] = getChartIdsInFilterScope(
         filterScope.scope,
         chartIds,
-        dashboardLayout,
+        chartLayoutItems,
       );
 
       const tabsInScope = findTabsWithChartsInScope(
-        dashboardLayout,
+        chartLayoutItems,
         chartsInScope,
       );
       return {

diff --git a/superset-frontend/src/dashboard/components/FiltersBadge/index.tsx b/superset-frontend/src/dashboard/components/FiltersBadge/index.tsx
@@ -39,6 +39,7 @@ import {
 } from '@superset-ui/core';
 import Icons from 'src/components/Icons';
 import { setDirectPathToChild } from 'src/dashboard/actions/dashboardState';
+import { useChartLayoutItems } from 'src/dashboard/util/useChartLayoutItems';
 import Badge from 'src/components/Badge';
 import DetailsPanelPopover from './DetailsPanel';
 import {
@@ -47,7 +48,7 @@ import {
   selectIndicatorsForChart,
   selectNativeIndicatorsForChart,
 } from '../nativeFilters/selectors';
-import { Chart, DashboardLayout, RootState } from '../../types';
+import { Chart, RootState } from '../../types';
 
 export interface FiltersBadgeProps {
   chartId: number;
@@ -126,9 +127,7 @@ export const FiltersBadge = ({ chartId }: FiltersBadgeProps) => {
     state => state.dashboardInfo.metadata?.chart_configuration,
   );
   const chart = useSelector<RootState, Chart>(state => state.charts[chartId]);
-  const present = useSelector<RootState, DashboardLayout>(
-    state => state.dashboardLayout.present,
-  );
+  const chartLayoutItems = useChartLayoutItems();
   const dataMask = useSelector<RootState, DataMaskStateWithId>(
     state => state.dataMask,
   );
@@ -207,7 +206,7 @@ export const FiltersBadge = ({ chartId }: FiltersBadgeProps) => {
   ]);
 
   const prevNativeFilters = usePrevious(nativeFilters);
-  const prevDashboardLayout = usePrevious(present);
+  const prevChartLayoutItems = usePrevious(chartLayoutItems);
   const prevDataMask = usePrevious(dataMask);
   const prevChartConfig = usePrevious(chartConfiguration);
 
@@ -221,7 +220,7 @@ export const FiltersBadge = ({ chartId }: FiltersBadgeProps) => {
         chart?.queriesResponse?.[0]?.applied_filters !==
           prevChart?.queriesResponse?.[0]?.applied_filters ||
         nativeFilters !== prevNativeFilters ||
-        present !== prevDashboardLayout ||
+        chartLayoutItems !== prevChartLayoutItems ||
         dataMask !== prevDataMask ||
         prevChartConfig !== chartConfiguration
       ) {
@@ -231,7 +230,7 @@ export const FiltersBadge = ({ chartId }: FiltersBadgeProps) => {
             dataMask,
             chartId,
             chart,
-            present,
+            chartLayoutItems,
             chartConfiguration,
           ),
         );
@@ -244,14 +243,14 @@ export const FiltersBadge = ({ chartId }: FiltersBadgeProps) => {
     dataMask,
     nativeFilters,
     nativeIndicators.length,
-    present,
     prevChart?.queriesResponse,
     prevChartConfig,
     prevChartStatus,
-    prevDashboardLayout,
     prevDataMask,
     prevNativeFilters,
     showIndicators,
+    chartLayoutItems,
+    prevChartLayoutItems,
   ]);
 
   const indicators = useMemo(

diff --git a/...hboard/components/nativeFilters/FilterBar/CrossFilters/ScopingModal/ScopingModal.test.tsx b/...hboard/components/nativeFilters/FilterBar/CrossFilters/ScopingModal/ScopingModal.test.tsx
@@ -39,6 +39,9 @@ const INITIAL_STATE = {
     3: { id: 3 },
     4: { id: 4 },
   },
+  dashboardState: {
+    sliceIds: [1, 2, 3, 4],
+  },
   dashboardInfo: {
     id: 1,
     metadata: {

diff --git a/...c/dashboard/components/nativeFilters/FilterBar/CrossFilters/ScopingModal/ScopingModal.tsx b/...c/dashboard/components/nativeFilters/FilterBar/CrossFilters/ScopingModal/ScopingModal.tsx
@@ -22,7 +22,6 @@ import { isDefined, NativeFilterScope, t } from '@superset-ui/core';
 import Modal from 'src/components/Modal';
 import {
   ChartConfiguration,
-  Layout,
   RootState,
   isCrossFilterScopeGlobal,
   GlobalChartCrossFilterConfig,
@@ -32,6 +31,7 @@ import { getChartIdsInFilterScope } from 'src/dashboard/util/getChartIdsInFilter
 import { useChartIds } from 'src/dashboard/util/charts/useChartIds';
 import { saveChartConfiguration } from 'src/dashboard/actions/dashboardInfo';
 import { DEFAULT_CROSS_FILTER_SCOPING } from 'src/dashboard/constants';
+import { useChartLayoutItems } from 'src/dashboard/util/useChartLayoutItems';
 import { ScopingModalContent } from './ScopingModalContent';
 import { NEW_CHART_SCOPING_ID } from './constants';
 
@@ -76,9 +76,7 @@ export const ScopingModal = ({
   closeModal,
 }: ScopingModalProps) => {
   const dispatch = useDispatch();
-  const layout = useSelector<RootState, Layout>(
-    state => state.dashboardLayout.present,
-  );
+  const chartLayoutItems = useChartLayoutItems();
   const chartIds = useChartIds();
   const [currentChartId, setCurrentChartId] = useState(initialChartId);
   const initialChartConfig = useSelector<RootState, ChartConfiguration>(
@@ -154,15 +152,19 @@ export const ScopingModal = ({
             id: currentChartId,
             crossFilters: {
               scope,
-              chartsInScope: getChartIdsInFilterScope(scope, chartIds, layout),
+              chartsInScope: getChartIdsInFilterScope(
+                scope,
+                chartIds,
+                chartLayoutItems,
+              ),
             },
           },
         }));
       } else {
         const globalChartsInScope = getChartIdsInFilterScope(
           scope,
           chartIds,
-          layout,
+          chartLayoutItems,
         );
         setGlobalChartConfig({
           scope,
@@ -176,7 +178,7 @@ export const ScopingModal = ({
         );
       }
     },
-    [currentChartId, chartIds, layout],
+    [currentChartId, chartIds, chartLayoutItems],
   );
 
   const removeCustomScope = useCallback(
@@ -241,7 +243,11 @@ export const ScopingModal = ({
           id: newChartId,
           crossFilters: {
             scope: newScope,
-            chartsInScope: getChartIdsInFilterScope(newScope, chartIds, layout),
+            chartsInScope: getChartIdsInFilterScope(
+              newScope,
+              chartIds,
+              chartLayoutItems,
+            ),
           },
         };
 
@@ -275,7 +281,7 @@ export const ScopingModal = ({
       currentChartId,
       globalChartConfig.chartsInScope,
       globalChartConfig.scope,
-      layout,
+      chartLayoutItems,
     ],
   );
 

diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/CrossFilters/Vertical.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/CrossFilters/Vertical.tsx
@@ -17,9 +17,11 @@
  * under the License.
  */
 
-import { DataMaskStateWithId, JsonObject } from '@superset-ui/core';
+import { DataMaskStateWithId } from '@superset-ui/core';
 import { useSelector } from 'react-redux';
-import { DashboardLayout, RootState } from 'src/dashboard/types';
+import { RootState } from 'src/dashboard/types';
+import { useChartLayoutItems } from 'src/dashboard/util/useChartLayoutItems';
+import { useChartIds } from 'src/dashboard/util/charts/useChartIds';
 import crossFiltersSelector from './selectors';
 import VerticalCollapse from './VerticalCollapse';
 import { useChartsVerboseMaps } from '../utils';
@@ -28,17 +30,13 @@ const CrossFiltersVertical = () => {
   const dataMask = useSelector<RootState, DataMaskStateWithId>(
     state => state.dataMask,
   );
-  const chartConfiguration = useSelector<RootState, JsonObject>(
-    state => state.dashboardInfo.metadata?.chart_configuration,
-  );
-  const dashboardLayout = useSelector<RootState, DashboardLayout>(
-    state => state.dashboardLayout.present,
-  );
+  const chartIds = useChartIds();
+  const chartLayoutItems = useChartLayoutItems();
   const verboseMaps = useChartsVerboseMaps();
   const selectedCrossFilters = crossFiltersSelector({
     dataMask,
-    chartConfiguration,
-    dashboardLayout,
+    chartIds,
+    chartLayoutItems,
     verboseMaps,
   });
 

diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/CrossFilters/selectors.ts b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/CrossFilters/selectors.ts
@@ -21,36 +21,37 @@ import {
   DataMaskStateWithId,
   getColumnLabel,
   isDefined,
-  JsonObject,
 } from '@superset-ui/core';
-import { DashboardLayout } from 'src/dashboard/types';
+import { LayoutItem } from 'src/dashboard/types';
 import { CrossFilterIndicator, getCrossFilterIndicator } from '../../selectors';
 
 export const crossFiltersSelector = (props: {
   dataMask: DataMaskStateWithId;
-  chartConfiguration: JsonObject;
-  dashboardLayout: DashboardLayout;
+  chartIds: number[];
+  chartLayoutItems: LayoutItem[];
   verboseMaps: { [key: string]: Record<string, string> };
 }): CrossFilterIndicator[] => {
-  const { dataMask, chartConfiguration, dashboardLayout, verboseMaps } = props;
-  const chartsIds = Object.keys(chartConfiguration || {});
+  const { dataMask, chartIds, chartLayoutItems, verboseMaps } = props;
 
-  return chartsIds
+  return chartIds
     .map(chartId => {
-      const id = Number(chartId);
       const filterIndicator = getCrossFilterIndicator(
-        id,
-        dataMask[id],
-        dashboardLayout,
+        chartId,
+        dataMask[chartId],
+        chartLayoutItems,
       );
       if (
         isDefined(filterIndicator.column) &&
         isDefined(filterIndicator.value)
       ) {
         const verboseColName =
-          verboseMaps[id]?.[getColumnLabel(filterIndicator.column)] ||
+          verboseMaps[chartId]?.[getColumnLabel(filterIndicator.column)] ||
           filterIndicator.column;
-        return { ...filterIndicator, column: verboseColName, emitterId: id };
+        return {
+          ...filterIndicator,
+          column: verboseColName,
+          emitterId: chartId,
+        };
       }
       return null;
     })