feat(native-filters): apply scoping of native filters to dashboard (#12716)

* feat: scoping native filters in dashboard

* test: add tests / fix reducer

* test: fix tests

* chore: merge with master

* fix: fix undefined cases

* fix: fix code according cypress

* refactor: fix mocks according CRs

* chore: re run pipeline

superset-frontend/spec/fixtures/mockNativeFilters.ts

@@ -88,3 +88,117 @@ export const nativeFilters: NativeFiltersState = {
     },
   },
 };
+
+export const extraFormData = {
+  append_form_data: {
+    filters: [
+      {
+        col: 'ethnic_minority',
+        op: 'IN',
+        val: 'No, not an ethnic minority',
+      },
+    ],
+  },
+};
+
+export const NATIVE_FILTER_ID = 'NATIVE_FILTER-p4LImrSgA';
+
+export const singleNativeFiltersState = {
+  filters: {
+    [NATIVE_FILTER_ID]: {
+      id: [NATIVE_FILTER_ID],
+      name: 'eth',
+      type: 'text',
+      targets: [{ datasetId: 13, column: { name: 'ethnic_minority' } }],
+      defaultValue: null,
+      cascadeParentIds: [],
+      scope: { rootPath: ['ROOT_ID'], excluded: [227, 229] },
+      inverseSelection: false,
+      isInstant: true,
+      allowsMultipleValues: false,
+      isRequired: false,
+    },
+  },
+  filtersState: {
+    [NATIVE_FILTER_ID]: {
+      id: NATIVE_FILTER_ID,
+      extraFormData,
+    },
+  },
+};
+
+export const layoutForSingleNativeFilter = {
+  'CHART-ZHVS7YasaQ': {
+    children: [],
+    id: 'CHART-ZHVS7YasaQ',
+    meta: {
+      chartId: 230,
+      height: 50,
+      sliceName: 'Pie Chart',
+      uuid: '05ef6145-3950-4f59-891f-160852613eca',
+      width: 12,
+    },
+    parents: ['ROOT_ID', 'GRID_ID', 'ROW-NweUz7oC0'],
+    type: 'CHART',
+  },
+  'CHART-gsGu8NIKQT': {
+    children: [],
+    id: 'CHART-gsGu8NIKQT',
+    meta: {
+      chartId: 227,
+      height: 50,
+      sliceName: 'Another Chart',
+      uuid: 'ddb78f6c-7876-47fc-ae98-70183b05ba90',
+      width: 4,
+    },
+    parents: ['ROOT_ID', 'GRID_ID', 'ROW-QkiTjeZGs'],
+    type: 'CHART',
+  },
+  'CHART-hgYjD8axJX': {
+    children: [],
+    id: 'CHART-hgYjD8axJX',
+    meta: {
+      chartId: 229,
+      height: 47,
+      sliceName: 'Bar Chart',
+      uuid: 'e1501e54-d632-4fdc-ae16-07cafee31093',
+      width: 12,
+    },
+    parents: ['ROOT_ID', 'GRID_ID', 'ROW-mcdVZi0rL'],
+    type: 'CHART',
+  },
+  DASHBOARD_VERSION_KEY: 'v2',
+  GRID_ID: {
+    children: ['ROW-mcdVZi0rL', 'ROW-NweUz7oC0', 'ROW-QkiTjeZGs'],
+    id: 'GRID_ID',
+    parents: ['ROOT_ID'],
+    type: 'GRID',
+  },
+  HEADER_ID: {
+    id: 'HEADER_ID',
+    type: 'HEADER',
+    meta: { text: 'My Native Filter Dashboard' },
+  },
+  ROOT_ID: { children: ['GRID_ID'], id: 'ROOT_ID', type: 'ROOT' },
+  'ROW-NweUz7oC0': {
+    children: ['CHART-ZHVS7YasaQ'],
+    id: 'ROW-NweUz7oC0',
+    meta: { background: 'BACKGROUND_TRANSPARENT' },
+    parents: ['ROOT_ID', 'GRID_ID'],
+    type: 'ROW',
+  },
+  'ROW-QkiTjeZGs': {
+    children: ['CHART-gsGu8NIKQT'],
+    id: 'ROW-QkiTjeZGs',
+    meta: { background: 'BACKGROUND_TRANSPARENT' },
+    parents: ['ROOT_ID', 'GRID_ID'],
+    type: 'ROW',
+  },
+  'ROW-mcdVZi0rL': {
+    children: ['CHART-hgYjD8axJX'],
+    id: 'ROW-mcdVZi0rL',
+    meta: { '0': 'ROOT_ID', background: 'BACKGROUND_TRANSPARENT' },
+    parents: ['ROOT_ID', 'GRID_ID'],
+    type: 'ROW',
+  },
+};

superset-frontend/spec/javascripts/dashboard/components/Dashboard_spec.jsx

@@ -28,10 +28,17 @@ import newComponentFactory from 'src/dashboard/util/newComponentFactory';
 // mock data
 import chartQueries from 'spec/fixtures/mockChartQueries';
 import datasources from 'spec/fixtures/mockDatasource';
+import {
+  extraFormData,
+  NATIVE_FILTER_ID,
+  layoutForSingleNativeFilter,
+  singleNativeFiltersState,
+} from 'spec/fixtures/mockNativeFilters';
 import dashboardInfo from 'spec/fixtures/mockDashboardInfo';
 import { dashboardLayout } from 'spec/fixtures/mockDashboardLayout';
 import dashboardState from 'spec/fixtures/mockDashboardState';
 import { sliceEntitiesForChart as sliceEntities } from 'spec/fixtures/mockSliceEntities';
+import { getActiveNativeFilters } from 'src/dashboard/util/activeDashboardNativeFilters';
 
 describe('Dashboard', () => {
   const props = {
@@ -141,6 +148,27 @@ describe('Dashboard', () => {
       expect(wrapper.instance().appliedFilters).toBe(OVERRIDE_FILTERS);
     });
 
+    it('should call refresh when native filters changed', () => {
+      wrapper.setProps({
+        activeFilters: {
+          ...OVERRIDE_FILTERS,
+          ...getActiveNativeFilters({
+            nativeFilters: singleNativeFiltersState,
+            layout: layoutForSingleNativeFilter,
+          }),
+        },
+      });
+      wrapper.instance().componentDidUpdate(prevProps);
+      expect(refreshSpy.callCount).toBe(1);
+      expect(wrapper.instance().appliedFilters).toEqual({
+        ...OVERRIDE_FILTERS,
+        [NATIVE_FILTER_ID]: {
+          scope: [230],
+          values: [extraFormData],
+        },
+      });
+    });
+
     it('should call refresh if a filter is added', () => {
       const newFilter = {
         gender: { values: ['boy', 'girl'], scope: [1] },

superset-frontend/spec/javascripts/dashboard/util/getFormDataWithExtraFilters_spec.ts

@@ -16,11 +16,18 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-import getFormDataWithExtraFilters from 'src/dashboard/util/charts/getFormDataWithExtraFilters';
+import getFormDataWithExtraFilters, {
+  GetFormDataWithExtraFiltersArguments,
+} from 'src/dashboard/util/charts/getFormDataWithExtraFilters';
+import { DASHBOARD_ROOT_ID } from 'src/dashboard/util/constants';
+import { Filter } from 'src/dashboard/components/nativeFilters/types';
+import { LayoutItem } from 'src/dashboard/types';
+import { dashboardLayout } from '../../../fixtures/mockDashboardLayout';
+import { sliceId as chartId } from '../../../fixtures/mockChartQueries';
 
 describe('getFormDataWithExtraFilters', () => {
-  const chartId = 8675309;
-  const mockArgs = {
+  const filterId = 'native-filter-1';
+  const mockArgs: GetFormDataWithExtraFiltersArguments = {
     chart: {
       id: chartId,
       formData: {
@@ -37,11 +44,27 @@ describe('getFormDataWithExtraFilters', () => {
       region: ['Spain'],
       color: ['pink', 'purple'],
     },
+    sliceId: chartId,
     nativeFilters: {
-      filters: {},
-      filtersState: {},
+      filters: {
+        [filterId]: ({
+          id: filterId,
+          scope: {
+            rootPath: [DASHBOARD_ROOT_ID],
+            excluded: [],
+          },
+        } as unknown) as Filter,
+      },
+      filtersState: {
+        [filterId]: {
+          id: filterId,
+          extraFormData: {},
+        },
+      },
+    },
+    layout: (dashboardLayout.present as unknown) as {
+      [key: string]: LayoutItem;
     },
-    sliceId: chartId,
   };
 
   it('should include filters from the passed filters', () => {

superset-frontend/src/dashboard/components/Dashboard.jsx

@@ -144,15 +144,11 @@ class Dashboard extends React.PureComponent {
     }
   }
 
-  componentDidUpdate(prevProps) {
+  componentDidUpdate() {
     const { hasUnsavedChanges, editMode } = this.props.dashboardState;
 
     const { appliedFilters } = this;
-    const { activeFilters, nativeFilters } = this.props;
-    // do not apply filter when dashboard in edit mode
-    if (!areObjectsEqual(prevProps.nativeFilters, nativeFilters)) {
-      this.refreshCharts(this.getAllCharts().map(chart => chart.id));
-    }
+    const { activeFilters } = this.props;
     if (!editMode && !areObjectsEqual(appliedFilters, activeFilters)) {
       this.applyFilters();
     }

superset-frontend/src/dashboard/containers/Chart.jsx

@@ -43,6 +43,7 @@ function mapStateToProps(
     charts: chartQueries,
     dashboardInfo,
     dashboardState,
+    dashboardLayout,
     datasources,
     sliceEntities,
     nativeFilters,
@@ -57,6 +58,7 @@ function mapStateToProps(
 
   // note: this method caches filters if possible to prevent render cascades
   const formData = getFormDataWithExtraFilters({
+    layout: dashboardLayout.present,
     chart,
     filters: getAppliedFilterValues(id),
     colorScheme,

superset-frontend/src/dashboard/containers/Dashboard.jsx

@@ -28,6 +28,7 @@ import {
 import { triggerQuery } from '../../chart/chartAction';
 import { logEvent } from '../../logger/actions';
 import { getActiveFilters } from '../util/activeDashboardFilters';
+import { getActiveNativeFilters } from '../util/activeDashboardNativeFilters';
 
 function mapStateToProps(state) {
   const {
@@ -54,10 +55,15 @@ function mapStateToProps(state) {
     // When dashboard is first loaded into browser,
     // its value is from preselect_filters that dashboard owner saved in dashboard's meta data
     // When user start interacting with dashboard, it will be user picked values from all filter_box
-    activeFilters: getActiveFilters(),
+    activeFilters: {
+      ...getActiveFilters(),
+      ...getActiveNativeFilters({
+        nativeFilters,
+        layout: dashboardLayout.present,
+      }),
+    },
     slices: sliceEntities.slices,
     layout: dashboardLayout.present,
-    nativeFilters,
     impressionId,
   };
 }

superset-frontend/src/dashboard/reducers/nativeFilters.ts

@@ -36,6 +36,7 @@ export function getInitialFilterState(id: string): FilterState {
 
 export function getInitialState(
   filterConfig: FilterConfiguration,
+  prevFiltersState: { [filterId: string]: FilterState },
 ): NativeFiltersState {
   const filters = {};
   const filtersState = {};
@@ -43,7 +44,7 @@ export function getInitialState(
   filterConfig.forEach(filter => {
     const { id } = filter;
     filters[id] = filter;
-    filtersState[id] = getInitialFilterState(id);
+    filtersState[id] = prevFiltersState?.[id] || getInitialFilterState(id);
   });
   return state;
 }
@@ -67,7 +68,7 @@ export default function nativeFilterReducer(
       };
 
     case SET_FILTER_CONFIG_COMPLETE:
-      return getInitialState(action.filterConfig);
+      return getInitialState(action.filterConfig, filtersState);
 
     // TODO handle SET_FILTER_CONFIG_FAIL action
     default:

superset-frontend/src/dashboard/types.ts

@@ -25,6 +25,7 @@ export type ChartReducerInitialState = typeof chart;
 // chart query built from initialState
 // Ref: https://github.com/apache/superset/blob/dcac860f3e5528ecbc39e58f045c7388adb5c3d0/superset-frontend/src/dashboard/reducers/getInitialState.js#L120
 export interface ChartQueryPayload extends Partial<ChartReducerInitialState> {
+  id: number;
   formData: ChartProps['formData'];
   form_data?: ChartProps['rawFormData'];
   [key: string]: unknown;
@@ -69,3 +70,12 @@ export type LayoutItem = {
     width: number;
   };
 };
+
+type ActiveFilter = {
+  scope: number[];
+  values: any[];
+};
+
+export type ActiveFilters = {
+  [key: string]: ActiveFilter;
+};

superset-frontend/src/dashboard/util/activeDashboardNativeFilters.ts

+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { CHART_TYPE } from './componentTypes';
+import { NativeFiltersState, Scope } from '../components/nativeFilters/types';
+import { ActiveFilters, LayoutItem } from '../types';
+
+// Looking for affected chart scopes and values
+export const findAffectedCharts = ({
+  child,
+  layout,
+  scope,
+  activeNativeFilters,
+  filterId,
+  extraFormData,
+}: {
+  child: string;
+  layout: { [key: string]: LayoutItem };
+  scope: Scope;
+  activeNativeFilters: ActiveFilters;
+  filterId: string;
+  extraFormData: any;
+}) => {
+  const chartId = layout[child]?.meta?.chartId;
+  if (layout[child].type === CHART_TYPE) {
+    // Ignore excluded charts
+    if (scope.excluded.includes(chartId)) {
+      return;
+    }
+    if (!activeNativeFilters[filterId]) {
+      // Small mutation but simplify logic
+      // eslint-disable-next-line no-param-reassign
+      activeNativeFilters[filterId] = {
+        scope: [],
+        values: [],
+      };
+    }
+    // Add not excluded chart scopes(to know what charts refresh) and values(refresh only if its value changed)
+    activeNativeFilters[filterId].scope.push(chartId);
+    activeNativeFilters[filterId].values.push(extraFormData);
+    return;
+  }
+  // If child is not chart, recursive iterate over its children
+  layout[child].children.forEach((child: string) =>
+    findAffectedCharts({
+      child,
+      layout,
+      scope,
+      activeNativeFilters,
+      filterId,
+      extraFormData,
+    }),
+  );
+};
+
+export const getActiveNativeFilters = ({
+  nativeFilters,
+  layout,
+}: {
+  nativeFilters: NativeFiltersState;
+  layout: { [key: string]: LayoutItem };
+}): ActiveFilters => {
+  const activeNativeFilters = {};
+  if (!nativeFilters?.filtersState) {
+    return activeNativeFilters;
+  }
+  Object.values(nativeFilters.filtersState).forEach(
+    ({ id: filterId, extraFormData }) => {
+      const scope = nativeFilters?.filters?.[filterId]?.scope;
+      if (!scope) {
+        return;
+      }
+      // Iterate over all roots to find all affected charts
+      scope.rootPath.forEach(layoutItemId => {
+        layout[layoutItemId].children.forEach((child: string) => {
+          // Need exclude from affected charts, charts that located in scope `excluded`
+          findAffectedCharts({
+            child,
+            layout,
+            scope,
+            activeNativeFilters,
+            filterId,
+            extraFormData,
+          });
+        });
+      });
+    },
+  );
+  return activeNativeFilters;
+};

superset-frontend/src/dashboard/util/charts/getFormDataWithExtraFilters.ts

@@ -21,19 +21,21 @@ import {
   CategoricalColorNamespace,
   DataRecordFilters,
 } from '@superset-ui/core';
-import { ChartQueryPayload } from 'src/dashboard/types';
+import { ChartQueryPayload, LayoutItem } from 'src/dashboard/types';
 import { NativeFiltersState } from 'src/dashboard/components/nativeFilters/types';
 import { getExtraFormData } from 'src/dashboard/components/nativeFilters/utils';
 import getEffectiveExtraFilters from './getEffectiveExtraFilters';
+import { getActiveNativeFilters } from '../activeDashboardNativeFilters';
 
 // We cache formData objects so that our connected container components don't always trigger
 // render cascades. we cannot leverage the reselect library because our cache size is >1
 const cachedFiltersByChart = {};
 const cachedFormdataByChart = {};
 
-interface GetFormDataWithExtraFiltersArguments {
+export interface GetFormDataWithExtraFiltersArguments {
   chart: ChartQueryPayload;
   filters: DataRecordFilters;
+  layout: { [key: string]: LayoutItem };
   colorScheme?: string;
   colorNamespace?: string;
   sliceId: number;
@@ -49,6 +51,7 @@ export default function getFormDataWithExtraFilters({
   colorScheme,
   colorNamespace,
   sliceId,
+  layout,
   nativeFilters,
 }: GetFormDataWithExtraFiltersArguments) {
   // Propagate color mapping to chart
@@ -68,12 +71,23 @@ export default function getFormDataWithExtraFilters({
     return cachedFormdataByChart[sliceId];
   }
 
+  let extraData = {};
+  const activeNativeFilters = getActiveNativeFilters({ nativeFilters, layout });
+  const isAffectedChart = Object.values(activeNativeFilters).some(({ scope }) =>
+    scope.includes(chart.id),
+  );
+  if (isAffectedChart) {
+    extraData = {
+      extra_form_data: getExtraFormData(nativeFilters),
+    };
+  }
+
   const formData = {
     ...chart.formData,
     ...(colorScheme && { color_scheme: colorScheme }),
     label_colors: labelColors,
     extra_filters: getEffectiveExtraFilters(filters),
-    extra_form_data: getExtraFormData(nativeFilters),
+    ...extraData,
   };
   cachedFiltersByChart[sliceId] = filters;
   cachedFormdataByChart[sliceId] = formData;