You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by jl...@apache.org on 2026/08/28 16:26:35 UTC
(superset) branch fix-hidden-section-stash-drops-saved-controls-4 created (now 903ae7ff433)
This is an automated email from the ASF dual-hosted git repository.
sadpandajoe pushed a change to branch fix-hidden-section-stash-drops-saved-controls-4
in repository https://gitbox.apache.org/repos/asf/superset.git
at 903ae7ff433 fix(explore): don't drop stashed control values from saved chart params
This branch includes the following new commits:
new 903ae7ff433 fix(explore): don't drop stashed control values from saved chart params
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
(superset) 01/01: fix(explore): don't drop stashed control values from saved chart params
Posted by jl...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
sadpandajoe pushed a commit to branch fix-hidden-section-stash-drops-saved-controls-4
in repository https://gitbox.apache.org/repos/asf/superset.git
commit 903ae7ff433335ac4ebcfb0a5997977a803d9790
Author: sadpandajoe <jc...@gmail.com>
AuthorDate: Fri Aug 28 16:26:13 2026 +0000
fix(explore): don't drop stashed control values from saved chart params
When a control-panel section becomes hidden (e.g. Advanced analytics'
time-comparison controls, hidden once the x-axis column loses its
temporal type), StashFormDataContainer moves that section's values out
of form_data so they stop affecting the live query. Saving the chart
while the section was hidden built the save payload directly from that
same stashed form_data, so the hidden keys (e.g. time_compare and
comparison_type) were permanently deleted from the saved chart with no
warning.
Merge the stashed values back in when building the save payload in
updateSlice/createSlice, so hiding a section no longer destroys
previously-saved configuration; the values still don't affect the live
query, since that path is unaffected by this change.
Co-Authored-By: Claude Sonnet 5 <no...@anthropic.com>
---
.../src/explore/actions/saveModalActions.test.ts | 86 +++++++++++++++++++---
.../src/explore/actions/saveModalActions.ts | 17 ++++-
2 files changed, 92 insertions(+), 11 deletions(-)
diff --git a/superset-frontend/src/explore/actions/saveModalActions.test.ts b/superset-frontend/src/explore/actions/saveModalActions.test.ts
index b60312ce52b..0ac39e098d7 100644
--- a/superset-frontend/src/explore/actions/saveModalActions.test.ts
+++ b/superset-frontend/src/explore/actions/saveModalActions.test.ts
@@ -250,6 +250,50 @@ test('matches normalization metadata against finalized payload filters', async (
]);
});
+test('updateSlice keeps values stashed by a hidden control section in the saved params', async () => {
+ fetchMock.put(updateSliceEndpoint, sliceResponsePayload, {
+ name: updateSliceEndpoint,
+ });
+ const dispatch = jest.fn();
+ const getState = () => ({
+ explore: {
+ // The Advanced analytics section is hidden (e.g. the x-axis column
+ // lost is_dttm), so StashFormDataContainer removed time_compare and
+ // comparison_type from form_data...
+ form_data: {
+ datasource: `${datasourceId}__${datasourceType}`,
+ viz_type: vizType,
+ },
+ // ...and holds their previously-saved values here until the section
+ // becomes visible again.
+ hiddenFormData: {
+ time_compare: ['1 year ago'],
+ comparison_type: 'values',
+ },
+ },
+ });
+
+ await updateSlice(
+ {
+ ...sliceResponsePayload,
+ slice_id: sliceId,
+ form_data: {
+ ...formData,
+ time_compare: ['1 year ago'],
+ comparison_type: 'values',
+ },
+ } as never,
+ sliceName,
+ [],
+ )(dispatch, getState);
+
+ const request = fetchMock.callHistory.lastCall(updateSliceEndpoint);
+ const body = JSON.parse(request?.options.body as string);
+ const savedParams = JSON.parse(body.params);
+ expect(savedParams.time_compare).toEqual(['1 year ago']);
+ expect(savedParams.comparison_type).toEqual('values');
+});
+
/**
* Tests updateSlice action
*/
@@ -377,6 +421,33 @@ test('createSlice handles success', async () => {
expect(slice).toEqual(sliceResponsePayload);
});
+test('createSlice keeps values stashed by a hidden control section in the saved params', async () => {
+ fetchMock.post(createSliceEndpoint, sliceResponsePayload, {
+ name: createSliceEndpoint,
+ });
+ const dispatch = jest.fn();
+ const getState = () => ({
+ explore: {
+ form_data: {
+ datasource: `${datasourceId}__${datasourceType}`,
+ viz_type: vizType,
+ },
+ hiddenFormData: {
+ time_compare: ['1 year ago'],
+ comparison_type: 'values',
+ },
+ },
+ });
+
+ await createSlice(sliceName, [])(dispatch, getState as never);
+
+ const request = fetchMock.callHistory.lastCall(createSliceEndpoint);
+ const body = JSON.parse(request?.options.body as string);
+ const savedParams = JSON.parse(body.params);
+ expect(savedParams.time_compare).toEqual(['1 year ago']);
+ expect(savedParams.comparison_type).toEqual('values');
+});
+
test('createSlice handles failure', async () => {
fetchMock.post(createSliceEndpoint, { throws: sampleError });
@@ -885,7 +956,7 @@ describe('getSlicePayload', () => {
});
});
-test('existing-chart overwrite covers stash-removed keys as drop transitions', async () => {
+test('existing-chart overwrite restores a stash-held value instead of dropping it', async () => {
mockedIsFeatureEnabled.mockReturnValue(true);
fetchMock.put(updateSliceEndpoint, sliceResponsePayload, {
name: updateSliceEndpoint,
@@ -926,14 +997,11 @@ test('existing-chart overwrite covers stash-removed keys as drop transitions', a
const request = fetchMock.callHistory.lastCall(updateSliceEndpoint);
const body = JSON.parse(request?.options.body as string);
- expect(body.normalization_changes).toEqual([
- {
- control: 'order_desc',
- from_present: true,
- from_value: true,
- to_present: false,
- },
- ]);
+ const savedParams = JSON.parse(body.params);
+ // The stashed value is written back into the saved params, so there is no
+ // drop for the normalization tracker to report.
+ expect(savedParams.order_desc).toBe(true);
+ expect(body.normalization_changes).toBeUndefined();
});
test('a stashed value the user changed before hiding is not covered', async () => {
diff --git a/superset-frontend/src/explore/actions/saveModalActions.ts b/superset-frontend/src/explore/actions/saveModalActions.ts
index db40fea1f3c..380d9634db3 100644
--- a/superset-frontend/src/explore/actions/saveModalActions.ts
+++ b/superset-frontend/src/explore/actions/saveModalActions.ts
@@ -263,8 +263,14 @@ export const updateSlice =
) => {
const { slice_id: sliceId, editors, form_data: formDataFromSlice } = slice;
const initialState = getState();
+ // Controls hidden by a form-section's visibility rule are stashed out of
+ // form_data so they don't affect the live query, but that hiding must not
+ // permanently delete the user's saved configuration when they save.
const formData = JSON.parse(
- JSON.stringify(initialState.explore?.form_data ?? {}),
+ JSON.stringify({
+ ...initialState.explore?.hiddenFormData,
+ ...initialState.explore?.form_data,
+ }),
) as QueryFormData;
const tracking = initialState.versionHistory?.chartNormalization;
const saveAttemptId = nanoid();
@@ -346,7 +352,14 @@ export const createSlice =
},
) =>
async (dispatch: Dispatch, getState: () => Partial<QueryFormData>) => {
- const formData = getState().explore?.form_data;
+ const exploreState = getState().explore;
+ // See the comment in updateSlice: stashed values from a hidden control
+ // section must not be dropped from the saved chart just because the
+ // section is currently hidden.
+ const formData = {
+ ...exploreState?.hiddenFormData,
+ ...exploreState?.form_data,
+ };
try {
const response = await SupersetClient.post({
endpoint: `/api/v1/chart/`,