This is an automated email from the ASF dual-hosted git repository.
justinpark pushed a commit to branch feat--add-explore-dashboard-to-core-api-6.2
in repository https://gitbox.apache.org/repos/asf/superset.git
The following commit(s) were added to
refs/heads/feat--add-explore-dashboard-to-core-api-6.2 by this push:
new 3c3f99fe4a7 feat(extensions): validate control name/value in Explore
core API's setControlValues
3c3f99fe4a7 is described below
commit 3c3f99fe4a76bf93a920300f8d772d7b82f402a0
Author: justinpark <[email protected]>
AuthorDate: Wed Sep 30 16:35:14 2026 -0700
feat(extensions): validate control name/value in Explore core API's
setControlValues
Throws a descriptive error naming the valid controls for the current viz
type when a control name doesn't exist, and runs each control's own
validators against the given value before applying it. When an unknown
name matches a raw ECharts option not exposed as a discrete control, the
error points to the echart_options control instead. All entries are
validated before any are dispatched, so a rejected call never applies a
partial set of changes.
Co-Authored-By: Claude Sonnet 5 <[email protected]>
---
.../packages/superset-core/src/explore/index.ts | 10 +++
superset-frontend/src/core/explore/index.test.ts | 77 +++++++++++++++++-
superset-frontend/src/core/explore/index.ts | 95 ++++++++++++++++++++++
3 files changed, 181 insertions(+), 1 deletion(-)
diff --git a/superset-frontend/packages/superset-core/src/explore/index.ts
b/superset-frontend/packages/superset-core/src/explore/index.ts
index e2b6f9939c9..1da9bd60317 100644
--- a/superset-frontend/packages/superset-core/src/explore/index.ts
+++ b/superset-frontend/packages/superset-core/src/explore/index.ts
@@ -83,9 +83,19 @@ export declare function getControlValue(name: string):
unknown;
* `renderTrigger` (e.g. `echart_options`) re-render an instant preview in
* place — no save and no new data query required.
*
+ * Every entry is validated before any of them is applied, so a rejected
+ * call never applies a partial set of changes.
+ *
* @param values A map of control name to the value it should be set to.
* @returns Promise that resolves once the values have been applied.
*
+ * @throws If `values` names a control that doesn't exist on the chart's
+ * current viz type. When the name matches a setting only reachable through
+ * the raw ECharts override (e.g. `title`, `legend`, `tooltip`), the error
+ * points to `echart_options` instead.
+ * @throws If a value fails the target control's own validators (e.g. a
+ * non-numeric `row_limit`). The error includes each validator's message.
+ *
* @example
* ```typescript
* await explore.setControlValues({
diff --git a/superset-frontend/src/core/explore/index.test.ts
b/superset-frontend/src/core/explore/index.test.ts
index 0601f1e4a03..8906f276807 100644
--- a/superset-frontend/src/core/explore/index.test.ts
+++ b/superset-frontend/src/core/explore/index.test.ts
@@ -55,6 +55,10 @@ const mockGetState = store.getState as jest.Mock;
const mockDispatch = store.dispatch as jest.Mock;
const mockRequestChartDataResolved = requestChartDataResolved as jest.Mock;
+const mockRowLimitValidator = jest.fn((value: unknown) =>
+ Number.isInteger(value) ? false : 'is expected to be an integer',
+);
+
// Mirrors the current controls Explore would be querying with, matching the
// pre-normalization `form_data` slice used elsewhere in these tests.
const defaultControls = {
@@ -62,6 +66,15 @@ const defaultControls = {
viz_type: { value: 'echarts_timeseries_bar' },
};
+// A real viz type's `controls` slice (see getAllControlsState) has one
+// entry per control the control panel defines, which is what lets
+// setControlValues tell an unknown control name from a known one.
+const controlsWithEchartOptionsAndRowLimit = {
+ ...defaultControls,
+ echart_options: { value: '{}', validators: [] },
+ row_limit: { value: 10000, validators: [mockRowLimitValidator] },
+};
+
function activateExplore(overrides: Record<string, unknown> = {}) {
mockGetPage.mockReturnValue('explore');
mockGetState.mockReturnValue({
@@ -133,7 +146,7 @@ test('setControlValues throws when active but no chart is
loaded (no datasource)
});
test('setControlValues dispatches a programmatic setControlValue per entry',
async () => {
- activateExplore();
+ activateExplore({ controls: controlsWithEchartOptionsAndRowLimit });
await explore.setControlValues({
echart_options: '{ "title": "Revenue" }',
@@ -152,6 +165,68 @@ test('setControlValues dispatches a programmatic
setControlValue per entry', asy
expect(mockDispatch).toHaveBeenCalledTimes(2);
});
+test('setControlValues throws on an unknown control name, listing the valid
ones', async () => {
+ activateExplore();
+
+ // The valid-controls list must come from `controls` (every control the
+ // current viz type's panel defines), not from getControlValues()/form_data,
+ // which only reflects currently-applied values and can omit hidden or
+ // untouched controls.
+ await expect(
+ explore.setControlValues({ not_a_real_control: 1 }),
+ ).rejects.toThrow(
+ '"not_a_real_control" is not a valid control for the chart currently ' +
+ 'loaded in Explore. Valid controls for this chart type are: ' +
+ 'datasource, viz_type.',
+ );
+ expect(mockDispatch).not.toHaveBeenCalled();
+});
+
+test('setControlValues hints at echart_options for an unknown ECharts-shaped
name', async () => {
+ activateExplore({ controls: controlsWithEchartOptionsAndRowLimit });
+
+ await expect(
+ explore.setControlValues({ tooltip_trigger: 'item' }),
+ ).rejects.toThrow('"tooltip_trigger" is not a control on this chart');
+ await expect(
+ explore.setControlValues({ tooltip_trigger: 'item' }),
+ ).rejects.toThrow('echart_options');
+ expect(mockDispatch).not.toHaveBeenCalled();
+});
+
+test('setControlValues does not hint at echart_options when the chart has no
such control', async () => {
+ activateExplore({
+ controls: {
+ datasource: { value: '1__table' },
+ viz_type: { value: 'table' },
+ },
+ });
+
+ await expect(explore.setControlValues({ title: 'x' })).rejects.toThrow(
+ '"title" is not a valid control',
+ );
+});
+
+test("setControlValues throws when a value fails the control's own validator",
async () => {
+ activateExplore({ controls: controlsWithEchartOptionsAndRowLimit });
+
+ await expect(
+ explore.setControlValues({ row_limit: 'not-a-number' }),
+ ).rejects.toThrow(
+ 'Invalid value for control "row_limit": is expected to be an integer',
+ );
+ expect(mockDispatch).not.toHaveBeenCalled();
+});
+
+test('setControlValues validates every entry before dispatching any of them',
async () => {
+ activateExplore({ controls: controlsWithEchartOptionsAndRowLimit });
+
+ await expect(
+ explore.setControlValues({ row_limit: 100, not_a_real_control: 1 }),
+ ).rejects.toThrow('"not_a_real_control" is not a valid control');
+ expect(mockDispatch).not.toHaveBeenCalled();
+});
+
test('getQuery throws when Explore is not the active surface', async () => {
await expect(explore.getQuery()).rejects.toThrow(
'No chart is currently loaded in Explore',
diff --git a/superset-frontend/src/core/explore/index.ts
b/superset-frontend/src/core/explore/index.ts
index 8a8afc398da..d5705b814d7 100644
--- a/superset-frontend/src/core/explore/index.ts
+++ b/superset-frontend/src/core/explore/index.ts
@@ -22,6 +22,11 @@ import type {
JsonObject,
QueryFormData,
} from '@superset-ui/core';
+import type {
+ ControlState,
+ ControlStateMapping,
+ ControlValueValidator,
+} from '@superset-ui/chart-controls';
import { setControlValue } from 'src/explore/actions/exploreActions';
import { getVisibleFormDataFromControls } from 'src/explore/controlUtils';
import { requestChartDataResolved } from 'src/components/Chart/chartAction';
@@ -77,10 +82,100 @@ const requireFormData = (): QueryFormData => {
return formData;
};
+// Top-level ECharts option keys Superset never exposes as a discrete
+// control (see sharedControls.tsx's `echart_options`). A caller trying to
+// set one of these by name isn't naming a typo — they're reaching for a
+// setting that only exists through that JS-override control, so the error
+// should point there instead of just saying "unknown control".
+const ECHART_OPTIONS_HINT_KEYS = new Set([
+ 'title',
+ 'legend',
+ 'grid',
+ 'tooltip',
+ 'toolbox',
+ 'datazoom',
+ 'visualmap',
+ 'animation',
+ 'backgroundcolor',
+ 'textstyle',
+ 'graphic',
+ 'polar',
+ 'radiusaxis',
+ 'angleaxis',
+ 'radar',
+ 'geo',
+ 'parallel',
+ 'parallelaxis',
+ 'singleaxis',
+ 'timeline',
+ 'calendar',
+ 'dataset',
+ 'aria',
+ 'brush',
+]);
+
+const matchesEchartOptionsKey = (controlName: string): boolean => {
+ const [firstSegment] = controlName.split(/[._]/);
+ return (
+ ECHART_OPTIONS_HINT_KEYS.has(controlName.toLowerCase()) ||
+ ECHART_OPTIONS_HINT_KEYS.has(firstSegment.toLowerCase())
+ );
+};
+
+const buildUnknownControlMessage = (
+ controlName: string,
+ controls: ControlStateMapping,
+): string => {
+ if (controls.echart_options && matchesEchartOptionsKey(controlName)) {
+ return (
+ `"${controlName}" is not a control on this chart, but it looks like an
ECharts ` +
+ `option. Try setting it through the "echart_options" control instead,
e.g. ` +
+ `setControlValues({ echart_options: '{
"${controlName.split(/[._]/)[0]}": { ... } }' }).`
+ );
+ }
+ // getControlValues() only reflects form_data — the currently-applied
+ // values, missing controls that are hidden or untouched — so it can't
+ // stand in for the full set of controls this viz type supports. `controls`
+ // (unlike form_data) has one entry per control the control panel defines,
+ // so list those names directly instead of pointing at the wrong API.
+ const validNames = Object.keys(controls).sort().join(', ');
+ return (
+ `"${controlName}" is not a valid control for the chart currently loaded in
` +
+ `Explore. Valid controls for this chart type are: ${validNames}.`
+ );
+};
+
+const validateControlValue = (
+ controlName: string,
+ value: unknown,
+ control: ControlState,
+): void => {
+ const validators = (control.validators ?? []) as ControlValueValidator[];
+ const processedState = { ...control, value } as ControlState;
+ const errors = validators
+ .map(validator => validator.call(control, value, processedState))
+ .filter((error): error is string => typeof error === 'string' && !!error);
+ if (errors.length > 0) {
+ throw new Error(
+ `Invalid value for control "${controlName}": ${errors.join('; ')}`,
+ );
+ }
+};
+
const setControlValues: typeof exploreApi.setControlValues = async (
values: Record<string, unknown>,
) => {
requireFormData();
+ const controls = getExploreState().controls ?? ({} as ControlStateMapping);
+ // Validate every entry before dispatching any of them, so a bad entry
+ // later in the object can't leave earlier ones applied.
+ Object.entries(values).forEach(([controlName, value]) => {
+ const control = controls[controlName];
+ if (!control) {
+ throw new Error(buildUnknownControlMessage(controlName, controls));
+ }
+ validateControlValue(controlName, value, control);
+ });
Object.entries(values).forEach(([controlName, value]) => {
store.dispatch(
setControlValue(controlName, value, undefined, { programmatic: true }),
