diff --git a/superset-frontend/src/dashboard/components/Dashboard.jsx b/superset-frontend/src/dashboard/components/Dashboard.jsx index 1953889c5d418..1a852edda89d3 100644 --- a/superset-frontend/src/dashboard/components/Dashboard.jsx +++ b/superset-frontend/src/dashboard/components/Dashboard.jsx @@ -212,7 +212,7 @@ class Dashboard extends PureComponent { applyFilters() { const { appliedFilters } = this; - const { activeFilters, ownDataCharts, datasources, slices } = this.props; + const { activeFilters, ownDataCharts, slices } = this.props; // refresh charts if a filter was removed, added, or changed @@ -231,22 +231,12 @@ class Dashboard extends PureComponent { ) { // filterKey is removed? affectedChartIds.push( - ...getRelatedCharts( - appliedFilters, - activeFilters, - slices, - datasources, - )[filterKey], + ...getRelatedCharts(appliedFilters, activeFilters, slices)[filterKey], ); } else if (!appliedFilterKeys.includes(filterKey)) { // filterKey is newly added? affectedChartIds.push( - ...getRelatedCharts( - activeFilters, - appliedFilters, - slices, - datasources, - )[filterKey], + ...getRelatedCharts(activeFilters, appliedFilters, slices)[filterKey], ); } else { // if filterKey changes value, @@ -261,12 +251,9 @@ class Dashboard extends PureComponent { ) ) { affectedChartIds.push( - ...getRelatedCharts( - activeFilters, - appliedFilters, - slices, - datasources, - )[filterKey], + ...getRelatedCharts(activeFilters, appliedFilters, slices)[ + filterKey + ], ); } diff --git a/superset-frontend/src/dashboard/util/getRelatedCharts.test.ts b/superset-frontend/src/dashboard/util/getRelatedCharts.test.ts index 94762e8f780dd..874b912d90ff8 100644 --- a/superset-frontend/src/dashboard/util/getRelatedCharts.test.ts +++ b/superset-frontend/src/dashboard/util/getRelatedCharts.test.ts @@ -19,11 +19,9 @@ import { AppliedCrossFilterType, - DatasourceType, Filter, NativeFilterType, } from '@superset-ui/core'; -import { DatasourcesState } from '../types'; import { getRelatedCharts } from './getRelatedCharts'; const slices = { @@ -32,48 +30,11 @@ const slices = { '3': { datasource: 'ds1', slice_id: 3 }, } as any; -const datasources: DatasourcesState = { - ds1: { - uid: 'ds1', - datasource_name: 'ds1', - table_name: 'table1', - description: '', - id: 100, - columns: [{ column_name: 'column1' }, { column_name: 'column2' }], - column_names: ['column1', '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_names: ['column3', '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 with native filter', () => { +test('Return all chart ids in global scope with native filters', () => { const filters = { filterKey1: { filterType: 'filter_select', - chartsInScope: [1, 2], + chartsInScope: [1, 2, 3], scope: { excluded: [], rootPath: [], @@ -88,167 +49,67 @@ test('Return chart ids matching the dataset id with native filter', () => { } as unknown as Filter, }; - const result = getRelatedCharts(filters, null, slices, datasources); - expect(result).toEqual({ - filterKey1: [1], - }); -}); - -test('Return chart ids matching the dataset id with cross filter', () => { - const filters = { - '3': { - filterType: undefined, - scope: [1, 2], - targets: [], - values: null, - } as AppliedCrossFilterType, - }; - - const result = getRelatedCharts(filters, null, slices, datasources); + const result = getRelatedCharts(filters, null, slices); expect(result).toEqual({ - '3': [1], + filterKey1: [1, 2, 3], }); }); -test('Return chart ids matching the column name with native filter', () => { +test('Return only chart ids in specific scope with native filters', () => { const filters = { filterKey1: { filterType: 'filter_select', - chartsInScope: [1, 2], + chartsInScope: [1, 3], scope: { excluded: [], rootPath: [], }, targets: [ { - column: { name: 'column3' }, - datasetId: 999, + column: { name: 'column1' }, + datasetId: 100, }, ], type: NativeFilterType.NativeFilter, } as unknown as Filter, }; - const result = getRelatedCharts(filters, null, slices, datasources); + const result = getRelatedCharts(filters, null, slices); expect(result).toEqual({ - filterKey1: [2], + filterKey1: [1, 3], }); }); -test('Return chart ids matching the column name with cross filter', () => { +test('Return all chart ids with cross filter in global scope', () => { const filters = { - '1': { + '3': { filterType: undefined, - scope: [1, 2], + scope: [1, 2, 3], targets: [], - values: { - filters: [{ col: 'column3' }], - }, + values: null, } as AppliedCrossFilterType, }; - const result = getRelatedCharts(filters, null, slices, datasources); - expect(result).toEqual({ - '1': [2], - }); -}); - -test('Return chart ids when column display name matches with native filter', () => { - const filters = { - filterKey1: { - filterType: 'filter_select', - chartsInScope: [1, 2], - scope: { - excluded: [], - rootPath: [], - }, - targets: [ - { - column: { name: 'column4', displayName: 'column4' }, - datasetId: 999, - }, - ], - type: NativeFilterType.NativeFilter, - } as unknown as Filter, - }; - - const result = getRelatedCharts(filters, null, slices, datasources); + const result = getRelatedCharts(filters, null, slices); expect(result).toEqual({ - filterKey1: [2], + '3': [1, 2], }); }); -test('Return chart ids when column display name matches with cross filter', () => { +test('Return only chart ids in specific scope with cross filter', () => { const filters = { '1': { filterType: undefined, scope: [1, 2], targets: [], values: { - filters: [{ col: 'column4' }], + filters: [{ col: 'column3' }], }, } as AppliedCrossFilterType, }; - const result = getRelatedCharts(filters, null, slices, datasources); + const result = getRelatedCharts(filters, null, slices); expect(result).toEqual({ '1': [2], }); }); - -test('Return scope when filterType is not filter_select', () => { - const filters = { - filterKey1: { - filterType: 'filter_time', - chartsInScope: [3, 4], - } as Filter, - }; - - const result = getRelatedCharts(filters, null, slices, datasources); - expect(result).toEqual({ - filterKey1: [3, 4], - }); -}); - -test('Return an empty array if no matching charts found with native filter', () => { - const filters = { - filterKey1: { - filterType: 'filter_select', - chartsInScope: [1, 2], - scope: { - excluded: [], - rootPath: [], - }, - targets: [ - { - column: { name: 'nonexistent_column' }, - datasetId: 300, - }, - ], - type: NativeFilterType.NativeFilter, - } as unknown as Filter, - }; - - const result = getRelatedCharts(filters, null, slices, datasources); - expect(result).toEqual({ - filterKey1: [], - }); -}); - -test('Return an empty array if no matching charts found with cross filter', () => { - const filters = { - '1': { - filterType: undefined, - scope: [1, 2], - targets: [], - values: { - filters: [{ col: 'nonexistent_column' }], - }, - } as AppliedCrossFilterType, - }; - - const result = getRelatedCharts(filters, null, slices, datasources); - expect(result).toEqual({ - '1': [], - }); -}); diff --git a/superset-frontend/src/dashboard/util/getRelatedCharts.ts b/superset-frontend/src/dashboard/util/getRelatedCharts.ts index c019baf98eb99..a75d9a7173c99 100644 --- a/superset-frontend/src/dashboard/util/getRelatedCharts.ts +++ b/superset-frontend/src/dashboard/util/getRelatedCharts.ts @@ -20,57 +20,31 @@ import { AppliedCrossFilterType, AppliedNativeFilterType, - ensureIsArray, Filter, isAppliedCrossFilterType, isAppliedNativeFilterType, isNativeFilter, } from '@superset-ui/core'; import { Slice } from 'src/types/Chart'; -import { DatasourcesState } from '../types'; -function checkForExpression(formData?: Record) { - const groupby = ensureIsArray(formData?.groupby); - const allColumns = ensureIsArray(formData?.all_columns); - const adhocFilters = ensureIsArray(formData?.adhoc_filters); - const checkExpressions = groupby.concat(allColumns).concat(adhocFilters); - return checkExpressions.some( - (ex: string | Record) => - ex && typeof ex === 'object' && ex.expressionType === 'SQL', - ); +function isGlobalScope(scope: number[], slices: Record) { + return scope.length === Object.keys(slices).length; } function getRelatedChartsForSelectFilter( nativeFilter: AppliedNativeFilterType | Filter, slices: Record, chartsInScope: number[], - datasources: DatasourcesState, ) { return Object.values(slices) .filter(slice => { - const { datasource, slice_id } = slice; - // not in scope, ignore - if (!chartsInScope.includes(slice_id)) return false; + const { slice_id } = slice; + // all have been selected, always apply + if (isGlobalScope(chartsInScope, slices)) return true; + // hand-picked in scope, always apply + if (chartsInScope.includes(slice_id)) return true; - const chartDatasource = datasource - ? datasources[datasource] - : Object.values(datasources).find(ds => ds.id === slice.datasource_id); - - const { column, datasetId } = nativeFilter.targets?.[0] ?? {}; - const datasourceColumnNames = chartDatasource?.column_names ?? []; - - // same datasource, always apply - if (chartDatasource?.id === datasetId) return true; - - // let backend deal with adhoc columns and jinja - const hasSqlExpression = checkForExpression(slice.form_data); - if (hasSqlExpression) { - return true; - } - - return datasourceColumnNames.some( - col => col === column?.name || col === column?.displayName, - ); + return false; }) .map(slice => slice.slice_id); } @@ -79,52 +53,23 @@ function getRelatedChartsForCrossFilter( crossFilter: AppliedCrossFilterType, slices: Record, scope: number[], - datasources: DatasourcesState, ): number[] { const sourceSlice = slices[filterKey]; - const filters = crossFilter?.values?.filters ?? []; if (!sourceSlice) return []; - const sourceDatasource = sourceSlice.datasource - ? datasources[sourceSlice.datasource] - : Object.values(datasources).find( - ds => ds.id === sourceSlice.datasource_id, - ); - return Object.values(slices) .filter(slice => { // cross-filter emitter if (slice.slice_id === Number(filterKey)) return false; - // not in scope, ignore - if (!scope.includes(slice.slice_id)) return false; - - const targetDatasource = slice.datasource - ? datasources[slice.datasource] - : Object.values(datasources).find(ds => ds.id === slice.datasource_id); - - // same datasource, always apply - if (targetDatasource === sourceDatasource) return true; - - const targetColumnNames = targetDatasource?.column_names ?? []; - - // let backend deal with adhoc columns and jinja - const hasSqlExpression = checkForExpression(slice.form_data); - if (hasSqlExpression) { - return true; - } - - for (const filter of filters) { - // let backend deal with adhoc columns - if ( - typeof filter.col !== 'string' && - filter.col.expressionType !== undefined - ) { - return true; - } - // shared column names, different datasources, apply filter - if (targetColumnNames.includes(filter.col)) return true; - } + // hand-picked in the scope, always apply + const fullScope = [ + ...scope.filter(s => String(s) !== filterKey), + Number(filterKey), + ]; + if (isGlobalScope(fullScope, slices)) return true; + // hand-picked in the scope, always apply + if (scope.includes(slice.slice_id)) return true; return false; }) @@ -141,7 +86,6 @@ export function getRelatedCharts( AppliedNativeFilterType | AppliedCrossFilterType | Filter > | null, slices: Record, - datasources: DatasourcesState, ) { const related = Object.entries(filters).reduce((acc, [filterKey, filter]) => { const isCrossFilter = @@ -169,7 +113,6 @@ export function getRelatedCharts( actualCrossFilter, slices, chartsInScope, - datasources, ), }; } @@ -187,7 +130,6 @@ export function getRelatedCharts( nativeFilter, slices, chartsInScope, - datasources, ), }; } diff --git a/superset-frontend/src/dashboard/util/useFilterFocusHighlightStyles.ts b/superset-frontend/src/dashboard/util/useFilterFocusHighlightStyles.ts index 74d31f60d6567..b28d0a3771eec 100644 --- a/superset-frontend/src/dashboard/util/useFilterFocusHighlightStyles.ts +++ b/superset-frontend/src/dashboard/util/useFilterFocusHighlightStyles.ts @@ -51,8 +51,6 @@ const useFilterFocusHighlightStyles = (chartId: number) => { dashboardFilters, ); - const datasources = - useSelector((state: RootState) => state.datasources) || {}; const slices = useSelector((state: RootState) => state.sliceEntities.slices) || {}; @@ -60,7 +58,6 @@ const useFilterFocusHighlightStyles = (chartId: number) => { nativeFilters.filters as Record, null, slices, - datasources, ); const highlightedFilterId =