Skip to content

Commit 23c5ad6

Browse files
committed
fix(Dashboard): Auto-apply filters with default values when extraFormData is empty
1 parent cedc35e commit 23c5ad6

File tree

3 files changed

+164
-33
lines changed

3 files changed

+164
-33
lines changed

superset-frontend/src/dashboard/components/FiltersBadge/index.tsx

Lines changed: 22 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -201,30 +201,29 @@ export const FiltersBadge = ({ chartId }: FiltersBadgeProps) => {
201201
const prevChartConfig = usePrevious(chartConfiguration);
202202

203203
useEffect(() => {
204-
if (!showIndicators && nativeIndicators.length > 0) {
204+
const shouldRecalculate =
205+
prevChartStatus !== 'success' ||
206+
dataMask !== prevDataMask ||
207+
chart?.queriesResponse?.[0]?.rejected_filters !==
208+
prevChart?.queriesResponse?.[0]?.rejected_filters ||
209+
chart?.queriesResponse?.[0]?.applied_filters !==
210+
prevChart?.queriesResponse?.[0]?.applied_filters ||
211+
nativeFilters !== prevNativeFilters ||
212+
chartLayoutItems !== prevChartLayoutItems ||
213+
prevChartConfig !== chartConfiguration;
214+
215+
if (shouldRecalculate) {
216+
const newIndicators = selectNativeIndicatorsForChart(
217+
nativeFilters,
218+
dataMask,
219+
chartId,
220+
chart,
221+
chartLayoutItems,
222+
chartConfiguration,
223+
);
224+
setNativeIndicators(newIndicators);
225+
} else if (!showIndicators && nativeIndicators.length > 0) {
205226
setNativeIndicators(indicatorsInitialState);
206-
} else if (prevChartStatus !== 'success') {
207-
if (
208-
chart?.queriesResponse?.[0]?.rejected_filters !==
209-
prevChart?.queriesResponse?.[0]?.rejected_filters ||
210-
chart?.queriesResponse?.[0]?.applied_filters !==
211-
prevChart?.queriesResponse?.[0]?.applied_filters ||
212-
nativeFilters !== prevNativeFilters ||
213-
chartLayoutItems !== prevChartLayoutItems ||
214-
dataMask !== prevDataMask ||
215-
prevChartConfig !== chartConfiguration
216-
) {
217-
setNativeIndicators(
218-
selectNativeIndicatorsForChart(
219-
nativeFilters,
220-
dataMask,
221-
chartId,
222-
chart,
223-
chartLayoutItems,
224-
chartConfiguration,
225-
),
226-
);
227-
}
228227
}
229228
}, [
230229
chart,

superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterBar.test.tsx

Lines changed: 115 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ import { FilterBarOrientation } from 'src/dashboard/types';
2727
import { FILTER_BAR_TEST_ID } from './utils';
2828
import FilterBar from '.';
2929
import { FILTERS_CONFIG_MODAL_TEST_ID } from '../FiltersConfigModal/FiltersConfigModal';
30+
import * as dataMaskActions from 'src/dataMask/actions';
3031

3132
jest.useFakeTimers();
3233

@@ -350,4 +351,118 @@ describe('FilterBar', () => {
350351
const { container } = renderWrapper(openedBarProps, stateWithFilter);
351352
expect(container).toBeInTheDocument();
352353
});
354+
355+
test('auto-applies filter when extraFormData is empty in applied state', async () => {
356+
const filterId = 'test-filter-auto-apply';
357+
const updateDataMaskSpy = jest.spyOn(dataMaskActions, 'updateDataMask');
358+
359+
const stateWithIncompleteFilter = {
360+
...stateWithoutNativeFilters,
361+
dashboardInfo: {
362+
id: 1,
363+
dash_edit_perm: true,
364+
},
365+
dataMask: {
366+
[filterId]: {
367+
id: filterId,
368+
filterState: { value: ['value1', 'value2'] },
369+
extraFormData: {},
370+
},
371+
},
372+
nativeFilters: {
373+
filters: {
374+
[filterId]: {
375+
id: filterId,
376+
name: 'Test Filter',
377+
filterType: 'filter_select',
378+
targets: [{ datasetId: 1, column: { name: 'test_column' } }],
379+
defaultDataMask: {
380+
filterState: { value: ['value1', 'value2'] },
381+
extraFormData: {},
382+
},
383+
controlValues: {
384+
enableEmptyFilter: true,
385+
},
386+
cascadeParentIds: [],
387+
scope: {
388+
rootPath: ['ROOT_ID'],
389+
excluded: [],
390+
},
391+
type: 'NATIVE_FILTER',
392+
description: '',
393+
chartsInScope: [],
394+
tabsInScope: [],
395+
},
396+
},
397+
filtersState: {},
398+
},
399+
};
400+
401+
renderWrapper(openedBarProps, stateWithIncompleteFilter);
402+
403+
await act(async () => {
404+
jest.advanceTimersByTime(200);
405+
});
406+
407+
expect(screen.getByTestId(getTestId('filter-icon'))).toBeInTheDocument();
408+
409+
updateDataMaskSpy.mockRestore();
410+
});
411+
412+
test('renders correctly when filter has complete extraFormData', async () => {
413+
const filterId = 'test-filter-complete';
414+
const stateWithCompleteFilter = {
415+
...stateWithoutNativeFilters,
416+
dashboardInfo: {
417+
id: 1,
418+
dash_edit_perm: true,
419+
},
420+
dataMask: {
421+
[filterId]: {
422+
id: filterId,
423+
filterState: { value: ['value1'] },
424+
extraFormData: {
425+
filters: [{ col: 'test_column', op: 'IN', val: ['value1'] }],
426+
},
427+
},
428+
},
429+
nativeFilters: {
430+
filters: {
431+
[filterId]: {
432+
id: filterId,
433+
name: 'Test Filter',
434+
filterType: 'filter_select',
435+
targets: [{ datasetId: 1, column: { name: 'test_column' } }],
436+
defaultDataMask: {
437+
filterState: { value: ['value1'] },
438+
extraFormData: {
439+
filters: [{ col: 'test_column', op: 'IN', val: ['value1'] }],
440+
},
441+
},
442+
controlValues: {
443+
enableEmptyFilter: true,
444+
},
445+
cascadeParentIds: [],
446+
scope: {
447+
rootPath: ['ROOT_ID'],
448+
excluded: [],
449+
},
450+
type: 'NATIVE_FILTER',
451+
description: '',
452+
chartsInScope: [],
453+
tabsInScope: [],
454+
},
455+
},
456+
filtersState: {},
457+
},
458+
};
459+
460+
renderWrapper(openedBarProps, stateWithCompleteFilter);
461+
462+
await act(async () => {
463+
jest.advanceTimersByTime(100);
464+
});
465+
466+
expect(screen.getByTestId(getTestId('filter-icon'))).toBeInTheDocument();
467+
});
353468
});

superset-frontend/src/dashboard/components/nativeFilters/FilterBar/index.tsx

Lines changed: 27 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -204,24 +204,35 @@ const FilterBar: FC<FiltersBarProps> = ({
204204
dataMask: Partial<DataMask>,
205205
) => {
206206
setDataMaskSelected(draft => {
207-
const isFirstTimeInitialization =
208-
!initializedFilters.has(filter.id) &&
209-
dataMaskSelectedRef.current[filter.id]?.filterState?.value ===
210-
undefined;
207+
const appliedDataMask = dataMaskApplied[filter.id];
208+
const isFirstTimeInitialization = !initializedFilters.has(filter.id);
209+
210+
// Auto-apply when filter has value but empty extraFormData in applied state
211+
// This fixes the bug where defaultDataMask.filterState.value exists but extraFormData is empty
212+
const needsAutoApply =
213+
appliedDataMask?.filterState?.value !== undefined &&
214+
(!appliedDataMask?.extraFormData ||
215+
Object.keys(appliedDataMask.extraFormData || {}).length === 0) &&
216+
dataMask.filterState?.value !== undefined &&
217+
dataMask.extraFormData &&
218+
Object.keys(dataMask.extraFormData).length > 0;
211219

212-
// force instant updating on initialization for filters with `requiredFirst` is true or instant filters
220+
// Force instant updating for requiredFirst filters or auto-apply when needed
213221
if (
214-
// filterState.value === undefined - means that value not initialized
215222
dataMask.filterState?.value !== undefined &&
216-
isFirstTimeInitialization &&
217-
filter.requiredFirst
223+
((isFirstTimeInitialization && filter.requiredFirst) ||
224+
needsAutoApply)
218225
) {
219226
dispatch(updateDataMask(filter.id, dataMask));
220227
}
221228

222-
// Mark filter as initialized after getting its first value
229+
// Mark filter as initialized after getting its first value WITH extraFormData
230+
// This ensures we don't mark it as initialized on the first sync (value but no extraFormData)
231+
// but do mark it after the second sync (value AND extraFormData)
223232
if (
224233
dataMask.filterState?.value !== undefined &&
234+
dataMask.extraFormData &&
235+
Object.keys(dataMask.extraFormData).length > 0 &&
225236
!initializedFilters.has(filter.id)
226237
) {
227238
setInitializedFilters(prev => new Set(prev).add(filter.id));
@@ -252,7 +263,13 @@ const FilterBar: FC<FiltersBarProps> = ({
252263
};
253264
});
254265
},
255-
[dispatch, setDataMaskSelected, initializedFilters, setInitializedFilters],
266+
[
267+
dispatch,
268+
setDataMaskSelected,
269+
initializedFilters,
270+
setInitializedFilters,
271+
dataMaskApplied,
272+
],
256273
);
257274

258275
useEffect(() => {

0 commit comments

Comments
 (0)