From 88a14f2ba0dc7a1068ac32b6c1883b3d17fc9007 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20S=C3=A1nchez?= Date: Wed, 11 Feb 2026 10:33:32 -0300 Subject: [PATCH] fix(FiltersBadge): world map wont show filter icon after refresh page (#37260) --- superset-frontend/spec/fixtures/mockStore.js | 11 + .../FiltersBadge/FiltersBadge.test.tsx | 4 +- .../FilterBar/FilterBar.test.tsx | 183 +++++++- .../nativeFilters/FilterBar/index.tsx | 12 +- .../nativeFilters/selectors.test.ts | 425 +++++++++++++++++- .../components/nativeFilters/selectors.ts | 60 ++- superset/common/query_context_processor.py | 12 + .../common/test_query_context_processor.py | 68 +++ 8 files changed, 763 insertions(+), 12 deletions(-) diff --git a/superset-frontend/spec/fixtures/mockStore.js b/superset-frontend/spec/fixtures/mockStore.js index df464f7b7b2..fef0aeca97c 100644 --- a/superset-frontend/spec/fixtures/mockStore.js +++ b/superset-frontend/spec/fixtures/mockStore.js @@ -137,6 +137,17 @@ export const getMockStoreWithNativeFilters = () => initialState: stateWithNativeFilters, }); +export const stateWithNativeFiltersButNoValues = { + ...stateWithNativeFilters, + dataMask: {}, +}; + +export const getMockStoreWithNativeFiltersButNoValues = () => + setupStore({ + disableDebugger: true, + initialState: stateWithNativeFiltersButNoValues, + }); + export const stateWithoutNativeFilters = { ...mockState, charts: { diff --git a/superset-frontend/src/dashboard/components/FiltersBadge/FiltersBadge.test.tsx b/superset-frontend/src/dashboard/components/FiltersBadge/FiltersBadge.test.tsx index 879891a7309..9a49e751b62 100644 --- a/superset-frontend/src/dashboard/components/FiltersBadge/FiltersBadge.test.tsx +++ b/superset-frontend/src/dashboard/components/FiltersBadge/FiltersBadge.test.tsx @@ -28,6 +28,7 @@ import { FiltersBadge } from 'src/dashboard/components/FiltersBadge'; import { getMockStoreWithFilters, getMockStoreWithNativeFilters, + getMockStoreWithNativeFiltersButNoValues, } from 'spec/fixtures/mockStore'; import { sliceId } from 'spec/fixtures/mockChartQueries'; import { dashboardFilters } from 'spec/fixtures/mockDashboardFilters'; @@ -100,8 +101,7 @@ describe('for dashboard filters', () => { // eslint-disable-next-line no-restricted-globals -- TODO: Migrate from describe blocks describe('for native filters', () => { test('does not show number when there are no active filters', () => { - const store = getMockStoreWithNativeFilters(); - // start with basic dashboard state, dispatch an event to simulate query completion + const store = getMockStoreWithNativeFiltersButNoValues(); store.dispatch({ type: CHART_UPDATE_SUCCEEDED, key: sliceId, 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 0d0a81a16e4..43ffe05cdb4 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterBar.test.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterBar.test.tsx @@ -21,7 +21,11 @@ import { act, render, screen, userEvent } from 'spec/helpers/testing-library'; import { stateWithoutNativeFilters } from 'spec/fixtures/mockStore'; import { testWithId } from 'src/utils/testUtils'; import { Preset, makeApi } from '@superset-ui/core'; -import { TimeFilterPlugin, SelectFilterPlugin } from 'src/filters/components'; +import { + TimeFilterPlugin, + SelectFilterPlugin, + RangeFilterPlugin, +} from 'src/filters/components'; import fetchMock from 'fetch-mock'; import { FilterBarOrientation } from 'src/dashboard/types'; import { FILTER_BAR_TEST_ID } from './utils'; @@ -45,6 +49,7 @@ class MainPreset extends Preset { plugins: [ new TimeFilterPlugin().configure({ key: 'filter_time' }), new SelectFilterPlugin().configure({ key: 'filter_select' }), + new RangeFilterPlugin().configure({ key: 'filter_range' }), ], }); } @@ -487,4 +492,180 @@ describe('FilterBar', () => { expect(screen.getByTestId(getTestId('filter-icon'))).toBeInTheDocument(); }); + + test('handleClearAll dispatches updateDataMask with value null for filter_select', async () => { + const filterId = 'NATIVE_FILTER-clear-select'; + const updateDataMaskSpy = jest.spyOn(dataMaskActions, 'updateDataMask'); + const selectFilterConfig = { + id: filterId, + name: 'Region', + filterType: 'filter_select', + targets: [{ datasetId: 7, column: { name: 'region' } }], + defaultDataMask: { filterState: { value: null }, extraFormData: {} }, + cascadeParentIds: [], + scope: { rootPath: ['ROOT_ID'], excluded: [] }, + type: 'NATIVE_FILTER', + description: '', + chartsInScope: [18], + tabsInScope: [], + }; + const stateWithSelect = { + ...stateWithoutNativeFilters, + dashboardInfo: { + id: 1, + dash_edit_perm: true, + filterBarOrientation: FilterBarOrientation.Vertical, + metadata: { + native_filter_configuration: [selectFilterConfig], + chart_configuration: {}, + }, + }, + dataMask: { + [filterId]: { + id: filterId, + filterState: { value: ['East'] }, + extraFormData: {}, + }, + }, + }; + + renderWrapper(openedBarProps, stateWithSelect); + await act(async () => { + jest.advanceTimersByTime(300); + }); + + const clearBtn = screen.getByTestId(getTestId('clear-button')); + expect(clearBtn).not.toBeDisabled(); + await act(async () => { + userEvent.click(clearBtn); + }); + + expect(updateDataMaskSpy).toHaveBeenCalledWith(filterId, { + filterState: { value: undefined }, + extraFormData: {}, + }); + updateDataMaskSpy.mockRestore(); + }); + + test('handleClearAll dispatches updateDataMask with value [null, null] for filter_range', async () => { + fetchMock.post('glob:*/api/v1/chart/data', { + result: [{ data: [{ min: 0, max: 100 }] }], + }); + const filterId = 'NATIVE_FILTER-clear-range'; + const updateDataMaskSpy = jest.spyOn(dataMaskActions, 'updateDataMask'); + const rangeFilterConfig = { + id: filterId, + name: 'Age', + filterType: 'filter_range', + targets: [{ datasetId: 7, column: { name: 'age' } }], + defaultDataMask: { filterState: { value: null }, extraFormData: {} }, + cascadeParentIds: [], + scope: { rootPath: ['ROOT_ID'], excluded: [] }, + type: 'NATIVE_FILTER', + description: '', + chartsInScope: [18], + tabsInScope: [], + }; + const stateWithRange = { + ...stateWithoutNativeFilters, + dashboardInfo: { + id: 1, + dash_edit_perm: true, + filterBarOrientation: FilterBarOrientation.Vertical, + metadata: { + native_filter_configuration: [rangeFilterConfig], + chart_configuration: {}, + }, + }, + dataMask: { + [filterId]: { + id: filterId, + filterState: { value: [10, 50] }, + extraFormData: {}, + }, + }, + }; + + renderWrapper(openedBarProps, stateWithRange); + await act(async () => { + jest.advanceTimersByTime(300); + }); + + const clearBtn = screen.getByTestId(getTestId('clear-button')); + expect(clearBtn).not.toBeDisabled(); + await act(async () => { + userEvent.click(clearBtn); + }); + + expect(updateDataMaskSpy).toHaveBeenCalledWith(filterId, { + filterState: { value: [null, null] }, + extraFormData: {}, + }); + updateDataMaskSpy.mockRestore(); + }); + + test('handleClearAll only dispatches for filters present in dataMask', async () => { + const idInMask = 'NATIVE_FILTER-has-value'; + const idNotInMask = 'NATIVE_FILTER-no-value'; + const updateDataMaskSpy = jest.spyOn(dataMaskActions, 'updateDataMask'); + const baseFilter = { + targets: [{ datasetId: 7, column: { name: 'x' } }], + defaultDataMask: { filterState: { value: null }, extraFormData: {} }, + cascadeParentIds: [], + scope: { rootPath: ['ROOT_ID'], excluded: [] }, + type: 'NATIVE_FILTER', + description: '', + chartsInScope: [18], + tabsInScope: [], + }; + const stateWithTwoFiltersOneInMask = { + ...stateWithoutNativeFilters, + dashboardInfo: { + id: 1, + dash_edit_perm: true, + filterBarOrientation: FilterBarOrientation.Vertical, + metadata: { + native_filter_configuration: [ + { + ...baseFilter, + id: idInMask, + name: 'A', + filterType: 'filter_select', + }, + { + ...baseFilter, + id: idNotInMask, + name: 'B', + filterType: 'filter_select', + }, + ], + chart_configuration: {}, + }, + }, + dataMask: { + [idInMask]: { + id: idInMask, + filterState: { value: ['v'] }, + extraFormData: {}, + }, + }, + }; + + renderWrapper(openedBarProps, stateWithTwoFiltersOneInMask); + await act(async () => { + jest.advanceTimersByTime(300); + }); + + const clearBtn = screen.getByTestId(getTestId('clear-button')); + await act(async () => { + userEvent.click(clearBtn); + }); + + expect(updateDataMaskSpy).toHaveBeenCalledTimes(1); + expect(updateDataMaskSpy).toHaveBeenCalledWith(idInMask, { + filterState: { value: undefined }, + extraFormData: {}, + }); + updateDataMaskSpy.mockRestore(); + }); }); diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/index.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/index.tsx index 8e70b1b7d92..2db15214c64 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/index.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/index.tsx @@ -416,11 +416,19 @@ const FilterBar: FC = ({ const handleClearAll = useCallback(() => { const newClearAllTriggers = { ...clearAllTriggers }; nativeFilterValues.forEach(filter => { - const { id } = filter; + const { id, filterType } = filter; + // Range filters use [null, null] as the cleared value; others use undefined + const clearedValue = + filterType === 'filter_range' ? [null, null] : undefined; + const clearedDataMask = { + filterState: { value: clearedValue }, + extraFormData: {}, + }; if (dataMaskSelected[id]) { + dispatch(updateDataMask(id, clearedDataMask)); setDataMaskSelected(draft => { if (draft[id].filterState?.value !== undefined) { - draft[id].filterState!.value = undefined; + draft[id].filterState!.value = clearedValue; } draft[id].extraFormData = {}; }); diff --git a/superset-frontend/src/dashboard/components/nativeFilters/selectors.test.ts b/superset-frontend/src/dashboard/components/nativeFilters/selectors.test.ts index 52a549c95ba..77fe6b7b0c7 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/selectors.test.ts +++ b/superset-frontend/src/dashboard/components/nativeFilters/selectors.test.ts @@ -17,7 +17,12 @@ * under the License. */ import { CHART_TYPE } from 'src/dashboard/util/componentTypes'; -import { getCrossFilterIndicator } from './selectors'; +import { NativeFilterType } from '@superset-ui/core'; +import { + extractLabel, + getAppliedColumnsWithFallback, + getCrossFilterIndicator, +} from './selectors'; // eslint-disable-next-line no-restricted-globals -- TODO: Migrate from describe blocks describe('getCrossFilterIndicator', () => { @@ -142,3 +147,421 @@ describe('getCrossFilterIndicator', () => { }); }); }); + +test('extractLabel returns label when filter has label and it does not include undefined', () => { + expect(extractLabel({ label: 'My Label', value: 'x' })).toBe('My Label'); + expect(extractLabel({ label: 'Only label' })).toBe('Only label'); +}); + +test('extractLabel returns value joined by ", " when filter has non-empty array value and no label', () => { + expect(extractLabel({ value: ['a', 'b'] })).toBe('a, b'); + expect(extractLabel({ value: ['single'] })).toBe('single'); +}); + +test('extractLabel returns value when filter has non-array value (ensureIsArray wraps it)', () => { + expect(extractLabel({ value: 'scalar' })).toBe('scalar'); + expect(extractLabel({ value: 42 })).toBe('42'); +}); + +test('extractLabel returns null when filter is undefined or has no label and no value', () => { + expect(extractLabel(undefined)).toBe(null); + expect(extractLabel({})).toBe(null); + expect(extractLabel({ label: '' })).toBe(null); +}); + +test('extractLabel returns null when filter.value is null or undefined', () => { + expect(extractLabel({ value: null })).toBe(null); + expect(extractLabel({ value: undefined })).toBe(null); +}); + +test('extractLabel does not return ", " or "null, null" for arrays of only null, undefined, or empty string', () => { + expect(extractLabel({ value: [null, null] })).toBe(null); + expect(extractLabel({ value: [null] })).toBe(null); + expect(extractLabel({ value: [''] })).toBe(null); + expect(extractLabel({ value: ['', ''] })).toBe(null); + expect(extractLabel({ value: [null, ''] })).toBe(null); + expect(extractLabel({ value: [undefined, undefined] })).toBe(null); + expect(extractLabel({ value: [null, undefined, ''] })).toBe(null); + expect(extractLabel({ value: [null, null] })).not.toBe(', '); + expect(extractLabel({ value: [null, ''] })).not.toBe(', '); +}); + +test('extractLabel returns only non-empty items when array has mix of empty and non-empty', () => { + expect(extractLabel({ value: [null, 'a', '', 'b', undefined] })).toBe('a, b'); + expect(extractLabel({ value: ['', 'x', ''] })).toBe('x'); +}); + +test('extractLabel uses value when label is undefined', () => { + expect(extractLabel({ label: undefined, value: ['a'] })).toBe('a'); +}); + +test('getAppliedColumnsWithFallback returns columns from query response when available', () => { + const chart = { + queriesResponse: [ + { + applied_filters: [{ column: 'age' }, { column: 'name' }], + }, + ], + }; + const result = getAppliedColumnsWithFallback(chart); + expect(result).toEqual(new Set(['age', 'name'])); +}); + +test('getAppliedColumnsWithFallback returns empty set when query response has no applied_filters and no fallback params', () => { + const chart = { + queriesResponse: [{ applied_filters: [] }], + }; + const result = getAppliedColumnsWithFallback(chart); + expect(result).toEqual(new Set()); +}); + +test('getAppliedColumnsWithFallback derives columns from native filters when query response is empty', () => { + const chart = { + queriesResponse: [{ applied_filters: [] }], + }; + const nativeFilters = { + filter1: { + id: 'filter1', + type: NativeFilterType.NativeFilter, + chartsInScope: [123], + targets: [{ column: { name: 'age' } }], + }, + filter2: { + id: 'filter2', + type: NativeFilterType.NativeFilter, + chartsInScope: [123], + targets: [{ column: { name: 'name' } }], + }, + } as any; + const dataMask = { + filter1: { + id: 'filter1', + filterState: { value: '25' }, + extraFormData: {}, + }, + filter2: { + id: 'filter2', + filterState: { value: 'John' }, + extraFormData: {}, + }, + } as any; + const result = getAppliedColumnsWithFallback( + chart, + nativeFilters, + dataMask, + 123, + ); + expect(result).toEqual(new Set(['age', 'name'])); +}); + +test('getAppliedColumnsWithFallback excludes filters not in chart scope', () => { + const chart = { + queriesResponse: [{ applied_filters: [] }], + }; + const nativeFilters = { + filter1: { + id: 'filter1', + type: NativeFilterType.NativeFilter, + chartsInScope: [123], + targets: [{ column: { name: 'age' } }], + }, + filter2: { + id: 'filter2', + type: NativeFilterType.NativeFilter, + chartsInScope: [456], // Different chart + targets: [{ column: { name: 'name' } }], + }, + } as any; + const dataMask = { + filter1: { + id: 'filter1', + filterState: { value: '25' }, + extraFormData: {}, + }, + filter2: { + id: 'filter2', + filterState: { value: 'John' }, + extraFormData: {}, + }, + } as any; + const result = getAppliedColumnsWithFallback( + chart, + nativeFilters, + dataMask, + 123, + ); + expect(result).toEqual(new Set(['age'])); +}); + +test('getAppliedColumnsWithFallback excludes filters without values', () => { + const chart = { + queriesResponse: [{ applied_filters: [] }], + }; + const nativeFilters = { + filter1: { + id: 'filter1', + type: NativeFilterType.NativeFilter, + chartsInScope: [123], + targets: [{ column: { name: 'age' } }], + }, + filter2: { + id: 'filter2', + type: NativeFilterType.NativeFilter, + chartsInScope: [123], + targets: [{ column: { name: 'name' } }], + }, + } as any; + const dataMask = { + filter1: { + id: 'filter1', + filterState: { value: '25' }, + extraFormData: {}, + }, + filter2: { + id: 'filter2', + filterState: { value: null }, + extraFormData: {}, + }, + } as any; + const result = getAppliedColumnsWithFallback( + chart, + nativeFilters, + dataMask, + 123, + ); + expect(result).toEqual(new Set(['age'])); +}); + +test('getAppliedColumnsWithFallback excludes filters without targets', () => { + const chart = { + queriesResponse: [{ applied_filters: [] }], + }; + const nativeFilters = { + filter1: { + id: 'filter1', + type: NativeFilterType.NativeFilter, + chartsInScope: [123], + targets: [{ column: { name: 'age' } }], + }, + filter2: { + id: 'filter2', + type: NativeFilterType.NativeFilter, + chartsInScope: [123], + targets: [], + }, + } as any; + const dataMask = { + filter1: { + id: 'filter1', + filterState: { value: '25' }, + extraFormData: {}, + }, + filter2: { + id: 'filter2', + filterState: { value: 'John' }, + extraFormData: {}, + }, + } as any; + const result = getAppliedColumnsWithFallback( + chart, + nativeFilters, + dataMask, + 123, + ); + expect(result).toEqual(new Set(['age'])); +}); + +test('getAppliedColumnsWithFallback excludes non-native filter types', () => { + const chart = { + queriesResponse: [{ applied_filters: [] }], + }; + const nativeFilters = { + filter1: { + id: 'filter1', + type: NativeFilterType.NativeFilter, + chartsInScope: [123], + targets: [{ column: { name: 'age' } }], + }, + filter2: { + id: 'filter2', + type: 'other_type' as any, + chartsInScope: [123], + targets: [{ column: { name: 'name' } }], + }, + } as any; + const dataMask = { + filter1: { + id: 'filter1', + filterState: { value: '25' }, + extraFormData: {}, + }, + filter2: { + id: 'filter2', + filterState: { value: 'John' }, + extraFormData: {}, + }, + } as any; + const result = getAppliedColumnsWithFallback( + chart, + nativeFilters, + dataMask, + 123, + ); + expect(result).toEqual(new Set(['age'])); +}); + +test('getAppliedColumnsWithFallback handles missing dataMask entry for filter', () => { + const chart = { + queriesResponse: [{ applied_filters: [] }], + }; + const nativeFilters = { + filter1: { + id: 'filter1', + type: NativeFilterType.NativeFilter, + chartsInScope: [123], + targets: [{ column: { name: 'age' } }], + }, + } as any; + const dataMask = { + // filter1 is missing + } as any; + const result = getAppliedColumnsWithFallback( + chart, + nativeFilters, + dataMask, + 123, + ); + expect(result).toEqual(new Set()); +}); + +test('getAppliedColumnsWithFallback handles empty array values in filterState', () => { + const chart = { + queriesResponse: [{ applied_filters: [] }], + }; + const nativeFilters = { + filter1: { + id: 'filter1', + type: NativeFilterType.NativeFilter, + chartsInScope: [123], + targets: [{ column: { name: 'age' } }], + }, + } as any; + const dataMask = { + filter1: { + id: 'filter1', + filterState: { value: [] }, + extraFormData: {}, + }, + } as any; + const result = getAppliedColumnsWithFallback( + chart, + nativeFilters, + dataMask, + 123, + ); + expect(result).toEqual(new Set()); +}); + +test('getAppliedColumnsWithFallback handles null values in filterState', () => { + const chart = { + queriesResponse: [{ applied_filters: [] }], + }; + const nativeFilters = { + filter1: { + id: 'filter1', + type: NativeFilterType.NativeFilter, + chartsInScope: [123], + targets: [{ column: { name: 'age' } }], + }, + } as any; + const dataMask = { + filter1: { + id: 'filter1', + filterState: { value: [null, null] }, + extraFormData: {}, + }, + } as any; + const result = getAppliedColumnsWithFallback( + chart, + nativeFilters, + dataMask, + 123, + ); + expect(result).toEqual(new Set()); +}); + +test('getAppliedColumnsWithFallback returns empty set when chart is undefined', () => { + const result = getAppliedColumnsWithFallback(undefined); + expect(result).toEqual(new Set()); +}); + +test('getAppliedColumnsWithFallback returns empty set when chart has no queriesResponse', () => { + const chart = {}; + const result = getAppliedColumnsWithFallback(chart); + expect(result).toEqual(new Set()); +}); + +test('getAppliedColumnsWithFallback returns empty set when fallback params are incomplete', () => { + const chart = { + queriesResponse: [{ applied_filters: [] }], + }; + const nativeFilters = { + filter1: { + id: 'filter1', + type: NativeFilterType.NativeFilter, + chartsInScope: [123], + targets: [{ column: { name: 'age' } }], + }, + } as any; + const dataMask = { + filter1: { + id: 'filter1', + filterState: { value: '25' }, + extraFormData: {}, + }, + } as any; + // Missing chartId + expect(getAppliedColumnsWithFallback(chart, nativeFilters, dataMask)).toEqual( + new Set(), + ); + // Missing dataMask + expect( + getAppliedColumnsWithFallback(chart, nativeFilters, undefined, 123), + ).toEqual(new Set()); + // Missing nativeFilters + expect( + getAppliedColumnsWithFallback(chart, undefined, dataMask, 123), + ).toEqual(new Set()); +}); + +test('getAppliedColumnsWithFallback prioritizes query response over fallback', () => { + const chart = { + queriesResponse: [ + { + applied_filters: [{ column: 'query_column' }], + }, + ], + }; + const nativeFilters = { + filter1: { + id: 'filter1', + type: NativeFilterType.NativeFilter, + chartsInScope: [123], + targets: [{ column: { name: 'fallback_column' } }], + }, + } as any; + const dataMask = { + filter1: { + id: 'filter1', + filterState: { value: '25' }, + extraFormData: {}, + }, + } as any; + const result = getAppliedColumnsWithFallback( + chart, + nativeFilters, + dataMask, + 123, + ); + expect(result).toEqual(new Set(['query_column'])); +}); diff --git a/superset-frontend/src/dashboard/components/nativeFilters/selectors.ts b/superset-frontend/src/dashboard/components/nativeFilters/selectors.ts index a6cfb55cd7f..13f3852beb1 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/selectors.ts +++ b/superset-frontend/src/dashboard/components/nativeFilters/selectors.ts @@ -65,7 +65,11 @@ export const extractLabel = (filter?: FilterState): string | null => { return filter.label; } if (filter?.value) { - return ensureIsArray(filter?.value).join(', '); + const arr = ensureIsArray(filter.value); + // To avoid returning an array with a simple comma ", " or similar + const nonEmpty = arr.filter(v => v != null && v !== ''); + if (nonEmpty.length === 0) return null; + return nonEmpty.join(', '); } return null; }; @@ -144,6 +148,47 @@ const getAppliedColumns = (chart: any): Set => ), ); +/** + * Get applied columns with fallback to derive from native filters when query response + * is missing applied_filters (e.g., from old cache entries). + * This ensures filter indicators show correctly even when backend cache doesn't have + * applied_filter_columns populated. + */ +export const getAppliedColumnsWithFallback = ( + chart: any, + nativeFilters?: Filters, + dataMask?: DataMaskStateWithId, + chartId?: number, +): Set => { + // First try to get from query response (preferred source of truth) + const queryAppliedFilters = + chart?.queriesResponse?.[0]?.applied_filters || []; + if (queryAppliedFilters.length > 0) { + return new Set(queryAppliedFilters.map((filter: any) => filter.column)); + } + + // Fallback: derive from native filters and dataMask when query response is empty + // This handles cases where cache entries are missing applied_filter_columns + if (nativeFilters && dataMask && chartId) { + const derivedColumns = new Set(); + Object.values(nativeFilters).forEach(filter => { + if ( + filter.type === NativeFilterType.NativeFilter && + filter.chartsInScope?.includes(chartId) + ) { + const filterState = dataMask[filter.id]?.filterState; + const hasValue = extractLabel(filterState) !== null; + if (hasValue && filter.targets?.[0]?.column?.name) { + derivedColumns.add(filter.targets[0].column.name); + } + } + }); + return derivedColumns; + } + + return new Set(); +}; + const getRejectedColumns = (chart: any): Set => new Set( (chart?.queriesResponse?.[0]?.rejected_filters || []).map((filter: any) => @@ -281,9 +326,7 @@ const getStatus = ({ } if (column && rejectedColumns?.has(column)) return IndicatorStatus.Incompatible; - if (column && appliedColumns?.has(column) && hasValue) { - return APPLIED_STATUS; - } + if (column && appliedColumns?.has(column) && hasValue) return APPLIED_STATUS; return IndicatorStatus.Unset; }; @@ -322,8 +365,8 @@ export const selectChartCrossFilters = ( ? getColumnLabel(filterIndicator.column) : undefined, type: DataMaskType.CrossFilters, - appliedColumns, rejectedColumns, + appliedColumns, }); return { ...filterIndicator, status: filterStatus }; @@ -354,7 +397,12 @@ export const selectNativeIndicatorsForChart = ( chartLayoutItems: LayoutItem[], chartConfiguration: ChartConfiguration = defaultChartConfig, ): Indicator[] => { - const appliedColumns = getAppliedColumns(chart); + const appliedColumns = getAppliedColumnsWithFallback( + chart, + nativeFilters, + dataMask, + chartId, + ); const rejectedColumns = getRejectedColumns(chart); const cachedFilterData = cachedNativeFilterDataForChart[chartId]; diff --git a/superset/common/query_context_processor.py b/superset/common/query_context_processor.py index 637b2dba1fa..7106d581b78 100644 --- a/superset/common/query_context_processor.py +++ b/superset/common/query_context_processor.py @@ -98,6 +98,18 @@ class QueryContextProcessor: force_cached=force_cached, ) + # If cache is loaded but missing applied_filter_columns and query has filters, + # treat as cache miss to ensure fresh query with proper applied_filter_columns + if ( + query_obj + and cache_key + and cache.is_loaded + and not cache.applied_filter_columns + and query_obj.filter + and len(query_obj.filter) > 0 + ): + cache.is_loaded = False + if query_obj and cache_key and not cache.is_loaded: try: if invalid_columns := [ diff --git a/tests/unit_tests/common/test_query_context_processor.py b/tests/unit_tests/common/test_query_context_processor.py index c83d5b728ba..a1d3814a02e 100644 --- a/tests/unit_tests/common/test_query_context_processor.py +++ b/tests/unit_tests/common/test_query_context_processor.py @@ -1311,3 +1311,71 @@ def test_force_cached_normalizes_totals_query_row_limit(): assert captured_limits == [None], "Totals query should be normalized before caching" mock_query_context.get_query_result.assert_not_called() + + +def test_get_df_payload_invalidates_cache_missing_applied_filter_columns(): + """ + Test that get_df_payload invalidates cache when cache is loaded but missing + applied_filter_columns and query has filters. + + This ensures that old cache entries without applied_filter_columns are + invalidated and fresh queries are executed to populate the field correctly. + """ + from superset.common.query_object import QueryObject + + # Minimal setup + mock_query_context = MagicMock() + mock_query_context.force = False + mock_datasource = MagicMock() + mock_datasource.column_names = ["col1"] + + processor = QueryContextProcessor(mock_query_context) + processor._qc_datasource = mock_datasource + + # Create query object with filters (note: `filters` kwarg, not `filter`) + query_obj = QueryObject( + datasource=mock_datasource, + columns=["col1"], + filters=[{"col": "col1", "op": "IN", "val": ["value1"]}], + ) + + # Simple cache class that tracks is_loaded changes + class MockCache: + def __init__(self): + self.is_loaded = True + self.applied_filter_columns = [] # Empty = missing + self.df = pd.DataFrame() + self.query = "" + self.status = "success" + self.cache_dttm = "2024-01-01T00:00:00" + self.queried_dttm = "2024-01-01T00:00:00" + self.stacktrace = None + self.error_message = None + self.is_cached = True + self.sql_rowcount = 0 + self.cache_value = None + self.cache_timeout = 3600 + self.datasource_uid = "test_datasource" + self.applied_template_filters = [] + self.rejected_filter_columns = [] + self.annotation_data = {} + self.set_query_result = MagicMock() + + mock_cache = MockCache() + + with patch( + "superset.common.query_context_processor.QueryCacheManager" + ) as mock_cache_manager: + mock_cache_manager.get.return_value = mock_cache + + # Prevent validate from doing any heavy work; it shouldn't modify filters + with patch.object(query_obj, "validate", return_value=None): + with patch.object(processor, "query_cache_key", return_value="key"): + with patch.object(processor, "get_cache_timeout", return_value=3600): + # Call get_df_payload - should invalidate cache + processor.get_df_payload(query_obj, force_cached=False) + + # Verify cache was invalidated + assert mock_cache.is_loaded is False, ( + "Cache should be inv when no applied_filter_columns and query has filters" + )