fix: Table time comparison breaking after form data update (#29525)

apache · Jul 9, 2024 · 3d06651 · 3d06651
1 parent b4560d4
commit 3d06651
Show file tree

Hide file tree

Showing 5 changed files with 70 additions and 4 deletions.
diff --git a/superset-frontend/packages/superset-ui-chart-controls/src/sections/timeComparison.tsx b/superset-frontend/packages/superset-ui-chart-controls/src/sections/timeComparison.tsx
@@ -118,7 +118,7 @@ export const timeComparisonControls: ({
               'difference between the main time series and each time shift; ' +
               'as the percentage change; or as the ratio between series and time shifts.',
           ),
-          visibility: () => Boolean(showCalculationType),
+          hidden: () => Boolean(showCalculationType),
         },
       },
     ],

diff --git a/superset-frontend/packages/superset-ui-chart-controls/src/types.ts b/superset-frontend/packages/superset-ui-chart-controls/src/types.ts
@@ -258,6 +258,9 @@ export interface BaseControlConfig<
     props: ControlPanelsContainerProps,
     controlData: AnyDict,
   ) => boolean;
+  hidden?:
+    | boolean
+    | ((props: ControlPanelsContainerProps, controlData: AnyDict) => boolean);
 }
 
 export interface ControlValueValidator<

diff --git a/superset-frontend/src/explore/components/ControlPanelsContainer.test.tsx b/superset-frontend/src/explore/components/ControlPanelsContainer.test.tsx
@@ -203,4 +203,60 @@ describe('ControlPanelsContainer', () => {
       'percent_metrics',
     );
   });
+
+  test('hidden state of controls is correctly applied', async () => {
+    getChartControlPanelRegistry().registerValue('table', {
+      controlPanelSections: [
+        {
+          label: t('Time Comparison'),
+          expanded: true,
+          controlSetRows: [
+            [
+              {
+                name: 'time_compare',
+                config: {
+                  type: 'SelectControl',
+                  freeForm: true,
+                  label: t('Time shift'),
+                  choices: [],
+                },
+              },
+            ],
+            [
+              {
+                name: 'start_date_offset',
+                config: {
+                  type: 'SelectControl',
+                  choices: [],
+                  label: t('Shift start date'),
+                  hidden: true,
+                },
+              },
+            ],
+            [
+              {
+                name: 'comparison_type',
+                config: {
+                  type: 'SelectControl',
+                  label: t('Calculation type'),
+                  default: 'values',
+                  choices: [],
+                  hidden: () => true,
+                },
+              },
+            ],
+          ],
+        },
+      ],
+    });
+    render(<ControlPanelsContainer {...getDefaultProps()} />, {
+      useRedux: true,
+    });
+
+    expect(screen.getByText('Time shift')).toBeInTheDocument();
+    expect(screen.getByText('Shift start date')).toBeInTheDocument();
+    expect(screen.getByText('Calculation type')).toBeInTheDocument();
+    expect(screen.getByText('Shift start date')).not.toBeVisible();
+    expect(screen.getByText('Calculation type')).not.toBeVisible();
+  });
 });
diff --git a/superset-frontend/src/explore/components/ControlPanelsContainer.tsx b/superset-frontend/src/explore/components/ControlPanelsContainer.tsx
@@ -448,13 +448,13 @@ export const ControlPanelsContainer = (props: ControlPanelsContainerProps) => {
 
   const renderControl = ({ name, config }: CustomControlItem) => {
     const { controls, chart, exploreState } = props;
-    const { visibility } = config;
+    const { visibility, hidden, ...restConfig } = config;
 
     // If the control item is not an object, we have to look up the control data from
     // the centralized controls file.
     // When it is an object we read control data straight from `config` instead
     const controlData = {
-      ...config,
+      ...restConfig,
       ...controls[name],
       ...(shouldRecalculateControlState({ name, config })
         ? config?.mapStateToProps?.(exploreState, controls[name], chart)
@@ -476,6 +476,11 @@ export const ControlPanelsContainer = (props: ControlPanelsContainerProps) => {
       ? visibility.call(config, props, controlData)
       : undefined;
 
+    const isHidden =
+      typeof hidden === 'function'
+        ? hidden.call(config, props, controlData)
+        : hidden;
+
     const label =
       typeof baseLabel === 'function'
         ? baseLabel(exploreState, controls[name], chart)
@@ -536,6 +541,7 @@ export const ControlPanelsContainer = (props: ControlPanelsContainerProps) => {
           validationErrors={validationErrors}
           actions={props.actions}
           isVisible={isVisible}
+          hidden={isHidden}
           {...restProps}
         />
       </StashFormDataContainer>

diff --git a/superset-frontend/src/explore/components/DatasourcePanel/index.tsx b/superset-frontend/src/explore/components/DatasourcePanel/index.tsx
@@ -50,7 +50,7 @@ import { DndItemType } from '../DndItemType';
 import { DndItemValue } from './types';
 import { DropzoneContext } from '../ExploreContainer';
 
-interface DatasourceControl extends ControlConfig {
+interface DatasourceControl extends Omit<ControlConfig, 'hidden'> {
   datasource?: IDatasource;
 }
 export interface IDatasource {
@@ -389,6 +389,7 @@ export default function DataSourcePanel({
           formData={formData}
         />
       )}
+      {/* @ts-ignore */}
       <Control {...datasourceControl} name="datasource" actions={actions} />
       {datasource.id != null && mainBody}
     </DatasourceContainer>