diff --git a/superset-frontend/src/dashboard/actions/dashboardState.js b/superset-frontend/src/dashboard/actions/dashboardState.js index 8db1a500eee32..058b3700bf44f 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 2b550e0783f7d..8625a2952ac13 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 0000000000000..9dbdac4d241f5 --- /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 061ed82634ac9..862db89798f81 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; +};