From 567e4b4d3a1f26d3162ec348793db42f864d7363 Mon Sep 17 00:00:00 2001 From: Kamil Gabryjelski Date: Sun, 7 May 2023 13:37:13 +0200 Subject: [PATCH 01/11] feat: Cross filters scoping modal --- .../DropdownSelectableIcon/index.tsx | 37 +- .../src/dashboard/actions/dashboardInfo.ts | 49 +- .../src/dashboard/actions/dashboardState.js | 30 +- .../src/dashboard/actions/hydrate.js | 13 +- .../DashboardBuilder/DashboardContainer.tsx | 9 +- .../components/SliceHeaderControls/index.tsx | 474 +++++++++--------- .../useCrossFiltersScopingModal.tsx | 160 ++++++ .../FilterBarSettings.test.tsx | 6 +- .../FilterBar/FilterBarSettings/index.tsx | 162 +++--- .../FilterCard/useFilterScope.ts | 37 +- .../components/nativeFilters/selectors.ts | 7 +- superset-frontend/src/dashboard/constants.ts | 7 +- .../src/dashboard/reducers/types.ts | 12 +- superset-frontend/src/dashboard/types.ts | 25 +- .../util/activeAllDashboardFilters.ts | 3 +- .../charts/getFormDataWithExtraFilters.ts | 3 +- .../src/dashboard/util/charts/useChartIds.ts | 28 ++ .../src/dashboard/util/crossFilters.test.ts | 133 +++-- .../src/dashboard/util/crossFilters.ts | 52 +- .../util/getChartIdsInFilterScope.ts | 30 +- .../src/types/DashboardContextForExplore.ts | 2 +- superset/dashboards/schemas.py | 3 + 22 files changed, 783 insertions(+), 499 deletions(-) create mode 100644 superset-frontend/src/dashboard/components/nativeFilters/FilterBar/CrossFilters/useCrossFiltersScopingModal.tsx create mode 100644 superset-frontend/src/dashboard/util/charts/useChartIds.ts diff --git a/superset-frontend/src/components/DropdownSelectableIcon/index.tsx b/superset-frontend/src/components/DropdownSelectableIcon/index.tsx index 85c4284439f19..582bc182e3c15 100644 --- a/superset-frontend/src/components/DropdownSelectableIcon/index.tsx +++ b/superset-frontend/src/components/DropdownSelectableIcon/index.tsx @@ -100,24 +100,25 @@ const StyleSubmenuItem = styled.div` export default (props: DropDownSelectableProps) => { const theme = useTheme(); const { icon, info, menuItems, selectedKeys, onSelect } = props; - const menuItem = ( - label: string | React.ReactNode, - key: string, - divider?: boolean, - ) => ( - - - {label} - {selectedKeys?.includes(key) && ( - - )} - - + const menuItem = useMemo( + () => (label: string | React.ReactNode, key: string, divider?: boolean) => + ( + + + {label} + {selectedKeys?.includes(key) && ( + + )} + + + ), + [selectedKeys, theme.colors.primary.base], ); + const overlayMenu = useMemo( () => ( @@ -141,7 +142,7 @@ export default (props: DropDownSelectableProps) => { )} ), - [info, menuItems], + [selectedKeys, onSelect, info, menuItems, menuItem], ); return ( diff --git a/superset-frontend/src/dashboard/actions/dashboardInfo.ts b/superset-frontend/src/dashboard/actions/dashboardInfo.ts index bbc06d37b0ad4..c918ffaecb900 100644 --- a/superset-frontend/src/dashboard/actions/dashboardInfo.ts +++ b/superset-frontend/src/dashboard/actions/dashboardInfo.ts @@ -22,11 +22,12 @@ import { isString } from 'lodash'; import { getErrorText } from 'src/utils/getClientErrorObject'; import { addDangerToast } from 'src/components/MessageToasts/actions'; import { + ChartConfiguration, DashboardInfo, FilterBarOrientation, + GlobalChartCrossFilterConfig, RootState, } from 'src/dashboard/types'; -import { ChartConfiguration } from 'src/dashboard/reducers/types'; import { onSave } from './dashboardState'; export const DASHBOARD_INFO_UPDATED = 'DASHBOARD_INFO_UPDATED'; @@ -67,26 +68,22 @@ export function dashboardInfoChanged(newInfo: { metadata: any }) { return { type: DASHBOARD_INFO_UPDATED, newInfo }; } export const SET_CHART_CONFIG_BEGIN = 'SET_CHART_CONFIG_BEGIN'; -export interface SetChartConfigBegin { - type: typeof SET_CHART_CONFIG_BEGIN; - chartConfiguration: ChartConfiguration; -} export const SET_CHART_CONFIG_COMPLETE = 'SET_CHART_CONFIG_COMPLETE'; -export interface SetChartConfigComplete { - type: typeof SET_CHART_CONFIG_COMPLETE; - chartConfiguration: ChartConfiguration; -} export const SET_CHART_CONFIG_FAIL = 'SET_CHART_CONFIG_FAIL'; -export interface SetChartConfigFail { - type: typeof SET_CHART_CONFIG_FAIL; - chartConfiguration: ChartConfiguration; -} + export const setChartConfiguration = - (chartConfiguration: ChartConfiguration) => - async (dispatch: Dispatch, getState: () => any) => { + ({ + chartConfiguration, + globalChartConfiguration, + }: { + chartConfiguration?: ChartConfiguration; + globalChartConfiguration?: GlobalChartCrossFilterConfig; + }) => + async (dispatch: Dispatch, getState: () => RootState) => { dispatch({ type: SET_CHART_CONFIG_BEGIN, chartConfiguration, + globalChartConfiguration, }); const { id, metadata } = getState().dashboardInfo; @@ -103,7 +100,10 @@ export const setChartConfiguration = const response = await updateDashboard({ json_metadata: JSON.stringify({ ...metadata, - chart_configuration: chartConfiguration, + chart_configuration: + chartConfiguration ?? metadata.chart_configuration, + global_chart_configuration: + globalChartConfiguration ?? metadata.global_chart_configuration, }), }); dispatch( @@ -114,17 +114,19 @@ export const setChartConfiguration = dispatch({ type: SET_CHART_CONFIG_COMPLETE, chartConfiguration, + globalChartConfiguration, }); } catch (err) { - dispatch({ type: SET_CHART_CONFIG_FAIL, chartConfiguration }); + dispatch({ + type: SET_CHART_CONFIG_FAIL, + chartConfiguration, + globalChartConfiguration, + }); } }; export const SET_FILTER_BAR_ORIENTATION = 'SET_FILTER_BAR_ORIENTATION'; -export interface SetFilterBarOrientation { - type: typeof SET_FILTER_BAR_ORIENTATION; - filterBarOrientation: FilterBarOrientation; -} + export function setFilterBarOrientation( filterBarOrientation: FilterBarOrientation, ) { @@ -132,10 +134,7 @@ export function setFilterBarOrientation( } export const SET_CROSS_FILTERS_ENABLED = 'SET_CROSS_FILTERS_ENABLED'; -export interface SetCrossFiltersEnabled { - type: typeof SET_CROSS_FILTERS_ENABLED; - crossFiltersEnabled: boolean; -} + export function setCrossFiltersEnabled(crossFiltersEnabled: boolean) { return { type: SET_CROSS_FILTERS_ENABLED, crossFiltersEnabled }; } diff --git a/superset-frontend/src/dashboard/actions/dashboardState.js b/superset-frontend/src/dashboard/actions/dashboardState.js index d3216411ebf7b..4781a7d76b838 100644 --- a/superset-frontend/src/dashboard/actions/dashboardState.js +++ b/superset-frontend/src/dashboard/actions/dashboardState.js @@ -89,7 +89,6 @@ export function toggleFaveStar(isStarred) { return { type: TOGGLE_FAVE_STAR, isStarred }; } -export const FETCH_FAVE_STAR = 'FETCH_FAVE_STAR'; export function fetchFaveStar(id) { return function fetchFaveStarThunk(dispatch) { return SupersetClient.get({ @@ -110,7 +109,6 @@ export function fetchFaveStar(id) { }; } -export const SAVE_FAVE_STAR = 'SAVE_FAVE_STAR'; export function saveFaveStar(id, isStarred) { return function saveFaveStarThunk(dispatch) { const endpoint = `/api/v1/dashboard/${id}/favorites/`; @@ -287,13 +285,11 @@ export function saveDashboardRequest(data, id, saveType) { const { dashboardLayout, charts, - dashboardInfo: { - metadata: { chart_configuration = {} }, - }, + dashboardInfo: { metadata }, } = getState(); return getCrossFiltersConfiguration( dashboardLayout.present, - chart_configuration, + metadata, charts, ); }; @@ -304,8 +300,14 @@ export function saveDashboardRequest(data, id, saveType) { dispatch(saveDashboardRequestSuccess(lastModifiedTime)); } if (isFeatureEnabled(FeatureFlag.DASHBOARD_CROSS_FILTERS)) { - const chartConfiguration = handleChartConfiguration(); - dispatch(setChartConfiguration(chartConfiguration)); + const { chartConfiguration, globalChartConfiguration } = + handleChartConfiguration(); + dispatch( + setChartConfiguration({ + chartConfiguration, + globalChartConfiguration, + }), + ); } dispatch(saveDashboardFinished()); dispatch(addSuccessToast(t('This dashboard was saved successfully.'))); @@ -373,8 +375,10 @@ export function saveDashboardRequest(data, id, saveType) { [SAVE_TYPE_OVERWRITE, SAVE_TYPE_OVERWRITE_CONFIRMED].includes(saveType) ) { let chartConfiguration = {}; + let globalChartConfiguration = {}; if (isFeatureEnabled(FeatureFlag.DASHBOARD_CROSS_FILTERS)) { - chartConfiguration = handleChartConfiguration(); + ({ chartConfiguration, globalChartConfiguration } = + handleChartConfiguration()); } const updatedDashboard = saveType === SAVE_TYPE_OVERWRITE_CONFIRMED @@ -392,6 +396,7 @@ export function saveDashboardRequest(data, id, saveType) { default_filters: safeStringify(serializedFilters), filter_scopes: serializedFilterScopes, chart_configuration: chartConfiguration, + global_chart_configuration: globalChartConfiguration, }), }; @@ -601,13 +606,6 @@ export function setColorScheme(colorScheme) { return { type: SET_COLOR_SCHEME, colorScheme }; } -export function setColorSchemeAndUnsavedChanges(colorScheme) { - return dispatch => { - dispatch(setColorScheme(colorScheme)); - dispatch(setUnsavedChanges(true)); - }; -} - export const SET_DIRECT_PATH = 'SET_DIRECT_PATH'; export function setDirectPathToChild(path) { return { type: SET_DIRECT_PATH, path }; diff --git a/superset-frontend/src/dashboard/actions/hydrate.js b/superset-frontend/src/dashboard/actions/hydrate.js index 4f1b94379b7eb..9cd45eced43ea 100644 --- a/superset-frontend/src/dashboard/actions/hydrate.js +++ b/superset-frontend/src/dashboard/actions/hydrate.js @@ -309,11 +309,14 @@ export const hydrateDashboard = }); if (isFeatureEnabled(FeatureFlag.DASHBOARD_CROSS_FILTERS)) { - metadata.chart_configuration = getCrossFiltersConfiguration( - dashboardLayout.present, - metadata.chart_configuration, - chartQueries, - ); + const { chartConfiguration, globalChartConfiguration } = + getCrossFiltersConfiguration( + dashboardLayout.present, + metadata, + chartQueries, + ); + metadata.chart_configuration = chartConfiguration; + metadata.global_chart_configuration = globalChartConfiguration; } const { roles } = user; diff --git a/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardContainer.tsx b/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardContainer.tsx index fc1570a975005..cc4e2db780e74 100644 --- a/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardContainer.tsx +++ b/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardContainer.tsx @@ -34,7 +34,6 @@ import pick from 'lodash/pick'; import Tabs from 'src/components/Tabs'; import DashboardGrid from 'src/dashboard/containers/DashboardGrid'; import { - ChartsState, DashboardInfo, DashboardLayout, LayoutItem, @@ -86,7 +85,9 @@ const DashboardContainer: FC = ({ topLevelTabs }) => { const directPathToChild = useSelector( state => state.dashboardState.directPathToChild, ); - const charts = useSelector(state => state.charts); + const chartIds = useSelector(state => + Object.values(state.charts).map(chart => chart.id), + ); const tabIndex = useMemo(() => { const nextTabIndex = findTabIndexByComponentId({ @@ -116,7 +117,7 @@ const DashboardContainer: FC = ({ topLevelTabs }) => { } const chartsInScope: number[] = getChartIdsInFilterScope( filterScope.scope, - charts, + chartIds, dashboardLayout, ); const tabsInScope = findTabsWithChartsInScope( @@ -207,7 +208,7 @@ const DashboardContainer: FC = ({ topLevelTabs }) => { } } } - }, [charts]); + }, [chartIds]); useComponentDidUpdate(verifyUpdateColorScheme); diff --git a/superset-frontend/src/dashboard/components/SliceHeaderControls/index.tsx b/superset-frontend/src/dashboard/components/SliceHeaderControls/index.tsx index 71a158503613b..857c72f3f1420 100644 --- a/superset-frontend/src/dashboard/components/SliceHeaderControls/index.tsx +++ b/superset-frontend/src/dashboard/components/SliceHeaderControls/index.tsx @@ -31,13 +31,16 @@ import { } from 'react-router-dom'; import moment from 'moment'; import { + Behavior, css, FeatureFlag, + getChartMetadataRegistry, QueryFormData, styled, t, useTheme, } from '@superset-ui/core'; +import { useSelector } from 'react-redux'; import { Menu } from 'src/components/Menu'; import { NoAnimationDropdown } from 'src/components/Dropdown'; import ShareMenuItems from 'src/dashboard/components/menu/ShareMenuItems'; @@ -53,6 +56,8 @@ import { ResultsPaneOnDashboard } from 'src/explore/components/DataTablesPane'; import Modal from 'src/components/Modal'; import { DrillDetailMenuItems } from 'src/components/Chart/DrillDetail'; import { LOG_ACTIONS_CHART_DOWNLOAD_AS_IMAGE } from 'src/logger/LogUtils'; +import { RootState } from 'src/dashboard/types'; +import { useCrossFiltersScopingModal } from '../nativeFilters/FilterBar/CrossFilters/useCrossFiltersScopingModal'; const MENU_KEYS = { DOWNLOAD_AS_IMAGE: 'download_as_image', @@ -65,6 +70,7 @@ const MENU_KEYS = { VIEW_QUERY: 'view_query', VIEW_RESULTS: 'view_results', DRILL_TO_DETAIL: 'drill_to_detail', + CROSS_FILTER_SCOPING: 'cross_filter_scoping', }; // TODO: replace 3 dots with an icon @@ -152,9 +158,6 @@ export interface SliceHeaderControlsProps { } type SliceHeaderControlsPropsWithRouter = SliceHeaderControlsProps & RouteComponentProps; -interface State { - showControls: boolean; -} const dropdownIconsStyles = css` &&.anticon > .anticon:first-child { @@ -240,66 +243,56 @@ const ViewResultsModalTrigger = ({ ); }; -class SliceHeaderControls extends React.PureComponent< - SliceHeaderControlsPropsWithRouter, - State -> { - constructor(props: SliceHeaderControlsPropsWithRouter) { - super(props); - this.toggleControls = this.toggleControls.bind(this); - this.refreshChart = this.refreshChart.bind(this); - this.handleMenuClick = this.handleMenuClick.bind(this); - - this.state = { - showControls: false, - }; - } +const SliceHeaderControls = (props: SliceHeaderControlsPropsWithRouter) => { + const [openScopingModal, scopingModal] = useCrossFiltersScopingModal( + props.slice.slice_id, + ); - refreshChart() { - if (this.props.updatedDttm) { - this.props.forceRefresh( - this.props.slice.slice_id, - this.props.dashboardId, - ); + const canEditCrossFilters = + useSelector( + ({ dashboardInfo }) => dashboardInfo.dash_edit_perm, + ) && + isFeatureEnabled(FeatureFlag.DASHBOARD_CROSS_FILTERS) && + getChartMetadataRegistry() + .get(props.slice.viz_type) + ?.behaviors?.includes(Behavior.INTERACTIVE_CHART); + + const refreshChart = () => { + if (props.updatedDttm) { + props.forceRefresh(props.slice.slice_id, props.dashboardId); } - } - - toggleControls() { - this.setState(prevState => ({ - showControls: !prevState.showControls, - })); - } + }; - handleMenuClick({ + const handleMenuClick = ({ key, domEvent, }: { key: Key; domEvent: MouseEvent; - }) { + }) => { switch (key) { case MENU_KEYS.FORCE_REFRESH: - this.refreshChart(); - this.props.addSuccessToast(t('Data refreshed')); + refreshChart(); + props.addSuccessToast(t('Data refreshed')); break; case MENU_KEYS.TOGGLE_CHART_DESCRIPTION: // eslint-disable-next-line no-unused-expressions - this.props.toggleExpandSlice?.(this.props.slice.slice_id); + props.toggleExpandSlice?.(props.slice.slice_id); break; case MENU_KEYS.EXPLORE_CHART: // eslint-disable-next-line no-unused-expressions - this.props.logExploreChart?.(this.props.slice.slice_id); + props.logExploreChart?.(props.slice.slice_id); break; case MENU_KEYS.EXPORT_CSV: // eslint-disable-next-line no-unused-expressions - this.props.exportCSV?.(this.props.slice.slice_id); + props.exportCSV?.(props.slice.slice_id); break; case MENU_KEYS.FULLSCREEN: - this.props.handleToggleFullSize(); + props.handleToggleFullSize(); break; case MENU_KEYS.EXPORT_FULL_CSV: // eslint-disable-next-line no-unused-expressions - this.props.exportFullCSV?.(this.props.slice.slice_id); + props.exportFullCSV?.(props.slice.slice_id); break; case MENU_KEYS.DOWNLOAD_AS_IMAGE: { // menu closes with a delay, we need to hide it manually, @@ -309,235 +302,238 @@ class SliceHeaderControls extends React.PureComponent< ) as HTMLElement; menu.style.visibility = 'hidden'; downloadAsImage( - getScreenshotNodeSelector(this.props.slice.slice_id), - this.props.slice.slice_name, + getScreenshotNodeSelector(props.slice.slice_id), + props.slice.slice_name, true, // @ts-ignore )(domEvent).then(() => { menu.style.visibility = 'visible'; }); - this.props.logEvent?.(LOG_ACTIONS_CHART_DOWNLOAD_AS_IMAGE, { - chartId: this.props.slice.slice_id, + props.logEvent?.(LOG_ACTIONS_CHART_DOWNLOAD_AS_IMAGE, { + chartId: props.slice.slice_id, }); break; } + case MENU_KEYS.CROSS_FILTER_SCOPING: { + openScopingModal(); + break; + } default: break; } - } + }; - render() { - const { - componentId, - dashboardId, - slice, - isFullSize, - cachedDttm = [], - updatedDttm = null, - addSuccessToast = () => {}, - addDangerToast = () => {}, - supersetCanShare = false, - isCached = [], - } = this.props; - const isTable = slice.viz_type === 'table'; - const cachedWhen = (cachedDttm || []).map(itemCachedDttm => - moment.utc(itemCachedDttm).fromNow(), - ); - const updatedWhen = updatedDttm ? moment.utc(updatedDttm).fromNow() : ''; - const getCachedTitle = (itemCached: boolean) => { - if (itemCached) { - return t('Cached %s', cachedWhen); - } - if (updatedWhen) { - return t('Fetched %s', updatedWhen); - } - return ''; - }; - const refreshTooltipData = [...new Set(isCached.map(getCachedTitle) || '')]; - // If all queries have same cache time we can unit them to one - const refreshTooltip = refreshTooltipData.map((item, index) => ( -
- {refreshTooltipData.length > 1 - ? t('Query %s: %s', index + 1, item) - : item} -
- )); - const fullscreenLabel = isFullSize - ? t('Exit fullscreen') - : t('Enter fullscreen'); - - const menu = ( - {}, + addDangerToast = () => {}, + supersetCanShare = false, + isCached = [], + } = props; + const isTable = slice.viz_type === 'table'; + const cachedWhen = (cachedDttm || []).map(itemCachedDttm => + moment.utc(itemCachedDttm).fromNow(), + ); + const updatedWhen = updatedDttm ? moment.utc(updatedDttm).fromNow() : ''; + const getCachedTitle = (itemCached: boolean) => { + if (itemCached) { + return t('Cached %s', cachedWhen); + } + if (updatedWhen) { + return t('Fetched %s', updatedWhen); + } + return ''; + }; + const refreshTooltipData = [...new Set(isCached.map(getCachedTitle) || '')]; + // If all queries have same cache time we can unit them to one + const refreshTooltip = refreshTooltipData.map((item, index) => ( +
+ {refreshTooltipData.length > 1 + ? t('Query %s: %s', index + 1, item) + : item} +
+ )); + const fullscreenLabel = isFullSize + ? t('Exit fullscreen') + : t('Enter fullscreen'); + + const menu = ( + + - - {t('Force refresh')} - - {refreshTooltip} - - + {t('Force refresh')} + + {refreshTooltip} + + - {fullscreenLabel} + {fullscreenLabel} - + - {slice.description && ( - - {this.props.isDescriptionExpanded - ? t('Hide chart description') - : t('Show chart description')} - - )} + {slice.description && ( + + {props.isDescriptionExpanded + ? t('Hide chart description') + : t('Show chart description')} + + )} + + {props.supersetCanExplore && ( + + + + {t('Edit chart')} + + + + )} - {this.props.supersetCanExplore && ( - - - - {t('Edit chart')} - - + {canEditCrossFilters && ( + <> + + {t('Cross-filtering scoping')} - )} + + + )} + + {props.supersetCanExplore && ( + + {t('View query')} + } + modalTitle={t('View query')} + modalBody={} + draggable + resizable + responsive + /> + + )} + + {props.supersetCanExplore && ( + + {t('View as table')} + } + modalTitle={t('Chart Data: %s', slice.slice_name)} + modalBody={ + + } + /> + + )} - {this.props.supersetCanExplore && ( - - {t('View query')} - } - modalTitle={t('View query')} - modalBody={ - - } - draggable - resizable - responsive - /> - + {isFeatureEnabled(FeatureFlag.DRILL_TO_DETAIL) && + props.supersetCanExplore && ( + )} - {this.props.supersetCanExplore && ( - - - {t('View as table')} - - } - modalTitle={t('Chart Data: %s', slice.slice_name)} - modalBody={ - - } - /> + {(slice.description || props.supersetCanExplore) && } + + {supersetCanShare && ( + + + + )} + + {props.slice.viz_type !== 'filter_box' && props.supersetCanCSV && ( + + } + > + {t('Export to .CSV')} - )} - - {isFeatureEnabled(FeatureFlag.DRILL_TO_DETAIL) && - this.props.supersetCanExplore && ( - - )} - {(slice.description || this.props.supersetCanExplore) && ( - - )} - - {supersetCanShare && ( - - - - )} - - {this.props.slice.viz_type !== 'filter_box' && - this.props.supersetCanCSV && ( - + {props.slice.viz_type !== 'filter_box' && + isFeatureEnabled(FeatureFlag.ALLOW_FULL_CSV_EXPORT) && + props.supersetCanCSV && + isTable && ( } > - {t('Export to .CSV')} + {t('Export to full .CSV')} + )} - {this.props.slice.viz_type !== 'filter_box' && - isFeatureEnabled(FeatureFlag.ALLOW_FULL_CSV_EXPORT) && - this.props.supersetCanCSV && - isTable && ( - } - > - {t('Export to full .CSV')} - - )} + } + > + {t('Download as image')} + + + )} + + ); - } - > - {t('Download as image')} - - - )} - - ); - - return ( - <> - {isFullSize && ( - { - this.props.handleToggleFullSize(); - }} - /> - )} - + {isFullSize && ( + { + props.handleToggleFullSize(); + }} + /> + )} + + - - - - - - ); - } -} + + + + {canEditCrossFilters && scopingModal} + + ); +}; export default withRouter(SliceHeaderControls); diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/CrossFilters/useCrossFiltersScopingModal.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/CrossFilters/useCrossFiltersScopingModal.tsx new file mode 100644 index 0000000000000..551487ff87621 --- /dev/null +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/CrossFilters/useCrossFiltersScopingModal.tsx @@ -0,0 +1,160 @@ +/** + * 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 React, { ReactNode, useCallback, useMemo, useState } from 'react'; +import { isDefined, NativeFilterScope, t } from '@superset-ui/core'; +import { useSelector, useDispatch } from 'react-redux'; +import Modal from 'src/components/Modal'; +import { noOp } from 'src/utils/common'; +import { + ChartConfiguration, + Layout, + RootState, + isCrossFilterScopeGlobal, + GlobalChartCrossFilterConfig, +} from 'src/dashboard/types'; +import { getChartIdsInFilterScope } from 'src/dashboard/util/getChartIdsInFilterScope'; +import { useChartIds } from 'src/dashboard/util/charts/useChartIds'; +import { setChartConfiguration } from 'src/dashboard/actions/dashboardInfo'; +import { DEFAULT_CROSS_FILTER_SCOPING } from 'src/dashboard/constants'; +import ScopingTree from '../../FiltersConfigModal/FiltersConfigForm/FilterScope/ScopingTree'; + +export const useCrossFiltersScopingModal = ( + chartId?: number, +): [() => void, ReactNode] => { + const dispatch = useDispatch(); + const layout = useSelector( + state => state.dashboardLayout.present, + ); + const chartIds = useChartIds(); + const [isVisible, setIsVisible] = useState(false); + const openModal = useCallback(() => setIsVisible(true), []); + const closeModal = useCallback(() => setIsVisible(false), []); + const initialChartConfig = useSelector( + state => state.dashboardInfo.metadata?.chart_configuration || {}, + ); + const defaultGlobalChartConfig = useMemo( + () => ({ + scope: DEFAULT_CROSS_FILTER_SCOPING, + chartsInScope: chartIds, + }), + [chartIds], + ); + + const initialGlobalChartConfig = useSelector< + RootState, + GlobalChartCrossFilterConfig + >( + state => + state.dashboardInfo.metadata?.global_chart_configuration || + defaultGlobalChartConfig, + ); + const [chartConfig, setChartConfig] = useState(initialChartConfig); + const [globalChartConfig, setGlobalChartConfig] = useState( + initialGlobalChartConfig, + ); + const saveScoping = useCallback(() => { + dispatch( + setChartConfiguration({ + chartConfiguration: chartConfig, + globalChartConfiguration: globalChartConfig, + }), + ); + closeModal(); + }, [chartConfig, closeModal, dispatch, globalChartConfig]); + + const handleScopeUpdate = useCallback( + ({ scope }: { scope: NativeFilterScope }) => { + if (isDefined(chartId)) { + setChartConfig(prevConfig => ({ + ...prevConfig, + [chartId]: { + id: chartId, + crossFilters: { + scope, + chartsInScope: getChartIdsInFilterScope(scope, chartIds, layout), + }, + }, + })); + } else { + const globalChartsInScope = getChartIdsInFilterScope( + scope, + chartIds, + layout, + ); + setGlobalChartConfig({ + scope, + chartsInScope: globalChartsInScope, + }); + setChartConfig(prevConfig => + Object.entries(prevConfig).reduce((acc, [id, config]) => { + if (config.crossFilters.scope === 'global') { + acc[id] = { + id, + crossFilters: { + scope: 'global' as const, + chartsInScope: globalChartsInScope.filter( + chartId => chartId !== Number(config.id), + ), + }, + }; + } else { + acc[id] = config; + } + return acc; + }, {}), + ); + } + }, + [chartId, chartIds, layout], + ); + + const scope = useMemo(() => { + const globalScope = globalChartConfig.scope; + if (!isDefined(chartId)) { + return globalScope; + } + if (isCrossFilterScopeGlobal(chartConfig[chartId]?.crossFilters?.scope)) { + return { + rootPath: globalScope.rootPath, + excluded: globalScope.excluded.filter(id => id !== chartId), + }; + } + return chartConfig[chartId]?.crossFilters?.scope as NativeFilterScope; + }, [chartConfig, chartId, globalChartConfig.scope]); + + return [ + openModal, + isVisible ? ( + + + + ) : null, + ]; +}; diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterBarSettings/FilterBarSettings.test.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterBarSettings/FilterBarSettings.test.tsx index 10fd66d7078a0..8c236c2714841 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterBarSettings/FilterBarSettings.test.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterBarSettings/FilterBarSettings.test.tsx @@ -34,6 +34,10 @@ const initialState: { dashboardInfo: DashboardInfo } = { metadata: { native_filter_configuration: {}, chart_configuration: {}, + global_chart_configuration: { + scope: { rootPath: ['ROOT_ID'], excluded: [] }, + chartsInScope: [], + }, color_scheme: '', color_namespace: '', color_scheme_domain: [], @@ -216,7 +220,7 @@ test('On selection change, send request and update checked value', async () => { within(screen.getAllByRole('menuitem')[2]).queryByLabelText('check'), ).not.toBeInTheDocument(); - userEvent.click(await screen.findByText('Horizontal (Top)')); + userEvent.click(screen.getByText('Horizontal (Top)')); // 1st check - checkmark appears immediately after click expect( diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterBarSettings/index.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterBarSettings/index.tsx index 75572d74e51a5..ddf88ebf9cd97 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterBarSettings/index.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterBarSettings/index.tsx @@ -38,6 +38,7 @@ import DropdownSelectableIcon, { } from 'src/components/DropdownSelectableIcon'; import Checkbox from 'src/components/Checkbox'; import { clearDataMaskState } from 'src/dataMask/actions'; +import { useCrossFiltersScopingModal } from '../CrossFilters/useCrossFiltersScopingModal'; type SelectedKey = FilterBarOrientation | string | number; @@ -62,6 +63,12 @@ const StyledCheckbox = styled(Checkbox)` `} `; +const CROSS_FILTERS_MENU_KEY = 'cross-filters-menu-key'; +const CROSS_FILTERS_SCOPING_MENU_KEY = 'cross-filters-scoping-menu-key'; + +const isOrientation = (o: SelectedKey): o is FilterBarOrientation => + o === FilterBarOrientation.VERTICAL || o === FilterBarOrientation.HORIZONTAL; + const FilterBarSettings = () => { const dispatch = useDispatch(); const theme = useTheme(); @@ -77,7 +84,7 @@ const FilterBarSettings = () => { FeatureFlag.DASHBOARD_CROSS_FILTERS, ); const shouldEnableCrossFilters = - !!isCrossFiltersEnabled && isCrossFiltersFeatureEnabled; + isCrossFiltersEnabled && isCrossFiltersFeatureEnabled; const [crossFiltersEnabled, setCrossFiltersEnabled] = useState( shouldEnableCrossFilters, ); @@ -86,10 +93,9 @@ const FilterBarSettings = () => { ); const canSetHorizontalFilterBar = canEdit && isFeatureEnabled(FeatureFlag.HORIZONTAL_FILTER_BAR); - const crossFiltersMenuKey = 'cross-filters-menu-key'; - const isOrientation = (o: SelectedKey): o is FilterBarOrientation => - o === FilterBarOrientation.VERTICAL || - o === FilterBarOrientation.HORIZONTAL; + + const [openScopingModal, scopingModal] = useCrossFiltersScopingModal(); + const updateCrossFiltersSetting = useCallback( async isEnabled => { if (!isEnabled) { @@ -97,36 +103,50 @@ const FilterBarSettings = () => { } await dispatch(saveCrossFiltersSetting(isEnabled)); }, - [dispatch, crossFiltersEnabled], + [dispatch], + ); + + const toggleCrossFiltering = useCallback(() => { + setCrossFiltersEnabled(!crossFiltersEnabled); + updateCrossFiltersSetting(!crossFiltersEnabled); + }, [crossFiltersEnabled, updateCrossFiltersSetting]); + + const toggleFilterBarOrientation = useCallback( + async (orientation: FilterBarOrientation) => { + if (orientation === filterBarOrientation) { + return; + } + // set displayed selection in local state for immediate visual response after clicking + setSelectedFilterBarOrientation(orientation); + try { + // save selection in Redux and backend + await dispatch(saveFilterBarOrientation(orientation)); + } catch { + // revert local state in case of error when saving + setSelectedFilterBarOrientation(filterBarOrientation); + } + }, + [dispatch, filterBarOrientation], ); - const changeFilterBarSettings = useCallback( - async ( + + const handleSelect = useCallback( + ( selection: Parameters< Required>['onSelect'] >[0], ) => { const selectedKey: SelectedKey = selection.key; - if (selectedKey === crossFiltersMenuKey) { - setCrossFiltersEnabled(!crossFiltersEnabled); - updateCrossFiltersSetting(!crossFiltersEnabled); - return; - } - if (isOrientation(selectedKey) && selectedKey !== filterBarOrientation) { - // set displayed selection in local state for immediate visual response after clicking - setSelectedFilterBarOrientation(selectedKey as FilterBarOrientation); - try { - // save selection in Redux and backend - await dispatch( - saveFilterBarOrientation(selection.key as FilterBarOrientation), - ); - } catch { - // revert local state in case of error when saving - setSelectedFilterBarOrientation(filterBarOrientation); - } + if (selectedKey === CROSS_FILTERS_MENU_KEY) { + toggleCrossFiltering(); + } else if (isOrientation(selectedKey)) { + toggleFilterBarOrientation(selectedKey); + } else if (selectedKey === CROSS_FILTERS_SCOPING_MENU_KEY) { + openScopingModal(); } }, - [dispatch, crossFiltersEnabled, filterBarOrientation], + [openScopingModal, toggleCrossFiltering, toggleFilterBarOrientation], ); + const crossFiltersMenuItem = useMemo( () => ( @@ -142,50 +162,66 @@ const FilterBarSettings = () => { ), [crossFiltersEnabled], ); - const menuItems: DropDownSelectableProps['menuItems'] = []; - - if (isCrossFiltersFeatureEnabled && canEdit) { - menuItems.unshift({ - key: crossFiltersMenuKey, - label: crossFiltersMenuItem, - divider: canSetHorizontalFilterBar, - }); - } - if (canSetHorizontalFilterBar) { - menuItems.push({ - key: 'placement', - label: t('Orientation of filter bar'), - children: [ - { - key: FilterBarOrientation.VERTICAL, - label: t('Vertical (Left)'), - }, - { - key: FilterBarOrientation.HORIZONTAL, - label: t('Horizontal (Top)'), - }, - ], - }); - } + const menuItems = useMemo(() => { + const items: DropDownSelectableProps['menuItems'] = []; + + if (isCrossFiltersFeatureEnabled && canEdit) { + items.push({ + key: CROSS_FILTERS_MENU_KEY, + label: crossFiltersMenuItem, + }); + items.push({ + key: CROSS_FILTERS_SCOPING_MENU_KEY, + label: t('Cross-filtering scoping'), + divider: canSetHorizontalFilterBar, + }); + } + + if (canSetHorizontalFilterBar) { + items.push({ + key: 'placement', + label: t('Orientation of filter bar'), + children: [ + { + key: FilterBarOrientation.VERTICAL, + label: t('Vertical (Left)'), + }, + { + key: FilterBarOrientation.HORIZONTAL, + label: t('Horizontal (Top)'), + }, + ], + }); + } + return items; + }, [ + canEdit, + canSetHorizontalFilterBar, + crossFiltersMenuItem, + isCrossFiltersFeatureEnabled, + ]); if (!menuItems.length) { return null; } return ( - - } - menuItems={menuItems} - selectedKeys={[selectedFilterBarOrientation]} - /> + <> + + } + menuItems={menuItems} + selectedKeys={[selectedFilterBarOrientation]} + /> + {scopingModal} + ); }; diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterCard/useFilterScope.ts b/superset-frontend/src/dashboard/components/nativeFilters/FilterCard/useFilterScope.ts index 35c84a8591d14..c93ce2426bd18 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterCard/useFilterScope.ts +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterCard/useFilterScope.ts @@ -16,33 +16,24 @@ * specific language governing permissions and limitations * under the License. */ -import { Filter, t } from '@superset-ui/core'; +import { useMemo } from 'react'; import { useSelector } from 'react-redux'; -import { - ChartsState, - Layout, - LayoutItem, - RootState, -} from 'src/dashboard/types'; +import { Filter, t } from '@superset-ui/core'; +import { Layout, LayoutItem, RootState } from 'src/dashboard/types'; import { DASHBOARD_ROOT_ID } from 'src/dashboard/util/constants'; import { CHART_TYPE } from 'src/dashboard/util/componentTypes'; -import { useMemo } from 'react'; +import { useChartIds } from 'src/dashboard/util/charts/useChartIds'; const extractTabLabel = (tab?: LayoutItem) => tab?.meta?.text || tab?.meta?.defaultText || ''; const extractChartLabel = (chart?: LayoutItem) => chart?.meta?.sliceNameOverride || chart?.meta?.sliceName || chart?.id || ''; -const useCharts = () => { - const charts = useSelector(state => state.charts); - return useMemo(() => Object.values(charts), [charts]); -}; - export const useFilterScope = (filter: Filter) => { const layout = useSelector( state => state.dashboardLayout.present, ); - const charts = useCharts(); + const chartIds = useChartIds(); return useMemo(() => { let topLevelTabs: string[] | undefined; @@ -87,11 +78,11 @@ export const useFilterScope = (filter: Filter) => { // returns "CHART1, CHART2" if (filter.scope.rootPath[0] === DASHBOARD_ROOT_ID) { return { - charts: charts - .filter(chart => !filter.scope.excluded.includes(chart.id)) - .map(chart => { + charts: chartIds + .filter(chartId => !filter.scope.excluded.includes(chartId)) + .map(chartId => { const layoutElement = layoutCharts.find( - layoutChart => layoutChart.meta.chartId === chart.id, + layoutChart => layoutChart.meta.chartId === chartId, ); return extractChartLabel(layoutElement); }) @@ -121,13 +112,13 @@ export const useFilterScope = (filter: Filter) => { } }); // Handle charts that are in scope but belong to excluded tabs. - const chartsInExcludedTabs = charts - .filter(chart => !filter.scope.excluded.includes(chart.id)) - .reduce((acc: LayoutItem[], chart) => { + const chartsInExcludedTabs = chartIds + .filter(chartId => !filter.scope.excluded.includes(chartId)) + .reduce((acc: LayoutItem[], chartId) => { const layoutChartElementInExcludedTab = layoutChartElementsInTabsInScope.find( element => - element.meta.chartId === chart.id && + element.meta.chartId === chartId && element.parents.every( parent => !topLevelTabsInFullScope.includes(parent), ), @@ -147,5 +138,5 @@ export const useFilterScope = (filter: Filter) => { } return undefined; - }, [charts, filter.scope.excluded, filter.scope.rootPath, layout]); + }, [chartIds, filter.scope.excluded, filter.scope.rootPath, layout]); }; diff --git a/superset-frontend/src/dashboard/components/nativeFilters/selectors.ts b/superset-frontend/src/dashboard/components/nativeFilters/selectors.ts index 59f00f1f7a428..680b918120345 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/selectors.ts +++ b/superset-frontend/src/dashboard/components/nativeFilters/selectors.ts @@ -32,8 +32,11 @@ import { } from '@superset-ui/core'; import { TIME_FILTER_MAP } from 'src/explore/constants'; import { getChartIdsInFilterBoxScope } from 'src/dashboard/util/activeDashboardFilters'; -import { ChartConfiguration } from 'src/dashboard/reducers/types'; -import { DashboardLayout, Layout } from 'src/dashboard/types'; +import { + ChartConfiguration, + DashboardLayout, + Layout, +} from 'src/dashboard/types'; import { areObjectsEqual } from 'src/reduxUtils'; export enum IndicatorStatus { diff --git a/superset-frontend/src/dashboard/constants.ts b/superset-frontend/src/dashboard/constants.ts index aa9f4d8bd3619..588e5b6cd5f71 100644 --- a/superset-frontend/src/dashboard/constants.ts +++ b/superset-frontend/src/dashboard/constants.ts @@ -1,6 +1,6 @@ -/* eslint-disable import/prefer-default-export */ import { DatasourceType } from '@superset-ui/core'; import { Datasource } from 'src/dashboard/types'; +import { DASHBOARD_ROOT_ID } from './util/constants'; /** * Licensed to the Apache Software Foundation (ASF) under one * or more contributor license agreements. See the NOTICE file @@ -42,3 +42,8 @@ export const FILTER_BAR_HEADER_HEIGHT = 80; export const FILTER_BAR_TABS_HEIGHT = 46; export const BUILDER_SIDEPANEL_WIDTH = 374; export const OVERWRITE_INSPECT_FIELDS = ['css', 'json_metadata.filter_scopes']; + +export const DEFAULT_CROSS_FILTER_SCOPING = { + rootPath: [DASHBOARD_ROOT_ID], + excluded: [], +}; diff --git a/superset-frontend/src/dashboard/reducers/types.ts b/superset-frontend/src/dashboard/reducers/types.ts index 9e9b4e8ca5fa0..56322670c1208 100644 --- a/superset-frontend/src/dashboard/reducers/types.ts +++ b/superset-frontend/src/dashboard/reducers/types.ts @@ -18,23 +18,13 @@ */ import componentTypes from 'src/dashboard/util/componentTypes'; -import { NativeFilterScope, JsonObject } from '@superset-ui/core'; +import { JsonObject } from '@superset-ui/core'; export enum Scoping { All = 'All', Specific = 'Specific', } -export type ChartConfiguration = { - [chartId: number]: { - id: number; - crossFilters: { - scope: NativeFilterScope; - chartsInScope: number[]; - }; - }; -}; - export type User = { email: string; firstName: string; diff --git a/superset-frontend/src/dashboard/types.ts b/superset-frontend/src/dashboard/types.ts index 7345f0b1aff19..5d4248334cc61 100644 --- a/superset-frontend/src/dashboard/types.ts +++ b/superset-frontend/src/dashboard/types.ts @@ -23,6 +23,7 @@ import { ExtraFormData, GenericDataType, JsonObject, + NativeFilterScope, NativeFiltersState, } from '@superset-ui/core'; import { Dataset } from '@superset-ui/chart-controls'; @@ -58,6 +59,27 @@ export enum FilterBarOrientation { HORIZONTAL = 'HORIZONTAL', } +// chart's cross filter scoping can have its custom value or point to the global configuration +export type ChartCrossFiltersGlobalScope = 'global'; +export type ChartCrossFiltersConfig = { + scope: NativeFilterScope | ChartCrossFiltersGlobalScope; + chartsInScope: number[]; +}; +export type GlobalChartCrossFilterConfig = { + scope: NativeFilterScope; + chartsInScope: number[]; +}; +export const isCrossFilterScopeGlobal = ( + scope: NativeFilterScope | ChartCrossFiltersGlobalScope, +): scope is ChartCrossFiltersGlobalScope => scope === 'global'; + +export type ChartConfiguration = { + [chartId: number]: { + id: number; + crossFilters: ChartCrossFiltersConfig; + }; +}; + export type ActiveTabs = string[]; export type DashboardLayout = { [key: string]: LayoutItem }; export type DashboardLayoutState = { present: DashboardLayout }; @@ -102,7 +124,8 @@ export type DashboardInfo = { json_metadata: string; metadata: { native_filter_configuration: JsonObject; - chart_configuration: JsonObject; + chart_configuration: ChartConfiguration; + global_chart_configuration: GlobalChartCrossFilterConfig; color_scheme: string; color_namespace: string; color_scheme_domain: string[]; diff --git a/superset-frontend/src/dashboard/util/activeAllDashboardFilters.ts b/superset-frontend/src/dashboard/util/activeAllDashboardFilters.ts index 14ae2d4ea083a..f9e98e85365cb 100644 --- a/superset-frontend/src/dashboard/util/activeAllDashboardFilters.ts +++ b/superset-frontend/src/dashboard/util/activeAllDashboardFilters.ts @@ -21,8 +21,7 @@ import { PartialFilters, JsonObject, } from '@superset-ui/core'; -import { ActiveFilters } from '../types'; -import { ChartConfiguration } from '../reducers/types'; +import { ActiveFilters, ChartConfiguration } from '../types'; export const getRelevantDataMask = ( dataMask: DataMaskStateWithId, diff --git a/superset-frontend/src/dashboard/util/charts/getFormDataWithExtraFilters.ts b/superset-frontend/src/dashboard/util/charts/getFormDataWithExtraFilters.ts index cbed55755a89e..0b7c6a9d7d8c0 100644 --- a/superset-frontend/src/dashboard/util/charts/getFormDataWithExtraFilters.ts +++ b/superset-frontend/src/dashboard/util/charts/getFormDataWithExtraFilters.ts @@ -22,11 +22,10 @@ import { JsonObject, PartialFilters, } from '@superset-ui/core'; -import { ChartQueryPayload } from 'src/dashboard/types'; +import { ChartConfiguration, ChartQueryPayload } from 'src/dashboard/types'; import { getExtraFormData } from 'src/dashboard/components/nativeFilters/utils'; import { areObjectsEqual } from 'src/reduxUtils'; import getEffectiveExtraFilters from './getEffectiveExtraFilters'; -import { ChartConfiguration } from '../../reducers/types'; import { getAllActiveFilters } from '../activeAllDashboardFilters'; // We cache formData objects so that our connected container components don't always trigger diff --git a/superset-frontend/src/dashboard/util/charts/useChartIds.ts b/superset-frontend/src/dashboard/util/charts/useChartIds.ts new file mode 100644 index 0000000000000..992447a46c31d --- /dev/null +++ b/superset-frontend/src/dashboard/util/charts/useChartIds.ts @@ -0,0 +1,28 @@ +/** + * 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 { useMemo } from 'react'; +import { useSelector } from 'react-redux'; +import { RootState } from 'src/dashboard/types'; + +export const useChartIds = () => { + const chartIds = useSelector(state => + Object.values(state.charts).map(chart => chart.id), + ); + return useMemo(() => chartIds, [chartIds]); +}; diff --git a/superset-frontend/src/dashboard/util/crossFilters.test.ts b/superset-frontend/src/dashboard/util/crossFilters.test.ts index 9dbdac4d241f5..45bcaaf0b6798 100644 --- a/superset-frontend/src/dashboard/util/crossFilters.test.ts +++ b/superset-frontend/src/dashboard/util/crossFilters.test.ts @@ -16,10 +16,11 @@ * specific language governing permissions and limitations * under the License. */ -import sinon from 'sinon'; +import sinon, { SinonStub } from 'sinon'; import { Behavior, FeatureFlag } from '@superset-ui/core'; import * as core from '@superset-ui/core'; import { getCrossFiltersConfiguration } from './crossFilters'; +import { DEFAULT_CROSS_FILTER_SCOPING } from '../constants'; const DASHBOARD_LAYOUT = { 'CHART-1': { @@ -101,19 +102,34 @@ const INITIAL_CHART_CONFIG = { crossFilters: { scope: { rootPath: ['ROOT_ID'], - excluded: [1], + excluded: [1, 2], }, - chartsInScope: [2], + chartsInScope: [], + }, + }, + '2': { + id: 2, + crossFilters: { + scope: 'global' as const, + chartsInScope: [1], }, }, }; -test('Generate correct cross filters configuration without initial configuration', () => { - // @ts-ignore - global.featureFlags = { - [FeatureFlag.DASHBOARD_CROSS_FILTERS]: true, - }; - const metadataRegistryStub = sinon +const GLOBAL_CHART_CONFIG = { + scope: DEFAULT_CROSS_FILTER_SCOPING, + chartsInScope: [1, 2], +}; + +const CHART_CONFIG_METADATA = { + chart_configuration: INITIAL_CHART_CONFIG, + global_chart_configuration: GLOBAL_CHART_CONFIG, +}; + +let metadataRegistryStub: SinonStub; + +beforeEach(() => { + metadataRegistryStub = sinon .stub(core, 'getChartMetadataRegistry') .callsFake(() => ({ // @ts-ignore @@ -121,29 +137,43 @@ test('Generate correct cross filters configuration without initial configuration behaviors: [Behavior.INTERACTIVE_CHART], }), })); - expect( - getCrossFiltersConfiguration(DASHBOARD_LAYOUT, undefined, CHARTS), - ).toEqual({ - '1': { - id: 1, - crossFilters: { - scope: { - rootPath: ['ROOT_ID'], - excluded: [1], +}); + +afterEach(() => { + metadataRegistryStub.restore(); +}); + +test('Generate correct cross filters configuration without initial configuration', () => { + // @ts-ignore + global.featureFlags = { + [FeatureFlag.DASHBOARD_CROSS_FILTERS]: true, + }; + + // @ts-ignore + expect(getCrossFiltersConfiguration(DASHBOARD_LAYOUT, {}, CHARTS)).toEqual({ + chartConfiguration: { + '1': { + id: 1, + crossFilters: { + scope: 'global', + chartsInScope: [2], }, - chartsInScope: [2], }, - }, - '2': { - id: 2, - crossFilters: { - scope: { - rootPath: ['ROOT_ID'], - excluded: [2], + '2': { + id: 2, + crossFilters: { + scope: 'global', + chartsInScope: [1], }, - chartsInScope: [1], }, }, + globalChartConfiguration: { + scope: { + excluded: [], + rootPath: ['ROOT_ID'], + }, + chartsInScope: [1, 2], + }, }); metadataRegistryStub.restore(); }); @@ -153,41 +183,40 @@ test('Generate correct cross filters configuration with initial configuration', 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, + CHART_CONFIG_METADATA, CHARTS, ), ).toEqual({ - '1': { - id: 1, - crossFilters: { - scope: { - rootPath: ['ROOT_ID'], - excluded: [1], + chartConfiguration: { + '1': { + id: 1, + crossFilters: { + scope: { + rootPath: ['ROOT_ID'], + excluded: [1, 2], + }, + chartsInScope: [], }, - chartsInScope: [2], }, - }, - '2': { - id: 2, - crossFilters: { - scope: { - rootPath: ['ROOT_ID'], - excluded: [2], + '2': { + id: 2, + crossFilters: { + scope: 'global', + chartsInScope: [1], }, - chartsInScope: [1], }, }, + globalChartConfiguration: { + scope: { + excluded: [], + rootPath: ['ROOT_ID'], + }, + chartsInScope: [1, 2], + }, }); metadataRegistryStub.restore(); }); @@ -200,7 +229,7 @@ test('Return undefined if DASHBOARD_CROSS_FILTERS feature flag is disabled', () expect( getCrossFiltersConfiguration( DASHBOARD_LAYOUT, - INITIAL_CHART_CONFIG, + CHART_CONFIG_METADATA, CHARTS, ), ).toEqual(undefined); diff --git a/superset-frontend/src/dashboard/util/crossFilters.ts b/superset-frontend/src/dashboard/util/crossFilters.ts index c166c10fbf4c0..80312c3fcc080 100644 --- a/superset-frontend/src/dashboard/util/crossFilters.ts +++ b/superset-frontend/src/dashboard/util/crossFilters.ts @@ -24,9 +24,14 @@ import { isDefined, isFeatureEnabled, } from '@superset-ui/core'; -import { DASHBOARD_ROOT_ID } from './constants'; import { getChartIdsInFilterScope } from './getChartIdsInFilterScope'; -import { ChartsState, DashboardInfo, DashboardLayout } from '../types'; +import { + ChartsState, + DashboardInfo, + DashboardLayout, + isCrossFilterScopeGlobal, +} from '../types'; +import { DEFAULT_CROSS_FILTER_SCOPING } from '../constants'; export const isCrossFiltersEnabled = ( metadataCrossFiltersEnabled: boolean | undefined, @@ -36,13 +41,24 @@ export const isCrossFiltersEnabled = ( export const getCrossFiltersConfiguration = ( dashboardLayout: DashboardLayout, - initialConfig: DashboardInfo['metadata']['chart_configuration'] = {}, + metadata: Pick< + DashboardInfo['metadata'], + 'chart_configuration' | 'global_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, + + const globalChartConfiguration = metadata.global_chart_configuration + ? cloneDeep(metadata.global_chart_configuration) + : { + scope: DEFAULT_CROSS_FILTER_SCOPING, + chartsInScope: Object.values(charts).map(chart => chart.id), + }; + + // If user just added cross filter to dashboard it's not saving its 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 => { @@ -59,28 +75,32 @@ export const getCrossFiltersConfiguration = ( )?.behaviors ?? []; if (behaviors.includes(Behavior.INTERACTIVE_CHART)) { - if (initialConfig[chartId]) { + if (metadata.chart_configuration?.[chartId]) { // We need to clone to avoid mutating Redux state - chartConfiguration[chartId] = cloneDeep(initialConfig[chartId]); + chartConfiguration[chartId] = cloneDeep( + metadata.chart_configuration[chartId], + ); } if (!chartConfiguration[chartId]) { chartConfiguration[chartId] = { id: chartId, crossFilters: { - scope: { - rootPath: [DASHBOARD_ROOT_ID], - excluded: [chartId], // By default it doesn't affects itself - }, + scope: 'global' as const, }, }; } chartConfiguration[chartId].crossFilters.chartsInScope = - getChartIdsInFilterScope( - chartConfiguration[chartId].crossFilters.scope, - charts, - dashboardLayout, - ); + isCrossFilterScopeGlobal(chartConfiguration[chartId].crossFilters.scope) + ? globalChartConfiguration.chartsInScope.filter( + id => id !== Number(chartId), + ) + : getChartIdsInFilterScope( + chartConfiguration[chartId].crossFilters.scope, + Object.values(charts).map(chart => chart.id), + dashboardLayout, + ); } }); - return chartConfiguration; + + return { chartConfiguration, globalChartConfiguration }; }; diff --git a/superset-frontend/src/dashboard/util/getChartIdsInFilterScope.ts b/superset-frontend/src/dashboard/util/getChartIdsInFilterScope.ts index 516bbf5045c6c..7e4b7b1273121 100644 --- a/superset-frontend/src/dashboard/util/getChartIdsInFilterScope.ts +++ b/superset-frontend/src/dashboard/util/getChartIdsInFilterScope.ts @@ -18,27 +18,23 @@ */ import { NativeFilterScope } from '@superset-ui/core'; import { CHART_TYPE } from './componentTypes'; -import { ChartsState, Layout } from '../types'; +import { Layout } from '../types'; export function getChartIdsInFilterScope( filterScope: NativeFilterScope, - charts: ChartsState, + chartIds: number[], layout: Layout, ) { const layoutItems = Object.values(layout); - return Object.values(charts) - .filter( - chart => - !filterScope.excluded.includes(chart.id) && - layoutItems - .find( - layoutItem => - layoutItem?.type === CHART_TYPE && - layoutItem.meta?.chartId === chart.id, - ) - ?.parents?.some(elementId => - filterScope.rootPath.includes(elementId), - ), - ) - .map(chart => chart.id); + return chartIds.filter( + chartId => + !filterScope.excluded.includes(chartId) && + layoutItems + .find( + layoutItem => + layoutItem?.type === CHART_TYPE && + layoutItem.meta?.chartId === chartId, + ) + ?.parents?.some(elementId => filterScope.rootPath.includes(elementId)), + ); } diff --git a/superset-frontend/src/types/DashboardContextForExplore.ts b/superset-frontend/src/types/DashboardContextForExplore.ts index b69ec3ca7f0ea..e5ce58056cffa 100644 --- a/superset-frontend/src/types/DashboardContextForExplore.ts +++ b/superset-frontend/src/types/DashboardContextForExplore.ts @@ -21,7 +21,7 @@ import { DataRecordValue, PartialFilters, } from '@superset-ui/core'; -import { ChartConfiguration } from 'src/dashboard/reducers/types'; +import { ChartConfiguration } from 'src/dashboard/types'; export interface DashboardContextForExplore { labelColors: Record; diff --git a/superset/dashboards/schemas.py b/superset/dashboards/schemas.py index 156a3ac1f9111..169d80613d330 100644 --- a/superset/dashboards/schemas.py +++ b/superset/dashboards/schemas.py @@ -112,6 +112,9 @@ class DashboardJSONMetadataSchema(Schema): native_filter_configuration = fields.List(fields.Dict(), allow_none=True) # chart_configuration for now keeps data about cross-filter scoping for charts chart_configuration = fields.Dict() + # global_chart_configuration keeps data about global cross-filter scoping + # for charts - can be overriden by chart_configuration for each chart + global_chart_configuration = fields.Dict() # filter_sets_configuration is for dashboard-native filters filter_sets_configuration = fields.List(fields.Dict(), allow_none=True) timed_refresh_immune_slices = fields.List(fields.Integer()) From e9164818245a43da3e4d8e80f6e2a2050ada78f7 Mon Sep 17 00:00:00 2001 From: Kamil Gabryjelski Date: Tue, 9 May 2023 09:12:21 +0200 Subject: [PATCH 02/11] Implement scoping modal --- .../src/components/Modal/Modal.tsx | 15 +- .../components/SliceHeaderControls/index.tsx | 2 +- .../ScopingModal/ChartsScopingListPanel.tsx | 176 ++++++++++ .../ScopingModal/ScopingModal.tsx | 313 ++++++++++++++++++ .../ScopingModal/ScopingModalContent.tsx | 81 +++++ .../ScopingModal/ScopingTreePanel.tsx | 197 +++++++++++ .../useCrossFiltersScopingModal.tsx | 40 +++ .../useCrossFiltersScopingModal.tsx | 160 --------- .../FilterBar/FilterBarSettings/index.tsx | 2 +- .../FilterTitleContainer.tsx | 3 +- .../FilterScope/ScopingTree.tsx | 4 +- .../components/nativeFilters/selectors.ts | 2 +- 12 files changed, 825 insertions(+), 170 deletions(-) create mode 100644 superset-frontend/src/dashboard/components/nativeFilters/FilterBar/CrossFilters/ScopingModal/ChartsScopingListPanel.tsx create mode 100644 superset-frontend/src/dashboard/components/nativeFilters/FilterBar/CrossFilters/ScopingModal/ScopingModal.tsx create mode 100644 superset-frontend/src/dashboard/components/nativeFilters/FilterBar/CrossFilters/ScopingModal/ScopingModalContent.tsx create mode 100644 superset-frontend/src/dashboard/components/nativeFilters/FilterBar/CrossFilters/ScopingModal/ScopingTreePanel.tsx create mode 100644 superset-frontend/src/dashboard/components/nativeFilters/FilterBar/CrossFilters/ScopingModal/useCrossFiltersScopingModal.tsx delete mode 100644 superset-frontend/src/dashboard/components/nativeFilters/FilterBar/CrossFilters/useCrossFiltersScopingModal.tsx diff --git a/superset-frontend/src/components/Modal/Modal.tsx b/superset-frontend/src/components/Modal/Modal.tsx index d1e1affcfbe98..fc9b82168397e 100644 --- a/superset-frontend/src/components/Modal/Modal.tsx +++ b/superset-frontend/src/components/Modal/Modal.tsx @@ -16,7 +16,13 @@ * specific language governing permissions and limitations * under the License. */ -import React, { useMemo, useRef, useState } from 'react'; +import React, { + CSSProperties, + ReactNode, + useMemo, + useRef, + useState, +} from 'react'; import { isNil } from 'lodash'; import { ModalFuncProps } from 'antd/lib/modal'; import { styled, t } from '@superset-ui/core'; @@ -33,7 +39,7 @@ import Draggable, { export interface ModalProps { className?: string; - children: React.ReactNode; + children: ReactNode; disablePrimaryButton?: boolean; primaryButtonLoading?: boolean; onHide: () => void; @@ -42,13 +48,13 @@ export interface ModalProps { primaryButtonType?: 'primary' | 'danger'; show: boolean; name?: string; - title: React.ReactNode; + title: ReactNode; width?: string; maxWidth?: string; responsive?: boolean; hideFooter?: boolean; centered?: boolean; - footer?: React.ReactNode; + footer?: ReactNode; wrapProps?: object; height?: string; closable?: boolean; @@ -59,6 +65,7 @@ export interface ModalProps { destroyOnClose?: boolean; maskClosable?: boolean; zIndex?: number; + bodyStyle?: CSSProperties; } interface StyledModalProps { diff --git a/superset-frontend/src/dashboard/components/SliceHeaderControls/index.tsx b/superset-frontend/src/dashboard/components/SliceHeaderControls/index.tsx index 857c72f3f1420..bba888710a54c 100644 --- a/superset-frontend/src/dashboard/components/SliceHeaderControls/index.tsx +++ b/superset-frontend/src/dashboard/components/SliceHeaderControls/index.tsx @@ -57,7 +57,7 @@ import Modal from 'src/components/Modal'; import { DrillDetailMenuItems } from 'src/components/Chart/DrillDetail'; import { LOG_ACTIONS_CHART_DOWNLOAD_AS_IMAGE } from 'src/logger/LogUtils'; import { RootState } from 'src/dashboard/types'; -import { useCrossFiltersScopingModal } from '../nativeFilters/FilterBar/CrossFilters/useCrossFiltersScopingModal'; +import { useCrossFiltersScopingModal } from '../nativeFilters/FilterBar/CrossFilters/ScopingModal/useCrossFiltersScopingModal'; const MENU_KEYS = { DOWNLOAD_AS_IMAGE: 'download_as_image', diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/CrossFilters/ScopingModal/ChartsScopingListPanel.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/CrossFilters/ScopingModal/ChartsScopingListPanel.tsx new file mode 100644 index 0000000000000..719ccee144cce --- /dev/null +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/CrossFilters/ScopingModal/ChartsScopingListPanel.tsx @@ -0,0 +1,176 @@ +/** + * 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 React, { ReactNode, useMemo } from 'react'; +import { css, styled, t, useTheme } from '@superset-ui/core'; +import { + ChartConfiguration, + DashboardLayout, + isCrossFilterScopeGlobal, + RootState, +} from 'src/dashboard/types'; +import { useSelector } from 'react-redux'; +import { CHART_TYPE } from 'src/dashboard/util/componentTypes'; +import Icons from 'src/components/Icons'; +import Button from 'src/components/Button'; +import { FilterTitle } from '../../../FiltersConfigModal/FilterTitleContainer'; + +const AddButtonContainer = styled.div` + ${({ theme }) => css` + margin-top: ${theme.gridUnit * 2}px; + + & button > [role='img']:first-of-type { + margin-right: ${theme.gridUnit}px; + line-height: 0; + } + + span[role='img'] { + padding-bottom: 1px; + } + + .ant-btn > .anticon + span { + margin-left: 0; + } + `} +`; + +const ScopingTitle = ({ + isActive, + onClick, + id, + label, + onRemove, +}: { + isActive: boolean; + onClick: (id: number) => void; + id: number; + label: ReactNode; + onRemove: (id: number) => void; +}) => { + const theme = useTheme(); + return ( + onClick(id)} + > + {label} + { + event.stopPropagation(); + onRemove(id); + }} + css={css` + margin-left: auto; + `} + /> + + ); +}; + +export const ChartsScopingListPanel = ({ + activeChartId, + chartConfigs, + setCurrentChartId, + removeCustomScope, + addNewCustomScope, +}: { + activeChartId: number | undefined; + chartConfigs: ChartConfiguration; + setCurrentChartId: (chartId: number | undefined) => void; + removeCustomScope: (chartId: number) => void; + addNewCustomScope: () => void; +}) => { + const theme = useTheme(); + const layout = useSelector( + state => state.dashboardLayout.present, + ); + const customScopedCharts = useMemo(() => { + const chartLayoutItems = Object.values(layout).filter( + item => item.type === CHART_TYPE, + ); + return Object.values(chartConfigs) + .filter( + config => + !isCrossFilterScopeGlobal(config.crossFilters.scope) && config.id > 0, + ) + .map(config => { + const chartLayoutItem = chartLayoutItems.find( + item => item.meta.chartId === config.id, + ); + return { + id: config.id, + label: + chartLayoutItem?.meta.sliceNameOverride || + chartLayoutItem?.meta.sliceName || + '', + }; + }); + }, [chartConfigs, layout]); + + const newScoping = Object.values(chartConfigs).find( + config => config.id === -1, + ); + return ( + <> + + + + setCurrentChartId(undefined)} + className={activeChartId === undefined ? 'active' : ''} + > + {t('All charts/global scoping')} + +
+ {customScopedCharts.map(chartInfo => ( + + ))} + {newScoping && ( + + )} + + ); +}; diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/CrossFilters/ScopingModal/ScopingModal.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/CrossFilters/ScopingModal/ScopingModal.tsx new file mode 100644 index 0000000000000..1321b364ee9ad --- /dev/null +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/CrossFilters/ScopingModal/ScopingModal.tsx @@ -0,0 +1,313 @@ +/** + * 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 React, { useCallback, useMemo, useState } from 'react'; +import { useDispatch, useSelector } from 'react-redux'; +import { isDefined, NativeFilterScope, t } from '@superset-ui/core'; +import Modal from 'src/components/Modal'; +import { + ChartConfiguration, + Layout, + RootState, + isCrossFilterScopeGlobal, + GlobalChartCrossFilterConfig, +} from 'src/dashboard/types'; +import { getChartIdsInFilterScope } from 'src/dashboard/util/getChartIdsInFilterScope'; +import { useChartIds } from 'src/dashboard/util/charts/useChartIds'; +import { setChartConfiguration } from 'src/dashboard/actions/dashboardInfo'; +import { DEFAULT_CROSS_FILTER_SCOPING } from 'src/dashboard/constants'; +import { ScopingModalContent } from './ScopingModalContent'; + +const getUpdatedGloballyScopedChartsInScope = ( + configs: ChartConfiguration, + globalChartsInScope: number[], +) => + Object.entries(configs).reduce((acc, [id, config]) => { + if (config.crossFilters.scope === 'global') { + acc[id] = { + id, + crossFilters: { + scope: 'global' as const, + chartsInScope: globalChartsInScope.filter( + chartId => chartId !== Number(config.id), + ), + }, + }; + } else { + acc[id] = config; + } + return acc; + }, {}); + +const getActualScopeFromGlobalScope = ( + chartId: number, + globalScope: NativeFilterScope, +) => ({ + rootPath: globalScope.rootPath, + excluded: globalScope.excluded.filter(id => id !== chartId), +}); + +export const ScopingModal = ({ + initialChartId, + isVisible, + closeModal, +}: { + initialChartId: number | undefined; + isVisible: boolean; + closeModal: () => void; +}) => { + const dispatch = useDispatch(); + const layout = useSelector( + state => state.dashboardLayout.present, + ); + const chartIds = useChartIds(); + const [currentChartId, setCurrentChartId] = useState(initialChartId); + const initialChartConfig = useSelector( + state => state.dashboardInfo.metadata?.chart_configuration || {}, + ); + const defaultGlobalChartConfig = useMemo( + () => ({ + scope: DEFAULT_CROSS_FILTER_SCOPING, + chartsInScope: chartIds, + }), + [chartIds], + ); + + const initialGlobalChartConfig = useSelector< + RootState, + GlobalChartCrossFilterConfig + >( + state => + state.dashboardInfo.metadata?.global_chart_configuration || + defaultGlobalChartConfig, + ); + + const getInitialChartConfig = () => { + if ( + isDefined(initialChartId) && + isCrossFilterScopeGlobal( + initialChartConfig[initialChartId]?.crossFilters.scope, + ) + ) { + return { + ...initialChartConfig, + [initialChartId]: { + id: initialChartId, + crossFilters: { + scope: getActualScopeFromGlobalScope( + initialChartId, + initialGlobalChartConfig.scope, + ), + chartsInScope: + initialChartConfig[initialChartId]?.crossFilters.chartsInScope, + }, + }, + }; + } + return initialChartConfig; + }; + + const [chartConfigs, setChartConfigs] = useState(getInitialChartConfig()); + const [globalChartConfig, setGlobalChartConfig] = useState( + initialGlobalChartConfig, + ); + + const saveScoping = useCallback(() => { + dispatch( + setChartConfiguration({ + chartConfiguration: chartConfigs, + globalChartConfiguration: globalChartConfig, + }), + ); + closeModal(); + }, [chartConfigs, closeModal, dispatch, globalChartConfig]); + + const handleScopeUpdate = useCallback( + ({ scope }: { scope: NativeFilterScope }) => { + if (isDefined(currentChartId)) { + setChartConfigs(prevConfig => ({ + ...prevConfig, + [currentChartId]: { + id: currentChartId, + crossFilters: { + scope, + chartsInScope: getChartIdsInFilterScope(scope, chartIds, layout), + }, + }, + })); + } else { + const globalChartsInScope = getChartIdsInFilterScope( + scope, + chartIds, + layout, + ); + setGlobalChartConfig({ + scope, + chartsInScope: globalChartsInScope, + }); + setChartConfigs(prevConfig => + getUpdatedGloballyScopedChartsInScope( + prevConfig, + globalChartsInScope, + ), + ); + } + }, + [currentChartId, chartIds, layout], + ); + + const removeCustomScope = useCallback( + (chartId: number) => { + setChartConfigs(prevConfigs => { + const newConfigs = { ...prevConfigs }; + if (chartId === -1) { + delete newConfigs[-1]; + } else { + newConfigs[chartId] = { + id: chartId, + crossFilters: { + scope: 'global', + chartsInScope: globalChartConfig.chartsInScope.filter( + id => id !== chartId, + ), + }, + }; + } + return newConfigs; + }); + if (currentChartId === chartId) { + setCurrentChartId(undefined); + } + }, + [currentChartId, globalChartConfig.chartsInScope], + ); + + const addNewCustomScope = useCallback(() => { + setCurrentChartId(-1); + if (!chartConfigs[-1]) { + setChartConfigs(prevConfigs => ({ + ...prevConfigs, + [-1]: { + id: -1, + crossFilters: { + scope: globalChartConfig.scope, + chartsInScope: globalChartConfig.chartsInScope, + }, + }, + })); + } + }, [chartConfigs, globalChartConfig.chartsInScope, globalChartConfig.scope]); + + const handleSelectChange = useCallback( + (newChartId: number) => { + if (isDefined(currentChartId)) { + const currentScope = + chartConfigs[currentChartId]?.crossFilters.scope !== 'global' + ? (chartConfigs[currentChartId].crossFilters + .scope as NativeFilterScope) + : globalChartConfig.scope; + const newScope = { + rootPath: currentScope.rootPath, + excluded: [ + ...currentScope.excluded.filter(id => id !== currentChartId), + newChartId, + ], + }; + const newCrossFiltersConfig = { + id: newChartId, + crossFilters: { + scope: newScope, + chartsInScope: getChartIdsInFilterScope(newScope, chartIds, layout), + }, + }; + + setChartConfigs(prevConfig => { + const newConfig = { + ...prevConfig, + [newChartId]: newCrossFiltersConfig, + }; + if (currentChartId < 0) { + delete newConfig[currentChartId]; + } else { + newConfig[currentChartId] = { + id: currentChartId, + crossFilters: { + scope: 'global', + chartsInScope: globalChartConfig.chartsInScope.filter( + id => id !== currentChartId, + ), + }, + }; + } + return newConfig; + }); + + setCurrentChartId(newChartId); + } + }, + [ + chartConfigs, + chartIds, + currentChartId, + globalChartConfig.chartsInScope, + globalChartConfig.scope, + layout, + ], + ); + + const scope = useMemo(() => { + const globalScope = globalChartConfig.scope; + if (!isDefined(currentChartId)) { + return globalScope; + } + if ( + isCrossFilterScopeGlobal( + chartConfigs[currentChartId]?.crossFilters?.scope, + ) + ) { + return getActualScopeFromGlobalScope(currentChartId, globalScope); + } + return chartConfigs[currentChartId]?.crossFilters + ?.scope as NativeFilterScope; + }, [chartConfigs, currentChartId, globalChartConfig.scope]); + + return ( + + + + ); +}; diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/CrossFilters/ScopingModal/ScopingModalContent.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/CrossFilters/ScopingModal/ScopingModalContent.tsx new file mode 100644 index 0000000000000..e4ff80896e5d3 --- /dev/null +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/CrossFilters/ScopingModal/ScopingModalContent.tsx @@ -0,0 +1,81 @@ +/** + * 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 React from 'react'; +import { css, NativeFilterScope, styled, useTheme } from '@superset-ui/core'; +import { ChartConfiguration } from 'src/dashboard/types'; +import { ScopingTreePanel } from './ScopingTreePanel'; +import { ChartsScopingListPanel } from './ChartsScopingListPanel'; + +export interface ScopingModalContentProps { + chartId: number | undefined; + currentScope: NativeFilterScope; + onScopeUpdate: ({ scope }: { scope: NativeFilterScope }) => void; + onSelectChange: (chartId: number) => void; + chartConfigs: ChartConfiguration; + setCurrentChartId: (chartId: number | undefined) => void; + removeCustomScope: (chartId: number) => void; + addNewCustomScope: () => void; +} + +const ModalContentContainer = styled.div` + ${({ theme }) => css` + display: flex; + & > div { + padding: ${theme.gridUnit * 4}px; + } + `} +`; + +export const ScopingModalContent = ({ + chartId, + currentScope, + onScopeUpdate, + onSelectChange, + chartConfigs, + setCurrentChartId, + removeCustomScope, + addNewCustomScope, +}: ScopingModalContentProps) => { + const theme = useTheme(); + return ( + +
+ +
+ + + ); +}; diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/CrossFilters/ScopingModal/ScopingTreePanel.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/CrossFilters/ScopingModal/ScopingTreePanel.tsx new file mode 100644 index 0000000000000..f06bf795f6fb3 --- /dev/null +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/CrossFilters/ScopingModal/ScopingTreePanel.tsx @@ -0,0 +1,197 @@ +/** + * 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 React, { useMemo } from 'react'; +import { + css, + isDefined, + NativeFilterScope, + styled, + t, + useTheme, +} from '@superset-ui/core'; +import { Select } from 'src/components'; +import { noOp } from 'src/utils/common'; +import ScopingTree from 'src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/FilterScope/ScopingTree'; +import { useSelector } from 'react-redux'; +import { + ChartConfiguration, + DashboardLayout, + isCrossFilterScopeGlobal, + RootState, +} from 'src/dashboard/types'; +import { CHART_TYPE } from 'src/dashboard/util/componentTypes'; +import { SelectOptionsType } from 'src/components/Select/types'; +import Icons from 'src/components/Icons'; +import { Tooltip } from 'src/components/Tooltip'; +import Alert from 'src/components/Alert'; + +interface ScopingTreePanelProps { + chartId: number | undefined; + currentScope: NativeFilterScope; + onScopeUpdate: ({ scope }: { scope: NativeFilterScope }) => void; + onSelectChange: (chartId: number) => void; + chartConfigs: ChartConfiguration; +} + +const InfoText = styled.div` + ${({ theme }) => css` + font-size: ${theme.typography.sizes.s}px; + color: ${theme.colors.grayscale.base}; + max-width: ${theme.gridUnit * 100}px; + margin-bottom: ${theme.gridUnit * 7}px; + `} +`; + +const ChartSelect = ({ + value, + onSelectChange, + chartConfigs, +}: { + value: number | undefined; + onSelectChange: (chartId: number) => void; + chartConfigs: ChartConfiguration; +}) => { + const theme = useTheme(); + const layout = useSelector( + state => state.dashboardLayout.present, + ); + const options: SelectOptionsType = useMemo(() => { + const chartLayoutItems = Object.values(layout).filter( + item => item.type === CHART_TYPE, + ); + return Object.values(chartConfigs) + .filter( + chartConfig => + isCrossFilterScopeGlobal(chartConfig.crossFilters.scope) || + (chartConfig.id === value && value !== -1), + ) + .map(chartConfig => { + const chartLayoutItem = chartLayoutItems.find( + item => item.meta.chartId === chartConfig.id, + ); + return { + value: chartConfig.id, + label: + chartLayoutItem?.meta.sliceNameOverride || + chartLayoutItem?.meta.sliceName || + '', + }; + }); + }, [chartConfigs, layout, value]); + + return ( +
+
+ {`${t('Chart')} *`} + + span { + line-height: 0; + } + `} + /> + +
+ { onSelectChange(Number(value)); }} diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/CrossFilters/ScopingModal/constants.ts b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/CrossFilters/ScopingModal/constants.ts new file mode 100644 index 0000000000000..abacb5cd02417 --- /dev/null +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/CrossFilters/ScopingModal/constants.ts @@ -0,0 +1,20 @@ +/** + * 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. + */ + +export const NEW_CHART_SCOPING_ID = -1; diff --git a/superset-frontend/src/dashboard/types.ts b/superset-frontend/src/dashboard/types.ts index 5d4248334cc61..aa261c0768393 100644 --- a/superset-frontend/src/dashboard/types.ts +++ b/superset-frontend/src/dashboard/types.ts @@ -60,9 +60,10 @@ export enum FilterBarOrientation { } // chart's cross filter scoping can have its custom value or point to the global configuration -export type ChartCrossFiltersGlobalScope = 'global'; +export const GLOBAL_SCOPE_POINTER = 'global'; +export type GlobalScopePointer = typeof GLOBAL_SCOPE_POINTER; export type ChartCrossFiltersConfig = { - scope: NativeFilterScope | ChartCrossFiltersGlobalScope; + scope: NativeFilterScope | GlobalScopePointer; chartsInScope: number[]; }; export type GlobalChartCrossFilterConfig = { @@ -70,8 +71,8 @@ export type GlobalChartCrossFilterConfig = { chartsInScope: number[]; }; export const isCrossFilterScopeGlobal = ( - scope: NativeFilterScope | ChartCrossFiltersGlobalScope, -): scope is ChartCrossFiltersGlobalScope => scope === 'global'; + scope: NativeFilterScope | GlobalScopePointer, +): scope is GlobalScopePointer => scope === GLOBAL_SCOPE_POINTER; export type ChartConfiguration = { [chartId: number]: { diff --git a/superset-frontend/src/dashboard/util/crossFilters.ts b/superset-frontend/src/dashboard/util/crossFilters.ts index 80312c3fcc080..77b2b36f35a5a 100644 --- a/superset-frontend/src/dashboard/util/crossFilters.ts +++ b/superset-frontend/src/dashboard/util/crossFilters.ts @@ -29,6 +29,7 @@ import { ChartsState, DashboardInfo, DashboardLayout, + GLOBAL_SCOPE_POINTER, isCrossFilterScopeGlobal, } from '../types'; import { DEFAULT_CROSS_FILTER_SCOPING } from '../constants'; @@ -85,7 +86,7 @@ export const getCrossFiltersConfiguration = ( chartConfiguration[chartId] = { id: chartId, crossFilters: { - scope: 'global' as const, + scope: GLOBAL_SCOPE_POINTER, }, }; } From 6cec269eaae9d13a716430a4214ef02b3785f1a1 Mon Sep 17 00:00:00 2001 From: Kamil Gabryjelski Date: Thu, 11 May 2023 13:35:25 +0200 Subject: [PATCH 04/11] add migration --- ...ea966691069_cross_filter_global_scoping.py | 105 ++++++++++++++++++ 1 file changed, 105 insertions(+) create mode 100644 superset/migrations/versions/2023-05-11_12-41_4ea966691069_cross_filter_global_scoping.py diff --git a/superset/migrations/versions/2023-05-11_12-41_4ea966691069_cross_filter_global_scoping.py b/superset/migrations/versions/2023-05-11_12-41_4ea966691069_cross_filter_global_scoping.py new file mode 100644 index 0000000000000..9072919f82f43 --- /dev/null +++ b/superset/migrations/versions/2023-05-11_12-41_4ea966691069_cross_filter_global_scoping.py @@ -0,0 +1,105 @@ +# 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. +"""cross-filter-global-scoping + +Revision ID: 4ea966691069 +Revises: 9c2a5681ddfd +Create Date: 2023-05-11 12:41:38.095717 + +""" + +# revision identifiers, used by Alembic. +revision = "4ea966691069" +down_revision = "9c2a5681ddfd" + +import copy +import json + +import sqlalchemy as sa +from alembic import op +from sqlalchemy.ext.declarative import declarative_base + +from superset import db + +Base = declarative_base() + + +class Dashboard(Base): + __tablename__ = "dashboards" + + id = sa.Column(sa.Integer, primary_key=True) + json_metadata = sa.Column(sa.Text) + + +def upgrade(): + bind = op.get_bind() + session = db.Session(bind=bind) + + for dashboard in session.query(Dashboard).all(): + try: + json_metadata = json.loads(dashboard.json_metadata) + new_chart_configuration = {} + for config in json_metadata.get("chart_configuration", {}).values(): + chart_id = int(config.get("id", 0)) + scope = config.get("crossFilters", {}).get("scope", {}) + new_chart_configuration[chart_id] = copy.deepcopy(config) + new_chart_configuration[chart_id]["id"] = chart_id + if scope.get("rootPath", []) == ["ROOT_ID"] and scope.get( + "excluded", [] + ) == [chart_id]: + new_chart_configuration[chart_id]["crossFilters"][ + "scope" + ] = "global" + + json_metadata.chart_configuration = new_chart_configuration + dashboard.json_metadata = json.dumps(json_metadata) + + except Exception: + pass + + session.commit() + session.close() + + +def downgrade(): + bind = op.get_bind() + session = db.Session(bind=bind) + + for dashboard in session.query(Dashboard).all(): + try: + json_metadata = json.loads(dashboard.json_metadata) + new_chart_configuration = {} + for config in json_metadata.get("chart_configuration", {}).values(): + chart_id = config.get("id") + if chart_id is None: + continue + scope = config.get("crossFilters", {}).get("scope", {}) + new_chart_configuration[chart_id] = copy.deepcopy(config) + if scope == "global": + new_chart_configuration[chart_id]["crossFilters"]["scope"] = { + "rootPath": ["ROOT_ID"], + "excluded": [chart_id], + } + + json_metadata.chart_configuration = new_chart_configuration + dashboard.json_metadata = json.dumps(json_metadata) + + except Exception: + pass + + session.commit() + session.close() From 7462456c4f92e32b5c00d71c0b60ddeb84301e80 Mon Sep 17 00:00:00 2001 From: Kamil Gabryjelski Date: Thu, 11 May 2023 14:42:15 +0200 Subject: [PATCH 05/11] fix migration --- ...1_4ea966691069_cross_filter_global_scoping.py | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/superset/migrations/versions/2023-05-11_12-41_4ea966691069_cross_filter_global_scoping.py b/superset/migrations/versions/2023-05-11_12-41_4ea966691069_cross_filter_global_scoping.py index 9072919f82f43..e8f35335fb9cc 100644 --- a/superset/migrations/versions/2023-05-11_12-41_4ea966691069_cross_filter_global_scoping.py +++ b/superset/migrations/versions/2023-05-11_12-41_4ea966691069_cross_filter_global_scoping.py @@ -48,7 +48,6 @@ class Dashboard(Base): def upgrade(): bind = op.get_bind() session = db.Session(bind=bind) - for dashboard in session.query(Dashboard).all(): try: json_metadata = json.loads(dashboard.json_metadata) @@ -56,16 +55,20 @@ def upgrade(): for config in json_metadata.get("chart_configuration", {}).values(): chart_id = int(config.get("id", 0)) scope = config.get("crossFilters", {}).get("scope", {}) + excluded = [ + int(excluded_id) for excluded_id in scope.get("excluded", []) + ] new_chart_configuration[chart_id] = copy.deepcopy(config) new_chart_configuration[chart_id]["id"] = chart_id - if scope.get("rootPath", []) == ["ROOT_ID"] and scope.get( - "excluded", [] - ) == [chart_id]: + new_chart_configuration[chart_id]["crossFilters"]["scope"][ + "excluded" + ] = excluded + if scope.get("rootPath") == ["ROOT_ID"] and excluded == [chart_id]: new_chart_configuration[chart_id]["crossFilters"][ "scope" ] = "global" - json_metadata.chart_configuration = new_chart_configuration + json_metadata["chart_configuration"] = new_chart_configuration dashboard.json_metadata = json.dumps(json_metadata) except Exception: @@ -95,7 +98,8 @@ def downgrade(): "excluded": [chart_id], } - json_metadata.chart_configuration = new_chart_configuration + json_metadata["chart_configuration"] = new_chart_configuration + del json_metadata["global_chart_configuration"] dashboard.json_metadata = json.dumps(json_metadata) except Exception: From e094215735af4fa3a9b6322825ff792e22850fd2 Mon Sep 17 00:00:00 2001 From: Kamil Gabryjelski Date: Fri, 12 May 2023 14:40:17 +0200 Subject: [PATCH 06/11] Add ut --- .../ChartsScopingListPanel.test.tsx | 175 ++++++++++ .../ScopingModal/ChartsScopingListPanel.tsx | 15 +- .../ScopingModal/ScopingModal.test.tsx | 322 ++++++++++++++++++ .../ScopingModal/ScopingModal.tsx | 15 +- .../ScopingModal/ScopingModalContent.tsx | 1 + .../ScopingModal/ScopingTreePanel.tsx | 5 +- .../useCrossFiltersScopingModal.test.ts | 40 +++ .../useCrossFiltersScopingModal.tsx | 4 +- 8 files changed, 560 insertions(+), 17 deletions(-) create mode 100644 superset-frontend/src/dashboard/components/nativeFilters/FilterBar/CrossFilters/ScopingModal/ChartsScopingListPanel.test.tsx create mode 100644 superset-frontend/src/dashboard/components/nativeFilters/FilterBar/CrossFilters/ScopingModal/ScopingModal.test.tsx create mode 100644 superset-frontend/src/dashboard/components/nativeFilters/FilterBar/CrossFilters/ScopingModal/useCrossFiltersScopingModal.test.ts diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/CrossFilters/ScopingModal/ChartsScopingListPanel.test.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/CrossFilters/ScopingModal/ChartsScopingListPanel.test.tsx new file mode 100644 index 0000000000000..997933d97e747 --- /dev/null +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/CrossFilters/ScopingModal/ChartsScopingListPanel.test.tsx @@ -0,0 +1,175 @@ +/** + * 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 React from 'react'; +import userEvent from '@testing-library/user-event'; +import { render, screen, within } from 'spec/helpers/testing-library'; +import { CHART_TYPE } from 'src/dashboard/util/componentTypes'; +import { + ChartsScopingListPanel, + ChartsScopingListPanelProps, +} from './ChartsScopingListPanel'; + +const DEFAULT_PROPS: ChartsScopingListPanelProps = { + addNewCustomScope: jest.fn(), + removeCustomScope: jest.fn(), + setCurrentChartId: jest.fn(), + activeChartId: undefined, + chartConfigs: { + 1: { + id: 1, + crossFilters: { + scope: 'global' as const, + chartsInScope: [2, 3, 4], + }, + }, + 2: { + id: 2, + crossFilters: { + scope: 'global' as const, + chartsInScope: [1, 3, 4], + }, + }, + 3: { + id: 3, + crossFilters: { + scope: { + rootPath: ['ROOT_ID'], + excluded: [1, 3], + }, + chartsInScope: [2, 4], + }, + }, + 4: { + id: 4, + crossFilters: { + scope: { + rootPath: ['ROOT_ID'], + excluded: [1, 4], + }, + chartsInScope: [2, 3], + }, + }, + }, +}; + +const INITIAL_STATE = { + dashboardLayout: { + past: [], + future: [], + present: { + CHART_1: { + id: 'CHART_1', + type: CHART_TYPE, + meta: { + chartId: 1, + sliceName: 'chart 1', + }, + }, + CHART_2: { + id: 'CHART_2', + type: CHART_TYPE, + meta: { + chartId: 2, + sliceName: 'chart 2', + }, + }, + CHART_3: { + id: 'CHART_3', + type: CHART_TYPE, + meta: { + chartId: 3, + sliceName: 'chart 3', + sliceNameOverride: 'Chart 3', + }, + }, + CHART_4: { + id: 'CHART_4', + type: CHART_TYPE, + meta: { + chartId: 4, + sliceName: 'chart 4', + }, + }, + }, + }, +}; + +const setup = (props = DEFAULT_PROPS) => + render(, { + useRedux: true, + initialState: INITIAL_STATE, + }); + +it('Renders charts scoping list panel', () => { + setup(); + expect(screen.getByText('Add custom scoping')).toBeVisible(); + expect(screen.getByText('All charts/global scoping')).toBeVisible(); + expect(screen.getByText('All charts/global scoping')).toHaveClass('active'); + expect(screen.queryByText('chart 1')).not.toBeInTheDocument(); + expect(screen.queryByText('chart 2')).not.toBeInTheDocument(); + expect(screen.getByText('Chart 3')).toBeVisible(); + expect(screen.getByText('chart 4')).toBeVisible(); + expect(screen.queryByText('[new custom scoping]')).not.toBeInTheDocument(); +}); + +it('Renders custom scoping item', () => { + setup({ + ...DEFAULT_PROPS, + activeChartId: -1, + chartConfigs: { + ...DEFAULT_PROPS.chartConfigs, + [-1]: { + id: -1, + crossFilters: { + scope: 'global', + chartsInScope: [1, 2, 3, 4], + }, + }, + }, + }); + expect(screen.getByText('All charts/global scoping')).toBeVisible(); + expect(screen.getByText('All charts/global scoping')).not.toHaveClass( + 'active', + ); + expect(screen.queryByText('chart 1')).not.toBeInTheDocument(); + expect(screen.queryByText('chart 2')).not.toBeInTheDocument(); + expect(screen.getByText('Chart 3')).toBeVisible(); + expect(screen.getByText('chart 4')).toBeVisible(); + expect(screen.getByText('[new custom scoping]')).toBeVisible(); + expect(screen.getByText('[new custom scoping]')).toHaveClass('active'); +}); + +it('Uses callbacks on click', () => { + setup(); + + userEvent.click(screen.getByText('Add custom scoping')); + expect(DEFAULT_PROPS.addNewCustomScope).toHaveBeenCalled(); + + userEvent.click(screen.getByText('All charts/global scoping')); + expect(DEFAULT_PROPS.setCurrentChartId).toHaveBeenCalledWith(undefined); + + userEvent.click(screen.getByText('Chart 3')); + expect(DEFAULT_PROPS.setCurrentChartId).toHaveBeenCalledWith(3); + + const chart4Container = screen.getByText('chart 4').closest('div'); + if (chart4Container) { + userEvent.click(within(chart4Container).getByLabelText('trash')); + } + expect(DEFAULT_PROPS.removeCustomScope).toHaveBeenCalledWith(4); +}); diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/CrossFilters/ScopingModal/ChartsScopingListPanel.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/CrossFilters/ScopingModal/ChartsScopingListPanel.tsx index 526e2823edc1f..7e2b90e7b6728 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/CrossFilters/ScopingModal/ChartsScopingListPanel.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/CrossFilters/ScopingModal/ChartsScopingListPanel.tsx @@ -85,19 +85,20 @@ const ScopingTitle = ({ ); }; +export interface ChartsScopingListPanelProps { + activeChartId: number | undefined; + chartConfigs: ChartConfiguration; + setCurrentChartId: (chartId: number | undefined) => void; + removeCustomScope: (chartId: number) => void; + addNewCustomScope: () => void; +} export const ChartsScopingListPanel = ({ activeChartId, chartConfigs, setCurrentChartId, removeCustomScope, addNewCustomScope, -}: { - activeChartId: number | undefined; - chartConfigs: ChartConfiguration; - setCurrentChartId: (chartId: number | undefined) => void; - removeCustomScope: (chartId: number) => void; - addNewCustomScope: () => void; -}) => { +}: ChartsScopingListPanelProps) => { const theme = useTheme(); const layout = useSelector( state => state.dashboardLayout.present, diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/CrossFilters/ScopingModal/ScopingModal.test.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/CrossFilters/ScopingModal/ScopingModal.test.tsx new file mode 100644 index 0000000000000..eef5d1fa07fff --- /dev/null +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/CrossFilters/ScopingModal/ScopingModal.test.tsx @@ -0,0 +1,322 @@ +/** + * 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 React from 'react'; +import fetchMock from 'fetch-mock'; +import userEvent from '@testing-library/user-event'; +import { render, screen, waitFor, within } from 'spec/helpers/testing-library'; +import { + CHART_TYPE, + DASHBOARD_ROOT_TYPE, +} from 'src/dashboard/util/componentTypes'; +import { DASHBOARD_ROOT_ID } from 'src/dashboard/util/constants'; +import { ScopingModal, ScopingModalProps } from './ScopingModal'; + +const INITIAL_STATE = { + charts: { + 1: { id: 1 }, + 2: { id: 2 }, + 3: { id: 3 }, + 4: { id: 4 }, + }, + dashboardInfo: { + id: 1, + metadata: { + chart_configuration: { + 1: { + id: 1, + crossFilters: { + scope: 'global' as const, + chartsInScope: [2, 3, 4], + }, + }, + 2: { + id: 2, + crossFilters: { + scope: 'global' as const, + chartsInScope: [1, 3, 4], + }, + }, + 3: { + id: 3, + crossFilters: { + scope: { + rootPath: ['ROOT_ID'], + excluded: [1, 3], + }, + chartsInScope: [2, 4], + }, + }, + 4: { + id: 4, + crossFilters: { + scope: { + rootPath: ['ROOT_ID'], + excluded: [1, 4], + }, + chartsInScope: [2, 3], + }, + }, + }, + global_chart_configuration: { + scope: { rootPath: ['ROOT_ID'], excluded: [] }, + chartsInScope: [1, 2, 3, 4], + }, + }, + }, + dashboardLayout: { + past: [], + future: [], + present: { + [DASHBOARD_ROOT_ID]: { + type: DASHBOARD_ROOT_TYPE, + id: DASHBOARD_ROOT_ID, + children: ['CHART_1', 'CHART_2', 'CHART_3', 'CHART_4'], + }, + CHART_1: { + id: 'CHART_1', + type: CHART_TYPE, + meta: { + chartId: 1, + sliceName: 'chart 1', + }, + parents: ['ROOT_ID'], + }, + CHART_2: { + id: 'CHART_2', + type: CHART_TYPE, + meta: { + chartId: 2, + sliceName: 'chart 2', + }, + parents: ['ROOT_ID'], + }, + CHART_3: { + id: 'CHART_3', + type: CHART_TYPE, + meta: { + chartId: 3, + sliceName: 'chart 3', + sliceNameOverride: 'Chart 3', + }, + parents: ['ROOT_ID'], + }, + CHART_4: { + id: 'CHART_4', + type: CHART_TYPE, + meta: { + chartId: 4, + sliceName: 'chart 4', + }, + parents: ['ROOT_ID'], + }, + }, + }, +}; + +const DEFAULT_PROPS: ScopingModalProps = { + closeModal: jest.fn(), + initialChartId: undefined, + isVisible: true, +}; + +const setup = (props = DEFAULT_PROPS) => + render(, { + useRedux: true, + initialState: INITIAL_STATE, + }); + +const DASHBOARD_UPDATE_URL = 'glob:*api/v1/dashboard/1'; +beforeEach(() => { + fetchMock.put(DASHBOARD_UPDATE_URL, 200); +}); + +afterEach(() => { + fetchMock.restore(); +}); + +it('renders modal', () => { + setup(); + expect(screen.getByRole('dialog')).toBeVisible(); + expect(screen.getByTestId('scoping-tree-panel')).toBeInTheDocument(); + expect(screen.getByTestId('scoping-list-panel')).toBeInTheDocument(); +}); + +it('switch currently edited chart scoping', async () => { + setup(); + const withinScopingList = within(screen.getByTestId('scoping-list-panel')); + expect(withinScopingList.getByText('All charts/global scoping')).toHaveClass( + 'active', + ); + userEvent.click(withinScopingList.getByText('Chart 3')); + await waitFor(() => { + expect(withinScopingList.getByText('Chart 3')).toHaveClass('active'); + expect( + withinScopingList.getByText('All charts/global scoping'), + ).not.toHaveClass('active'); + }); +}); + +it('scoping tree global and custom checks', () => { + setup(); + + expect( + document.querySelectorAll( + '[data-test="scoping-tree-panel"] .ant-tree-checkbox-checked', + ), + ).toHaveLength(5); + + userEvent.click( + within(screen.getByTestId('scoping-list-panel')).getByText('Chart 3'), + ); + + expect( + document.querySelectorAll( + '[data-test="scoping-tree-panel"] .ant-tree-checkbox-checked', + ), + ).toHaveLength(2); +}); + +it('add new custom scoping', async () => { + setup(); + + userEvent.click(screen.getByText('Add custom scoping')); + + expect(screen.getByText('[new custom scoping]')).toBeInTheDocument(); + expect(screen.getByText('[new custom scoping]')).toHaveClass('active'); + + await waitFor(() => + userEvent.click(screen.getByRole('combobox', { name: 'Select chart' })), + ); + await waitFor(() => { + userEvent.click( + within(document.querySelector('.rc-virtual-list')!).getByText('chart 1'), + ); + }); + + expect( + within(document.querySelector('.ant-select-selection-item')!).getByText( + 'chart 1', + ), + ).toBeInTheDocument(); + + expect( + document.querySelectorAll( + '[data-test="scoping-tree-panel"] .ant-tree-checkbox-checked', + ), + ).toHaveLength(4); + + userEvent.click( + within(document.querySelector('.ant-tree')!).getByText('chart 2'), + ); + + expect( + document.querySelectorAll( + '[data-test="scoping-tree-panel"] .ant-tree-checkbox-checked', + ), + ).toHaveLength(2); +}); + +it('edit scope and save', async () => { + setup(); + + // unselect chart 2 in global scoping + userEvent.click( + within(document.querySelector('.ant-tree')!).getByText('chart 2'), + ); + + userEvent.click( + within(screen.getByTestId('scoping-list-panel')).getByText('Chart 3'), + ); + + // select chart 1 in chart 3's custom scoping + userEvent.click( + within(document.querySelector('.ant-tree')!).getByText('chart 1'), + ); + + // create custom scoping for chart 1 with unselected chart 2 (from global) and chart 4 + userEvent.click(screen.getByText('Add custom scoping')); + await waitFor(() => + userEvent.click(screen.getByRole('combobox', { name: 'Select chart' })), + ); + await waitFor(() => { + userEvent.click( + within(document.querySelector('.rc-virtual-list')!).getByText('chart 1'), + ); + }); + userEvent.click( + within(document.querySelector('.ant-tree')!).getByText('chart 4'), + ); + + // remove custom scoping for chart 4 + userEvent.click( + within( + within(screen.getByTestId('scoping-list-panel')) + .getByText('chart 4') + .closest('div')!, + ).getByLabelText('trash'), + ); + expect( + within(screen.getByTestId('scoping-list-panel')).queryByText('chart 4'), + ).not.toBeInTheDocument(); + + userEvent.click(screen.getByText('Save')); + + await waitFor(() => fetchMock.called(DASHBOARD_UPDATE_URL)); + + expect( + JSON.parse( + JSON.parse(fetchMock.lastCall()?.[1]?.body as string).json_metadata, + ), + ).toEqual({ + chart_configuration: { + '1': { + id: 1, + crossFilters: { + scope: { rootPath: ['ROOT_ID'], excluded: [1, 2, 4] }, + chartsInScope: [3], + }, + }, + '2': { + id: 2, + crossFilters: { + scope: 'global', + chartsInScope: [1, 3, 4], + }, + }, + '3': { + id: 3, + crossFilters: { + scope: { rootPath: ['ROOT_ID'], excluded: [3] }, + chartsInScope: [1, 2, 4], + }, + }, + '4': { + id: 4, + crossFilters: { + scope: 'global', + chartsInScope: [1, 3], + }, + }, + }, + global_chart_configuration: { + scope: { rootPath: ['ROOT_ID'], excluded: [2] }, + chartsInScope: [1, 3, 4], + }, + }); +}); diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/CrossFilters/ScopingModal/ScopingModal.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/CrossFilters/ScopingModal/ScopingModal.tsx index a623b5ecf97b4..e48554c93ae8c 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/CrossFilters/ScopingModal/ScopingModal.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/CrossFilters/ScopingModal/ScopingModal.tsx @@ -42,7 +42,7 @@ const getUpdatedGloballyScopedChartsInScope = ( Object.entries(configs).reduce((acc, [id, config]) => { if (isCrossFilterScopeGlobal(config.crossFilters.scope)) { acc[id] = { - id, + id: Number(config.id), crossFilters: { scope: GLOBAL_SCOPE_POINTER, chartsInScope: globalChartsInScope.filter( @@ -64,15 +64,17 @@ const getActualScopeFromGlobalScope = ( excluded: globalScope.excluded.filter(id => id !== chartId), }); +export interface ScopingModalProps { + initialChartId: number | undefined; + isVisible: boolean; + closeModal: () => void; +} + export const ScopingModal = ({ initialChartId, isVisible, closeModal, -}: { - initialChartId: number | undefined; - isVisible: boolean; - closeModal: () => void; -}) => { +}: ScopingModalProps) => { const dispatch = useDispatch(); const layout = useSelector( state => state.dashboardLayout.present, @@ -299,6 +301,7 @@ export const ScopingModal = ({ show={isVisible} title={t('Cross-filtering scoping')} onHandledPrimaryAction={saveScoping} + primaryButtonName={t('Save')} responsive destroyOnClose bodyStyle={{ diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/CrossFilters/ScopingModal/ScopingModalContent.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/CrossFilters/ScopingModal/ScopingModalContent.tsx index e4ff80896e5d3..0b90f8687926f 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/CrossFilters/ScopingModal/ScopingModalContent.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/CrossFilters/ScopingModal/ScopingModalContent.tsx @@ -60,6 +60,7 @@ export const ScopingModalContent = ({ width: 30%; border-right: 1px solid ${theme.colors.grayscale.light2}; `} + data-test="scoping-list-panel" >