diff --git a/superset-frontend/packages/superset-ui-core/test/utils/copy.test.ts b/superset-frontend/packages/superset-ui-core/test/utils/copy.test.ts index 2ca9d3bec96..44c14958c76 100644 --- a/superset-frontend/packages/superset-ui-core/test/utils/copy.test.ts +++ b/superset-frontend/packages/superset-ui-core/test/utils/copy.test.ts @@ -25,7 +25,9 @@ const CHROME_UA = const makeGetText = (text: string) => () => Promise.resolve(text); -const globalWithClipboardItem = global as unknown as { ClipboardItem?: unknown }; +const globalWithClipboardItem = global as unknown as { + ClipboardItem?: unknown; +}; afterEach(() => { jest.restoreAllMocks(); @@ -94,9 +96,7 @@ function mockExecCommand(impl: (cmd: string) => boolean) { }); } -function setupFallbackMocks(options: { - selection: Partial | null; -}) { +function setupFallbackMocks(options: { selection: Partial | null }) { Object.defineProperty(navigator, 'userAgent', { value: CHROME_UA, configurable: true, @@ -117,7 +117,9 @@ function setupFallbackMocks(options: { jest .spyOn(document, 'getSelection') .mockReturnValue(options.selection as Selection | null); - jest.spyOn(document, 'createRange').mockReturnValue(mockRange as unknown as Range); + jest + .spyOn(document, 'createRange') + .mockReturnValue(mockRange as unknown as Range); jest.spyOn(document, 'createElement').mockReturnValue(mockSpan); jest.spyOn(document.body, 'appendChild').mockImplementation(() => mockSpan); jest.spyOn(document.body, 'removeChild').mockImplementation(() => mockSpan); @@ -168,7 +170,9 @@ test('rejects when execCommand returns false', async () => { }); mockExecCommand(() => false); - await expect(copyTextToClipboard(makeGetText('fail'))).rejects.toBeUndefined(); + await expect( + copyTextToClipboard(makeGetText('fail')), + ).rejects.toBeUndefined(); }); test('rejects when execCommand throws', async () => { @@ -187,7 +191,9 @@ test('rejects when execCommand throws', async () => { writable: true, }); - await expect(copyTextToClipboard(makeGetText('throw'))).rejects.toBeUndefined(); + await expect( + copyTextToClipboard(makeGetText('throw')), + ).rejects.toBeUndefined(); }); test('resolves without copying when getSelection returns null', async () => { diff --git a/superset-frontend/src/components/AlteredSliceTag/utils/index.ts b/superset-frontend/src/components/AlteredSliceTag/utils/index.ts index 9fa9830e785..3088d90aeb0 100644 --- a/superset-frontend/src/components/AlteredSliceTag/utils/index.ts +++ b/superset-frontend/src/components/AlteredSliceTag/utils/index.ts @@ -52,7 +52,9 @@ export const formatValueHandler = ( v.comparator && v.comparator.constructor === Array ? `[${v.comparator.join(', ')}]` : v.comparator; - return filterVal ? `${v.subject} ${v.operator} ${filterVal}` : `${v.subject} ${v.operator}`; + return filterVal + ? `${v.subject} ${v.operator} ${filterVal}` + : `${v.subject} ${v.operator}`; }) .join(', '); } diff --git a/superset-frontend/src/dashboard/actions/chartCustomizationActions.test.ts b/superset-frontend/src/dashboard/actions/chartCustomizationActions.test.ts new file mode 100644 index 00000000000..09defc6c3c0 --- /dev/null +++ b/superset-frontend/src/dashboard/actions/chartCustomizationActions.test.ts @@ -0,0 +1,273 @@ +/** + * 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 fetchMock from 'fetch-mock'; +import { ChartCustomization, ChartCustomizationType } from '@superset-ui/core'; +import { + setInScopeStatusOfCustomizations, + saveChartCustomization, +} from './chartCustomizationActions'; +import { SET_IN_SCOPE_STATUS_OF_FILTERS } from './nativeFilters'; +import { DASHBOARD_INFO_UPDATED } from './dashboardInfo'; + +beforeAll(() => fetchMock.mockGlobal()); +afterAll(() => fetchMock.hardReset()); +afterEach(() => fetchMock.clearHistory().removeRoutes()); + +function setup(stateOverrides: Record = {}) { + const state = { + nativeFilters: { + filters: {}, + }, + dashboardInfo: { + metadata: { + chart_customization_config: [], + }, + }, + ...stateOverrides, + }; + const getState = jest.fn(() => state) as unknown as () => any; + const dispatch = jest.fn(); + return { getState, dispatch, state }; +} + +test('setInScopeStatusOfCustomizations filters null entries from chart_customization_config', () => { + const { getState, dispatch } = setup({ + nativeFilters: { + filters: { + 'CUSTOM-1': { + id: 'CUSTOM-1', + name: 'Group By', + chartsInScope: [], + }, + }, + }, + dashboardInfo: { + metadata: { + chart_customization_config: [ + null, + { id: 'CUSTOM-1', name: 'Group By', chartsInScope: [] }, + null, + ], + }, + }, + }); + + const thunk = setInScopeStatusOfCustomizations([ + { + customizationId: 'CUSTOM-1', + chartsInScope: [10, 20], + tabsInScope: ['TAB-A'], + }, + ]); + thunk(dispatch, getState, null); + + // Should dispatch SET_IN_SCOPE_STATUS_OF_FILTERS for the matching filter + expect(dispatch).toHaveBeenCalledWith( + expect.objectContaining({ type: SET_IN_SCOPE_STATUS_OF_FILTERS }), + ); + + // Should dispatch DASHBOARD_INFO_UPDATED with only the valid config item (nulls stripped) + const infoUpdateCall = dispatch.mock.calls.find( + ([action]: [{ type: string }]) => action.type === DASHBOARD_INFO_UPDATED, + ); + expect(infoUpdateCall).toBeDefined(); + const updatedConfig = + infoUpdateCall[0].newInfo.metadata.chart_customization_config; + expect(updatedConfig).toHaveLength(1); + expect(updatedConfig[0].id).toBe('CUSTOM-1'); + expect(updatedConfig[0].chartsInScope).toEqual([10, 20]); + expect(updatedConfig[0].tabsInScope).toEqual(['TAB-A']); +}); + +test('setInScopeStatusOfCustomizations filters undefined entries from chart_customization_config', () => { + const { getState, dispatch } = setup({ + nativeFilters: { + filters: { + 'CUSTOM-1': { + id: 'CUSTOM-1', + name: 'Group By', + chartsInScope: [], + }, + }, + }, + dashboardInfo: { + metadata: { + chart_customization_config: [ + undefined, + { id: 'CUSTOM-1', name: 'Group By', chartsInScope: [] }, + undefined, + ], + }, + }, + }); + + const thunk = setInScopeStatusOfCustomizations([ + { + customizationId: 'CUSTOM-1', + chartsInScope: [10, 20], + tabsInScope: ['TAB-A'], + }, + ]); + thunk(dispatch, getState, null); + + const infoUpdateCall = dispatch.mock.calls.find( + ([action]: [{ type: string }]) => action.type === DASHBOARD_INFO_UPDATED, + ); + expect(infoUpdateCall).toBeDefined(); + const updatedConfig = + infoUpdateCall[0].newInfo.metadata.chart_customization_config; + expect(updatedConfig).toHaveLength(1); + expect(updatedConfig[0].id).toBe('CUSTOM-1'); + expect(updatedConfig[0].chartsInScope).toEqual([10, 20]); +}); + +test('setInScopeStatusOfCustomizations handles config that is entirely null entries', () => { + const { getState, dispatch } = setup({ + nativeFilters: { filters: {} }, + dashboardInfo: { + metadata: { + chart_customization_config: [null, null], + }, + }, + }); + + const thunk = setInScopeStatusOfCustomizations([ + { + customizationId: 'CUSTOM-MISSING', + chartsInScope: [10], + tabsInScope: [], + }, + ]); + thunk(dispatch, getState, null); + + // Should still dispatch the info update with an empty config + const infoUpdateCall = dispatch.mock.calls.find( + ([action]: [{ type: string }]) => action.type === DASHBOARD_INFO_UPDATED, + ); + expect(infoUpdateCall).toBeDefined(); + expect( + infoUpdateCall[0].newInfo.metadata.chart_customization_config, + ).toHaveLength(0); +}); + +test('setInScopeStatusOfCustomizations works with undefined metadata', () => { + const { getState, dispatch } = setup({ + nativeFilters: { filters: {} }, + dashboardInfo: { metadata: undefined }, + }); + + const thunk = setInScopeStatusOfCustomizations([ + { + customizationId: 'CUSTOM-1', + chartsInScope: [10], + tabsInScope: [], + }, + ]); + thunk(dispatch, getState, null); + + const infoUpdateCall = dispatch.mock.calls.find( + ([action]: [{ type: string }]) => action.type === DASHBOARD_INFO_UPDATED, + ); + expect(infoUpdateCall).toBeDefined(); + expect( + infoUpdateCall[0].newInfo.metadata.chart_customization_config, + ).toHaveLength(0); +}); + +test('saveChartCustomization filters null entries from currentConfig before merging', async () => { + const customization: ChartCustomization = { + id: 'CUSTOM-1', + type: ChartCustomizationType.ChartCustomization, + name: 'Group By', + filterType: 'chart_customization_dynamic_groupby', + targets: [{}], + scope: { rootPath: ['ROOT_ID'], excluded: [] }, + chartsInScope: [10], + tabsInScope: ['TAB-A'], + defaultDataMask: {}, + controlValues: {}, + }; + + fetchMock.put('glob:*/api/v1/dashboard/1/chart_customizations', { + result: [customization], + }); + + const { getState, dispatch } = setup({ + nativeFilters: { filters: {} }, + dashboardInfo: { + id: 1, + metadata: { + chart_customization_config: [ + null, + { id: 'CUSTOM-1', name: 'Group By', chartsInScope: [10] }, + null, + ], + }, + }, + }); + + const thunk = saveChartCustomization([customization], [], [], false); + await thunk(dispatch, getState, null); + + // DASHBOARD_INFO_UPDATED should have merged config without nulls + const infoUpdateCall = dispatch.mock.calls.find( + ([action]: [{ type: string }]) => action.type === DASHBOARD_INFO_UPDATED, + ); + expect(infoUpdateCall).toBeDefined(); + const config = infoUpdateCall[0].newInfo.metadata.chart_customization_config; + expect(config.every((item: unknown) => item !== null)).toBe(true); +}); + +test('saveChartCustomization filters null entries from oldConfig when resetDataMask is true', async () => { + const customization: ChartCustomization = { + id: 'CUSTOM-1', + type: ChartCustomizationType.ChartCustomization, + name: 'Group By', + filterType: 'chart_customization_dynamic_groupby', + targets: [{}], + scope: { rootPath: ['ROOT_ID'], excluded: [] }, + chartsInScope: [10], + tabsInScope: ['TAB-A'], + defaultDataMask: {}, + controlValues: {}, + }; + + fetchMock.put('glob:*/api/v1/dashboard/1/chart_customizations', { + result: [customization], + }); + + const { getState, dispatch } = setup({ + nativeFilters: { filters: {} }, + dashboardInfo: { + id: 1, + metadata: { + chart_customization_config: [ + null, + { id: 'CUSTOM-1', name: 'Group By', chartsInScope: [10] }, + null, + ], + }, + }, + }); + + const thunk = saveChartCustomization([customization], [], [], true); + + // Should not throw when building oldCustomizationsById from null-containing config + await expect(thunk(dispatch, getState, null)).resolves.toBeDefined(); +}); diff --git a/superset-frontend/src/dashboard/actions/chartCustomizationActions.ts b/superset-frontend/src/dashboard/actions/chartCustomizationActions.ts index 348f65c9b86..6a282a03d71 100644 --- a/superset-frontend/src/dashboard/actions/chartCustomizationActions.ts +++ b/superset-frontend/src/dashboard/actions/chartCustomizationActions.ts @@ -107,7 +107,8 @@ export function saveChartCustomization( }); const currentMetadata = getState().dashboardInfo.metadata; - const currentConfig = currentMetadata?.chart_customization_config || []; + const currentConfig = + currentMetadata?.chart_customization_config?.filter(Boolean) || []; const mergedResult = response.result.map( (item: ChartCustomization | ChartCustomizationDivider) => { @@ -148,7 +149,8 @@ export function saveChartCustomization( ); if (resetDataMask) { - const oldConfig = metadata?.chart_customization_config || []; + const oldConfig = + metadata?.chart_customization_config?.filter(Boolean) || []; const oldCustomizationsById = oldConfig.reduce< Record >((acc, customization) => { @@ -333,7 +335,8 @@ export function setInScopeStatusOfCustomizations( } const { metadata } = getState().dashboardInfo; - const customizationConfig = metadata?.chart_customization_config || []; + const customizationConfig = + metadata?.chart_customization_config?.filter(Boolean) || []; const scopeMap = new Map( customizationScopes.map( diff --git a/superset-frontend/src/dashboard/actions/hydrate.ts b/superset-frontend/src/dashboard/actions/hydrate.ts index 4639589ec24..f28803a0dec 100644 --- a/superset-frontend/src/dashboard/actions/hydrate.ts +++ b/superset-frontend/src/dashboard/actions/hydrate.ts @@ -293,15 +293,17 @@ export const hydrateDashboard = directPathToChild.push(directLinkComponentId); } - const rawChartCustomizations = - (metadata?.chart_customization_config as JsonObject[]) || []; + const rawChartCustomizations = ( + (metadata?.chart_customization_config as JsonObject[]) || [] + ).filter(Boolean); const chartCustomizations = migrateChartCustomizationArray( rawChartCustomizations, ); - const filters = - (metadata?.native_filter_configuration as JsonObject[]) || []; + const filters = ( + (metadata?.native_filter_configuration as JsonObject[]) || [] + ).filter(Boolean); const combinedFilters = [...filters, ...chartCustomizations]; const nativeFilters = getInitialNativeFilterState({ diff --git a/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardContainer.test.tsx b/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardContainer.test.tsx index cd144fb9799..563cedf4198 100644 --- a/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardContainer.test.tsx +++ b/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardContainer.test.tsx @@ -16,7 +16,7 @@ * specific language governing permissions and limitations * under the License. */ -import { render, waitFor } from 'spec/helpers/testing-library'; +import { render, screen, waitFor } from 'spec/helpers/testing-library'; import fetchMock from 'fetch-mock'; import { storeWithState } from 'spec/fixtures/mockStore'; import mockState from 'spec/fixtures/mockState'; @@ -700,6 +700,136 @@ test('returns empty scope data for chart customization dividers', async () => { } }); +test('does not crash when chart_customization_config contains a legacy item with customization: null', async () => { + const nullCustomizationId = 'CHART_CUSTOMIZATION-null-1'; + const originalFn = chartCustomizationActions.setInScopeStatusOfCustomizations; + const spy = jest.spyOn( + chartCustomizationActions, + 'setInScopeStatusOfCustomizations', + ); + spy.mockImplementation(args => originalFn(args)); + + try { + const state = { + dashboardInfo: { + ...mockState.dashboardInfo, + metadata: { + ...mockState.dashboardInfo.metadata, + native_filter_configuration: [], + chart_customization_config: [ + { + id: nullCustomizationId, + customization: null, + }, + ], + }, + }, + nativeFilters: { + filters: { + [nullCustomizationId]: { + id: nullCustomizationId, + chartsInScope: [], + }, + }, + }, + }; + + setup(state); + expect(screen.getByTestId('mock-dashboard-grid')).toBeInTheDocument(); + } finally { + spy.mockRestore(); + } +}); + +test('does not crash when chart_customization_config contains a null entry', async () => { + setup({ + dashboardInfo: { + ...mockState.dashboardInfo, + metadata: { + ...mockState.dashboardInfo.metadata, + native_filter_configuration: [], + chart_customization_config: [null], + }, + }, + nativeFilters: { filters: {} }, + }); + expect(screen.getByTestId('mock-dashboard-grid')).toBeInTheDocument(); +}); + +test('does not crash when chart_customization_config contains an undefined entry', async () => { + setup({ + dashboardInfo: { + ...mockState.dashboardInfo, + metadata: { + ...mockState.dashboardInfo.metadata, + native_filter_configuration: [], + chart_customization_config: [undefined], + }, + }, + nativeFilters: { filters: {} }, + }); + expect(screen.getByTestId('mock-dashboard-grid')).toBeInTheDocument(); +}); + +test('does not crash when chart_customization_config mixes null and new-format items', async () => { + const customizationId = 'CHART_CUSTOMIZATION-new-format-1'; + const originalFn = chartCustomizationActions.setInScopeStatusOfCustomizations; + const spy = jest.spyOn( + chartCustomizationActions, + 'setInScopeStatusOfCustomizations', + ); + spy.mockImplementation(args => originalFn(args)); + + try { + setup({ + dashboardInfo: { + ...mockState.dashboardInfo, + metadata: { + ...mockState.dashboardInfo.metadata, + native_filter_configuration: [], + chart_customization_config: [ + null, + { + id: customizationId, + type: 'CHART_CUSTOMIZATION', + name: 'Dynamic Group By', + filterType: 'chart_customization_dynamic_groupby', + targets: [{ datasetId: 1, column: { name: 'status' } }], + scope: { rootPath: ['ROOT_ID'], excluded: [] }, + chartsInScope: [], + defaultDataMask: {}, + controlValues: {}, + }, + ], + }, + }, + nativeFilters: { + filters: { + [customizationId]: { + id: customizationId, + type: 'CHART_CUSTOMIZATION', + chartsInScope: [], + }, + }, + }, + }); + expect(screen.getByTestId('mock-dashboard-grid')).toBeInTheDocument(); + + await waitFor(() => { + expect(spy).toHaveBeenCalledWith( + expect.arrayContaining([ + expect.objectContaining({ + customizationId, + chartsInScope: [sliceId], + }), + ]), + ); + }); + } finally { + spy.mockRestore(); + } +}); + test('does not dispatch setInScopeStatusOfCustomizations when chart_customization_config is empty', async () => { const spy = jest.spyOn( chartCustomizationActions, diff --git a/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardContainer.tsx b/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardContainer.tsx index a9c954aecbf..84ca3581bfa 100644 --- a/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardContainer.tsx +++ b/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardContainer.tsx @@ -93,11 +93,13 @@ function normalizeChartCustomizationsForScopeCalculation( chartCustomizations: ChartCustomizationConfiguration, chartIds: number[], ): ChartCustomizationConfiguration { - if (!chartCustomizations.some(isLegacyChartCustomizationFormat)) { - return chartCustomizations; + const truthyCustomizations = chartCustomizations.filter(Boolean); + + if (!truthyCustomizations.some(isLegacyChartCustomizationFormat)) { + return truthyCustomizations; } - return chartCustomizations.map(item => { + return truthyCustomizations.map(item => { if (!isLegacyChartCustomizationFormat(item)) { return item; } diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/FilterScope/utils.test.ts b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/FilterScope/utils.test.ts index 2c73e920512..94c1b3cdc85 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/FilterScope/utils.test.ts +++ b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/FilterScope/utils.test.ts @@ -18075,7 +18075,12 @@ describe('Ensure buildTree does not throw runtime errors when encountering an in }; const rootNode = circularLayout.ROOT_ID; - const rootTreeItem: TreeItem = { key: 'ROOT_ID', title: 'Root', children: [], nodeType: 'TAB' }; + const rootTreeItem: TreeItem = { + key: 'ROOT_ID', + title: 'Root', + children: [], + nodeType: 'TAB', + }; expect(() => { buildTree( diff --git a/superset-frontend/src/dashboard/components/nativeFilters/state.test.ts b/superset-frontend/src/dashboard/components/nativeFilters/state.test.ts index 782e0fff6bf..a97360ea54e 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/state.test.ts +++ b/superset-frontend/src/dashboard/components/nativeFilters/state.test.ts @@ -19,8 +19,18 @@ import { renderHook } from '@testing-library/react-hooks'; import { useSelector } from 'react-redux'; -import { NativeFilterType, Filter, Divider } from '@superset-ui/core'; -import { useIsFilterInScope, useSelectFiltersInScope } from './state'; +import { + NativeFilterType, + Filter, + Divider, + ChartCustomizationType, + type ChartCustomization, +} from '@superset-ui/core'; +import { + useChartCustomizationConfiguration, + useIsFilterInScope, + useSelectFiltersInScope, +} from './state'; jest.mock('react-redux', () => { const actual = jest.requireActual('react-redux'); @@ -507,3 +517,87 @@ test('filter with mix of existing and non-existent charts in chartsInScope', () const { result } = renderHook(() => useIsFilterInScope()); expect(result.current(filter)).toBe(true); }); + +test('useChartCustomizationConfiguration ignores null items in metadata', () => { + (useSelector as jest.Mock).mockImplementation((selector: Function) => + selector({ + dashboardInfo: { + metadata: { + chart_customization_config: [ + null, + { + id: 'CHART_CUSTOMIZATION-1', + name: 'Dynamic Group By', + type: ChartCustomizationType.ChartCustomization, + filterType: 'chart_customization_dynamic_groupby', + chartsInScope: [101], + scope: { rootPath: ['ROOT_ID'], excluded: [] }, + controlValues: { canSelectMultiple: false }, + defaultDataMask: {}, + cascadeParentIds: [], + targets: [{ column: { name: 'status' }, datasetId: 1 }], + description: 'valid customization', + } satisfies ChartCustomization, + ], + }, + }, + dashboardLayout: { + present: { + 'CHART-101': { + type: 'CHART', + meta: { chartId: 101 }, + }, + }, + }, + }), + ); + + const { result } = renderHook(() => useChartCustomizationConfiguration()); + + expect(result.current).toHaveLength(1); + expect(result.current[0]).toEqual( + expect.objectContaining({ id: 'CHART_CUSTOMIZATION-1' }), + ); +}); + +test('useChartCustomizationConfiguration ignores undefined items in metadata', () => { + (useSelector as jest.Mock).mockImplementation((selector: Function) => + selector({ + dashboardInfo: { + metadata: { + chart_customization_config: [ + undefined, + { + id: 'CHART_CUSTOMIZATION-1', + name: 'Dynamic Group By', + type: ChartCustomizationType.ChartCustomization, + filterType: 'chart_customization_dynamic_groupby', + chartsInScope: [101], + scope: { rootPath: ['ROOT_ID'], excluded: [] }, + controlValues: { canSelectMultiple: false }, + defaultDataMask: {}, + cascadeParentIds: [], + targets: [{ column: { name: 'status' }, datasetId: 1 }], + description: 'valid customization', + } satisfies ChartCustomization, + ], + }, + }, + dashboardLayout: { + present: { + 'CHART-101': { + type: 'CHART', + meta: { chartId: 101 }, + }, + }, + }, + }), + ); + + const { result } = renderHook(() => useChartCustomizationConfiguration()); + + expect(result.current).toHaveLength(1); + expect(result.current[0]).toEqual( + expect.objectContaining({ id: 'CHART_CUSTOMIZATION-1' }), + ); +}); diff --git a/superset-frontend/src/dashboard/components/nativeFilters/state.ts b/superset-frontend/src/dashboard/components/nativeFilters/state.ts index 11034ab1da0..cfb58d0f442 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/state.ts +++ b/superset-frontend/src/dashboard/components/nativeFilters/state.ts @@ -101,13 +101,15 @@ const selectChartCustomizationConfiguration = createSelector( selectDashboardChartIds, ], (allCustomizations, dashboardChartIds): ChartCustomizationConfiguration => { - const hasLegacyFormat = allCustomizations.some(item => + const truthyCustomizations = allCustomizations.filter(Boolean); + + const hasLegacyFormat = truthyCustomizations.some(item => isLegacyChartCustomizationFormat(item), ); const migratedCustomizations = hasLegacyFormat - ? migrateChartCustomizationArray(allCustomizations) - : (allCustomizations as ChartCustomizationConfiguration); + ? migrateChartCustomizationArray(truthyCustomizations) + : (truthyCustomizations as ChartCustomizationConfiguration); return migratedCustomizations.filter(customization => { if ( diff --git a/superset-frontend/src/dashboard/reducers/dashboardInfo.test.ts b/superset-frontend/src/dashboard/reducers/dashboardInfo.test.ts index 5d57ae4ee9e..8bf3e5b774d 100644 --- a/superset-frontend/src/dashboard/reducers/dashboardInfo.test.ts +++ b/superset-frontend/src/dashboard/reducers/dashboardInfo.test.ts @@ -20,6 +20,7 @@ import { DASHBOARD_INFO_UPDATED, dashboardInfoChanged, } from '../actions/dashboardInfo'; +import { CLEAR_ALL_CHART_CUSTOMIZATIONS } from '../actions/chartCustomizationActions'; import type { DashboardInfo } from '../types'; import dashboardInfoReducer from './dashboardInfo'; @@ -154,3 +155,147 @@ test('does not affect metadata when DASHBOARD_INFO_UPDATED has no metadata key', // Metadata untouched — same reference expect(result.metadata).toBe(stateWithScopes.metadata); }); + +test('preserveScopes filters out null entries from both existing and incoming config', () => { + const stateWithNulls = { + id: 1, + metadata: { + native_filter_configuration: [ + null, + { + id: 'FILTER-1', + name: 'Region', + chartsInScope: [10, 20], + tabsInScope: ['TAB-A'], + targets: [{ datasetId: 1 }], + }, + ], + chart_customization_config: [ + null, + { + id: 'CUSTOM-1', + chartsInScope: [100], + tabsInScope: ['TAB-X'], + }, + ], + }, + } as unknown as Partial; + + const serverMetadata = { + native_filter_configuration: [ + null, + { id: 'FILTER-1', name: 'Region (updated)', targets: [{ datasetId: 1 }] }, + ], + chart_customization_config: [null, { id: 'CUSTOM-1' }], + } as unknown as DashboardInfo['metadata']; + + const action = dashboardInfoChanged({ metadata: serverMetadata }); + const result = dashboardInfoReducer(stateWithNulls, action); + + const filters = result.metadata?.native_filter_configuration; + expect(filters).toHaveLength(1); + expect(filters![0].id).toBe('FILTER-1'); + expect(filters![0].chartsInScope).toEqual([10, 20]); + + const customizations = result.metadata?.chart_customization_config; + expect(customizations).toHaveLength(1); + expect(customizations![0].id).toBe('CUSTOM-1'); + expect(customizations![0].chartsInScope).toEqual([100]); +}); + +test('preserveScopes filters out undefined entries from both existing and incoming config', () => { + const stateWithUndefined = { + id: 1, + metadata: { + native_filter_configuration: [ + undefined, + { + id: 'FILTER-1', + name: 'Region', + chartsInScope: [10, 20], + tabsInScope: ['TAB-A'], + targets: [{ datasetId: 1 }], + }, + ], + chart_customization_config: [ + undefined, + { + id: 'CUSTOM-1', + chartsInScope: [100], + tabsInScope: ['TAB-X'], + }, + ], + }, + } as unknown as Partial; + + const serverMetadata = { + native_filter_configuration: [ + undefined, + { id: 'FILTER-1', name: 'Region (updated)', targets: [{ datasetId: 1 }] }, + ], + chart_customization_config: [undefined, { id: 'CUSTOM-1' }], + } as unknown as DashboardInfo['metadata']; + + const action = dashboardInfoChanged({ metadata: serverMetadata }); + const result = dashboardInfoReducer(stateWithUndefined, action); + + const filters = result.metadata?.native_filter_configuration; + expect(filters).toHaveLength(1); + expect(filters![0].id).toBe('FILTER-1'); + expect(filters![0].chartsInScope).toEqual([10, 20]); + + const customizations = result.metadata?.chart_customization_config; + expect(customizations).toHaveLength(1); + expect(customizations![0].id).toBe('CUSTOM-1'); + expect(customizations![0].chartsInScope).toEqual([100]); +}); + +test('CLEAR_ALL_CHART_CUSTOMIZATIONS filters out null entries before mapping', () => { + const stateWithNullConfig = { + id: 1, + metadata: { + chart_customization_config: [ + null, + { + id: 'CUSTOM-1', + targets: [{ datasetId: 1, column: { name: 'status' } }], + chartsInScope: [10], + }, + null, + ], + }, + } as unknown as Partial; + + const action = { type: CLEAR_ALL_CHART_CUSTOMIZATIONS }; + const result = dashboardInfoReducer(stateWithNullConfig, action); + + const config = result.metadata?.chart_customization_config; + expect(config).toHaveLength(1); + expect(config![0].id).toBe('CUSTOM-1'); + expect(config![0].targets).toEqual([{ datasetId: 1 }]); +}); + +test('CLEAR_ALL_CHART_CUSTOMIZATIONS filters out undefined entries before mapping', () => { + const stateWithUndefinedConfig = { + id: 1, + metadata: { + chart_customization_config: [ + undefined, + { + id: 'CUSTOM-1', + targets: [{ datasetId: 1, column: { name: 'status' } }], + chartsInScope: [10], + }, + undefined, + ], + }, + } as unknown as Partial; + + const action = { type: CLEAR_ALL_CHART_CUSTOMIZATIONS }; + const result = dashboardInfoReducer(stateWithUndefinedConfig, action); + + const config = result.metadata?.chart_customization_config; + expect(config).toHaveLength(1); + expect(config![0].id).toBe('CUSTOM-1'); + expect(config![0].targets).toEqual([{ datasetId: 1 }]); +}); diff --git a/superset-frontend/src/dashboard/reducers/dashboardInfo.ts b/superset-frontend/src/dashboard/reducers/dashboardInfo.ts index f6dbe8b7bd8..94a1f671058 100644 --- a/superset-frontend/src/dashboard/reducers/dashboardInfo.ts +++ b/superset-frontend/src/dashboard/reducers/dashboardInfo.ts @@ -89,7 +89,10 @@ function preserveScopes( existingConfig: T[] | undefined, incomingConfig: T[] | undefined, ): T[] { - const existingScopesMap = (existingConfig || []).reduce< + const truthyExistingConfig = (existingConfig || []).filter(Boolean); + const truthyIncomingConfig = (incomingConfig || []).filter(Boolean); + + const existingScopesMap = truthyExistingConfig.reduce< Record >((acc, item) => { if (item.chartsInScope != null || item.tabsInScope != null) { @@ -101,7 +104,7 @@ function preserveScopes( return acc; }, {}); - return (incomingConfig || []).map(item => { + return truthyIncomingConfig.map(item => { const existingScopes = existingScopesMap[item.id]; if (item.chartsInScope == null && existingScopes) { return { @@ -289,8 +292,9 @@ export default function dashboardInfoReducer( pendingChartCustomizations: {}, }; case CLEAR_ALL_CHART_CUSTOMIZATIONS: { - const customizationConfig = - state.metadata?.chart_customization_config || []; + const customizationConfig = ( + state.metadata?.chart_customization_config || [] + ).filter(Boolean); return { ...state, metadata: { diff --git a/superset-frontend/src/dashboard/reducers/nativeFilters.test.ts b/superset-frontend/src/dashboard/reducers/nativeFilters.test.ts index ceac844317b..31c2aee28ca 100644 --- a/superset-frontend/src/dashboard/reducers/nativeFilters.test.ts +++ b/superset-frontend/src/dashboard/reducers/nativeFilters.test.ts @@ -22,9 +22,10 @@ import { ChartCustomization, ChartCustomizationType, } from '@superset-ui/core'; -import nativeFilterReducer from './nativeFilters'; +import nativeFilterReducer, { getInitialState } from './nativeFilters'; import { SET_NATIVE_FILTERS_CONFIG_COMPLETE } from '../actions/nativeFilters'; import { HYDRATE_DASHBOARD } from '../actions/hydrate'; +import { migrateChartCustomizationArray } from '../util/migrateChartCustomization'; const createMockFilter = ( id: string, @@ -415,3 +416,94 @@ test('SET_NATIVE_FILTERS_CONFIG_COMPLETE treats backend response as source of tr expect(result.filters.customization1.chartsInScope).toEqual([12, 13]); expect(result.filters.customization1.tabsInScope).toEqual(['tab6']); }); + +test('getInitialState builds filters map from combined filter and customization config', () => { + const filter = createMockFilter('filter1', [1, 2], ['tab1']); + const customization = createMockChartCustomization( + 'custom1', + [3, 4], + ['tab2'], + ); + + const result = getInitialState({ + filterConfig: [filter, customization], + }); + + expect(Object.keys(result.filters)).toHaveLength(2); + expect(result.filters.filter1).toBeDefined(); + expect(result.filters.custom1).toBeDefined(); + expect(result.filters.filter1.chartsInScope).toEqual([1, 2]); + expect(result.filters.custom1.chartsInScope).toEqual([3, 4]); +}); + +test('getInitialState crashes on null entries in filterConfig (hydrate must pre-filter)', () => { + // This proves why hydrate.ts MUST call .filter(Boolean) before passing + // chart_customization_config to getInitialState — null entries cause a + // TypeError when accessing .id on null. + expect(() => + getInitialState({ + filterConfig: [ + null as unknown as Filter, + createMockFilter('filter1', [1], []), + ], + }), + ).toThrow(); +}); + +test('hydrate pipeline: filter(Boolean) + migrate + getInitialState succeeds with null entries', () => { + // Reproduces the exact pipeline in hydrate.ts (lines 296-312): + // rawConfig.filter(Boolean) → migrateChartCustomizationArray → getInitialState + const rawConfig = [ + null, + { + id: 'CHART_CUSTOMIZATION-1', + type: ChartCustomizationType.ChartCustomization, + name: 'Dynamic Group By', + filterType: 'chart_customization_dynamic_groupby', + targets: [{ datasetId: 1, column: { name: 'status' } }], + scope: { rootPath: ['ROOT_ID'], excluded: [] }, + chartsInScope: [10], + defaultDataMask: { filterState: { value: null } }, + controlValues: {}, + cascadeParentIds: [], + description: '', + }, + null, + ]; + + // Step 1: filter(Boolean) — as hydrate.ts does + const filtered = rawConfig.filter(Boolean); + // Step 2: migrate — as hydrate.ts does + const migrated = migrateChartCustomizationArray(filtered); + // Step 3: combine with native filters and pass to getInitialState + const nativeFilters = [createMockFilter('filter1', [1, 2], ['tab1'])]; + const result = getInitialState({ + filterConfig: [...nativeFilters, ...migrated] as Filter[], + }); + + expect(Object.keys(result.filters)).toHaveLength(2); + expect(result.filters.filter1).toBeDefined(); + expect(result.filters['CHART_CUSTOMIZATION-1']).toBeDefined(); + expect(result.filters['CHART_CUSTOMIZATION-1'].chartsInScope).toEqual([10]); +}); + +test('hydrate pipeline: native_filter_configuration with null entries does not crash getInitialState', () => { + // Reproduces the native_filter_configuration path in hydrate.ts (L305): + // native_filter_configuration may contain null entries from corrupted metadata. + // Without .filter(Boolean), these nulls reach getInitialState which reads .id → crash. + const nativeFilters = [ + null, + createMockFilter('filter1', [1, 2], ['tab1']), + null, + ]; + + // After .filter(Boolean), only valid filters remain + const filtered = nativeFilters.filter(Boolean); + const result = getInitialState({ + filterConfig: filtered as Filter[], + }); + + expect(Object.keys(result.filters)).toHaveLength(1); + expect(result.filters.filter1).toBeDefined(); + expect(result.filters.filter1.chartsInScope).toEqual([1, 2]); +}); diff --git a/superset-frontend/src/dashboard/util/charts/getFormDataWithExtraFilters.ts b/superset-frontend/src/dashboard/util/charts/getFormDataWithExtraFilters.ts index 45756b77481..161e0edddb3 100644 --- a/superset-frontend/src/dashboard/util/charts/getFormDataWithExtraFilters.ts +++ b/superset-frontend/src/dashboard/util/charts/getFormDataWithExtraFilters.ts @@ -22,9 +22,11 @@ import { DataRecordFilters, DataRecordValue, ensureIsArray, + getColumnLabel, JsonObject, PartialFilters, ChartCustomization, + QueryFormColumn, } from '@superset-ui/core'; import { ChartConfiguration, @@ -122,8 +124,13 @@ function extractColumnNames(columns: unknown[]): string[] { columns.forEach((col: unknown) => { if (typeof col === 'string') { columnNames.push(col); - } else if (col && typeof col === 'object' && 'column_name' in col) { - columnNames.push((col as { column_name: string }).column_name); + } else if (col && typeof col === 'object') { + if ('column_name' in col) { + columnNames.push((col as { column_name: string }).column_name); + } else if ('sqlExpression' in col) { + const label = getColumnLabel(col as QueryFormColumn); + if (label) columnNames.push(label); + } } }); } @@ -134,6 +141,9 @@ function buildExistingColumnsSet(chart: ChartQueryPayload): Set { const existingColumns = new Set(); const chartType = chart.form_data?.viz_type; + const existingGroupBy = ensureIsArray(chart.form_data?.groupby); + extractColumnNames(existingGroupBy).forEach(col => existingColumns.add(col)); + const xAxisColumn = chart.form_data?.x_axis; if (xAxisColumn && chartType !== 'heatmap' && chartType !== 'heatmap_v2') { existingColumns.add(xAxisColumn); diff --git a/superset-frontend/src/dashboard/util/getFormDataWithExtraFilters.test.ts b/superset-frontend/src/dashboard/util/getFormDataWithExtraFilters.test.ts index 3e090134bee..61adf3e3ad4 100644 --- a/superset-frontend/src/dashboard/util/getFormDataWithExtraFilters.test.ts +++ b/superset-frontend/src/dashboard/util/getFormDataWithExtraFilters.test.ts @@ -433,6 +433,147 @@ test('dataset mismatch: display control for a different dataset does not affect expectGroupBy(result, ['original_column']); }); +test('dynamic group by with overlapping selection preserves multi-column base groupby', () => { + const result = getFormDataWithExtraFilters( + makeGroupByArgs(['status'], ['status', 'category']), + ); + expectGroupBy(result, ['status', 'category']); +}); + +test('timeseries chart: overlapping selection preserves multi-column base groupby', () => { + const customizationId = 'CHART_CUSTOMIZATION-groupby-1'; + const result = getFormDataWithExtraFilters({ + ...mockArgs, + chart: { + ...mockChart, + form_data: { + ...mockChart.form_data, + viz_type: 'echarts_timeseries_line', + datasource: '3__table', + groupby: ['series_col', 'breakdown_col'], + x_axis: 'time_col', + }, + }, + dataMask: { + [customizationId]: { + id: customizationId, + extraFormData: {}, + filterState: { value: ['series_col'] }, + ownState: {}, + }, + }, + chartCustomizationItems: [ + createChartCustomization({ id: customizationId }), + ], + }); + expectGroupBy(result, ['series_col', 'breakdown_col']); +}); + +test('partial overlap: only non-existing columns pass through as customization override', () => { + const result = getFormDataWithExtraFilters( + makeGroupByArgs(['status', 'new_col'], ['status', 'category']), + ); + expectGroupBy(result, ['new_col']); +}); + +test('object-typed groupby entries (AdhocColumn) are recognized as existing columns', () => { + const customizationId = 'CHART_CUSTOMIZATION-groupby-1'; + const result = getFormDataWithExtraFilters({ + ...mockArgs, + chart: { + ...mockChart, + form_data: { + ...mockChart.form_data, + viz_type: 'table', + datasource: '3__table', + groupby: [{ column_name: 'status' }, 'category'], + }, + }, + dataMask: { + [customizationId]: { + id: customizationId, + extraFormData: {}, + filterState: { value: ['status', 'new_col'] }, + ownState: {}, + }, + }, + chartCustomizationItems: [ + createChartCustomization({ id: customizationId }), + ], + }); + // 'status' is already in groupby (as an object with column_name), so only 'new_col' passes through + expectGroupBy(result, ['new_col']); +}); + +test('AdhocColumn with sqlExpression (no column_name) is recognized as existing column', () => { + const customizationId = 'CHART_CUSTOMIZATION-groupby-1'; + const result = getFormDataWithExtraFilters({ + ...mockArgs, + chart: { + ...mockChart, + form_data: { + ...mockChart.form_data, + viz_type: 'table', + datasource: '3__table', + groupby: [ + { + sqlExpression: 'CASE WHEN status = 1 THEN "active" END', + label: 'status_label', + expressionType: 'SQL', + }, + 'category', + ], + }, + }, + dataMask: { + [customizationId]: { + id: customizationId, + extraFormData: {}, + filterState: { value: ['status_label', 'new_col'] }, + ownState: {}, + }, + }, + chartCustomizationItems: [ + createChartCustomization({ id: customizationId }), + ], + }); + // 'status_label' matches the AdhocColumn's label, so only 'new_col' passes through + expectGroupBy(result, ['new_col']); +}); + +test('AdhocColumn with empty label falls back to sqlExpression for identity', () => { + const customizationId = 'CHART_CUSTOMIZATION-groupby-1'; + const sqlExpr = 'CASE WHEN status = 1 THEN "active" END'; + const result = getFormDataWithExtraFilters({ + ...mockArgs, + chart: { + ...mockChart, + form_data: { + ...mockChart.form_data, + viz_type: 'table', + datasource: '3__table', + groupby: [ + { sqlExpression: sqlExpr, label: '', expressionType: 'SQL' }, + 'category', + ], + }, + }, + dataMask: { + [customizationId]: { + id: customizationId, + extraFormData: {}, + filterState: { value: [sqlExpr, 'new_col'] }, + ownState: {}, + }, + }, + chartCustomizationItems: [ + createChartCustomization({ id: customizationId }), + ], + }); + // Empty label → falls back to sqlExpression; sqlExpr is in existingColumns, only 'new_col' passes + expectGroupBy(result, ['new_col']); +}); + test('Scope boundary: display control with chartsInScope:[] does not affect the chart', () => { const customizationId = 'CHART_CUSTOMIZATION-groupby-out-of-scope'; const argsOutOfScope: GetFormDataWithExtraFiltersArguments = { diff --git a/superset-frontend/src/dashboard/util/migrateChartCustomization.test.ts b/superset-frontend/src/dashboard/util/migrateChartCustomization.test.ts index 28784c7378a..3932a417544 100644 --- a/superset-frontend/src/dashboard/util/migrateChartCustomization.test.ts +++ b/superset-frontend/src/dashboard/util/migrateChartCustomization.test.ts @@ -418,6 +418,89 @@ test('migrateChartCustomizationArray migrates mixed array', () => { expect(result[1].name).toBe('Already Migrated'); }); +test('isLegacyChartCustomizationFormat rejects item with customization: null', () => { + const item = { id: 'CUSTOMIZATION-NULL', customization: null }; + expect(isLegacyChartCustomizationFormat(item)).toBe(false); +}); + +test('migrateChartCustomizationArray drops item with customization: null', () => { + const items = [ + { id: 'CUSTOMIZATION-NULL', customization: null }, + { + id: 'CUSTOMIZATION-VALID', + customization: { + name: 'Valid', + dataset: 1, + column: 'country', + }, + }, + ]; + const result = migrateChartCustomizationArray(items); + // Malformed item is dropped; valid legacy item is migrated + expect(result).toHaveLength(1); + expect(result[0].name).toBe('Valid'); + expect(result[0].type).toBe(ChartCustomizationType.ChartCustomization); +}); + +test('isLegacyChartCustomizationFormat rejects item with customization: undefined', () => { + const item = { id: 'CUSTOMIZATION-UNDEF', customization: undefined }; + expect(isLegacyChartCustomizationFormat(item)).toBe(false); +}); + +test('migrateChartCustomizationArray drops null entries', () => { + const items = [ + null, + { + id: 'CUSTOMIZATION-VALID', + customization: { + name: 'Valid', + dataset: 1, + column: 'country', + }, + }, + ]; + const result = migrateChartCustomizationArray(items); + expect(result).toHaveLength(1); + expect(result[0].name).toBe('Valid'); + expect(result[0].type).toBe(ChartCustomizationType.ChartCustomization); +}); + +test('migrateChartCustomizationArray drops undefined entries', () => { + const items = [ + undefined, + { + id: 'CUSTOMIZATION-VALID', + customization: { + name: 'Valid', + dataset: 1, + column: 'country', + }, + }, + ]; + const result = migrateChartCustomizationArray(items); + expect(result).toHaveLength(1); + expect(result[0].name).toBe('Valid'); + expect(result[0].type).toBe(ChartCustomizationType.ChartCustomization); +}); + +test('migrateChartCustomizationArray drops item with customization: undefined', () => { + const items = [ + { id: 'CUSTOMIZATION-UNDEF', customization: undefined }, + { + id: 'CUSTOMIZATION-VALID', + customization: { + name: 'Valid', + dataset: 1, + column: 'country', + }, + }, + ]; + const result = migrateChartCustomizationArray(items); + expect(result).toHaveLength(1); + expect(result[0].name).toBe('Valid'); + expect(result[0].type).toBe(ChartCustomizationType.ChartCustomization); +}); + test('migrateChartCustomizationArray handles empty array', () => { const result = migrateChartCustomizationArray([]); expect(result).toEqual([]); diff --git a/superset-frontend/src/dashboard/util/migrateChartCustomization.ts b/superset-frontend/src/dashboard/util/migrateChartCustomization.ts index e658c57c0f4..9e05495487a 100644 --- a/superset-frontend/src/dashboard/util/migrateChartCustomization.ts +++ b/superset-frontend/src/dashboard/util/migrateChartCustomization.ts @@ -32,6 +32,7 @@ export function isLegacyChartCustomizationFormat( typeof item === 'object' && item !== null && 'customization' in item && + (item as Record).customization != null && !('type' in item) ); } @@ -146,10 +147,17 @@ export function migrateChartCustomization( export function migrateChartCustomizationArray( items: unknown[], ): ChartCustomization[] { - return items.map(item => { - if (isLegacyChartCustomizationFormat(item)) { - return migrateChartCustomization(item); - } - return item as ChartCustomization; - }); + return items + .filter( + item => + item != null && + (isLegacyChartCustomizationFormat(item) || + (typeof item === 'object' && 'type' in item)), + ) + .map(item => { + if (isLegacyChartCustomizationFormat(item)) { + return migrateChartCustomization(item); + } + return item as ChartCustomization; + }); } diff --git a/superset-frontend/src/dataMask/reducer.test.ts b/superset-frontend/src/dataMask/reducer.test.ts index 00f5e88d446..072fefe8948 100644 --- a/superset-frontend/src/dataMask/reducer.test.ts +++ b/superset-frontend/src/dataMask/reducer.test.ts @@ -23,11 +23,15 @@ import { type SetDataMaskForFilterChangesComplete, } from './actions'; import { + type ChartCustomization, type DataMaskStateWithId, type Filter, + type FilterConfiguration, type NativeFilterTarget, NativeFilterType, + ChartCustomizationType, } from '@superset-ui/core'; +import { HYDRATE_DASHBOARD } from 'src/dashboard/actions/hydrate'; // Helper to create minimal filter for testing const createFilter = ( @@ -116,3 +120,66 @@ test('when user edits a filter without changing targets, their selection is pres '1 year ago', ); }); + +// Runtime data from the server can contain null entries in +// chart_customization_config even though the TS type does not include | null +// yet. These helpers build HYDRATE_DASHBOARD actions that mirror that reality. +function hydrateAction( + chartCustomizationConfig: unknown[], + nativeFilterConfig: FilterConfiguration = [], +) { + return { + type: HYDRATE_DASHBOARD as typeof HYDRATE_DASHBOARD, + data: { + dashboardInfo: { + metadata: { + native_filter_configuration: nativeFilterConfig, + chart_customization_config: + chartCustomizationConfig as ChartCustomization[], + }, + }, + dataMask: {}, + }, + }; +} + +test('HYDRATE_DASHBOARD filters null entries from chart_customization_config', () => { + const customizationId = 'CHART_CUSTOMIZATION-group-1'; + const action = hydrateAction([ + null, + { + id: customizationId, + type: ChartCustomizationType.ChartCustomization, + name: 'Dynamic Group By', + filterType: 'chart_customization_dynamic_groupby', + targets: [{ datasetId: 1, column: { name: 'status' } }], + scope: { rootPath: ['ROOT_ID'], excluded: [] }, + chartsInScope: [10], + defaultDataMask: { + extraFormData: {}, + filterState: { value: ['status'] }, + }, + controlValues: {}, + cascadeParentIds: [], + description: '', + }, + null, + ]); + + const result = reducer({}, action); + + expect(result[customizationId]).toBeDefined(); + expect(result[customizationId].filterState?.value).toEqual(['status']); +}); + +test('HYDRATE_DASHBOARD handles chart_customization_config that is entirely null', () => { + const action = hydrateAction([null, null]); + + const result = reducer({}, action); + + // Should not crash; no customization keys should appear + const customizationKeys = Object.keys(result).filter(k => + k.startsWith('CHART_CUSTOMIZATION'), + ); + expect(customizationKeys).toHaveLength(0); +}); diff --git a/superset-frontend/src/dataMask/reducer.ts b/superset-frontend/src/dataMask/reducer.ts index 90051cbe54a..5ae614eb048 100644 --- a/superset-frontend/src/dataMask/reducer.ts +++ b/superset-frontend/src/dataMask/reducer.ts @@ -230,8 +230,9 @@ const dataMaskReducer = produce( loadedDataMask, ); - const rawChartCustomizationConfig = - metadata?.chart_customization_config || []; + const rawChartCustomizationConfig = ( + metadata?.chart_customization_config || [] + ).filter(Boolean); const hasLegacyFormat = rawChartCustomizationConfig.some(item => isLegacyChartCustomizationFormat(item), diff --git a/superset-frontend/src/explore/components/controls/ConditionalFormattingControl/ConditionalFormattingControl.test.tsx b/superset-frontend/src/explore/components/controls/ConditionalFormattingControl/ConditionalFormattingControl.test.tsx index 11716aa43a1..b40806bbe71 100644 --- a/superset-frontend/src/explore/components/controls/ConditionalFormattingControl/ConditionalFormattingControl.test.tsx +++ b/superset-frontend/src/explore/components/controls/ConditionalFormattingControl/ConditionalFormattingControl.test.tsx @@ -38,7 +38,11 @@ const defaultProps = { test('renders "is false" operator label without trailing undefined', () => { const value: ConditionalFormattingConfig[] = [ - { column: 'my_col', operator: Comparator.IsFalse, colorScheme: 'colorSuccess' }, + { + column: 'my_col', + operator: Comparator.IsFalse, + colorScheme: 'colorSuccess', + }, ]; render(); expect(screen.getByText('my_col is false')).toBeInTheDocument(); @@ -46,7 +50,11 @@ test('renders "is false" operator label without trailing undefined', () => { test('renders "is true" operator label without trailing undefined', () => { const value: ConditionalFormattingConfig[] = [ - { column: 'my_col', operator: Comparator.IsTrue, colorScheme: 'colorSuccess' }, + { + column: 'my_col', + operator: Comparator.IsTrue, + colorScheme: 'colorSuccess', + }, ]; render(); expect(screen.getByText('my_col is true')).toBeInTheDocument(); @@ -54,7 +62,11 @@ test('renders "is true" operator label without trailing undefined', () => { test('renders "is null" operator label without trailing undefined', () => { const value: ConditionalFormattingConfig[] = [ - { column: 'my_col', operator: Comparator.IsNull, colorScheme: 'colorSuccess' }, + { + column: 'my_col', + operator: Comparator.IsNull, + colorScheme: 'colorSuccess', + }, ]; render(); expect(screen.getByText('my_col is null')).toBeInTheDocument(); @@ -62,7 +74,11 @@ test('renders "is null" operator label without trailing undefined', () => { test('renders "is not null" operator label without trailing undefined', () => { const value: ConditionalFormattingConfig[] = [ - { column: 'my_col', operator: Comparator.IsNotNull, colorScheme: 'colorSuccess' }, + { + column: 'my_col', + operator: Comparator.IsNotNull, + colorScheme: 'colorSuccess', + }, ]; render(); expect(screen.getByText('my_col is not null')).toBeInTheDocument(); @@ -70,7 +86,11 @@ test('renders "is not null" operator label without trailing undefined', () => { test('renders verbose column name when available', () => { const value: ConditionalFormattingConfig[] = [ - { column: 'my_col', operator: Comparator.IsFalse, colorScheme: 'colorSuccess' }, + { + column: 'my_col', + operator: Comparator.IsFalse, + colorScheme: 'colorSuccess', + }, ]; render( = ({ addDangerToast(t('There was an error retrieving dashboard tabs.')); }); } - }, [dashboard, tabsEnabled, filtersEnabled, currentAlert?.extra, addDangerToast]); + }, [ + dashboard, + tabsEnabled, + filtersEnabled, + currentAlert?.extra, + addDangerToast, + ]); const databaseLabel = currentAlert?.database && !currentAlert.database.label; useEffect(() => { diff --git a/superset-frontend/src/utils/downloadAsImage.tsx b/superset-frontend/src/utils/downloadAsImage.tsx index cc1b3134e02..db55a35e036 100644 --- a/superset-frontend/src/utils/downloadAsImage.tsx +++ b/superset-frontend/src/utils/downloadAsImage.tsx @@ -350,8 +350,7 @@ export default function downloadAsImageOptimized( if (agContainer && agRootWrapper) { const api = agContainer._agGridApi; - const isFirstDataRendered = - agContainer._agGridFirstDataRendered === true; + const isFirstDataRendered = agContainer._agGridFirstDataRendered === true; if (!isFirstDataRendered) { addWarningToast(