chore(Dashboard): Implement affected logic to highlight

apache · sadpandajoe · Oct 15, 2024 · Sep 30, 2024 · Oct 3, 2024 · Oct 3, 2024
commit 0f661c6b291ab17a46f3a1acf8e1debd11328b82
diff --git a/superset-frontend/spec/fixtures/mockNativeFilters.ts b/superset-frontend/spec/fixtures/mockNativeFilters.ts
@@ -133,6  133,7 @@ export const singleNativeFiltersState = {
       id: [NATIVE_FILTER_ID],
       name: 'eth',
       type: 'text',
       filterType: 'filter_select',
       targets: [{ datasetId: 13, column: { name: 'ethnic_minority' } }],
       defaultDataMask: {
         filterState: {

diff --git a/superset-frontend/src/dashboard/components/Dashboard.test.jsx b/superset-frontend/src/dashboard/components/Dashboard.test.jsx
@@ -37,6  37,9 @@ import { dashboardLayout } from 'spec/fixtures/mockDashboardLayout';
 import dashboardState from 'spec/fixtures/mockDashboardState';
 import { sliceEntitiesForChart as sliceEntities } from 'spec/fixtures/mockSliceEntities';
 import { getAllActiveFilters } from 'src/dashboard/util/activeAllDashboardFilters';
 import { getRelatedCharts } from 'src/dashboard/util/getRelatedCharts';
 
 jest.mock('src/dashboard/util/getRelatedCharts');
 
 describe('Dashboard', () => {
   const props = {
@@ -130,6  133,7 @@ describe('Dashboard', () => {
 
     afterEach(() => {
       refreshSpy.restore();
       jest.clearAllMocks();
     });
 
     it('should not call refresh when is editMode', () => {
@@ -153,6  157,9 @@ describe('Dashboard', () => {
     });
 
     it('should call refresh when native filters changed', () => {
       getRelatedCharts.mockReturnValue({
         [NATIVE_FILTER_ID]: [230],
       });
       wrapper.setProps({
         activeFilters: {
           ...OVERRIDE_FILTERS,
@@ -170,7  177,7 @@ describe('Dashboard', () => {
         [NATIVE_FILTER_ID]: {
           scope: [230],
           values: extraFormData,
-          filterType: null,
           filterType: 'filter_select',
           targets: [
             {
               datasetId: 13,
@@ -184,6  191,13 @@ describe('Dashboard', () => {
     });
 
     it('should call refresh if a filter is added', () => {
       getRelatedCharts.mockReturnValue({
         '1_region': [1],
         '2_country_name': [1, 2],
         '3_region': [1],
         '3_country_name': [],
         gender: [1],
       });
       const newFilter = {
         gender: { values: ['boy', 'girl'], scope: [1] },
       };
@@ -195,6  209,12 @@ describe('Dashboard', () => {
     });
 
     it('should call refresh if a filter is removed', () => {
       getRelatedCharts.mockReturnValue({
         '1_region': [1],
         '2_country_name': [1, 2],
         '3_region': [1],
         '3_country_name': [],
       });
       wrapper.setProps({
         activeFilters: {},
       });

diff --git a/superset-frontend/src/dashboard/util/activeAllDashboardFilters.ts b/superset-frontend/src/dashboard/util/activeAllDashboardFilters.ts
@@ -54,7  54,7 @@ export const getAllActiveFilters = ({
       chartConfiguration?.[filterId]?.crossFilters?.chartsInScope ??
       allSliceIds ??
       [];
-    const filterType = nativeFilters?.[filterId]?.filterType ?? null;
     const filterType = nativeFilters?.[filterId]?.filterType;
     const targets = nativeFilters?.[filterId]?.targets ?? scope;
     // Iterate over all roots to find all affected charts
     activeFilters[filterId] = {

diff --git a/superset-frontend/src/dashboard/util/getRelatedCharts.test.ts b/superset-frontend/src/dashboard/util/getRelatedCharts.test.ts
@@ -21,134  21,132 @@ import { DatasourceType } from '@superset-ui/core';
 import { DatasourcesState } from '../types';
 import { getRelatedCharts } from './getRelatedCharts';
 
-describe('getRelatedCharts', () => {
-  const slices = [
-    { datasource: 'ds1', slice_id: 1 },
-    { datasource: 'ds2', slice_id: 2 },
-  ];
 const slices = {
   '1': { datasource: 'ds1', slice_id: 1 },
   '2': { datasource: 'ds2', slice_id: 2 },
 };
 
-  const datasources: DatasourcesState = {
-    ds1: {
-      uid: 'ds1',
-      datasource_name: 'ds1',
-      table_name: 'table1',
-      description: '',
-      id: 100,
-      columns: [{ column_name: 'column1' }, { column_name: 'column2' }],
-      column_types: [],
-      type: DatasourceType.Table,
-      metrics: [],
-      column_formats: {},
-      currency_formats: {},
-      verbose_map: {},
-      main_dttm_col: '',
-      filter_select_enabled: true,
-    },
-    ds2: {
-      uid: 'ds2',
-      datasource_name: 'ds2',
-      table_name: 'table2',
-      description: '',
-      id: 200,
-      columns: [{ column_name: 'column3' }, { column_name: 'column4' }],
-      column_types: [],
-      type: DatasourceType.Table,
-      metrics: [],
-      column_formats: {},
-      currency_formats: {},
-      verbose_map: {},
-      main_dttm_col: '',
-      filter_select_enabled: true,
 const datasources: DatasourcesState = {
   ds1: {
     uid: 'ds1',
     datasource_name: 'ds1',
     table_name: 'table1',
     description: '',
     id: 100,
     columns: [{ column_name: 'column1' }, { column_name: 'column2' }],
     column_types: [],
     type: DatasourceType.Table,
     metrics: [],
     column_formats: {},
     currency_formats: {},
     verbose_map: {},
     main_dttm_col: '',
     filter_select_enabled: true,
   },
   ds2: {
     uid: 'ds2',
     datasource_name: 'ds2',
     table_name: 'table2',
     description: '',
     id: 200,
     columns: [{ column_name: 'column3' }, { column_name: 'column4' }],
     column_types: [],
     type: DatasourceType.Table,
     metrics: [],
     column_formats: {},
     currency_formats: {},
     verbose_map: {},
     main_dttm_col: '',
     filter_select_enabled: true,
   },
 };
 
 test('Return chart ids matching the dataset id', () => {
   const filters = {
     filterKey1: {
       filterType: 'filter_select',
       targets: [
         {
           column: { name: 'column1' },
           datasetId: 100,
         },
       ],
     },
   };
 
-  it('should return chart ids matching the dataset id', () => {
-    const filters = {
-      filterKey1: {
-        filterType: 'filter_select',
-        targets: [
-          {
-            column: { name: 'column1' },
-            datasetId: 100,
-          },
-        ],
-      },
-    };
-
-    const result = getRelatedCharts(filters, slices, datasources);
-    expect(result).toEqual({
-      filterKey1: [1],
-    });
   const result = getRelatedCharts(filters, slices, datasources);
   expect(result).toEqual({
     filterKey1: [1],
   });
 });
 
-  it('should return chart ids matching the column name', () => {
-    const filters = {
-      filterKey1: {
-        filterType: 'filter_select',
-        targets: [
-          {
-            column: { name: 'column3' },
-            datasetId: 999,
-          },
-        ],
-      },
-    };
 test('Return chart ids matching the column name', () => {
   const filters = {
     filterKey1: {
       filterType: 'filter_select',
       targets: [
         {
           column: { name: 'column3' },
           datasetId: 999,
         },
       ],
     },
   };
 
-    const result = getRelatedCharts(filters, slices, datasources);
-    expect(result).toEqual({
-      filterKey1: [2],
-    });
   const result = getRelatedCharts(filters, slices, datasources);
   expect(result).toEqual({
     filterKey1: [2],
   });
 });
 
-  it('should return chart ids when column display name matches', () => {
-    const filters = {
-      filterKey1: {
-        filterType: 'filter_select',
-        targets: [
-          {
-            column: { displayName: 'column4' },
-            datasetId: 999,
-          },
-        ],
-      },
-    };
 test('Return chart ids when column display name matches', () => {
   const filters = {
     filterKey1: {
       filterType: 'filter_select',
       targets: [
         {
           column: { displayName: 'column4' },
           datasetId: 999,
         },
       ],
     },
   };
 
-    const result = getRelatedCharts(filters, slices, datasources);
-    expect(result).toEqual({
-      filterKey1: [2],
-    });
   const result = getRelatedCharts(filters, slices, datasources);
   expect(result).toEqual({
     filterKey1: [2],
   });
 });
 
-  it('should return scope when filterType is not filter_select', () => {
-    const filters = {
-      filterKey1: {
-        filterType: 'filter_time',
-        scope: [3, 4],
-      },
-    };
 test('Return scope when filterType is not filter_select', () => {
   const filters = {
     filterKey1: {
       filterType: 'filter_time',
       scope: [3, 4],
     },
   };
 
-    const result = getRelatedCharts(filters, slices, datasources);
-    expect(result).toEqual({
-      filterKey1: [3, 4],
-    });
   const result = getRelatedCharts(filters, slices, datasources);
   expect(result).toEqual({
     filterKey1: [3, 4],
   });
 });
 
-  it('should return an empty array if no matching charts found', () => {
-    const filters = {
-      filterKey1: {
-        filterType: 'filter_select',
-        targets: [
-          {
-            column: { name: 'nonexistent_column' },
-            datasetId: 300,
-          },
-        ],
-      },
-    };
 test('Return an empty array if no matching charts found', () => {
   const filters = {
     filterKey1: {
       filterType: 'filter_select',
       targets: [
         {
           column: { name: 'nonexistent_column' },
           datasetId: 300,
         },
       ],
     },
   };
 
-    const result = getRelatedCharts(filters, slices, datasources);
-    expect(result).toEqual({
-      filterKey1: [],
-    });
   const result = getRelatedCharts(filters, slices, datasources);
   expect(result).toEqual({
     filterKey1: [],
   });
 });
diff --git a/superset-frontend/src/dashboard/util/getRelatedCharts.ts b/superset-frontend/src/dashboard/util/getRelatedCharts.ts
@@ -27,14  27,14 @@ type Slice = {
 
 function getRelatedChartsForSelectFilter(
   filter: Filter,
-  slices: Slice[],
   slices: Record<string, Slice>,
   datasources: DatasourcesState,
 ) {
   return Object.values(slices)
     .filter(({ datasource }) => {
       const chartDatasource = datasources[datasource];
       if (!chartDatasource) return false;
-      const { column, datasetId } = filter.targets[0];
       const { column, datasetId } = filter.targets[0] ?? {};
 
       return (
         chartDatasource.id === datasetId ||
@@ -49,7  49,7 @@ function getRelatedChartsForSelectFilter(
 }
 function getRelatedChartsForCrossFilter(
   filterKey: string,
-  slices: Slice[],
   slices: Record<string, Slice>,
   datasources: DatasourcesState,
 ): number[] {
   const sourceSlice = slices[filterKey];
@@ -60,7  60,7 @@ function getRelatedChartsForCrossFilter(
 
   return Object.values(slices)
     .filter(slice => {
-      if (slice.slice_id === Number[filterKey]) return false;
       if (slice.slice_id === Number(filterKey)) return false;
       const targetDatasource = datasources[slice.datasource];
       if (!targetDatasource) return false;
       if (targetDatasource === sourceDatasource) return true;
@@ -76,7  76,7 @@ function getRelatedChartsForCrossFilter(
 
 export function getRelatedCharts(
   filters: Object,
-  slices: Slice[],
   slices: Record<string, Slice>,
   datasources: DatasourcesState,
 ) {
   return Object.entries(filters).reduce((acc, [filterKey, filter]) => {
@@ -90,7  90,7 @@ export function getRelatedCharts(
         ),
       };
     }
-    if (filter.filterType === null) {
     if (!filter.filterType) {
       return {
         ...acc,
         [filterKey]: getRelatedChartsForCrossFilter(
@@ -102,7  102,11 @@ export function getRelatedCharts(
     }
     return {
       ...acc,
-      [filterKey]: filter.scope,
       [filterKey]: filter.chartsInScope
         ? filter.chartsInScope.filter(
             (chartId: number) => !filter.scope.excluded.includes(chartId),
           )
         : filter.scope,
     };
   }, {});
 }