This is an automated email from the ASF dual-hosted git repository.
jli pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git
The following commit(s) were added to refs/heads/master by this push:
new 362948324c fix(Filters): Apply native & cross filters on common
columns (#30438)
362948324c is described below
commit 362948324c7718e74c0a9655332249c0e1328703
Author: Geido <[email protected]>
AuthorDate: Wed Oct 16 02:13:05 2024 +0300
fix(Filters): Apply native & cross filters on common columns (#30438)
---
docs/static/resources/openapi.json | 6 +
.../superset-ui-chart-controls/src/types.ts | 1 +
.../superset-ui-core/src/query/types/Dashboard.ts | 36 +++
.../test/query/types/Dashboard.test.ts | 28 +++
.../spec/fixtures/mockDashboardInfo.js | 1 +
.../spec/fixtures/mockNativeFilters.ts | 1 +
.../src/dashboard/actions/dashboardState.js | 13 +-
.../src/dashboard/components/Dashboard.jsx | 31 ++-
.../src/dashboard/components/Dashboard.test.jsx | 29 +++
.../OverwriteConfirmModal.test.tsx | 5 +
.../src/dashboard/fixtures/mockNativeFilters.ts | 1 +
.../dashboard/util/activeAllDashboardFilters.ts | 4 +
.../src/dashboard/util/crossFilters.test.ts | 4 +-
.../src/dashboard/util/crossFilters.ts | 19 +-
.../src/dashboard/util/getRelatedCharts.test.ts | 254 +++++++++++++++++++++
.../src/dashboard/util/getRelatedCharts.ts | 200 ++++++++++++++++
.../util/useFilterFocusHighlightStyles.test.tsx | 22 ++
.../util/useFilterFocusHighlightStyles.ts | 22 +-
superset-frontend/src/types/Chart.ts | 2 +
superset/connectors/sqla/models.py | 14 +-
superset/dashboards/schemas.py | 1 +
21 files changed, 656 insertions(+), 38 deletions(-)
diff --git a/docs/static/resources/openapi.json
b/docs/static/resources/openapi.json
index 43781ac965..c2cadfd1f5 100644
--- a/docs/static/resources/openapi.json
+++ b/docs/static/resources/openapi.json
@@ -3053,6 +3053,12 @@
},
"type": "array"
},
+ "column_names": {
+ "items": {
+ "type": "string"
+ },
+ "type": "array"
+ },
"currency_formats": {
"type": "object"
},
diff --git a/superset-frontend/packages/superset-ui-chart-controls/src/types.ts
b/superset-frontend/packages/superset-ui-chart-controls/src/types.ts
index d4170be17c..86e7b1c04e 100644
--- a/superset-frontend/packages/superset-ui-chart-controls/src/types.ts
+++ b/superset-frontend/packages/superset-ui-chart-controls/src/types.ts
@@ -83,6 +83,7 @@ export interface Dataset {
owners?: Owner[];
filter_select?: boolean;
filter_select_enabled?: boolean;
+ column_names?: string[];
}
export interface ControlPanelState {
diff --git
a/superset-frontend/packages/superset-ui-core/src/query/types/Dashboard.ts
b/superset-frontend/packages/superset-ui-core/src/query/types/Dashboard.ts
index cb299c3ac9..ac4b19cae5 100644
--- a/superset-frontend/packages/superset-ui-core/src/query/types/Dashboard.ts
+++ b/superset-frontend/packages/superset-ui-core/src/query/types/Dashboard.ts
@@ -80,6 +80,24 @@ export type Filter = {
description: string;
};
+export type AppliedFilter = {
+ values: {
+ filters: Record<string, any>[];
+ } | null;
+};
+
+export type AppliedCrossFilterType = {
+ filterType: undefined;
+ targets: number[];
+ scope: number[];
+} & AppliedFilter;
+
+export type AppliedNativeFilterType = {
+ filterType: 'filter_select';
+ scope: number[];
+ targets: Partial<NativeFilterTarget>[];
+} & AppliedFilter;
+
export type FilterWithDataMask = Filter & { dataMask: DataMaskWithId };
export type Divider = Partial<Omit<Filter, 'id' | 'type'>> & {
@@ -89,6 +107,24 @@ export type Divider = Partial<Omit<Filter, 'id' | 'type'>>
& {
type: typeof NativeFilterType.Divider;
};
+export function isAppliedCrossFilterType(
+ filterElement: AppliedCrossFilterType | AppliedNativeFilterType | Filter,
+): filterElement is AppliedCrossFilterType {
+ return (
+ filterElement.filterType === undefined &&
+ filterElement.hasOwnProperty('values')
+ );
+}
+
+export function isAppliedNativeFilterType(
+ filterElement: AppliedCrossFilterType | AppliedNativeFilterType | Filter,
+): filterElement is AppliedNativeFilterType {
+ return (
+ filterElement.filterType === 'filter_select' &&
+ filterElement.hasOwnProperty('values')
+ );
+}
+
export function isNativeFilter(
filterElement: Filter | Divider,
): filterElement is Filter {
diff --git
a/superset-frontend/packages/superset-ui-core/test/query/types/Dashboard.test.ts
b/superset-frontend/packages/superset-ui-core/test/query/types/Dashboard.test.ts
index 79d7980940..c1c7143957 100644
---
a/superset-frontend/packages/superset-ui-core/test/query/types/Dashboard.test.ts
+++
b/superset-frontend/packages/superset-ui-core/test/query/types/Dashboard.test.ts
@@ -24,6 +24,10 @@ import {
FilterWithDataMask,
Divider,
isNativeFilterWithDataMask,
+ isAppliedCrossFilterType,
+ isAppliedNativeFilterType,
+ AppliedCrossFilterType,
+ AppliedNativeFilterType,
} from '@superset-ui/core';
const filter: Filter = {
@@ -51,6 +55,20 @@ const filterDivider: Divider = {
description: 'Divider description.',
};
+const appliedCrossFilter: AppliedCrossFilterType = {
+ filterType: undefined,
+ targets: [1, 2],
+ scope: [1, 2],
+ values: null,
+};
+
+const appliedNativeFilter: AppliedNativeFilterType = {
+ filterType: 'filter_select',
+ scope: [1, 2],
+ targets: [{}],
+ values: null,
+};
+
test('filter type guard', () => {
expect(isNativeFilter(filter)).toBeTruthy();
expect(isNativeFilter(filterWithDataMask)).toBeTruthy();
@@ -68,3 +86,13 @@ test('filter divider type guard', () => {
expect(isFilterDivider(filterWithDataMask)).toBeFalsy();
expect(isFilterDivider(filterDivider)).toBeTruthy();
});
+
+test('applied cross filter type guard', () => {
+ expect(isAppliedCrossFilterType(appliedCrossFilter)).toBeTruthy();
+ expect(isAppliedCrossFilterType(appliedNativeFilter)).toBeFalsy();
+});
+
+test('applied native filter type guard', () => {
+ expect(isAppliedNativeFilterType(appliedNativeFilter)).toBeTruthy();
+ expect(isAppliedNativeFilterType(appliedCrossFilter)).toBeFalsy();
+});
diff --git a/superset-frontend/spec/fixtures/mockDashboardInfo.js
b/superset-frontend/spec/fixtures/mockDashboardInfo.js
index 2f747fd07b..a046a554d0 100644
--- a/superset-frontend/spec/fixtures/mockDashboardInfo.js
+++ b/superset-frontend/spec/fixtures/mockDashboardInfo.js
@@ -26,6 +26,7 @@ export default {
{
id: 'DefaultsID',
filterType: 'filter_select',
+ chartsInScope: [],
targets: [{}],
cascadeParentIds: [],
},
diff --git a/superset-frontend/spec/fixtures/mockNativeFilters.ts
b/superset-frontend/spec/fixtures/mockNativeFilters.ts
index 070f48ab06..b83cdcc8dc 100644
--- 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/actions/dashboardState.js
b/superset-frontend/src/dashboard/actions/dashboardState.js
index 9b0c181821..945dda32a3 100644
--- a/superset-frontend/src/dashboard/actions/dashboardState.js
+++ b/superset-frontend/src/dashboard/actions/dashboardState.js
@@ -61,7 +61,7 @@ import {
dashboardInfoChanged,
SAVE_CHART_CONFIG_COMPLETE,
} from './dashboardInfo';
-import { fetchDatasourceMetadata } from './datasources';
+import { fetchDatasourceMetadata, setDatasources } from './datasources';
import { updateDirectPathToFilter } from './dashboardFilters';
import { SET_FILTER_CONFIG_COMPLETE } from './nativeFilters';
import getOverwriteItems from '../util/getOverwriteItems';
@@ -341,6 +341,17 @@ export function saveDashboardRequest(data, id, saveType) {
filterConfig: metadata.native_filter_configuration,
});
}
+
+ // fetch datasets to make sure they are up to date
+ SupersetClient.get({
+ endpoint: `/api/v1/dashboard/${id}/datasets`,
+ headers: { 'Content-Type': 'application/json' },
+ }).then(({ json }) => {
+ const datasources = json?.result ?? [];
+ if (datasources.length) {
+ dispatch(setDatasources(datasources));
+ }
+ });
}
if (lastModifiedTime) {
dispatch(saveDashboardRequestSuccess(lastModifiedTime));
diff --git a/superset-frontend/src/dashboard/components/Dashboard.jsx
b/superset-frontend/src/dashboard/components/Dashboard.jsx
index 8cc0d81030..1953889c5d 100644
--- a/superset-frontend/src/dashboard/components/Dashboard.jsx
+++ b/superset-frontend/src/dashboard/components/Dashboard.jsx
@@ -41,6 +41,7 @@ import { areObjectsEqual } from '../../reduxUtils';
import getLocationHash from '../util/getLocationHash';
import isDashboardEmpty from '../util/isDashboardEmpty';
import { getAffectedOwnDataCharts } from '../util/charts/getOwnDataCharts';
+import { getRelatedCharts } from '../util/getRelatedCharts';
const propTypes = {
actions: PropTypes.shape({
@@ -211,9 +212,10 @@ class Dashboard extends PureComponent {
applyFilters() {
const { appliedFilters } = this;
- const { activeFilters, ownDataCharts } = this.props;
+ const { activeFilters, ownDataCharts, datasources, slices } = this.props;
// refresh charts if a filter was removed, added, or changed
+
const currFilterKeys = Object.keys(activeFilters);
const appliedFilterKeys = Object.keys(appliedFilters);
@@ -228,10 +230,24 @@ class Dashboard extends PureComponent {
appliedFilterKeys.includes(filterKey)
) {
// filterKey is removed?
- affectedChartIds.push(...appliedFilters[filterKey].scope);
+ affectedChartIds.push(
+ ...getRelatedCharts(
+ appliedFilters,
+ activeFilters,
+ slices,
+ datasources,
+ )[filterKey],
+ );
} else if (!appliedFilterKeys.includes(filterKey)) {
// filterKey is newly added?
- affectedChartIds.push(...activeFilters[filterKey].scope);
+ affectedChartIds.push(
+ ...getRelatedCharts(
+ activeFilters,
+ appliedFilters,
+ slices,
+ datasources,
+ )[filterKey],
+ );
} else {
// if filterKey changes value,
// update charts in its scope
@@ -244,7 +260,14 @@ class Dashboard extends PureComponent {
},
)
) {
- affectedChartIds.push(...activeFilters[filterKey].scope);
+ affectedChartIds.push(
+ ...getRelatedCharts(
+ activeFilters,
+ appliedFilters,
+ slices,
+ datasources,
+ )[filterKey],
+ );
}
// if filterKey changes scope,
diff --git a/superset-frontend/src/dashboard/components/Dashboard.test.jsx
b/superset-frontend/src/dashboard/components/Dashboard.test.jsx
index b33fe9f638..cd76787a21 100644
--- 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,11 +177,27 @@ describe('Dashboard', () => {
[NATIVE_FILTER_ID]: {
scope: [230],
values: extraFormData,
+ filterType: 'filter_select',
+ targets: [
+ {
+ datasetId: 13,
+ column: {
+ name: 'ethnic_minority',
+ },
+ },
+ ],
},
});
});
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] },
};
@@ -186,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/components/OverwriteConfirm/OverwriteConfirmModal.test.tsx
b/superset-frontend/src/dashboard/components/OverwriteConfirm/OverwriteConfirmModal.test.tsx
index 01e9e825bf..5d93fd6900 100644
---
a/superset-frontend/src/dashboard/components/OverwriteConfirm/OverwriteConfirmModal.test.tsx
+++
b/superset-frontend/src/dashboard/components/OverwriteConfirm/OverwriteConfirmModal.test.tsx
@@ -51,6 +51,11 @@ test('renders diff viewer when it contains
overwriteConfirmMetadata', async () =
test('requests update dashboard api when save button is clicked', async () => {
const updateDashboardEndpoint =
`glob:*/api/v1/dashboard/${overwriteConfirmMetadata.dashboardId}`;
+ const fetchDatasetsEndpoint =
`glob:*/api/v1/dashboard/${overwriteConfirmMetadata.dashboardId}/datasets`;
+
+ // mock fetch datasets
+ fetchMock.get(fetchDatasetsEndpoint, []);
+
fetchMock.put(updateDashboardEndpoint, {
id: overwriteConfirmMetadata.dashboardId,
last_modified_time: +new Date(),
diff --git a/superset-frontend/src/dashboard/fixtures/mockNativeFilters.ts
b/superset-frontend/src/dashboard/fixtures/mockNativeFilters.ts
index d1d37cbf6a..32f54cbe26 100644
--- a/superset-frontend/src/dashboard/fixtures/mockNativeFilters.ts
+++ b/superset-frontend/src/dashboard/fixtures/mockNativeFilters.ts
@@ -39,6 +39,7 @@ export const nativeFiltersInfo: NativeFiltersState = {
id: 'DefaultsID',
name: 'test',
filterType: 'filter_select',
+ chartsInScope: [],
targets: [
{
datasetId: 0,
diff --git a/superset-frontend/src/dashboard/util/activeAllDashboardFilters.ts
b/superset-frontend/src/dashboard/util/activeAllDashboardFilters.ts
index f9e98e8536..3bde1d2e6f 100644
--- a/superset-frontend/src/dashboard/util/activeAllDashboardFilters.ts
+++ b/superset-frontend/src/dashboard/util/activeAllDashboardFilters.ts
@@ -54,9 +54,13 @@ export const getAllActiveFilters = ({
chartConfiguration?.[filterId]?.crossFilters?.chartsInScope ??
allSliceIds ??
[];
+ const filterType = nativeFilters?.[filterId]?.filterType;
+ const targets = nativeFilters?.[filterId]?.targets ?? scope;
// Iterate over all roots to find all affected charts
activeFilters[filterId] = {
scope,
+ filterType,
+ targets,
values: extraFormData,
};
});
diff --git a/superset-frontend/src/dashboard/util/crossFilters.test.ts
b/superset-frontend/src/dashboard/util/crossFilters.test.ts
index e811856a4a..b88579cc84 100644
--- a/superset-frontend/src/dashboard/util/crossFilters.test.ts
+++ b/superset-frontend/src/dashboard/util/crossFilters.test.ts
@@ -294,14 +294,14 @@ test('Recalculate charts in global filter scope when
charts change', () => {
id: 2,
crossFilters: {
scope: 'global',
- chartsInScope: [1],
+ chartsInScope: [1, 3],
},
},
'3': {
id: 3,
crossFilters: {
scope: 'global',
- chartsInScope: [],
+ chartsInScope: [1, 2],
},
},
},
diff --git a/superset-frontend/src/dashboard/util/crossFilters.ts
b/superset-frontend/src/dashboard/util/crossFilters.ts
index 880b826870..903a1f74fb 100644
--- a/superset-frontend/src/dashboard/util/crossFilters.ts
+++ b/superset-frontend/src/dashboard/util/crossFilters.ts
@@ -52,20 +52,6 @@ export const getCrossFiltersConfiguration = (
return undefined;
}
- const chartsByDataSource: Record<string, Set<number>> = Object.values(
- charts,
- ).reduce((acc: Record<string, Set<number>>, chart) => {
- if (!chart.form_data) {
- return acc;
- }
- const { datasource } = chart.form_data;
- if (!acc[datasource]) {
- acc[datasource] = new Set();
- }
- acc[datasource].add(chart.id);
- return acc;
- }, {});
-
const globalChartConfiguration = metadata.global_chart_configuration?.scope
? {
scope: metadata.global_chart_configuration.scope,
@@ -111,13 +97,10 @@ export const getCrossFiltersConfiguration = (
},
};
}
- const chartDataSource = charts[chartId].form_data.datasource;
chartConfiguration[chartId].crossFilters.chartsInScope =
isCrossFilterScopeGlobal(chartConfiguration[chartId].crossFilters.scope)
? globalChartConfiguration.chartsInScope.filter(
- id =>
- id !== Number(chartId) &&
- chartsByDataSource[chartDataSource]?.has(id),
+ id => id !== Number(chartId),
)
: getChartIdsInFilterScope(
chartConfiguration[chartId].crossFilters.scope,
diff --git a/superset-frontend/src/dashboard/util/getRelatedCharts.test.ts
b/superset-frontend/src/dashboard/util/getRelatedCharts.test.ts
new file mode 100644
index 0000000000..94762e8f78
--- /dev/null
+++ b/superset-frontend/src/dashboard/util/getRelatedCharts.test.ts
@@ -0,0 +1,254 @@
+/**
+ * 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 {
+ AppliedCrossFilterType,
+ DatasourceType,
+ Filter,
+ NativeFilterType,
+} from '@superset-ui/core';
+import { DatasourcesState } from '../types';
+import { getRelatedCharts } from './getRelatedCharts';
+
+const slices = {
+ '1': { datasource: 'ds1', slice_id: 1 },
+ '2': { datasource: 'ds2', slice_id: 2 },
+ '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', () => {
+ const filters = {
+ filterKey1: {
+ filterType: 'filter_select',
+ chartsInScope: [1, 2],
+ scope: {
+ excluded: [],
+ rootPath: [],
+ },
+ targets: [
+ {
+ column: { name: 'column1' },
+ datasetId: 100,
+ },
+ ],
+ type: NativeFilterType.NativeFilter,
+ } 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);
+ expect(result).toEqual({
+ '3': [1],
+ });
+});
+
+test('Return chart ids matching the column name with native filter', () => {
+ const filters = {
+ filterKey1: {
+ filterType: 'filter_select',
+ chartsInScope: [1, 2],
+ scope: {
+ excluded: [],
+ rootPath: [],
+ },
+ targets: [
+ {
+ column: { name: 'column3' },
+ datasetId: 999,
+ },
+ ],
+ type: NativeFilterType.NativeFilter,
+ } as unknown as Filter,
+ };
+
+ const result = getRelatedCharts(filters, null, slices, datasources);
+ expect(result).toEqual({
+ filterKey1: [2],
+ });
+});
+
+test('Return chart ids matching the column name with cross filter', () => {
+ const filters = {
+ '1': {
+ filterType: undefined,
+ scope: [1, 2],
+ targets: [],
+ values: {
+ filters: [{ col: 'column3' }],
+ },
+ } 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);
+ expect(result).toEqual({
+ filterKey1: [2],
+ });
+});
+
+test('Return chart ids when column display name matches with cross filter', ()
=> {
+ const filters = {
+ '1': {
+ filterType: undefined,
+ scope: [1, 2],
+ targets: [],
+ values: {
+ filters: [{ col: 'column4' }],
+ },
+ } as AppliedCrossFilterType,
+ };
+
+ const result = getRelatedCharts(filters, null, slices, datasources);
+ 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
new file mode 100644
index 0000000000..aba56ba3b6
--- /dev/null
+++ b/superset-frontend/src/dashboard/util/getRelatedCharts.ts
@@ -0,0 +1,200 @@
+/**
+ * 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 {
+ 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<string, any>) {
+ const groupby = ensureIsArray(formData?.groupby) ?? [];
+ const allColumns = ensureIsArray(formData?.all_columns) ?? [];
+ const checkColumns = groupby.concat(allColumns);
+ return checkColumns.some(
+ (col: string | Record<string, any>) =>
+ typeof col !== 'string' && col.expressionType !== undefined,
+ );
+}
+
+function getRelatedChartsForSelectFilter(
+ nativeFilter: AppliedNativeFilterType | Filter,
+ slices: Record<string, Slice>,
+ 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 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,
+ );
+ })
+ .map(slice => slice.slice_id);
+}
+function getRelatedChartsForCrossFilter(
+ filterKey: string,
+ crossFilter: AppliedCrossFilterType,
+ slices: Record<string, Slice>,
+ 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;
+ }
+
+ return false;
+ })
+ .map(slice => slice.slice_id);
+}
+
+export function getRelatedCharts(
+ filters: Record<
+ string,
+ AppliedNativeFilterType | AppliedCrossFilterType | Filter
+ >,
+ checkFilters: Record<
+ string,
+ AppliedNativeFilterType | AppliedCrossFilterType | Filter
+ > | null,
+ slices: Record<string, Slice>,
+ datasources: DatasourcesState,
+) {
+ const related = Object.entries(filters).reduce((acc, [filterKey, filter]) =>
{
+ const isCrossFilter =
+ Object.keys(slices).includes(filterKey) &&
+ isAppliedCrossFilterType(filter);
+
+ const chartsInScope = Array.isArray(filter.scope)
+ ? filter.scope
+ : ((filter as Filter).chartsInScope ?? []);
+
+ if (isCrossFilter) {
+ const checkFilter = checkFilters?.[filterKey] as AppliedCrossFilterType;
+ const crossFilter = filter as AppliedCrossFilterType;
+ const wasRemoved = !!(
+ ((filter.values && filter.values.filters === undefined) ||
+ filter.values?.filters?.length === 0) &&
+ checkFilter?.values?.filters?.length
+ );
+ const actualCrossFilter = wasRemoved ? checkFilter : crossFilter;
+
+ return {
+ ...acc,
+ [filterKey]: getRelatedChartsForCrossFilter(
+ filterKey,
+ actualCrossFilter,
+ slices,
+ chartsInScope,
+ datasources,
+ ),
+ };
+ }
+
+ const nativeFilter = filter as AppliedNativeFilterType | Filter;
+ // on highlight, a standard native filter is passed
+ // on apply, an applied native filter is passed
+ if (
+ isAppliedNativeFilterType(nativeFilter) ||
+ isNativeFilter(nativeFilter)
+ ) {
+ return {
+ ...acc,
+ [filterKey]: getRelatedChartsForSelectFilter(
+ nativeFilter,
+ slices,
+ chartsInScope,
+ datasources,
+ ),
+ };
+ }
+ return {
+ ...acc,
+ [filterKey]: chartsInScope,
+ };
+ }, {});
+
+ return related;
+}
diff --git
a/superset-frontend/src/dashboard/util/useFilterFocusHighlightStyles.test.tsx
b/superset-frontend/src/dashboard/util/useFilterFocusHighlightStyles.test.tsx
index d3010376da..ca19c63d51 100644
---
a/superset-frontend/src/dashboard/util/useFilterFocusHighlightStyles.test.tsx
+++
b/superset-frontend/src/dashboard/util/useFilterFocusHighlightStyles.test.tsx
@@ -24,6 +24,9 @@ import reducerIndex from 'spec/helpers/reducerIndex';
import { screen, render } from 'spec/helpers/testing-library';
import { initialState } from 'src/SqlLab/fixtures';
import useFilterFocusHighlightStyles from './useFilterFocusHighlightStyles';
+import { getRelatedCharts } from './getRelatedCharts';
+
+jest.mock('./getRelatedCharts');
const TestComponent = ({ chartId }: { chartId: number }) => {
const styles = useFilterFocusHighlightStyles(chartId);
@@ -38,6 +41,7 @@ describe('useFilterFocusHighlightStyles', () => {
{ ...mockState, ...(initialState as any), ...customState },
compose(applyMiddleware(thunk)),
);
+ const mockGetRelatedCharts = getRelatedCharts as jest.Mock;
const renderWrapper = (chartId: number, store = createMockStore()) =>
render(<TestComponent chartId={chartId} />, {
@@ -57,6 +61,9 @@ describe('useFilterFocusHighlightStyles', () => {
});
it('should return unfocused styles if chart is not in scope of focused
native filter', async () => {
+ mockGetRelatedCharts.mockReturnValue({
+ 'test-filter': [],
+ });
const store = createMockStore({
nativeFilters: {
focusedFilterId: 'test-filter',
@@ -76,6 +83,9 @@ describe('useFilterFocusHighlightStyles', () => {
});
it('should return unfocused styles if chart is not in scope of hovered
native filter', async () => {
+ mockGetRelatedCharts.mockReturnValue({
+ 'test-filter': [],
+ });
const store = createMockStore({
nativeFilters: {
hoveredFilterId: 'test-filter',
@@ -96,6 +106,9 @@ describe('useFilterFocusHighlightStyles', () => {
it('should return focused styles if chart is in scope of focused native
filter', async () => {
const chartId = 18;
+ mockGetRelatedCharts.mockReturnValue({
+ testFilter: [chartId],
+ });
const store = createMockStore({
nativeFilters: {
focusedFilterId: 'testFilter',
@@ -116,6 +129,9 @@ describe('useFilterFocusHighlightStyles', () => {
it('should return focused styles if chart is in scope of hovered native
filter', async () => {
const chartId = 18;
+ mockGetRelatedCharts.mockReturnValue({
+ testFilter: [chartId],
+ });
const store = createMockStore({
nativeFilters: {
hoveredFilterId: 'testFilter',
@@ -136,6 +152,9 @@ describe('useFilterFocusHighlightStyles', () => {
it('should return unfocused styles if focusedFilterField is targeting a
different chart', async () => {
const chartId = 18;
+ mockGetRelatedCharts.mockReturnValue({
+ testFilter: [],
+ });
const store = createMockStore({
dashboardState: {
focusedFilterField: {
@@ -159,6 +178,9 @@ describe('useFilterFocusHighlightStyles', () => {
it('should return focused styles if focusedFilterField chart equals our
own', async () => {
const chartId = 18;
+ mockGetRelatedCharts.mockReturnValue({
+ testFilter: [chartId],
+ });
const store = createMockStore({
dashboardState: {
focusedFilterField: {
diff --git
a/superset-frontend/src/dashboard/util/useFilterFocusHighlightStyles.ts
b/superset-frontend/src/dashboard/util/useFilterFocusHighlightStyles.ts
index f1f428240c..74d31f60d6 100644
--- a/superset-frontend/src/dashboard/util/useFilterFocusHighlightStyles.ts
+++ b/superset-frontend/src/dashboard/util/useFilterFocusHighlightStyles.ts
@@ -16,11 +16,12 @@
* specific language governing permissions and limitations
* under the License.
*/
-import { useTheme } from '@superset-ui/core';
+import { Filter, useTheme } from '@superset-ui/core';
import { useSelector } from 'react-redux';
import { getChartIdsInFilterScope } from
'src/dashboard/util/activeDashboardFilters';
import { DashboardState, RootState } from 'src/dashboard/types';
+import { getRelatedCharts } from './getRelatedCharts';
const selectFocusedFilterScope = (
dashboardState: DashboardState,
@@ -41,6 +42,7 @@ const useFilterFocusHighlightStyles = (chartId: number) => {
const dashboardState = useSelector(
(state: RootState) => state.dashboardState,
);
+
const dashboardFilters = useSelector(
(state: RootState) => state.dashboardFilters,
);
@@ -49,6 +51,18 @@ const useFilterFocusHighlightStyles = (chartId: number) => {
dashboardFilters,
);
+ const datasources =
+ useSelector((state: RootState) => state.datasources) || {};
+ const slices =
+ useSelector((state: RootState) => state.sliceEntities.slices) || {};
+
+ const relatedCharts = getRelatedCharts(
+ nativeFilters.filters as Record<string, Filter>,
+ null,
+ slices,
+ datasources,
+ );
+
const highlightedFilterId =
nativeFilters?.focusedFilterId || nativeFilters?.hoveredFilterId;
if (!(focusedFilterScope || highlightedFilterId)) {
@@ -69,11 +83,7 @@ const useFilterFocusHighlightStyles = (chartId: number) => {
};
if (highlightedFilterId) {
- if (
- nativeFilters.filters[highlightedFilterId]?.chartsInScope?.includes(
- chartId,
- )
- ) {
+ if (relatedCharts[highlightedFilterId]?.includes(chartId)) {
return focusedChartStyles;
}
} else if (
diff --git a/superset-frontend/src/types/Chart.ts
b/superset-frontend/src/types/Chart.ts
index f26525cd33..b7c439cb21 100644
--- a/superset-frontend/src/types/Chart.ts
+++ b/superset-frontend/src/types/Chart.ts
@@ -74,6 +74,8 @@ export type Slice = {
query_context?: object;
is_managed_externally: boolean;
owners?: number[];
+ datasource?: string;
+ datasource_id?: number;
};
export default Chart;
diff --git a/superset/connectors/sqla/models.py
b/superset/connectors/sqla/models.py
index 82980cef16..e8aa0d705b 100644
--- a/superset/connectors/sqla/models.py
+++ b/superset/connectors/sqla/models.py
@@ -489,6 +489,11 @@ class BaseDatasource(AuditMixinNullable,
ImportExportMixin): # pylint: disable=
del data["description"]
data.update({"metrics": filtered_metrics})
data.update({"columns": filtered_columns})
+
+ all_columns = {
+ column_["column_name"]: column_["verbose_name"] or
column_["column_name"]
+ for column_ in filtered_columns
+ }
verbose_map = {"__timestamp": "Time"}
verbose_map.update(
{
@@ -496,14 +501,9 @@ class BaseDatasource(AuditMixinNullable,
ImportExportMixin): # pylint: disable=
for metric in filtered_metrics
}
)
- verbose_map.update(
- {
- column_["column_name"]: column_["verbose_name"]
- or column_["column_name"]
- for column_ in filtered_columns
- }
- )
+ verbose_map.update(all_columns)
data["verbose_map"] = verbose_map
+ data["column_names"] = set(all_columns.values()) |
set(self.column_names)
return data
diff --git a/superset/dashboards/schemas.py b/superset/dashboards/schemas.py
index b0a47aba41..d63e79336c 100644
--- a/superset/dashboards/schemas.py
+++ b/superset/dashboards/schemas.py
@@ -262,6 +262,7 @@ class DashboardDatasetSchema(Schema):
owners = fields.List(fields.Dict())
columns = fields.List(fields.Dict())
column_types = fields.List(fields.Int())
+ column_names = fields.List(fields.Str())
metrics = fields.List(fields.Dict())
order_by_choices = fields.List(fields.List(fields.Str()))
verbose_map = fields.Dict(fields.Str(), fields.Str())
