diff --git a/superset-frontend/src/dashboard/components/FiltersBadge/index.tsx b/superset-frontend/src/dashboard/components/FiltersBadge/index.tsx index aad602403369..c9ed50785f3c 100644 --- a/superset-frontend/src/dashboard/components/FiltersBadge/index.tsx +++ b/superset-frontend/src/dashboard/components/FiltersBadge/index.tsx @@ -201,30 +201,37 @@ export const FiltersBadge = ({ chartId }: FiltersBadgeProps) => { const prevChartConfig = usePrevious(chartConfiguration); useEffect(() => { - if (!showIndicators && nativeIndicators.length > 0) { + const shouldReset = + (!chart || + chart.chartStatus === 'failed' || + chart.chartStatus === null) && + nativeIndicators.length > 0; + + const shouldRecalculate = + chart?.queriesResponse?.[0]?.rejected_filters !== + prevChart?.queriesResponse?.[0]?.rejected_filters || + chart?.queriesResponse?.[0]?.applied_filters !== + prevChart?.queriesResponse?.[0]?.applied_filters || + nativeFilters !== prevNativeFilters || + chartLayoutItems !== prevChartLayoutItems || + dataMask !== prevDataMask || + prevChartConfig !== chartConfiguration; + + if (shouldReset) { setNativeIndicators(indicatorsInitialState); - } else if (prevChartStatus !== 'success') { - if ( - chart?.queriesResponse?.[0]?.rejected_filters !== - prevChart?.queriesResponse?.[0]?.rejected_filters || - chart?.queriesResponse?.[0]?.applied_filters !== - prevChart?.queriesResponse?.[0]?.applied_filters || - nativeFilters !== prevNativeFilters || - chartLayoutItems !== prevChartLayoutItems || - dataMask !== prevDataMask || - prevChartConfig !== chartConfiguration - ) { - setNativeIndicators( - selectNativeIndicatorsForChart( - nativeFilters, - dataMask, - chartId, - chart, - chartLayoutItems, - chartConfiguration, - ), - ); - } + } else if ( + showIndicators && + (shouldRecalculate || nativeIndicators.length === 0) + ) { + const newIndicators = selectNativeIndicatorsForChart( + nativeFilters, + dataMask, + chartId, + chart, + chartLayoutItems, + chartConfiguration, + ); + setNativeIndicators(newIndicators); } }, [ chart, diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterBar.test.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterBar.test.tsx index 96e2770603ef..17ed9cba224b 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterBar.test.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterBar.test.tsx @@ -27,6 +27,7 @@ import { FilterBarOrientation } from 'src/dashboard/types'; import { FILTER_BAR_TEST_ID } from './utils'; import FilterBar from '.'; import { FILTERS_CONFIG_MODAL_TEST_ID } from '../FiltersConfigModal/FiltersConfigModal'; +import * as dataMaskActions from 'src/dataMask/actions'; jest.useFakeTimers(); @@ -350,4 +351,118 @@ describe('FilterBar', () => { const { container } = renderWrapper(openedBarProps, stateWithFilter); expect(container).toBeInTheDocument(); }); + + test('auto-applies filter when extraFormData is empty in applied state', async () => { + const filterId = 'test-filter-auto-apply'; + const updateDataMaskSpy = jest.spyOn(dataMaskActions, 'updateDataMask'); + + const stateWithIncompleteFilter = { + ...stateWithoutNativeFilters, + dashboardInfo: { + id: 1, + dash_edit_perm: true, + }, + dataMask: { + [filterId]: { + id: filterId, + filterState: { value: ['value1', 'value2'] }, + extraFormData: {}, + }, + }, + nativeFilters: { + filters: { + [filterId]: { + id: filterId, + name: 'Test Filter', + filterType: 'filter_select', + targets: [{ datasetId: 1, column: { name: 'test_column' } }], + defaultDataMask: { + filterState: { value: ['value1', 'value2'] }, + extraFormData: {}, + }, + controlValues: { + enableEmptyFilter: true, + }, + cascadeParentIds: [], + scope: { + rootPath: ['ROOT_ID'], + excluded: [], + }, + type: 'NATIVE_FILTER', + description: '', + chartsInScope: [], + tabsInScope: [], + }, + }, + filtersState: {}, + }, + }; + + renderWrapper(openedBarProps, stateWithIncompleteFilter); + + await act(async () => { + jest.advanceTimersByTime(200); + }); + + expect(screen.getByTestId(getTestId('filter-icon'))).toBeInTheDocument(); + + updateDataMaskSpy.mockRestore(); + }); + + test('renders correctly when filter has complete extraFormData', async () => { + const filterId = 'test-filter-complete'; + const stateWithCompleteFilter = { + ...stateWithoutNativeFilters, + dashboardInfo: { + id: 1, + dash_edit_perm: true, + }, + dataMask: { + [filterId]: { + id: filterId, + filterState: { value: ['value1'] }, + extraFormData: { + filters: [{ col: 'test_column', op: 'IN', val: ['value1'] }], + }, + }, + }, + nativeFilters: { + filters: { + [filterId]: { + id: filterId, + name: 'Test Filter', + filterType: 'filter_select', + targets: [{ datasetId: 1, column: { name: 'test_column' } }], + defaultDataMask: { + filterState: { value: ['value1'] }, + extraFormData: { + filters: [{ col: 'test_column', op: 'IN', val: ['value1'] }], + }, + }, + controlValues: { + enableEmptyFilter: true, + }, + cascadeParentIds: [], + scope: { + rootPath: ['ROOT_ID'], + excluded: [], + }, + type: 'NATIVE_FILTER', + description: '', + chartsInScope: [], + tabsInScope: [], + }, + }, + filtersState: {}, + }, + }; + + renderWrapper(openedBarProps, stateWithCompleteFilter); + + await act(async () => { + jest.advanceTimersByTime(100); + }); + + expect(screen.getByTestId(getTestId('filter-icon'))).toBeInTheDocument(); + }); }); diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/index.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/index.tsx index 74d90cbb9926..d7f787254ba9 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/index.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/index.tsx @@ -204,24 +204,41 @@ const FilterBar: FC = ({ dataMask: Partial, ) => { setDataMaskSelected(draft => { - const isFirstTimeInitialization = - !initializedFilters.has(filter.id) && - dataMaskSelectedRef.current[filter.id]?.filterState?.value === - undefined; - - // force instant updating on initialization for filters with `requiredFirst` is true or instant filters - if ( - // filterState.value === undefined - means that value not initialized + const appliedDataMask = dataMaskApplied[filter.id]; + const isFirstTimeInitialization = !initializedFilters.has(filter.id); + + // Auto-apply when filter has value but empty extraFormData in applied state + // This fixes the bug where defaultDataMask.filterState.value exists but extraFormData is empty + // Only auto-apply if: value matches what's applied AND extraFormData is missing in applied but present in incoming + const needsAutoApply = + appliedDataMask?.filterState?.value !== undefined && dataMask.filterState?.value !== undefined && - isFirstTimeInitialization && - filter.requiredFirst - ) { + isEqual( + appliedDataMask.filterState.value, + dataMask.filterState.value, + ) && + (!appliedDataMask?.extraFormData || + Object.keys(appliedDataMask.extraFormData || {}).length === 0) && + dataMask.extraFormData && + Object.keys(dataMask.extraFormData).length > 0; + + // Force instant updating for requiredFirst filters or auto-apply when needed + const shouldDispatch = + dataMask.filterState?.value !== undefined && + ((isFirstTimeInitialization && filter.requiredFirst) || + needsAutoApply); + + if (shouldDispatch) { dispatch(updateDataMask(filter.id, dataMask)); } - // Mark filter as initialized after getting its first value + // Mark filter as initialized after getting its first value WITH extraFormData + // This ensures we don't mark it as initialized on the first sync (value but no extraFormData) + // but do mark it after the second sync (value AND extraFormData) if ( dataMask.filterState?.value !== undefined && + dataMask.extraFormData && + Object.keys(dataMask.extraFormData).length > 0 && !initializedFilters.has(filter.id) ) { setInitializedFilters(prev => new Set(prev).add(filter.id)); @@ -252,7 +269,13 @@ const FilterBar: FC = ({ }; }); }, - [dispatch, setDataMaskSelected, initializedFilters, setInitializedFilters], + [ + dispatch, + setDataMaskSelected, + initializedFilters, + setInitializedFilters, + dataMaskApplied, + ], ); useEffect(() => {