This is an automated email from the ASF dual-hosted git repository. sadpandajoe pushed a commit to branch explore-rtl-control-deps in repository https://gitbox.apache.org/repos/asf/superset.git
commit 24e0548a4016b832e3c8d79b606467777628d78e Author: Joe Li <[email protected]> AuthorDate: Wed Sep 30 16:55:40 2026 -0700 test(explore): cover viz switch and control dependency logic Add tests for the explore reducer's SET_FIELD_VALUE handling of validationDependencies and rerender, the viz_type switch carry-over through StandardizedFormData, ControlPanelsContainer re-rendering against the real reducer, and getSectionsToRender datasource filtering. Co-Authored-By: Claude Opus 5.5 <[email protected]> --- .../components/ControlPanelsContainer.test.tsx | 156 +++++++++++++++- .../controlUtils/getSectionsToRender.test.ts | 121 ++++++++++++ .../src/explore/reducers/exploreReducer.test.ts | 208 ++++++++++++++++++++- 3 files changed, 482 insertions(+), 3 deletions(-) diff --git a/superset-frontend/src/explore/components/ControlPanelsContainer.test.tsx b/superset-frontend/src/explore/components/ControlPanelsContainer.test.tsx index de05550a9cd..1e0cd5e630d 100644 --- a/superset-frontend/src/explore/components/ControlPanelsContainer.test.tsx +++ b/superset-frontend/src/explore/components/ControlPanelsContainer.test.tsx @@ -16,13 +16,16 @@ * specific language governing permissions and limitations * under the License. */ -import { useSelector } from 'react-redux'; +import { useDispatch, useSelector } from 'react-redux'; +import { configureStore } from '@reduxjs/toolkit'; import { + act, render, screen, userEvent, waitFor, } from 'spec/helpers/testing-library'; +import reducerIndex from 'spec/helpers/reducerIndex'; import { t } from '@apache-superset/core/translation'; import { DatasourceType, @@ -30,8 +33,13 @@ import { isFeatureEnabled, FeatureFlag, } from '@superset-ui/core'; -import { defaultControls, defaultState } from 'src/explore/store'; +import { + defaultControls, + defaultState, + getControlsState, +} from 'src/explore/store'; import { ExplorePageState } from 'src/explore/types'; +import { setControlValue } from 'src/explore/actions/exploreActions'; import { getFormDataFromControls } from 'src/explore/controlUtils'; import { ControlPanelsContainer, @@ -53,6 +61,42 @@ const FormDataMock = () => { return <div data-test="mock-formdata">{Object.keys(formData).join(':')}</div>; }; +// Mirrors the props ExploreViewContainer derives from the store, so the real +// reducer and the real container run against each other. +const StoreBackedControlPanelsContainer = () => { + const dispatch = useDispatch(); + const exploreState = useSelector((state: ExplorePageState) => state.explore); + const actions = { + setControlValue: ( + ...args: Parameters<typeof setControlValue> + ): ReturnType<typeof setControlValue> => + dispatch(setControlValue(...args)) as unknown as ReturnType< + typeof setControlValue + >, + }; + return ( + <ControlPanelsContainer + exploreState={exploreState} + actions={actions} + datasource_type={DatasourceType.Table} + controls={exploreState.controls} + form_data={getFormDataFromControls(exploreState.controls)} + isDatasourceMetaLoading={false} + chart={ + { + queriesResponse: null, + chartStatus: 'success', + } as ControlPanelsContainerProps['chart'] + } + onQuery={jest.fn()} + onStop={jest.fn()} + canStopQuery={false} + chartIsStale={false} + errorMessage={null} + /> + ); +}; + // eslint-disable-next-line no-restricted-globals -- TODO: Migrate from describe blocks describe('ControlPanelsContainer', () => { const defaultTableConfig = { @@ -97,8 +141,24 @@ describe('ControlPanelsContainer', () => { ], }; + // Reuses the 'Query' section label of the table config so the section stays + // expanded after a switch (the accordion only honors its initial open keys). + const switchTargetConfig = { + controlPanelSections: [ + { + label: t('Query'), + expanded: true, + controlSetRows: [['groupby'], ['metrics'], ['limit']], + }, + ], + }; + beforeEach(() => { getChartControlPanelRegistry().registerValue('table', defaultTableConfig); + getChartControlPanelRegistry().registerValue( + 'switch-target', + switchTargetConfig, + ); jest.clearAllMocks(); // Default: feature disabled mockIsFeatureEnabled.mockReturnValue(false); @@ -106,6 +166,7 @@ describe('ControlPanelsContainer', () => { afterEach(() => { getChartControlPanelRegistry().remove('table'); + getChartControlPanelRegistry().remove('switch-target'); jest.clearAllMocks(); }); @@ -623,4 +684,95 @@ describe('ControlPanelsContainer', () => { getChartControlPanelRegistry().remove('bar'); getChartControlPanelRegistry().remove('pie'); }); + + function createStoreBackedExplore() { + const datasource = { + id: 1, + type: DatasourceType.Table, + columns: [{ column_name: 'name' }, { column_name: 'gender' }], + metrics: [{ metric_name: 'count' }], + verbose_map: {}, + column_formats: {}, + }; + const baseState = { datasource }; + const formData = { + viz_type: 'table', + datasource: '1__table', + groupby: ['name', 'gender'], + metrics: ['count'], + all_columns: ['name'], + row_limit: 100, + }; + const controls = getControlsState( + baseState, + formData as unknown as Parameters<typeof getControlsState>[1], + ); + // Control states carry mapStateToProps functions, so the default + // serializability check would only add noise. + return configureStore({ + reducer: reducerIndex, + preloadedState: { + explore: { + ...baseState, + controls, + form_data: getFormDataFromControls( + controls as ControlPanelsContainerProps['controls'], + ), + }, + } as never, + middleware: getDefaultMiddleware => + getDefaultMiddleware({ + serializableCheck: false, + immutableCheck: false, + }), + devTools: false, + }); + } + + test('switching viz_type re-renders the control sections of the new viz', async () => { + const store = createStoreBackedExplore(); + render(<StoreBackedControlPanelsContainer />, { store }); + + expect( + await screen.findByRole('button', { name: /group by/i }), + ).toBeInTheDocument(); + expect(screen.queryByText(/series limit/i)).not.toBeInTheDocument(); + + act(() => { + store.dispatch(setControlValue('viz_type', 'switch-target')); + }); + + // `limit` exists only in the target viz, so its control is rendered from + // the control state the reducer built for that viz. + expect( + (await screen.findAllByText(/series limit/i)).length, + ).toBeGreaterThan(0); + expect( + screen.queryByRole('button', { name: /group by/i }), + ).not.toBeInTheDocument(); + }); + + test('switching viz_type carries shared metrics and groupby over and drops controls the new viz lacks', async () => { + const store = createStoreBackedExplore(); + render(<StoreBackedControlPanelsContainer />, { store }); + await screen.findByRole('button', { name: /group by/i }); + + act(() => { + store.dispatch(setControlValue('viz_type', 'switch-target')); + }); + await screen.findAllByText(/series limit/i); + + const { form_data: formData, controls } = ( + store.getState() as ExplorePageState + ).explore; + expect(formData.viz_type).toBe('switch-target'); + expect(formData.groupby).toEqual(['name', 'gender']); + expect(formData.metrics).toEqual(['count']); + expect(formData.standardizedFormData.controls).toEqual({ + metrics: ['count'], + columns: ['name', 'gender'], + }); + expect(controls.all_columns).toBeUndefined(); + expect(formData.all_columns).toBeUndefined(); + }); }); diff --git a/superset-frontend/src/explore/controlUtils/getSectionsToRender.test.ts b/superset-frontend/src/explore/controlUtils/getSectionsToRender.test.ts new file mode 100644 index 00000000000..fba9e0af1f2 --- /dev/null +++ b/superset-frontend/src/explore/controlUtils/getSectionsToRender.test.ts @@ -0,0 +1,121 @@ +/** + * 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 { + DatasourceType, + getChartControlPanelRegistry, +} from '@superset-ui/core'; +import { ControlPanelConfig } from '@superset-ui/chart-controls'; +import { getSectionsToRender } from './getSectionsToRender'; + +const VIZ_TYPE = 'sections-to-render-test-viz'; +const SECTION_LABEL = 'Time section under test'; + +type ExpandedRow = Array<string | { name?: string } | null | undefined>; + +const controlConfig: ControlPanelConfig = { + controlPanelSections: [ + { + label: SECTION_LABEL, + expanded: true, + controlSetRows: [ + ['granularity'], + ['granularity_sqla', 'time_grain_sqla'], + ['metrics'], + ], + }, + ], +}; + +function namesInSection(datasourceType: DatasourceType): string[] { + const section = getSectionsToRender(VIZ_TYPE, datasourceType).find( + item => item?.label === SECTION_LABEL, + ); + if (!section) { + throw new Error(`Section "${SECTION_LABEL}" was not rendered`); + } + return (section.controlSetRows as ExpandedRow[]) + .flat() + .map(item => (typeof item === 'string' ? item : item?.name)) + .filter((name): name is string => Boolean(name)); +} + +beforeEach(() => { + getChartControlPanelRegistry().registerValue(VIZ_TYPE, controlConfig); +}); + +afterEach(() => { + getChartControlPanelRegistry().remove(VIZ_TYPE); +}); + +test.each([ + DatasourceType.Table, + DatasourceType.Query, + DatasourceType.SemanticView, +])( + 'hides the legacy granularity control and keeps the sqla time controls for %s datasources', + datasourceType => { + const names = namesInSection(datasourceType); + + expect(names).not.toContain('granularity'); + expect(names).toEqual( + expect.arrayContaining(['granularity_sqla', 'time_grain_sqla']), + ); + }, +); + +test.each([ + DatasourceType.Dataset, + DatasourceType.SlTable, + DatasourceType.SavedQuery, +])( + 'hides granularity_sqla and time_grain_sqla and keeps granularity for %s datasources', + datasourceType => { + const names = namesInSection(datasourceType); + + expect(names).toContain('granularity'); + expect(names).not.toContain('granularity_sqla'); + expect(names).not.toContain('time_grain_sqla'); + }, +); + +test('keeps controls that are not datasource specific for every datasource type', () => { + [DatasourceType.Table, DatasourceType.Dataset].forEach(datasourceType => { + expect(namesInSection(datasourceType)).toContain('metrics'); + }); +}); + +test('always leads with the datasource and viz type section', () => { + const [first] = getSectionsToRender(VIZ_TYPE, DatasourceType.Table); + const rows = (first?.controlSetRows ?? []) as ExpandedRow[]; + const names = rows + .flat() + .map(item => (typeof item === 'string' ? item : item?.name)); + + expect(names).toEqual(expect.arrayContaining(['datasource', 'viz_type'])); +}); + +test('returns only the default sections for an unregistered viz type', () => { + const sections = getSectionsToRender( + 'unregistered-viz-type', + DatasourceType.Table, + ); + + expect(sections.map(section => section?.label)).not.toContain(SECTION_LABEL); + expect(sections.length).toBeGreaterThan(0); +}); diff --git a/superset-frontend/src/explore/reducers/exploreReducer.test.ts b/superset-frontend/src/explore/reducers/exploreReducer.test.ts index 59611707364..9bdd03de970 100644 --- a/superset-frontend/src/explore/reducers/exploreReducer.test.ts +++ b/superset-frontend/src/explore/reducers/exploreReducer.test.ts @@ -18,7 +18,14 @@ */ import { QueryFormData } from '@superset-ui/core'; -import { sections, CustomControlItem } from '@superset-ui/chart-controls'; +import { + ControlConfig, + ControlPanelState, + ControlState, + CustomControlItem, + sections, + sharedControls, +} from '@superset-ui/chart-controls'; import { getControlStateFromControlConfig } from 'src/explore/controlUtils'; import exploreReducer, { ExploreState } from './exploreReducer'; import { @@ -168,3 +175,202 @@ test('SET_FIELD_VALUE clears the custom-shift date error when time_compare leave ); expect(afterSwitch.controls.start_date_offset.validationErrors).toEqual([]); }); + +type ReducerAction = Parameters<typeof exploreReducer>[1]; + +type MirrorControlState = ControlState & { mirroredValue?: unknown }; + +// A control whose derived props depend on another control's current value. +const mirrorOf = (sourceControl: string): ControlConfig => + ({ + type: 'TextControl', + mapStateToProps: (state: ControlPanelState) => ({ + mirroredValue: state.controls?.[sourceControl]?.value, + }), + }) as unknown as ControlConfig; + +const buildControl = ( + config: ControlConfig, + formData: QueryFormData, + value: unknown, + controls: Record<string, ControlState> = {}, +) => + getControlStateFromControlConfig( + config, + { controls, form_data: formData }, + value as never, + )!; + +test('SET_FIELD_VALUE re-derives controls listed in `rerender` from the updated control value', () => { + const form_data = { + matrixify_mode_rows: 'disabled', + matrixify_mode_columns: 'disabled', + } as unknown as QueryFormData; + const initialState: ExploreState = { + form_data, + controls: { + matrixify_mode_rows: buildControl( + sharedControls.matrixify_mode_rows as unknown as ControlConfig, + form_data, + 'disabled', + ), + matrixify_mode_columns: buildControl( + sharedControls.matrixify_mode_columns as unknown as ControlConfig, + form_data, + 'disabled', + ), + matrixify_dimension_rows: buildControl( + sharedControls.matrixify_dimension_rows as unknown as ControlConfig, + form_data, + { dimension: '', values: [] }, + ), + }, + }; + const metricsOption = (state: ExploreState) => + ( + state.controls.matrixify_mode_columns.options as Array<{ + value: string; + disabled?: boolean; + }> + ).find(option => option.value === 'metrics'); + + // Columns offer "metrics" while the rows axis is not using it. + expect(metricsOption(initialState)?.disabled).toBe(false); + + const newState = exploreReducer( + initialState, + setControlValue('matrixify_mode_rows', 'metrics') as ReducerAction, + ); + + // The rows axis now owns "metrics", so the columns control must be + // re-derived without the user touching it. + expect(metricsOption(newState)?.disabled).toBe(true); + expect(newState.controls.matrixify_mode_columns.value).toBe('disabled'); +}); + +test('SET_FIELD_VALUE leaves controls that are not listed in `rerender` stale', () => { + const form_data = { source: 'a' } as unknown as QueryFormData; + const plain = { type: 'TextControl' } as unknown as ControlConfig; + const buildState = (rerender: string[]): ExploreState => { + const source = buildControl( + { ...plain, rerender } as unknown as ControlConfig, + form_data, + 'a', + ); + const controls = { source }; + return { + form_data, + controls: { + source, + listed: buildControl(mirrorOf('source'), form_data, null, controls), + unlisted: buildControl(mirrorOf('source'), form_data, null, controls), + }, + }; + }; + const mirrored = (state: ExploreState, name: string) => + (state.controls[name] as MirrorControlState).mirroredValue; + + const initialState = buildState(['listed']); + expect(mirrored(initialState, 'listed')).toBe('a'); + expect(mirrored(initialState, 'unlisted')).toBe('a'); + + const newState = exploreReducer( + initialState, + setControlValue('source', 'b') as ReducerAction, + ); + + expect(mirrored(newState, 'listed')).toBe('b'); + expect(mirrored(newState, 'unlisted')).toBe('a'); +}); + +test('SET_FIELD_VALUE only re-validates controls that declare the changed control in `validationDependencies`', () => { + const STALE_ERROR = 'Driven by the changed control'; + const requiresOk = (declaresDependency: boolean): ControlConfig => + ({ + type: 'TextControl', + mapStateToProps: (state: ControlPanelState) => ({ + externalValidationErrors: + state.form_data.source === 'bad' ? [STALE_ERROR] : [], + }), + ...(declaresDependency && { validationDependencies: ['source'] }), + }) as unknown as ControlConfig; + + const form_data = { source: 'bad' } as unknown as QueryFormData; + const initialState: ExploreState = { + form_data, + controls: { + source: buildControl( + { type: 'TextControl' } as unknown as ControlConfig, + form_data, + 'bad', + ), + declared: buildControl(requiresOk(true), form_data, 'x'), + undeclared: buildControl(requiresOk(false), form_data, 'x'), + }, + }; + + // Both dependents start out in error. + expect(initialState.controls.declared.validationErrors).toEqual([ + STALE_ERROR, + ]); + expect(initialState.controls.undeclared.validationErrors).toEqual([ + STALE_ERROR, + ]); + + const newState = exploreReducer( + initialState, + setControlValue('source', 'good') as ReducerAction, + ); + + // The declared dependent re-validates against the new value; the other one + // keeps the error because nothing tells the reducer to re-run it. + expect(newState.controls.declared.validationErrors).toEqual([]); + expect(newState.controls.undeclared.validationErrors).toEqual([STALE_ERROR]); +}); + +test('SET_FIELD_VALUE raises a dependent control error when the changed control makes it invalid', () => { + const REQUIRED_DATE_ERROR = 'A date is required when using custom date shift'; + const timeComparisonSection = sections.timeComparisonControls({ + multi: false, + showCalculationType: false, + showFullChoices: false, + }); + const timeCompareConfig = ( + timeComparisonSection.controlSetRows[0][0] as CustomControlItem + ).config; + const startDateOffsetConfig = ( + timeComparisonSection.controlSetRows[1][0] as CustomControlItem + ).config; + const form_data = { + time_compare: '1 week ago', + start_date_offset: '', + } as unknown as QueryFormData; + const controlPanelState = { controls: {}, form_data }; + const initialState: ExploreState = { + form_data, + controls: { + time_compare: getControlStateFromControlConfig( + timeCompareConfig, + controlPanelState, + '1 week ago', + )!, + start_date_offset: getControlStateFromControlConfig( + startDateOffsetConfig, + controlPanelState, + '', + )!, + }, + }; + + // An empty start date is fine until the shift becomes "custom". + expect(initialState.controls.start_date_offset.validationErrors).toEqual([]); + + const newState = exploreReducer( + initialState, + setControlValue('time_compare', 'custom') as ReducerAction, + ); + + expect(newState.controls.start_date_offset.validationErrors).toEqual([ + REQUIRED_DATE_ERROR, + ]); +});
