fix(explore): hide advanced analytics for non temporal xaxis (#28312)

superset-frontend/packages/superset-ui-chart-controls/src/sections/advancedAnalytics.tsx

@@ -21,7 +21,7 @@ import { t, RollingType, ComparisonType } from '@superset-ui/core';
 
 import { ControlSubSectionHeader } from '../components/ControlSubSectionHeader';
 import { ControlPanelSectionConfig } from '../types';
-import { formatSelectOptions } from '../utils';
+import { formatSelectOptions, displayTimeRelatedControls } from '../utils';
 
 export const advancedAnalyticsControls: ControlPanelSectionConfig = {
   label: t('Advanced analytics'),
@@ -31,6 +31,7 @@ export const advancedAnalyticsControls: ControlPanelSectionConfig = {
       'that allow for advanced analytical post processing ' +
       'of query results',
   ),
+  visibility: displayTimeRelatedControls,
   controlSetRows: [
     [<ControlSubSectionHeader>{t('Rolling window')}</ControlSubSectionHeader>],
     [

superset-frontend/packages/superset-ui-chart-controls/src/sections/forecastInterval.tsx

@@ -22,6 +22,7 @@ import {
   t,
 } from '@superset-ui/core';
 import { ControlPanelSectionConfig } from '../types';
+import { displayTimeRelatedControls } from '../utils';
 
 export const FORECAST_DEFAULT_DATA = {
   forecastEnabled: false,
@@ -35,6 +36,7 @@ export const FORECAST_DEFAULT_DATA = {
 export const forecastIntervalControls: ControlPanelSectionConfig = {
   label: t('Predictive Analytics'),
   expanded: false,
+  visibility: displayTimeRelatedControls,
   controlSetRows: [
     [
       {

superset-frontend/packages/superset-ui-chart-controls/src/shared-controls/sharedControls.tsx

@@ -41,8 +41,6 @@ import {
   SequentialScheme,
   legacyValidateInteger,
   ComparisonType,
-  isAdhocColumn,
-  isPhysicalColumn,
   ensureIsArray,
   isDefined,
   NO_TIME_RANGE,
@@ -51,6 +49,7 @@ import {
 
 import {
   formatSelectOptions,
+  displayTimeRelatedControls,
   D3_FORMAT_OPTIONS,
   D3_FORMAT_DOCS,
   D3_TIME_FORMAT_OPTIONS,
@@ -62,7 +61,6 @@ import { DEFAULT_MAX_ROW, TIME_FILTER_LABELS } from '../constants';
 import {
   SharedControlConfig,
   Dataset,
-  ColumnMeta,
   ControlState,
   ControlPanelState,
 } from '../types';
@@ -203,23 +201,7 @@ const time_grain_sqla: SharedControlConfig<'SelectControl'> = {
   mapStateToProps: ({ datasource }) => ({
     choices: (datasource as Dataset)?.time_grain_sqla || [],
   }),
-  visibility: ({ controls }) => {
-    if (!controls?.x_axis) {
-      return true;
-    }
-
-    const xAxis = controls?.x_axis;
-    const xAxisValue = xAxis?.value;
-    if (isAdhocColumn(xAxisValue)) {
-      return true;
-    }
-    if (isPhysicalColumn(xAxisValue)) {
-      return !!(xAxis?.options ?? []).find(
-        (col: ColumnMeta) => col?.column_name === xAxisValue,
-      )?.is_dttm;
-    }
-    return false;
-  },
+  visibility: displayTimeRelatedControls,
 };
 
 const time_range: SharedControlConfig<'DateFilterControl'> = {

superset-frontend/packages/superset-ui-chart-controls/src/types.ts

@@ -376,6 +376,10 @@ export interface ControlPanelSectionConfig {
   expanded?: boolean;
   tabOverride?: TabOverride;
   controlSetRows: ControlSetRow[];
+  visibility?: (
+    props: ControlPanelsContainerProps,
+    controlData: AnyDict,
+  ) => boolean;
 }
 
 export interface StandardizedControls {

superset-frontend/packages/superset-ui-chart-controls/src/utils/displayTimeRelatedControls.ts

@@ -0,0 +1,40 @@
+/**
+ * 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 { isAdhocColumn, isPhysicalColumn } from '@superset-ui/core';
+import type { ColumnMeta, ControlPanelsContainerProps } from '../types';
+
+export default function displayTimeRelatedControls({
+  controls,
+}: ControlPanelsContainerProps) {
+  if (!controls?.x_axis) {
+    return true;
+  }
+
+  const xAxis = controls?.x_axis;
+  const xAxisValue = xAxis?.value;
+  if (isAdhocColumn(xAxisValue)) {
+    return true;
+  }
+  if (isPhysicalColumn(xAxisValue)) {
+    return !!(xAxis?.options ?? []).find(
+      (col: ColumnMeta) => col?.column_name === xAxisValue,
+    )?.is_dttm;
+  }
+  return false;
+}

superset-frontend/packages/superset-ui-chart-controls/src/utils/index.ts

@@ -26,3 +26,4 @@ export { default as columnChoices } from './columnChoices';
 export * from './defineSavedMetrics';
 export * from './getStandardizedControls';
 export * from './getTemporalColumns';
+export { default as displayTimeRelatedControls } from './displayTimeRelatedControls';

superset-frontend/packages/superset-ui-chart-controls/test/utils/displayTimeRelatedControls.test.ts

@@ -0,0 +1,118 @@
+/**
+ * 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 { displayTimeRelatedControls } from '../../src';
+
+const mockData = {
+  actions: {
+    setDatasource: jest.fn(),
+  },
+  controls: {
+    x_axis: {
+      type: 'SelectControl' as const,
+      value: 'not_temporal',
+      options: [
+        { column_name: 'not_temporal', is_dttm: false },
+        { column_name: 'ds', is_dttm: true },
+      ],
+    },
+  },
+  exportState: {},
+  form_data: {
+    datasource: '22__table',
+    viz_type: 'table',
+  },
+};
+
+test('returns true when no x-axis exists', () => {
+  expect(
+    displayTimeRelatedControls({
+      ...mockData,
+      controls: {
+        control_options: {
+          type: 'SelectControl',
+          value: 'not_temporal',
+          options: [],
+        },
+      },
+    }),
+  ).toBeTruthy();
+});
+
+test('returns false when x-axis value is not temporal', () => {
+  expect(displayTimeRelatedControls(mockData)).toBeFalsy();
+});
+test('returns true when x-axis value is temporal', () => {
+  expect(
+    displayTimeRelatedControls({
+      ...mockData,
+      controls: {
+        x_axis: {
+          ...mockData.controls.x_axis,
+          value: 'ds',
+        },
+      },
+    }),
+  ).toBeTruthy();
+});
+
+test('returns false when x-axis value without options', () => {
+  expect(
+    displayTimeRelatedControls({
+      ...mockData,
+      controls: {
+        x_axis: {
+          type: 'SelectControl' as const,
+          value: 'not_temporal',
+        },
+      },
+    }),
+  ).toBeFalsy();
+});
+
+test('returns true when x-axis is ad-hoc column', () => {
+  expect(
+    displayTimeRelatedControls({
+      ...mockData,
+      controls: {
+        x_axis: {
+          ...mockData.controls.x_axis,
+          value: {
+            sqlExpression: 'ds',
+            label: 'ds',
+            expressionType: 'SQL',
+          },
+        },
+      },
+    }),
+  ).toBeTruthy();
+});
+
+test('returns false when the x-axis is neither an ad-hoc column nor a physical column', () => {
+  expect(
+    displayTimeRelatedControls({
+      ...mockData,
+      controls: {
+        x_axis: {
+          ...mockData.controls.x_axis,
+          value: {},
+        },
+      },
+    }),
+  ).toBeFalsy();
+});

superset-frontend/src/explore/actions/exploreActions.test.js

@@ -217,4 +217,25 @@ describe('reducers', () => {
       expectedColumnConfig,
     );
   });
+
+  test('setStashFormData works as expected with fieldNames', () => {
+    const newState = exploreReducer(
+      defaultState,
+      actions.setStashFormData(true, ['y_axis_format']),
+    );
+    expect(newState.hiddenFormData).toEqual({
+      y_axis_format: defaultState.form_data.y_axis_format,
+    });
+    expect(newState.form_data.y_axis_format).toBeFalsy();
+    const updatedState = exploreReducer(
+      newState,
+      actions.setStashFormData(false, ['y_axis_format']),
+    );
+    expect(updatedState.hiddenFormData).toEqual({
+      y_axis_format: defaultState.form_data.y_axis_format,
+    });
+    expect(updatedState.form_data.y_axis_format).toEqual(
+      defaultState.form_data.y_axis_format,
+    );
+  });
 });

superset-frontend/src/explore/actions/exploreActions.ts

@@ -152,6 +152,18 @@ export function setForceQuery(force: boolean) {
   };
 }
 
+export const SET_STASH_FORM_DATA = 'SET_STASH_FORM_DATA';
+export function setStashFormData(
+  isHidden: boolean,
+  fieldNames: ReadonlyArray<string>,
+) {
+  return {
+    type: SET_STASH_FORM_DATA,
+    isHidden,
+    fieldNames,
+  };
+}
+
 export const exploreActions = {
   ...toastActions,
   fetchDatasourcesStarted,
@@ -161,6 +173,7 @@ export const exploreActions = {
   saveFaveStar,
   setControlValue,
   setExploreControls,
+  setStashFormData,
   updateChartTitle,
   createNewSlice,
   sliceUpdated,

superset-frontend/src/explore/components/ControlPanelsContainer.test.tsx

@@ -17,20 +17,30 @@
  * under the License.
  */
 import React from 'react';
+import { useSelector } from 'react-redux';
 import userEvent from '@testing-library/user-event';
 import { render, screen } from 'spec/helpers/testing-library';
 import {
   DatasourceType,
   getChartControlPanelRegistry,
   t,
 } from '@superset-ui/core';
-import { defaultControls } from 'src/explore/store';
+import { defaultControls, defaultState } from 'src/explore/store';
+import { ExplorePageState } from 'src/explore/types';
 import { getFormDataFromControls } from 'src/explore/controlUtils';
 import {
   ControlPanelsContainer,
   ControlPanelsContainerProps,
 } from 'src/explore/components/ControlPanelsContainer';
 
+const FormDataMock = () => {
+  const formData = useSelector(
+    (state: ExplorePageState) => state.explore.form_data,
+  );
+
+  return <div data-test="mock-formdata">{Object.keys(formData).join(':')}</div>;
+};
+
 describe('ControlPanelsContainer', () => {
   beforeAll(() => {
     getChartControlPanelRegistry().registerValue('table', {
@@ -144,4 +154,54 @@ describe('ControlPanelsContainer', () => {
       await screen.findAllByTestId('collapsible-control-panel-header'),
     ).toHaveLength(2);
   });
+
+  test('visibility of panels is correctly applied', async () => {
+    getChartControlPanelRegistry().registerValue('table', {
+      controlPanelSections: [
+        {
+          label: t('Advanced analytics'),
+          description: t('Advanced analytics post processing'),
+          expanded: true,
+          controlSetRows: [['groupby'], ['metrics'], ['percent_metrics']],
+          visibility: () => false,
+        },
+        {
+          label: t('Chart Title'),
+          visibility: () => true,
+          controlSetRows: [['timeseries_limit_metric', 'row_limit']],
+        },
+        {
+          label: t('Chart Options'),
+          controlSetRows: [['include_time', 'order_desc']],
+        },
+      ],
+    });
+    const { getByTestId } = render(
+      <>
+        <ControlPanelsContainer {...getDefaultProps()} />
+        <FormDataMock />
+      </>,
+      {
+        useRedux: true,
+        initialState: { explore: { form_data: defaultState.form_data } },
+      },
+    );
+
+    const disabledSection = screen.queryByRole('button', {
+      name: /advanced analytics/i,
+    });
+    expect(disabledSection).not.toBeInTheDocument();
+    expect(
+      screen.getByRole('button', { name: /chart title/i }),
+    ).toBeInTheDocument();
+    expect(
+      screen.queryByRole('button', { name: /chart options/i }),
+    ).toBeInTheDocument();
+
+    expect(getByTestId('mock-formdata')).not.toHaveTextContent('groupby');
+    expect(getByTestId('mock-formdata')).not.toHaveTextContent('metrics');
+    expect(getByTestId('mock-formdata')).not.toHaveTextContent(
+      'percent_metrics',
+    );
+  });
 });