You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by kg...@apache.org on 2023/03/01 10:47:29 UTC
[superset] branch master updated: fix(dashboard): Cross-filters not working properly for new dashboards (#23194)
This is an automated email from the ASF dual-hosted git repository.
kgabryje 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 7196e87833 fix(dashboard): Cross-filters not working properly for new dashboards (#23194)
7196e87833 is described below
commit 7196e878332ed57eb192b0cee560e2831ab077b0
Author: Kamil Gabryjelski <ka...@gmail.com>
AuthorDate: Wed Mar 1 11:47:15 2023 +0100
fix(dashboard): Cross-filters not working properly for new dashboards (#23194)
---
.../src/dashboard/actions/dashboardState.js | 25 +--
superset-frontend/src/dashboard/actions/hydrate.js | 66 ++-----
.../src/dashboard/util/crossFilters.test.ts | 207 +++++++++++++++++++++
.../src/dashboard/util/crossFilters.ts | 61 +++++-
4 files changed, 288 insertions(+), 71 deletions(-)
diff --git a/superset-frontend/src/dashboard/actions/dashboardState.js b/superset-frontend/src/dashboard/actions/dashboardState.js
index 8db1a500ee..058b3700bf 100644
--- a/superset-frontend/src/dashboard/actions/dashboardState.js
+++ b/superset-frontend/src/dashboard/actions/dashboardState.js
@@ -36,7 +36,10 @@ import {
SAVE_TYPE_OVERWRITE,
SAVE_TYPE_OVERWRITE_CONFIRMED,
} from 'src/dashboard/util/constants';
-import { isCrossFiltersEnabled } from 'src/dashboard/util/crossFilters';
+import {
+ getCrossFiltersConfiguration,
+ isCrossFiltersEnabled,
+} from 'src/dashboard/util/crossFilters';
import {
addSuccessToast,
addWarningToast,
@@ -277,25 +280,17 @@ export function saveDashboardRequest(data, id, saveType) {
const handleChartConfiguration = () => {
const {
+ dashboardLayout,
+ charts,
dashboardInfo: {
metadata: { chart_configuration = {} },
},
} = getState();
- const chartConfiguration = Object.values(chart_configuration).reduce(
- (prev, next) => {
- // If chart removed from dashboard - remove it from metadata
- if (
- Object.values(layout).find(
- layoutItem => layoutItem?.meta?.chartId === next.id,
- )
- ) {
- return { ...prev, [next.id]: next };
- }
- return prev;
- },
- {},
+ return getCrossFiltersConfiguration(
+ dashboardLayout.present,
+ chart_configuration,
+ charts,
);
- return chartConfiguration;
};
const onCopySuccess = response => {
diff --git a/superset-frontend/src/dashboard/actions/hydrate.js b/superset-frontend/src/dashboard/actions/hydrate.js
index 2b550e0783..8625a2952a 100644
--- a/superset-frontend/src/dashboard/actions/hydrate.js
+++ b/superset-frontend/src/dashboard/actions/hydrate.js
@@ -16,9 +16,6 @@
* specific language governing permissions and limitations
* under the License.
*/
-/* eslint-disable camelcase */
-import { Behavior, getChartMetadataRegistry } from '@superset-ui/core';
-
import { chart } from 'src/components/Chart/chartReducer';
import { initSliceEntities } from 'src/dashboard/reducers/sliceEntities';
import { getInitialState as getInitialNativeFilterState } from 'src/dashboard/reducers/nativeFilters';
@@ -26,7 +23,10 @@ import { applyDefaultFormData } from 'src/explore/store';
import { buildActiveFilters } from 'src/dashboard/util/activeDashboardFilters';
import { findPermission } from 'src/utils/findPermission';
import { canUserEditDashboard } from 'src/dashboard/util/permissionUtils';
-import { isCrossFiltersEnabled } from 'src/dashboard/util/crossFilters';
+import {
+ getCrossFiltersConfiguration,
+ isCrossFiltersEnabled,
+} from 'src/dashboard/util/crossFilters';
import {
DASHBOARD_FILTER_SCOPE_GLOBAL,
dashboardFilter,
@@ -54,7 +54,6 @@ import { ResourceStatus } from 'src/hooks/apiResources/apiResources';
import { FeatureFlag, isFeatureEnabled } from '../../featureFlags';
import extractUrlParams from '../util/extractUrlParams';
import { updateColorSchema } from './dashboardInfo';
-import { getChartIdsInFilterScope } from '../util/getChartIdsInFilterScope';
import updateComponentParentsList from '../util/updateComponentParentsList';
import { FilterBarOrientation } from '../types';
@@ -124,7 +123,7 @@ export const hydrateDashboard =
charts.forEach(slice => {
const key = slice.slice_id;
- const form_data = {
+ const formData = {
...slice.form_data,
url_params: {
...slice.form_data.url_params,
@@ -134,7 +133,7 @@ export const hydrateDashboard =
chartQueries[key] = {
...chart,
id: key,
- form_data: applyDefaultFormData(form_data),
+ form_data: applyDefaultFormData(formData),
};
slices[key] = {
@@ -311,54 +310,11 @@ export const hydrateDashboard =
);
if (isFeatureEnabled(FeatureFlag.DASHBOARD_CROSS_FILTERS)) {
- // If user just added cross filter to dashboard it's not saving it scope on server,
- // so we tweak it until user will update scope and will save it in server
- Object.values(dashboardLayout.present).forEach(layoutItem => {
- const chartId = layoutItem.meta?.chartId;
- const behaviors =
- (
- getChartMetadataRegistry().get(
- chartQueries[chartId]?.form_data?.viz_type,
- ) ?? {}
- )?.behaviors ?? [];
-
- if (!metadata.chart_configuration) {
- metadata.chart_configuration = {};
- }
- if (behaviors.includes(Behavior.INTERACTIVE_CHART)) {
- if (!metadata.chart_configuration[chartId]) {
- metadata.chart_configuration[chartId] = {
- id: chartId,
- crossFilters: {
- scope: {
- rootPath: [DASHBOARD_ROOT_ID],
- excluded: [chartId], // By default it doesn't affects itself
- },
- },
- };
- }
- metadata.chart_configuration[chartId].crossFilters.chartsInScope =
- getChartIdsInFilterScope(
- metadata.chart_configuration[chartId].crossFilters.scope,
- chartQueries,
- dashboardLayout.present,
- );
- }
- if (
- behaviors.includes(Behavior.INTERACTIVE_CHART) &&
- !metadata.chart_configuration[chartId]
- ) {
- metadata.chart_configuration[chartId] = {
- id: chartId,
- crossFilters: {
- scope: {
- rootPath: [DASHBOARD_ROOT_ID],
- excluded: [chartId], // By default it doesn't affects itself
- },
- },
- };
- }
- });
+ metadata.chart_configuration = getCrossFiltersConfiguration(
+ dashboardLayout.present,
+ metadata.chart_configuration,
+ chartQueries,
+ );
}
const { roles } = user;
diff --git a/superset-frontend/src/dashboard/util/crossFilters.test.ts b/superset-frontend/src/dashboard/util/crossFilters.test.ts
new file mode 100644
index 0000000000..9dbdac4d24
--- /dev/null
+++ b/superset-frontend/src/dashboard/util/crossFilters.test.ts
@@ -0,0 +1,207 @@
+/**
+ * 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 sinon from 'sinon';
+import { Behavior, FeatureFlag } from '@superset-ui/core';
+import * as core from '@superset-ui/core';
+import { getCrossFiltersConfiguration } from './crossFilters';
+
+const DASHBOARD_LAYOUT = {
+ 'CHART-1': {
+ children: [],
+ id: 'CHART-1',
+ meta: {
+ chartId: 1,
+ sliceName: 'Test chart 1',
+ height: 1,
+ width: 1,
+ uuid: '1',
+ },
+ parents: ['ROOT_ID', 'GRID_ID', 'ROW-6XUMf1rV76'],
+ type: 'CHART',
+ },
+ 'CHART-2': {
+ children: [],
+ id: 'CHART-2',
+ meta: {
+ chartId: 2,
+ sliceName: 'Test chart 2',
+ height: 1,
+ width: 1,
+ uuid: '2',
+ },
+ parents: ['ROOT_ID', 'GRID_ID', 'ROW-6XUMf1rV76'],
+ type: 'CHART',
+ },
+};
+
+const CHARTS = {
+ '1': {
+ id: 1,
+ form_data: {
+ datasource: '2__table',
+ viz_type: 'echarts_timeseries_line',
+ slice_id: 1,
+ },
+ chartAlert: null,
+ chartStatus: 'rendered' as const,
+ chartUpdateEndTime: 0,
+ chartUpdateStartTime: 0,
+ lastRendered: 0,
+ latestQueryFormData: {},
+ sliceFormData: {
+ datasource: '2__table',
+ viz_type: 'echarts_timeseries_line',
+ },
+ queryController: null,
+ queriesResponse: [{}],
+ triggerQuery: false,
+ },
+ '2': {
+ id: 2,
+ form_data: {
+ datasource: '2__table',
+ viz_type: 'echarts_timeseries_line',
+ slice_id: 2,
+ },
+ chartAlert: null,
+ chartStatus: 'rendered' as const,
+ chartUpdateEndTime: 0,
+ chartUpdateStartTime: 0,
+ lastRendered: 0,
+ latestQueryFormData: {},
+ sliceFormData: {
+ datasource: '2__table',
+ viz_type: 'echarts_timeseries_line',
+ },
+ queryController: null,
+ queriesResponse: [{}],
+ triggerQuery: false,
+ },
+};
+
+const INITIAL_CHART_CONFIG = {
+ '1': {
+ id: 1,
+ crossFilters: {
+ scope: {
+ rootPath: ['ROOT_ID'],
+ excluded: [1],
+ },
+ chartsInScope: [2],
+ },
+ },
+};
+
+test('Generate correct cross filters configuration without initial configuration', () => {
+ // @ts-ignore
+ global.featureFlags = {
+ [FeatureFlag.DASHBOARD_CROSS_FILTERS]: true,
+ };
+ const metadataRegistryStub = sinon
+ .stub(core, 'getChartMetadataRegistry')
+ .callsFake(() => ({
+ // @ts-ignore
+ get: () => ({
+ behaviors: [Behavior.INTERACTIVE_CHART],
+ }),
+ }));
+ expect(
+ getCrossFiltersConfiguration(DASHBOARD_LAYOUT, undefined, CHARTS),
+ ).toEqual({
+ '1': {
+ id: 1,
+ crossFilters: {
+ scope: {
+ rootPath: ['ROOT_ID'],
+ excluded: [1],
+ },
+ chartsInScope: [2],
+ },
+ },
+ '2': {
+ id: 2,
+ crossFilters: {
+ scope: {
+ rootPath: ['ROOT_ID'],
+ excluded: [2],
+ },
+ chartsInScope: [1],
+ },
+ },
+ });
+ metadataRegistryStub.restore();
+});
+
+test('Generate correct cross filters configuration with initial configuration', () => {
+ // @ts-ignore
+ global.featureFlags = {
+ [FeatureFlag.DASHBOARD_CROSS_FILTERS]: true,
+ };
+ const metadataRegistryStub = sinon
+ .stub(core, 'getChartMetadataRegistry')
+ .callsFake(() => ({
+ // @ts-ignore
+ get: () => ({
+ behaviors: [Behavior.INTERACTIVE_CHART],
+ }),
+ }));
+ expect(
+ getCrossFiltersConfiguration(
+ DASHBOARD_LAYOUT,
+ INITIAL_CHART_CONFIG,
+ CHARTS,
+ ),
+ ).toEqual({
+ '1': {
+ id: 1,
+ crossFilters: {
+ scope: {
+ rootPath: ['ROOT_ID'],
+ excluded: [1],
+ },
+ chartsInScope: [2],
+ },
+ },
+ '2': {
+ id: 2,
+ crossFilters: {
+ scope: {
+ rootPath: ['ROOT_ID'],
+ excluded: [2],
+ },
+ chartsInScope: [1],
+ },
+ },
+ });
+ metadataRegistryStub.restore();
+});
+
+test('Return undefined if DASHBOARD_CROSS_FILTERS feature flag is disabled', () => {
+ // @ts-ignore
+ global.featureFlags = {
+ [FeatureFlag.DASHBOARD_CROSS_FILTERS]: false,
+ };
+ expect(
+ getCrossFiltersConfiguration(
+ DASHBOARD_LAYOUT,
+ INITIAL_CHART_CONFIG,
+ CHARTS,
+ ),
+ ).toEqual(undefined);
+});
diff --git a/superset-frontend/src/dashboard/util/crossFilters.ts b/superset-frontend/src/dashboard/util/crossFilters.ts
index 061ed82634..862db89798 100644
--- a/superset-frontend/src/dashboard/util/crossFilters.ts
+++ b/superset-frontend/src/dashboard/util/crossFilters.ts
@@ -17,10 +17,69 @@
* under the License.
*/
-import { FeatureFlag, isFeatureEnabled } from '@superset-ui/core';
+import {
+ Behavior,
+ FeatureFlag,
+ getChartMetadataRegistry,
+ isDefined,
+ isFeatureEnabled,
+} from '@superset-ui/core';
+import { DASHBOARD_ROOT_ID } from './constants';
+import { getChartIdsInFilterScope } from './getChartIdsInFilterScope';
+import { ChartsState, DashboardInfo, DashboardLayout } from '../types';
export const isCrossFiltersEnabled = (
metadataCrossFiltersEnabled: boolean | undefined,
): boolean =>
isFeatureEnabled(FeatureFlag.DASHBOARD_CROSS_FILTERS) &&
(metadataCrossFiltersEnabled === undefined || metadataCrossFiltersEnabled);
+
+export const getCrossFiltersConfiguration = (
+ dashboardLayout: DashboardLayout,
+ initialConfig: DashboardInfo['metadata']['chart_configuration'] = {},
+ charts: ChartsState,
+) => {
+ if (!isFeatureEnabled(FeatureFlag.DASHBOARD_CROSS_FILTERS)) {
+ return undefined;
+ }
+ // If user just added cross filter to dashboard it's not saving it scope on server,
+ // so we tweak it until user will update scope and will save it in server
+ const chartConfiguration = {};
+ Object.values(dashboardLayout).forEach(layoutItem => {
+ const chartId = layoutItem.meta?.chartId;
+
+ if (!isDefined(chartId)) {
+ return;
+ }
+
+ const behaviors =
+ (
+ getChartMetadataRegistry().get(charts[chartId]?.form_data?.viz_type) ??
+ {}
+ )?.behaviors ?? [];
+
+ if (behaviors.includes(Behavior.INTERACTIVE_CHART)) {
+ if (initialConfig[chartId]) {
+ chartConfiguration[chartId] = initialConfig[chartId];
+ }
+ if (!chartConfiguration[chartId]) {
+ chartConfiguration[chartId] = {
+ id: chartId,
+ crossFilters: {
+ scope: {
+ rootPath: [DASHBOARD_ROOT_ID],
+ excluded: [chartId], // By default it doesn't affects itself
+ },
+ },
+ };
+ }
+ chartConfiguration[chartId].crossFilters.chartsInScope =
+ getChartIdsInFilterScope(
+ chartConfiguration[chartId].crossFilters.scope,
+ charts,
+ dashboardLayout,
+ );
+ }
+ });
+ return chartConfiguration;
+};