fix: add a fallback to chart state callback (#36327)

superset-frontend/src/dashboard/components/gridComponents/Chart/Chart.jsx

@@ -114,6 +114,33 @@ const SliceContainer = styled.div`
 const EMPTY_OBJECT = {};
 const EMPTY_ARRAY = [];
 
+// Helper function to get chart state with fallback
+const getChartStateWithFallback = (chartState, formData, vizType) => {
+  if (!hasChartStateConverter(vizType)) {
+    return null;
+  }
+
+  return (
+    chartState?.state || formData.table_state || formData.pivot_table_state
+  );
+};
+
+// Helper function to create own state with chart state conversion
+const createOwnStateWithChartState = (baseOwnState, chartState, vizType) => {
+  const state = getChartStateWithFallback(chartState, {}, vizType);
+
+  if (!state) {
+    return baseOwnState;
+  }
+
+  const convertedState = convertChartStateToOwnState(vizType, state);
+  return {
+    ...baseOwnState,
+    ...convertedState,
+    chartState: state,
+  };
+};
+
 const Chart = props => {
   const dispatch = useDispatch();
   const descriptionRef = useRef(null);
@@ -383,16 +410,11 @@ const Chart = props => {
 
   const ownState = useMemo(() => {
     const baseOwnState = dataMask[props.id]?.ownState || EMPTY_OBJECT;
-
-    if (hasChartStateConverter(slice.viz_type) && chartState?.state) {
-      return {
-        ...baseOwnState,
-        ...convertChartStateToOwnState(slice.viz_type, chartState.state),
-        chartState: chartState.state,
-      };
-    }
-
-    return baseOwnState;
+    return createOwnStateWithChartState(
+      baseOwnState,
+      chartState,
+      slice.viz_type,
+    );
   }, [
     dataMask[props.id]?.ownState,
     props.id,
@@ -486,19 +508,19 @@ const Chart = props => {
         const safeChartName = chartName.replace(/[^a-zA-Z0-9_-]/g, '_');
         filename = `${safeChartName}${timestamp}.csv`;
       }
-      let ownState = dataMask[props.id]?.ownState || {};
+      const baseOwnState = dataMask[props.id]?.ownState || {};
+      const state = getChartStateWithFallback(
+        chartState,
+        formData,
+        slice.viz_type,
+      );
 
-      // Convert chart-specific state to backend format using registered converter
-      if (hasChartStateConverter(slice.viz_type) && chartState?.state) {
-        const convertedState = convertChartStateToOwnState(
-          slice.viz_type,
-          chartState.state,
-        );
-        ownState = {
-          ...ownState,
-          ...convertedState,
-        };
-      }
+      const ownState = state
+        ? {
+            ...baseOwnState,
+            ...convertChartStateToOwnState(slice.viz_type, state),
+          }
+        : baseOwnState;
 
       exportChart({
         formData: exportFormData,
@@ -671,7 +693,17 @@ const Chart = props => {
           formData={formData}
           labelsColor={labelsColor}
           labelsColorMap={labelsColorMap}
-          ownState={ownState}
+          ownState={createOwnStateWithChartState(
+            dataMask[props.id]?.ownState || EMPTY_OBJECT,
+            {
+              state: getChartStateWithFallback(
+                chartState,
+                formData,
+                slice.viz_type,
+              ),
+            },
+            slice.viz_type,
+          )}
           filterState={dataMask[props.id]?.filterState}
           queriesResponse={chart.queriesResponse}
           timeout={timeout}

superset-frontend/src/dashboard/components/gridComponents/Chart/Chart.test.jsx

@@ -21,6 +21,7 @@ import { FeatureFlag, VizType } from '@superset-ui/core';
 import * as redux from 'redux';
 
 import * as exploreUtils from 'src/explore/exploreUtils';
+import * as chartStateConverter from '../../../util/chartStateConverter';
 import { sliceEntitiesForChart as sliceEntities } from 'spec/fixtures/mockSliceEntities';
 import mockDatasource from 'spec/fixtures/mockDatasource';
 import chartQueries, {
@@ -116,6 +117,10 @@ beforeAll(() => {
   }));
 });
 
+afterEach(() => {
+  jest.clearAllMocks();
+});
+
 test('should render a SliceHeader', () => {
   const { getByTestId, container } = setup();
   expect(getByTestId('slice-header')).toBeInTheDocument();
@@ -297,3 +302,122 @@ test('should re-render when cacheBusterProp changes', () => {
   rerender(<Chart {...props} isComponentVisible cacheBusterProp="v2" />);
   expect(getByTestId('chart-container')).toBeInTheDocument();
 });
+
+test('should handle chart state conversion when converter exists', () => {
+  const mockChartState = { sortColumn: 'column1', sortOrder: 'asc' };
+  const mockConvertedState = { sort: [{ columnId: 'column1', order: 'asc' }] };
+
+  jest
+    .spyOn(chartStateConverter, 'hasChartStateConverter')
+    .mockReturnValue(true);
+  jest
+    .spyOn(chartStateConverter, 'convertChartStateToOwnState')
+    .mockReturnValue(mockConvertedState);
+
+  const { getByTestId } = setup(
+    {},
+    {
+      ...defaultState,
+      dashboardState: {
+        ...defaultState.dashboardState,
+        chartStates: {
+          [queryId]: { state: mockChartState },
+        },
+      },
+    },
+  );
+
+  expect(getByTestId('chart-container')).toBeInTheDocument();
+  expect(chartStateConverter.hasChartStateConverter).toHaveBeenCalledWith(
+    VizType.Table,
+  );
+});
+
+test('should fallback to formData state when runtime state not available', () => {
+  const mockFormDataState = { sortColumn: 'column2', sortOrder: 'desc' };
+  const mockConvertedState = { sort: [{ columnId: 'column2', order: 'desc' }] };
+
+  jest
+    .spyOn(chartStateConverter, 'hasChartStateConverter')
+    .mockReturnValue(true);
+  jest
+    .spyOn(chartStateConverter, 'convertChartStateToOwnState')
+    .mockReturnValue(mockConvertedState);
+
+  const { getByTestId } = setup(
+    {},
+    {
+      ...defaultState,
+      charts: {
+        ...defaultState.charts,
+        [queryId]: {
+          ...defaultState.charts[queryId],
+          form_data: {
+            ...defaultState.charts[queryId].form_data,
+            table_state: mockFormDataState,
+          },
+        },
+      },
+    },
+  );
+
+  expect(getByTestId('chart-container')).toBeInTheDocument();
+});
+
+test('should handle chart state when no converter exists', () => {
+  jest
+    .spyOn(chartStateConverter, 'hasChartStateConverter')
+    .mockReturnValue(false);
+  jest.spyOn(chartStateConverter, 'convertChartStateToOwnState');
+
+  const { getByTestId } = setup(
+    {},
+    {
+      ...defaultState,
+      dashboardState: {
+        ...defaultState.dashboardState,
+        chartStates: {
+          [queryId]: { state: { someState: 'value' } },
+        },
+      },
+    },
+  );
+
+  expect(getByTestId('chart-container')).toBeInTheDocument();
+  expect(
+    chartStateConverter.convertChartStateToOwnState,
+  ).not.toHaveBeenCalled();
+});
+
+test('should merge base ownState with converted chart state', () => {
+  const baseOwnState = { existingProp: 'value' };
+  const mockChartState = { sortColumn: 'column1', sortOrder: 'asc' };
+  const mockConvertedState = { sort: [{ columnId: 'column1', order: 'asc' }] };
+
+  jest
+    .spyOn(chartStateConverter, 'hasChartStateConverter')
+    .mockReturnValue(true);
+  jest
+    .spyOn(chartStateConverter, 'convertChartStateToOwnState')
+    .mockReturnValue(mockConvertedState);
+
+  const { getByTestId } = setup(
+    {},
+    {
+      ...defaultState,
+      dataMask: {
+        [queryId]: {
+          ownState: baseOwnState,
+        },
+      },
+      dashboardState: {
+        ...defaultState.dashboardState,
+        chartStates: {
+          [queryId]: { state: mockChartState },
+        },
+      },
+    },
+  );
+
+  expect(getByTestId('chart-container')).toBeInTheDocument();
+});