diff --git a/superset-frontend/src/dashboard/components/GroupByBadge/index.tsx b/superset-frontend/src/dashboard/components/GroupByBadge/index.tsx index 8c1af0aecaa3..f73608666e7b 100644 --- a/superset-frontend/src/dashboard/components/GroupByBadge/index.tsx +++ b/superset-frontend/src/dashboard/components/GroupByBadge/index.tsx @@ -18,6 +18,7 @@ */ import { memo, useMemo, useState, useRef } from 'react'; import { useSelector } from 'react-redux'; +import { createSelector } from '@reduxjs/toolkit'; import { t } from '@superset-ui/core'; import { styled, useTheme } from '@apache-superset/core/ui'; import { Icons, Badge, Tooltip, Tag } from '@superset-ui/core/components'; @@ -26,6 +27,26 @@ import { ChartCustomizationItem } from '../nativeFilters/ChartCustomization/type import { RootState } from '../../types'; import { isChartWithoutGroupBy } from '../../util/charts/chartTypeLimitations'; +const makeSelectChartDataset = (chartId: number) => + createSelector( + (state: RootState) => state.charts[chartId]?.latestQueryFormData, + latestQueryFormData => { + if (!latestQueryFormData?.datasource) { + return null; + } + const chartDatasetParts = String(latestQueryFormData.datasource).split( + '__', + ); + return chartDatasetParts[0]; + }, + ); + +const makeSelectChartFormData = (chartId: number) => + createSelector( + (state: RootState) => state.charts[chartId]?.latestQueryFormData, + latestQueryFormData => latestQueryFormData, + ); + export interface GroupByBadgeProps { chartId: number; } @@ -142,16 +163,19 @@ export const GroupByBadge = ({ chartId }: GroupByBadgeProps) => { dashboardInfo.metadata?.chart_customization_config || [], ); - const chartDataset = useSelector(state => { - const chart = state.charts[chartId]; - if (!chart?.latestQueryFormData?.datasource) { - return null; - } - const chartDatasetParts = String( - chart.latestQueryFormData.datasource, - ).split('__'); - return chartDatasetParts[0]; - }); + // Use memoized selectors for chart data + const selectChartDataset = useMemo( + () => makeSelectChartDataset(chartId), + [chartId], + ); + const selectChartFormData = useMemo( + () => makeSelectChartFormData(chartId), + [chartId], + ); + + const chartDataset = useSelector(selectChartDataset); + const chartFormData = useSelector(selectChartFormData); + const chartType = chartFormData?.viz_type; const applicableGroupBys = useMemo(() => { if (!chartDataset) { @@ -173,9 +197,6 @@ export const GroupByBadge = ({ chartId }: GroupByBadgeProps) => { }); }, [chartCustomizationItems, chartDataset]); - const chart = useSelector(state => state.charts[chartId]); - const chartType = chart?.latestQueryFormData?.viz_type; - const effectiveGroupBys = useMemo(() => { if (!chartType || applicableGroupBys.length === 0) { return []; @@ -185,7 +206,6 @@ export const GroupByBadge = ({ chartId }: GroupByBadgeProps) => { return []; } - const chartFormData = chart?.latestQueryFormData; if (!chartFormData) { return applicableGroupBys; } @@ -278,7 +298,7 @@ export const GroupByBadge = ({ chartId }: GroupByBadgeProps) => { return columnNames.length > 0; }); - }, [applicableGroupBys, chartType, chart]); + }, [applicableGroupBys, chartType, chartFormData]); const groupByCount = effectiveGroupBys.length; diff --git a/superset-frontend/src/dashboard/components/gridComponents/Chart/Chart.jsx b/superset-frontend/src/dashboard/components/gridComponents/Chart/Chart.jsx index 353ae0077267..4fb02867d862 100644 --- a/superset-frontend/src/dashboard/components/gridComponents/Chart/Chart.jsx +++ b/superset-frontend/src/dashboard/components/gridComponents/Chart/Chart.jsx @@ -110,6 +110,7 @@ const SliceContainer = styled.div` `; const EMPTY_OBJECT = {}; +const EMPTY_ARRAY = []; const Chart = props => { const dispatch = useDispatch(); @@ -284,7 +285,8 @@ const Chart = props => { state => state.dashboardInfo.metadata?.chart_configuration, ); const chartCustomizationItems = useSelector( - state => state.dashboardInfo.metadata?.chart_customization_config || [], + state => + state.dashboardInfo.metadata?.chart_customization_config || EMPTY_ARRAY, ); const colorScheme = useSelector(state => state.dashboardState.colorScheme); const colorNamespace = useSelector( @@ -296,8 +298,8 @@ const Chart = props => { const allSliceIds = useSelector(state => state.dashboardState.sliceIds); const nativeFilters = useSelector(state => state.nativeFilters?.filters); const dataMask = useSelector(state => state.dataMask); - const chartStates = useSelector( - state => state.dashboardState.chartStates || EMPTY_OBJECT, + const chartState = useSelector( + state => state.dashboardState.chartStates?.[props.id], ); const labelsColor = useSelector( state => state.dashboardInfo?.metadata?.label_colors || EMPTY_OBJECT, @@ -314,7 +316,7 @@ const Chart = props => { const formData = useMemo( () => getFormDataWithExtraFilters({ - chart, + chart: { id: chart.id, form_data: chart.form_data }, // avoid passing the whole chart object chartConfiguration, chartCustomizationItems, filters: getAppliedFilterValues(props.id), @@ -331,7 +333,8 @@ const Chart = props => { ownColorScheme, }), [ - chart, + chart.id, + chart.form_data, chartConfiguration, chartCustomizationItems, props.id, @@ -350,6 +353,25 @@ const Chart = props => { formData.dashboardId = dashboardInfo.id; + const ownState = useMemo(() => { + const baseOwnState = dataMask[props.id]?.ownState || EMPTY_OBJECT; + + if (hasChartStateConverter(slice.viz_type) && chartState?.state) { + return { + ...baseOwnState, + ...convertChartStateToOwnState(slice.viz_type, chartState.state), + chartState: chartState.state, + }; + } + + return baseOwnState; + }, [ + dataMask[props.id]?.ownState, + props.id, + slice.viz_type, + chartState?.state, + ]); + const onExploreChart = useCallback( async clickEvent => { const isOpenInNewTab = @@ -406,13 +428,10 @@ const Chart = props => { let ownState = dataMask[props.id]?.ownState || {}; // Convert chart-specific state to backend format using registered converter - if ( - hasChartStateConverter(slice.viz_type) && - chartStates[props.id]?.state - ) { + if (hasChartStateConverter(slice.viz_type) && chartState?.state) { const convertedState = convertChartStateToOwnState( slice.viz_type, - chartStates[props.id].state, + chartState.state, ); ownState = { ...ownState, @@ -435,7 +454,7 @@ const Chart = props => { formData, maxRows, dataMask[props.id]?.ownState, - chartStates, + chartState, props.id, boundActionCreators.logEvent, ], @@ -577,19 +596,7 @@ const Chart = props => { formData={formData} labelsColor={labelsColor} labelsColorMap={labelsColorMap} - ownState={{ - ...dataMask[props.id]?.ownState, - ...(hasChartStateConverter(slice.viz_type) && - chartStates[props.id]?.state - ? { - ...convertChartStateToOwnState( - slice.viz_type, - chartStates[props.id].state, - ), - chartState: chartStates[props.id].state, - } - : {}), - }} + ownState={ownState} filterState={dataMask[props.id]?.filterState} queriesResponse={chart.queriesResponse} timeout={timeout} diff --git a/superset-frontend/src/dashboard/components/nativeFilters/ChartCustomization/selectors.ts b/superset-frontend/src/dashboard/components/nativeFilters/ChartCustomization/selectors.ts index f64dc36d5855..547db56cb4be 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/ChartCustomization/selectors.ts +++ b/superset-frontend/src/dashboard/components/nativeFilters/ChartCustomization/selectors.ts @@ -16,30 +16,32 @@ * specific language governing permissions and limitations * under the License. */ +import { createSelector } from '@reduxjs/toolkit'; import { RootState } from 'src/dashboard/types'; import { ChartCustomizationItem } from './types'; -export const selectChartCustomizationItems = ( - state: RootState, -): ChartCustomizationItem[] => { - const { metadata } = state.dashboardInfo; +const EMPTY_ARRAY: ChartCustomizationItem[] = []; - if ( - metadata?.chart_customization_config && - metadata.chart_customization_config.length > 0 - ) { - return metadata.chart_customization_config; - } +export const selectChartCustomizationItems = createSelector( + (state: RootState) => state.dashboardInfo.metadata, + (metadata): ChartCustomizationItem[] => { + if ( + metadata?.chart_customization_config && + metadata.chart_customization_config.length > 0 + ) { + return metadata.chart_customization_config; + } - const legacyCustomization = metadata?.native_filter_configuration?.find( - (item: any) => - item.type === 'CHART_CUSTOMIZATION' && - item.id === 'chart_customization_groupby', - ); + const legacyCustomization = metadata?.native_filter_configuration?.find( + (item: any) => + item.type === 'CHART_CUSTOMIZATION' && + item.id === 'chart_customization_groupby', + ); - if (legacyCustomization?.chart_customization) { - return legacyCustomization.chart_customization; - } + if (legacyCustomization?.chart_customization) { + return legacyCustomization.chart_customization; + } - return []; -}; + return EMPTY_ARRAY; + }, +); diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterControls.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterControls.tsx index 6c2d391c4910..9275de76da18 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterControls.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterControls.tsx @@ -54,7 +54,6 @@ import { import { Icons } from '@superset-ui/core/components/Icons'; import { useChartIds } from 'src/dashboard/util/charts/useChartIds'; import { useChartLayoutItems } from 'src/dashboard/util/useChartLayoutItems'; -import { ChartCustomizationItem } from 'src/dashboard/components/nativeFilters/ChartCustomization/types'; import { FiltersOutOfScopeCollapsible } from '../FiltersOutOfScopeCollapsible'; import { useFilterControlFactory } from '../useFilterControlFactory'; import { FiltersDropdownContent } from '../FiltersDropdownContent'; @@ -141,10 +140,7 @@ const FilterControls: FC = ({ const chartLayoutItems = useChartLayoutItems(); const verboseMaps = useChartsVerboseMaps(); - const chartCustomizationItems = useSelector< - RootState, - ChartCustomizationItem[] - >(state => selectChartCustomizationItems(state)); + const chartCustomizationItems = useSelector(selectChartCustomizationItems); const selectedCrossFilters = useMemo( () => diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/Horizontal.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/Horizontal.tsx index 3af13459fba5..dd610eda6191 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/Horizontal.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/Horizontal.tsx @@ -96,7 +96,7 @@ const HorizontalFilterBar: FC = ({ const chartCustomizationItems = useSelector< RootState, ChartCustomizationItem[] - >(state => selectChartCustomizationItems(state)); + >(selectChartCustomizationItems); const hasFilters = filterValues.length > 0 || diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/Vertical.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/Vertical.tsx index 76adfdfb4ebe..3a3bf0d66b84 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/Vertical.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/Vertical.tsx @@ -177,7 +177,7 @@ const VerticalFilterBar: FC = ({ const chartCustomizationItems = useSelector< RootState, ChartCustomizationItem[] - >(state => selectChartCustomizationItems(state)); + >(selectChartCustomizationItems); const dataMask = useSelector( state => state.dataMask, diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterCard/useFilterDependencies.ts b/superset-frontend/src/dashboard/components/nativeFilters/FilterCard/useFilterDependencies.ts index ee2cc9c6d2c3..9ce60bbc2e7c 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterCard/useFilterDependencies.ts +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterCard/useFilterDependencies.ts @@ -18,17 +18,32 @@ */ import { ensureIsArray, Filter } from '@superset-ui/core'; import { useSelector } from 'react-redux'; +import { useMemo } from 'react'; +import { createSelector } from '@reduxjs/toolkit'; import { RootState } from 'src/dashboard/types'; +const EMPTY_ARRAY: Filter[] = []; + +const makeSelectFilterDependencies = (filterDependencyIds: string[]) => + createSelector( + (state: RootState) => state.nativeFilters.filters, + (filters): Filter[] => { + if (filterDependencyIds.length === 0) { + return EMPTY_ARRAY; + } + return filterDependencyIds + .map(id => filters[id] as Filter) + .filter(Boolean); + }, + ); + export const useFilterDependencies = (filter: Filter) => { const filterDependencyIds = ensureIsArray(filter.cascadeParentIds); - return useSelector(state => { - if (filterDependencyIds.length === 0) { - return []; - } - return filterDependencyIds.reduce((acc: Filter[], filterDependencyId) => { - acc.push(state.nativeFilters.filters[filterDependencyId] as Filter); - return acc; - }, []); - }); + + const selectFilterDependencies = useMemo( + () => makeSelectFilterDependencies(filterDependencyIds), + [filterDependencyIds.join(',')], + ); + + return useSelector(selectFilterDependencies); }; diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigModal.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigModal.tsx index 85a4b6c2d940..ad5c154fc518 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigModal.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigModal.tsx @@ -311,24 +311,29 @@ function FiltersConfigModal({ [filterConfigMap, form, removedFilters], ); + const buildDependencyMap = useCallback(() => { + const dependencyMap = new Map(); + const filters = form.getFieldValue('filters'); + if (filters) { + Object.keys(filters).forEach(key => { + const formItem = filters[key]; + const configItem = filterConfigMap[key]; + let array: string[] = []; + if (formItem && 'dependencies' in formItem) { + array = [...formItem.dependencies]; + } else if (configItem?.cascadeParentIds) { + array = [...configItem.cascadeParentIds]; + } + dependencyMap.set(key, array); + }); + } + return dependencyMap; + }, [filterConfigMap, form]); + const getAvailableFilters = useCallback( (filterId: string) => { // Build current dependency map - const dependencyMap = new Map(); - const filters = form.getFieldValue('filters'); - if (filters) { - Object.keys(filters).forEach(key => { - const formItem = filters[key]; - const configItem = filterConfigMap[key]; - let array: string[] = []; - if (formItem && 'dependencies' in formItem) { - array = [...formItem.dependencies]; - } else if (configItem?.cascadeParentIds) { - array = [...configItem.cascadeParentIds]; - } - dependencyMap.set(key, array); - }); - } + const dependencyMap = buildDependencyMap(); return filterIds .filter(id => id !== filterId) @@ -348,12 +353,11 @@ function FiltersConfigModal({ })); }, [ + buildDependencyMap, canBeUsedAsDependency, filterConfigMap, filterIds, getFilterTitle, - form, - form.getFieldValue('filters'), ], ); @@ -515,25 +519,6 @@ function FiltersConfigModal({ })); }; - const buildDependencyMap = useCallback(() => { - const dependencyMap = new Map(); - const filters = form.getFieldValue('filters'); - if (filters) { - Object.keys(filters).forEach(key => { - const formItem = filters[key]; - const configItem = filterConfigMap[key]; - let array: string[] = []; - if (formItem && 'dependencies' in formItem) { - array = [...formItem.dependencies]; - } else if (configItem?.cascadeParentIds) { - array = [...configItem.cascadeParentIds]; - } - dependencyMap.set(key, array); - }); - } - return dependencyMap; - }, [filterConfigMap, form]); - const validateDependencies = useCallback(() => { const dependencyMap = buildDependencyMap(); filterIds diff --git a/superset-frontend/src/dashboard/components/nativeFilters/state.ts b/superset-frontend/src/dashboard/components/nativeFilters/state.ts index e506497a2af6..265e0ed5d2eb 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/state.ts +++ b/superset-frontend/src/dashboard/components/nativeFilters/state.ts @@ -18,27 +18,28 @@ */ import { useSelector } from 'react-redux'; import { useCallback, useMemo } from 'react'; -import { - Filter, - FilterConfiguration, - Divider, - isFilterDivider, -} from '@superset-ui/core'; +import { createSelector } from '@reduxjs/toolkit'; +import { Filter, Divider, isFilterDivider } from '@superset-ui/core'; import { ActiveTabs, DashboardLayout, RootState } from '../../types'; import { CHART_TYPE, TAB_TYPE } from '../../util/componentTypes'; const defaultFilterConfiguration: Filter[] = []; -export function useFilterConfiguration() { - return useSelector(state => { - const nativeFilterConfig = - state.dashboardInfo?.metadata?.native_filter_configuration || - defaultFilterConfiguration; - +const selectFilterConfiguration = createSelector( + (state: RootState) => + state.dashboardInfo?.metadata?.native_filter_configuration, + (nativeFilterConfig): (Filter | Divider)[] => { + if (!nativeFilterConfig) { + return defaultFilterConfiguration; + } return nativeFilterConfig.filter( (filter: any) => filter.type !== 'CHART_CUSTOMIZATION', ); - }); + }, +); + +export function useFilterConfiguration(): (Filter | Divider)[] { + return useSelector(selectFilterConfiguration); } /** diff --git a/superset-frontend/src/dashboard/util/charts/getFormDataWithExtraFilters.ts b/superset-frontend/src/dashboard/util/charts/getFormDataWithExtraFilters.ts index 77602818a399..e29ec9a7b842 100644 --- a/superset-frontend/src/dashboard/util/charts/getFormDataWithExtraFilters.ts +++ b/superset-frontend/src/dashboard/util/charts/getFormDataWithExtraFilters.ts @@ -21,6 +21,7 @@ import { DataMaskStateWithId, DataRecordFilters, DataRecordValue, + ensureIsArray, JsonObject, PartialFilters, QueryFormExtraFilter, @@ -81,7 +82,7 @@ const cachedFormdataByChart: Record< export interface GetFormDataWithExtraFiltersArguments { chartConfiguration: ChartConfiguration; chartCustomizationItems?: ChartCustomizationItem[]; - chart: ChartQueryPayload; + chart: Pick; filters: DataRecordFilters; colorScheme?: string; ownColorScheme?: string; @@ -132,11 +133,7 @@ function buildExistingColumnsSet(chart: ChartQueryPayload): Set { const existingColumns = new Set(); const chartType = chart.form_data?.viz_type; - const existingGroupBy = Array.isArray(chart.form_data?.groupby) - ? chart.form_data.groupby - : chart.form_data?.groupby - ? [chart.form_data.groupby] - : []; + const existingGroupBy = ensureIsArray(chart.form_data?.groupby); existingGroupBy.forEach((col: string) => existingColumns.add(col)); const xAxisColumn = chart.form_data?.x_axis; @@ -347,11 +344,7 @@ function processGroupByCustomizations( } const existingColumns = buildExistingColumnsSet(chart); - const existingGroupBy = Array.isArray(chart.form_data?.groupby) - ? chart.form_data.groupby - : chart.form_data?.groupby - ? [chart.form_data.groupby] - : []; + const existingGroupBy = ensureIsArray(chart.form_data?.groupby); const xAxisColumn = chart.form_data?.x_axis; const groupByColumns: string[] = [];