refactor: get Axis from a helper function (#21449)

superset-frontend/packages/superset-ui-chart-controls/src/operators/pivotOperator.ts

@@ -17,27 +17,26 @@
  * under the License.
  */
 import {
-  DTTM_ALIAS,
   ensureIsArray,
   getColumnLabel,
   getMetricLabel,
   PostProcessingPivot,
 } from '@superset-ui/core';
 import { PostProcessingFactory } from './types';
+import { getAxis } from './utils';
 
 export const pivotOperator: PostProcessingFactory<PostProcessingPivot> = (
   formData,
   queryObject,
 ) => {
   const metricLabels = ensureIsArray(queryObject.metrics).map(getMetricLabel);
-  const { x_axis: xAxis } = formData;
+  const xAxis = getAxis(formData);
 
-  if ((xAxis || queryObject.is_timeseries) && metricLabels.length) {
-    const index = [getColumnLabel(xAxis || DTTM_ALIAS)];
+  if (xAxis && metricLabels.length) {
     return {
       operation: 'pivot',
       options: {
-        index,
+        index: [xAxis],
         columns: ensureIsArray(queryObject.columns).map(getColumnLabel),
         // Create 'dummy' mean aggregates to assign cell values in pivot table
         // use the 'mean' aggregates to avoid drop NaN. PR: https://github.com/apache-superset/superset-ui/pull/1231

superset-frontend/packages/superset-ui-chart-controls/src/operators/prophetOperator.ts

@@ -16,20 +16,17 @@
  * specific language governing permissions and limitationsxw
  * under the License.
  */
-import {
-  DTTM_ALIAS,
-  getColumnLabel,
-  PostProcessingProphet,
-} from '@superset-ui/core';
+import { PostProcessingProphet } from '@superset-ui/core';
 import { PostProcessingFactory } from './types';
+import { getAxis } from './utils';
 
 /* eslint-disable @typescript-eslint/no-unused-vars */
 export const prophetOperator: PostProcessingFactory<PostProcessingProphet> = (
   formData,
   queryObject,
 ) => {
-  const index = getColumnLabel(formData.x_axis || DTTM_ALIAS);
-  if (formData.forecastEnabled) {
+  const xAxis = getAxis(formData);
+  if (formData.forecastEnabled && xAxis) {
     return {
       operation: 'prophet',
       options: {
@@ -39,7 +36,7 @@ export const prophetOperator: PostProcessingFactory<PostProcessingProphet> = (
         yearly_seasonality: formData.forecastSeasonalityYearly,
         weekly_seasonality: formData.forecastSeasonalityWeekly,
         daily_seasonality: formData.forecastSeasonalityDaily,
-        index,
+        index: xAxis,
       },
     };
   }

superset-frontend/packages/superset-ui-chart-controls/src/operators/timeComparePivotOperator.ts

@@ -18,33 +18,32 @@
  * under the License.
  */
 import {
-  DTTM_ALIAS,
   ensureIsArray,
   getColumnLabel,
   NumpyFunction,
   PostProcessingPivot,
 } from '@superset-ui/core';
-import { getMetricOffsetsMap, isTimeComparison } from './utils';
+import { getMetricOffsetsMap, isTimeComparison, getAxis } from './utils';
 import { PostProcessingFactory } from './types';
 
 export const timeComparePivotOperator: PostProcessingFactory<PostProcessingPivot> =
   (formData, queryObject) => {
     const metricOffsetMap = getMetricOffsetsMap(formData, queryObject);
+    const xAxis = getAxis(formData);
 
-    if (isTimeComparison(formData, queryObject)) {
+    if (isTimeComparison(formData, queryObject) && xAxis) {
       const aggregates = Object.fromEntries(
         [...metricOffsetMap.values(), ...metricOffsetMap.keys()].map(metric => [
           metric,
           // use the 'mean' aggregates to avoid drop NaN
           { operator: 'mean' as NumpyFunction },
         ]),
       );
-      const index = [getColumnLabel(formData.x_axis || DTTM_ALIAS)];
 
       return {
         operation: 'pivot',
         options: {
-          index,
+          index: [xAxis],
           columns: ensureIsArray(queryObject.columns).map(getColumnLabel),
           drop_missing_columns: !formData?.show_empty_columns,
           aggregates,

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

@@ -0,0 +1,35 @@
+/**
+ * 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 {
+  DTTM_ALIAS,
+  getColumnLabel,
+  isDefined,
+  QueryFormData,
+} from '@superset-ui/core';
+
+export const getAxis = (formData: QueryFormData): string | undefined => {
+  // The formData should be "raw form_data" -- the snake_case version of formData rather than camelCase.
+  if (!(formData.granularity_sqla || formData.x_axis)) {
+    return undefined;
+  }
+
+  return isDefined(formData.x_axis)
+    ? getColumnLabel(formData.x_axis)
+    : DTTM_ALIAS;
+};

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

@@ -21,3 +21,4 @@ export { getMetricOffsetsMap } from './getMetricOffsetsMap';
 export { isTimeComparison } from './isTimeComparison';
 export { isDerivedSeries } from './isDerivedSeries';
 export { TIME_COMPARISON_SEPARATOR } from './constants';
+export { getAxis } from './getAxis';

superset-frontend/packages/superset-ui-chart-controls/test/operators/pivotOperator.test.ts

@@ -56,21 +56,20 @@ const queryObject: QueryObject = {
 
 test('skip pivot', () => {
   expect(pivotOperator(formData, queryObject)).toEqual(undefined);
-  expect(
-    pivotOperator(formData, { ...queryObject, is_timeseries: false }),
-  ).toEqual(undefined);
   expect(
     pivotOperator(formData, {
       ...queryObject,
-      is_timeseries: true,
       metrics: [],
     }),
   ).toEqual(undefined);
 });
 
 test('pivot by __timestamp without groupby', () => {
   expect(
-    pivotOperator(formData, { ...queryObject, is_timeseries: true }),
+    pivotOperator(
+      { ...formData, granularity_sqla: 'time_column' },
+      queryObject,
+    ),
   ).toEqual({
     operation: 'pivot',
     options: {
@@ -87,11 +86,13 @@ test('pivot by __timestamp without groupby', () => {
 
 test('pivot by __timestamp with groupby', () => {
   expect(
-    pivotOperator(formData, {
-      ...queryObject,
-      columns: ['foo', 'bar'],
-      is_timeseries: true,
-    }),
+    pivotOperator(
+      { ...formData, granularity_sqla: 'time_column' },
+      {
+        ...queryObject,
+        columns: ['foo', 'bar'],
+      },
+    ),
   ).toEqual({
     operation: 'pivot',
     options: {

superset-frontend/packages/superset-ui-chart-controls/test/operators/prophetOperator.test.ts

@@ -47,6 +47,7 @@ test('should do prophetOperator with default index', () => {
     prophetOperator(
       {
         ...formData,
+        granularity_sqla: 'time_column',
         forecastEnabled: true,
         forecastPeriods: '3',
         forecastInterval: '5',

superset-frontend/packages/superset-ui-chart-controls/test/operators/timeComparePivotOperator.test.ts

@@ -72,10 +72,10 @@ test('should pivot on any type of timeCompare', () => {
           ...formData,
           comparison_type: cType,
           time_compare: ['1 year ago', '1 year later'],
+          granularity_sqla: 'time_column',
         },
         {
           ...queryObject,
-          is_timeseries: true,
         },
       ),
     ).toEqual({

superset-frontend/packages/superset-ui-core/src/query/normalizeTimeColumn.ts

@@ -32,6 +32,7 @@ export function normalizeTimeColumn(
   formData: QueryFormData,
   queryObject: QueryObject,
 ): QueryObject {
+  // The formData should be "raw form_data" -- the snake_case version of formData rather than camelCase.
   if (!(isFeatureEnabled(FeatureFlag.GENERIC_CHART_AXES) && formData.x_axis)) {
     return queryObject;
   }

superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts

@@ -20,18 +20,17 @@
 import {
   AnnotationLayer,
   CategoricalColorNamespace,
-  DTTM_ALIAS,
   GenericDataType,
-  getColumnLabel,
   getNumberFormatter,
   isEventAnnotationLayer,
   isFormulaAnnotationLayer,
   isIntervalAnnotationLayer,
   isTimeseriesAnnotationLayer,
   TimeseriesChartDataResponseResult,
   t,
+  DTTM_ALIAS,
 } from '@superset-ui/core';
-import { isDerivedSeries } from '@superset-ui/chart-controls';
+import { getAxis, isDerivedSeries } from '@superset-ui/chart-controls';
 import { EChartsCoreOption, SeriesOption } from 'echarts';
 import { ZRLineType } from 'echarts/types/src/util/types';
 import {
@@ -149,8 +148,9 @@ export default function transformProps(
 
   const colorScale = CategoricalColorNamespace.getScale(colorScheme as string);
   const rebasedData = rebaseForecastDatum(data, verboseMap);
+  // todo: if the both granularity_sqla and x_axis are `null`, should throw an error
   const xAxisCol =
-    verboseMap[xAxisOrig] || getColumnLabel(xAxisOrig || DTTM_ALIAS);
+    verboseMap[xAxisOrig] || getAxis(chartProps.rawFormData) || DTTM_ALIAS;
   const isHorizontal = orientation === OrientationType.horizontal;
   const { totalStackedValues, thresholdValues } = extractDataTotalValues(
     rebasedData,