fix(native-filters): Show incompatible native filters indicator (#12687)

* Show incompatible native filters indicator

* Add Native Filters mocks and tests to Filter Badge

* Compare filter names in deduplication logic

* Add indicator key

* Remove unnecessary import

superset-frontend/spec/fixtures/mockNativeFilters.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 { NativeFiltersState } from 'src/dashboard/components/nativeFilters/types';
+
+export const nativeFilters: NativeFiltersState = {
+  filters: {
+    'NATIVE_FILTER-e7Q8zKixx': {
+      id: 'NATIVE_FILTER-e7Q8zKixx',
+      name: 'region',
+      type: 'text',
+      targets: [
+        {
+          datasetId: 2,
+          column: {
+            name: 'region',
+          },
+        },
+      ],
+      defaultValue: null,
+      cascadeParentIds: [],
+      scope: {
+        rootPath: ['ROOT_ID'],
+        excluded: [],
+      },
+      inverseSelection: false,
+      isInstant: true,
+      allowsMultipleValues: false,
+      isRequired: false,
+    },
+    'NATIVE_FILTER-x9QPw0so1': {
+      id: 'NATIVE_FILTER-x9QPw0so1',
+      name: 'country_code',
+      type: 'text',
+      targets: [
+        {
+          datasetId: 2,
+          column: {
+            name: 'country_code',
+          },
+        },
+      ],
+      defaultValue: null,
+      cascadeParentIds: [],
+      scope: {
+        rootPath: ['ROOT_ID'],
+        excluded: [],
+      },
+      inverseSelection: false,
+      isInstant: true,
+      allowsMultipleValues: false,
+      isRequired: false,
+    },
+  },
+  filtersState: {
+    'NATIVE_FILTER-e7Q8zKixx': {
+      id: 'NATIVE_FILTER-e7Q8zKixx',
+      extraFormData: {
+        append_form_data: {
+          filters: [
+            {
+              col: 'region',
+              op: 'IN',
+              val: ['East Asia & Pacific'],
+            },
+          ],
+        },
+      },
+    },
+    'NATIVE_FILTER-x9QPw0so1': {
+      id: 'NATIVE_FILTER-x9QPw0so1',
+      extraFormData: {},
+    },
+  },
+};

superset-frontend/spec/fixtures/mockStore.js

@@ -25,6 +25,7 @@ import mockState from './mockState';
 import { dashboardLayoutWithTabs } from './mockDashboardLayout';
 import { sliceId } from './mockChartQueries';
 import { dashboardFilters } from './mockDashboardFilters';
+import { nativeFilters } from './mockNativeFilters';
 
 export const getMockStore = () =>
   createStore(rootReducer, mockState, compose(applyMiddleware(thunk)));
@@ -74,3 +75,28 @@ export const getMockStoreWithFilters = () =>
       },
     },
   });
+
+export const getMockStoreWithNativeFilters = () =>
+  createStore(rootReducer, {
+    ...mockState,
+    nativeFilters,
+    charts: {
+      ...mockState.charts,
+      [sliceIdWithAppliedFilter]: {
+        ...mockState.charts[sliceId],
+        queryResponse: {
+          status: 'success',
+          applied_filters: [{ column: 'region' }],
+          rejected_filters: [],
+        },
+      },
+      [sliceIdWithRejectedFilter]: {
+        ...mockState.charts[sliceId],
+        queryResponse: {
+          status: 'success',
+          applied_filters: [],
+          rejected_filters: [{ column: 'region', reason: 'not_in_datasource' }],
+        },
+      },
+    },
+  });

superset-frontend/spec/javascripts/dashboard/components/FiltersBadge_spec.tsx

@@ -24,7 +24,10 @@ import * as SupersetUI from '@superset-ui/core';
 import { CHART_UPDATE_SUCCEEDED } from 'src/chart/chartAction';
 import { buildActiveFilters } from 'src/dashboard/util/activeDashboardFilters';
 import FiltersBadge from 'src/dashboard/containers/FiltersBadge';
-import { getMockStoreWithFilters } from 'spec/fixtures/mockStore';
+import {
+  getMockStoreWithFilters,
+  getMockStoreWithNativeFilters,
+} from 'spec/fixtures/mockStore';
 import { sliceId } from 'spec/fixtures/mockChartQueries';
 import { dashboardFilters } from 'spec/fixtures/mockDashboardFilters';
 import { dashboardWithFilter } from 'spec/fixtures/mockDashboardLayout';
@@ -45,83 +48,166 @@ describe('FiltersBadge', () => {
     jest.spyOn(SupersetUI, 'useTheme').mockImplementation(() => supersetTheme);
   });
 
-  it("doesn't show number when there are no active filters", () => {
-    const store = getMockStoreWithFilters();
-    // start with basic dashboard state, dispatch an event to simulate query completion
-    store.dispatch({
-      type: CHART_UPDATE_SUCCEEDED,
-      key: sliceId,
-      queriesResponse: [
-        {
-          status: 'success',
-          applied_filters: [],
-          rejected_filters: [],
-        },
-      ],
-      dashboardFilters,
+  describe('for dashboard filters', () => {
+    it("doesn't show number when there are no active filters", () => {
+      const store = getMockStoreWithFilters();
+      // start with basic dashboard state, dispatch an event to simulate query completion
+      store.dispatch({
+        type: CHART_UPDATE_SUCCEEDED,
+        key: sliceId,
+        queriesResponse: [
+          {
+            status: 'success',
+            applied_filters: [],
+            rejected_filters: [],
+          },
+        ],
+        dashboardFilters,
+      });
+      const wrapper = shallow(
+        <Provider store={store}>
+          <FiltersBadge chartId={sliceId} />,
+        </Provider>,
+      );
+      expect(
+        wrapper.dive().find('[data-test="applied-filter-count"]'),
+      ).not.toExist();
+    });
+
+    it('shows the indicator when filters have been applied', () => {
+      const store = getMockStoreWithFilters();
+      // start with basic dashboard state, dispatch an event to simulate query completion
+      store.dispatch({
+        type: CHART_UPDATE_SUCCEEDED,
+        key: sliceId,
+        queriesResponse: [
+          {
+            status: 'success',
+            applied_filters: [{ column: 'region' }],
+            rejected_filters: [],
+          },
+        ],
+        dashboardFilters,
+      });
+      const wrapper = shallow(
+        <FiltersBadge {...{ store }} chartId={sliceId} />,
+      ).dive();
+      expect(wrapper.dive().find('DetailsPanelPopover')).toExist();
+      expect(
+        wrapper.dive().find('[data-test="applied-filter-count"]'),
+      ).toHaveText('1');
+      expect(wrapper.dive().find('WarningFilled')).not.toExist();
     });
-    const wrapper = shallow(
-      <Provider store={store}>
-        <FiltersBadge chartId={sliceId} />,
-      </Provider>,
-    );
-    expect(
-      wrapper.dive().find('[data-test="applied-filter-count"]'),
-    ).not.toExist();
-  });
 
-  it('shows the indicator when filters have been applied', () => {
-    const store = getMockStoreWithFilters();
-    // start with basic dashboard state, dispatch an event to simulate query completion
-    store.dispatch({
-      type: CHART_UPDATE_SUCCEEDED,
-      key: sliceId,
-      queriesResponse: [
-        {
-          status: 'success',
-          applied_filters: [{ column: 'region' }],
-          rejected_filters: [],
-        },
-      ],
-      dashboardFilters,
+    it("shows a warning when there's a rejected filter", () => {
+      const store = getMockStoreWithFilters();
+      // start with basic dashboard state, dispatch an event to simulate query completion
+      store.dispatch({
+        type: CHART_UPDATE_SUCCEEDED,
+        key: sliceId,
+        queriesResponse: [
+          {
+            status: 'success',
+            applied_filters: [],
+            rejected_filters: [
+              { column: 'region', reason: 'not_in_datasource' },
+            ],
+          },
+        ],
+        dashboardFilters,
+      });
+      const wrapper = shallow(
+        <FiltersBadge {...{ store }} chartId={sliceId} />,
+      ).dive();
+      expect(wrapper.dive().find('DetailsPanelPopover')).toExist();
+      expect(
+        wrapper.dive().find('[data-test="applied-filter-count"]'),
+      ).toHaveText('0');
+      expect(
+        wrapper.dive().find('[data-test="incompatible-filter-count"]'),
+      ).toHaveText('1');
+      // to look at the shape of the wrapper use:
+      // console.log(wrapper.dive().debug())
+      expect(wrapper.dive().find('Icon[name="alert-solid"]')).toExist();
     });
-    const wrapper = shallow(
-      <FiltersBadge {...{ store }} chartId={sliceId} />,
-    ).dive();
-    expect(wrapper.dive().find('DetailsPanelPopover')).toExist();
-    expect(
-      wrapper.dive().find('[data-test="applied-filter-count"]'),
-    ).toHaveText('1');
-    expect(wrapper.dive().find('WarningFilled')).not.toExist();
   });
 
-  it("shows a warning when there's a rejected filter", () => {
-    const store = getMockStoreWithFilters();
-    // start with basic dashboard state, dispatch an event to simulate query completion
-    store.dispatch({
-      type: CHART_UPDATE_SUCCEEDED,
-      key: sliceId,
-      queriesResponse: [
-        {
-          status: 'success',
-          applied_filters: [],
-          rejected_filters: [{ column: 'region', reason: 'not_in_datasource' }],
-        },
-      ],
-      dashboardFilters,
+  describe('for native filters', () => {
+    it("doesn't show number when there are no active filters", () => {
+      const store = getMockStoreWithNativeFilters();
+      // start with basic dashboard state, dispatch an event to simulate query completion
+      store.dispatch({
+        type: CHART_UPDATE_SUCCEEDED,
+        key: sliceId,
+        queriesResponse: [
+          {
+            status: 'success',
+            applied_filters: [],
+            rejected_filters: [],
+          },
+        ],
+      });
+      const wrapper = shallow(
+        <Provider store={store}>
+          <FiltersBadge chartId={sliceId} />,
+        </Provider>,
+      );
+      expect(
+        wrapper.dive().find('[data-test="applied-filter-count"]'),
+      ).not.toExist();
+    });
+
+    it('shows the indicator when filters have been applied', () => {
+      const store = getMockStoreWithNativeFilters();
+      // start with basic dashboard state, dispatch an event to simulate query completion
+      store.dispatch({
+        type: CHART_UPDATE_SUCCEEDED,
+        key: sliceId,
+        queriesResponse: [
+          {
+            status: 'success',
+            applied_filters: [{ column: 'region' }],
+            rejected_filters: [],
+          },
+        ],
+      });
+      const wrapper = shallow(
+        <FiltersBadge {...{ store }} chartId={sliceId} />,
+      ).dive();
+      expect(wrapper.dive().find('DetailsPanelPopover')).toExist();
+      expect(
+        wrapper.dive().find('[data-test="applied-filter-count"]'),
+      ).toHaveText('1');
+      expect(wrapper.dive().find('WarningFilled')).not.toExist();
+    });
+
+    it("shows a warning when there's a rejected filter", () => {
+      const store = getMockStoreWithNativeFilters();
+      // start with basic dashboard state, dispatch an event to simulate query completion
+      store.dispatch({
+        type: CHART_UPDATE_SUCCEEDED,
+        key: sliceId,
+        queriesResponse: [
+          {
+            status: 'success',
+            applied_filters: [],
+            rejected_filters: [
+              { column: 'region', reason: 'not_in_datasource' },
+            ],
+          },
+        ],
+      });
+      const wrapper = shallow(
+        <FiltersBadge {...{ store }} chartId={sliceId} />,
+      ).dive();
+      expect(wrapper.dive().find('DetailsPanelPopover')).toExist();
+      expect(
+        wrapper.dive().find('[data-test="applied-filter-count"]'),
+      ).toHaveText('0');
+      expect(
+        wrapper.dive().find('[data-test="incompatible-filter-count"]'),
+      ).toHaveText('1');
+      expect(wrapper.dive().find('Icon[name="alert-solid"]')).toExist();
     });
-    const wrapper = shallow(
-      <FiltersBadge {...{ store }} chartId={sliceId} />,
-    ).dive();
-    expect(wrapper.dive().find('DetailsPanelPopover')).toExist();
-    expect(
-      wrapper.dive().find('[data-test="applied-filter-count"]'),
-    ).toHaveText('0');
-    expect(
-      wrapper.dive().find('[data-test="incompatible-filter-count"]'),
-    ).toHaveText('1');
-    // to look at the shape of the wrapper use:
-    // console.log(wrapper.dive().debug())
-    expect(wrapper.dive().find('Icon[name="alert-solid"]')).toExist();
   });
 });

superset-frontend/src/dashboard/components/FiltersBadge/DetailsPanel.tsx

@@ -103,6 +103,9 @@ const DetailsPanelPopover = ({
     }
   }
 
+  const indicatorKey = (indicator: Indicator): string =>
+    `${indicator.column} - ${indicator.name}`;
+
   const content = (
     <Panel>
       <Global
@@ -173,7 +176,7 @@ const DetailsPanelPopover = ({
               <Indent>
                 {appliedIndicators.map(indicator => (
                   <Indicator
-                    key={indicator.column}
+                    key={indicatorKey(indicator)}
                     indicator={indicator}
                     onClick={onHighlightFilterSource}
                   />
@@ -197,7 +200,7 @@ const DetailsPanelPopover = ({
               <Indent>
                 {incompatibleIndicators.map(indicator => (
                   <Indicator
-                    key={indicator.column}
+                    key={indicatorKey(indicator)}
                     indicator={indicator}
                     onClick={onHighlightFilterSource}
                   />
@@ -219,7 +222,7 @@ const DetailsPanelPopover = ({
               <Indent>
                 {unsetIndicators.map(indicator => (
                   <Indicator
-                    key={indicator.column}
+                    key={indicatorKey(indicator)}
                     indicator={indicator}
                     onClick={onHighlightFilterSource}
                   />

superset-frontend/src/dashboard/components/FiltersBadge/selectors.ts

@@ -115,6 +115,20 @@ const selectIndicatorsForChartFromFilter = (
     }));
 };
 
+const getAppliedColumns = (chart: any): Set<string> =>
+  new Set(
+    (chart?.queriesResponse?.[0]?.applied_filters || []).map(
+      (filter: any) => filter.column,
+    ),
+  );
+
+const getRejectedColumns = (chart: any): Set<string> =>
+  new Set(
+    (chart?.queriesResponse?.[0]?.rejected_filters || []).map(
+      (filter: any) => filter.column,
+    ),
+  );
+
 export type Indicator = {
   column: string;
   name: string;
@@ -136,16 +150,9 @@ export const selectIndicatorsForChart = (
 
   // for now we only need to know which columns are compatible/incompatible,
   // so grab the columns from the applied/rejected filters
-  const appliedColumns: Set<string> = new Set(
-    (chart?.queriesResponse?.[0]?.applied_filters || []).map(
-      (filter: any) => filter.column,
-    ),
-  );
-  const rejectedColumns: Set<string> = new Set(
-    (chart?.queriesResponse?.[0]?.rejected_filters || []).map(
-      (filter: any) => filter.column,
-    ),
-  );
+  const appliedColumns = getAppliedColumns(chart);
+  const rejectedColumns = getRejectedColumns(chart);
+
   const indicators = Object.values(filters)
     .filter(filter => filter.chartId !== chartId)
     .reduce(
@@ -184,7 +191,22 @@ const selectNativeIndicatorValue = (
 
 export const selectNativeIndicatorsForChart = (
   nativeFilters: NativeFiltersState,
+  chartId: number,
+  charts: any,
 ): Indicator[] => {
+  const chart = charts[chartId];
+
+  const appliedColumns = getAppliedColumns(chart);
+  const rejectedColumns = getRejectedColumns(chart);
+
+  const getStatus = (column: string, value: string[]): IndicatorStatus => {
+    if (rejectedColumns.has(column)) return IndicatorStatus.Incompatible;
+    if (appliedColumns.has(column) && value.length > 0) {
+      return IndicatorStatus.Applied;
+    }
+    return IndicatorStatus.Unset;
+  };
+
   const indicators = Object.values(nativeFilters.filters).map(nativeFilter => {
     const column = nativeFilter.targets[0].column.name;
     const filterState = nativeFilters.filtersState[nativeFilter.id];
@@ -193,7 +215,7 @@ export const selectNativeIndicatorsForChart = (
       column,
       name: nativeFilter.name,
       path: [nativeFilter.id],
-      status: value.length ? IndicatorStatus.Applied : IndicatorStatus.Unset,
+      status: getStatus(column, value),
       value,
     };
   });

superset-frontend/src/dashboard/containers/FiltersBadge.tsx

@@ -62,12 +62,17 @@ const mapStateToProps = (
     charts,
   );
 
-  const nativeIndicators = selectNativeIndicatorsForChart(nativeFilters);
+  const nativeIndicators = selectNativeIndicatorsForChart(
+    nativeFilters,
+    chartId,
+    charts,
+  );
 
   const indicators = uniqWith(
     sortByStatus([...dashboardIndicators, ...nativeIndicators]),
     (ind1, ind2) =>
       ind1.column === ind2.column &&
+      ind1.name === ind2.name &&
       (ind1.status !== IndicatorStatus.Applied ||
         ind2.status !== IndicatorStatus.Applied),
   );