feat: generate consistent QueryObject whether GenericAxis is enabled …

…or disabled (#21519)

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

@@ -21,16 +21,16 @@ import {
   getColumnLabel,
   getMetricLabel,
   PostProcessingPivot,
+  getXAxis,
 } 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 xAxis = getAxis(formData);
+  const xAxis = getXAxis(formData);
 
   if (xAxis && metricLabels.length) {
     return {

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

@@ -16,16 +16,15 @@
  * specific language governing permissions and limitationsxw
  * under the License.
  */
-import { PostProcessingProphet } from '@superset-ui/core';
+import { PostProcessingProphet, getXAxis } 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 xAxis = getAxis(formData);
+  const xAxis = getXAxis(formData);
   if (formData.forecastEnabled && xAxis) {
     return {
       operation: 'prophet',

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

@@ -22,6 +22,7 @@ import {
   ensureIsArray,
   getMetricLabel,
   ComparisionType,
+  getXAxis,
 } from '@superset-ui/core';
 import { PostProcessingFactory } from './types';
 import { getMetricOffsetsMap, isTimeComparison } from './utils';
@@ -32,7 +33,8 @@ export const renameOperator: PostProcessingFactory<PostProcessingRename> = (
 ) => {
   const metrics = ensureIsArray(queryObject.metrics);
   const columns = ensureIsArray(queryObject.columns);
-  const { x_axis: xAxis, truncate_metric } = formData;
+  const { truncate_metric } = formData;
+  const xAxis = getXAxis(formData);
   // remove or rename top level of column name(metric name) in the MultiIndex when
   // 1) only 1 metric
   // 2) exist dimentsion
@@ -42,7 +44,7 @@ export const renameOperator: PostProcessingFactory<PostProcessingRename> = (
   if (
     metrics.length === 1 &&
     columns.length > 0 &&
-    (xAxis || queryObject.is_timeseries) &&
+    xAxis &&
     !(
       // todo: we should provide an approach to handle derived metrics
       (

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

@@ -22,14 +22,15 @@ import {
   getColumnLabel,
   NumpyFunction,
   PostProcessingPivot,
+  getXAxis,
 } from '@superset-ui/core';
-import { getMetricOffsetsMap, isTimeComparison, getAxis } from './utils';
+import { getMetricOffsetsMap, isTimeComparison } from './utils';
 import { PostProcessingFactory } from './types';
 
 export const timeComparePivotOperator: PostProcessingFactory<PostProcessingPivot> =
   (formData, queryObject) => {
     const metricOffsetMap = getMetricOffsetsMap(formData, queryObject);
-    const xAxis = getAxis(formData);
+    const xAxis = getXAxis(formData);
 
     if (isTimeComparison(formData, queryObject) && xAxis) {
       const aggregates = Object.fromEntries(

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

@@ -21,4 +21,3 @@ 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

@@ -141,7 +141,7 @@ test('pivot by adhoc x_axis', () => {
         x_axis: {
           label: 'my_case_expr',
           expressionType: 'SQL',
-          expression: 'case when a = 1 then 1 else 0 end',
+          sqlExpression: 'case when a = 1 then 1 else 0 end',
         },
       },
       {

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

@@ -108,7 +108,7 @@ test('should do prophetOperator over adhoc column', () => {
         x_axis: {
           label: 'my_case_expr',
           expressionType: 'SQL',
-          expression: 'case when a = 1 then 1 else 0 end',
+          sqlExpression: 'case when a = 1 then 1 else 0 end',
         },
         forecastEnabled: true,
         forecastPeriods: '3',

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

@@ -110,7 +110,7 @@ test('should add renameOperator if does not exist x_axis', () => {
     renameOperator(
       {
         ...formData,
-        ...{ x_axis: null },
+        ...{ x_axis: null, granularity_sqla: 'time column' },
       },
       queryObject,
     ),

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

@@ -146,7 +146,7 @@ test('should pivot on adhoc x-axis', () => {
         x_axis: {
           label: 'my_case_expr',
           expressionType: 'SQL',
-          expression: 'case when a = 1 then 1 else 0 end',
+          sqlExpression: 'case when a = 1 then 1 else 0 end',
         },
       },
       queryObject,

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

@@ -23,8 +23,8 @@ import { QueryFieldAliases, QueryFormData } from './types/QueryFormData';
 import { QueryContext, QueryObject } from './types/Query';
 import { SetDataMaskHook } from '../chart';
 import { JsonObject } from '../connection';
-import { isFeatureEnabled, FeatureFlag } from '../utils';
 import { normalizeTimeColumn } from './normalizeTimeColumn';
+import { isXAxisSet } from './getXAxis';
 
 const WRAP_IN_ARRAY = (baseQueryObject: QueryObject) => [baseQueryObject];
 
@@ -54,7 +54,7 @@ export default function buildQueryContext(
       query.post_processing = query.post_processing.filter(Boolean);
     }
   });
-  if (isFeatureEnabled(FeatureFlag.GENERIC_CHART_AXES)) {
+  if (isXAxisSet(formData)) {
     queries = queries.map(query => normalizeTimeColumn(formData, query));
   }
   return {

superset-frontend/packages/superset-ui-chart-controls/src/operators/utils/getAxis.ts → superset-frontend/packages/superset-ui-core/src/query/getXAxis.ts

@@ -19,17 +19,21 @@
 import {
   DTTM_ALIAS,
   getColumnLabel,
-  isDefined,
+  isQueryFormColumn,
   QueryFormData,
 } from '@superset-ui/core';
 
-export const getAxis = (formData: QueryFormData): string | undefined => {
+export const isXAxisSet = (formData: QueryFormData) =>
+  isQueryFormColumn(formData.x_axis);
+
+export const getXAxis = (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;
+  if (isXAxisSet(formData)) {
+    return getColumnLabel(formData.x_axis);
+  }
+  return DTTM_ALIAS;
 };

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

@@ -29,6 +29,7 @@ export { default as getMetricLabel } from './getMetricLabel';
 export { default as DatasourceKey } from './DatasourceKey';
 export { default as normalizeOrderBy } from './normalizeOrderBy';
 export { normalizeTimeColumn } from './normalizeTimeColumn';
+export { getXAxis, isXAxisSet } from './getXAxis';
 
 export * from './types/AnnotationLayer';
 export * from './types/QueryFormData';

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

@@ -26,14 +26,14 @@ import {
   QueryFormData,
   QueryObject,
 } from './types';
-import { FeatureFlag, isFeatureEnabled } from '../utils';
+import { isXAxisSet } from './getXAxis';
 
 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)) {
+  if (!isXAxisSet(formData)) {
     return queryObject;
   }
 

superset-frontend/packages/superset-ui-core/test/query/buildQueryContext.test.ts

@@ -123,7 +123,7 @@ describe('buildQueryContext', () => {
       },
     ]);
   });
-  it('should call normalizeTimeColumn if GENERIC_CHART_AXES is enabled', () => {
+  it('should call normalizeTimeColumn if GENERIC_CHART_AXES is enabled and has x_axis', () => {
     // @ts-ignore
     const spy = jest.spyOn(window, 'window', 'get').mockImplementation(() => ({
       featureFlags: {
@@ -139,6 +139,7 @@ describe('buildQueryContext', () => {
       {
         datasource: '5__table',
         viz_type: 'table',
+        x_axis: 'axis',
       },
       () => [{}],
     );

superset-frontend/packages/superset-ui-core/test/query/getAxis.test.ts

@@ -0,0 +1,71 @@
+/**
+ * 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 { isXAxisSet } from '@superset-ui/core';
+
+describe('GENERIC_CHART_AXES is enabled', () => {
+  let windowSpy: any;
+
+  beforeAll(() => {
+    // @ts-ignore
+    windowSpy = jest.spyOn(window, 'window', 'get').mockImplementation(() => ({
+      featureFlags: {
+        GENERIC_CHART_AXES: true,
+      },
+    }));
+  });
+
+  afterAll(() => {
+    windowSpy.mockRestore();
+  });
+
+  it('isEnabledAxies when FF is disabled', () => {
+    expect(
+      isXAxisSet({ datasource: '123', viz_type: 'table' }),
+    ).not.toBeTruthy();
+    expect(
+      isXAxisSet({ datasource: '123', viz_type: 'table', x_axis: 'axis' }),
+    ).toBeTruthy();
+  });
+});
+
+describe('GENERIC_CHART_AXES is disabled', () => {
+  let windowSpy: any;
+
+  beforeAll(() => {
+    // @ts-ignore
+    windowSpy = jest.spyOn(window, 'window', 'get').mockImplementation(() => ({
+      featureFlags: {
+        GENERIC_CHART_AXES: false,
+      },
+    }));
+  });
+
+  afterAll(() => {
+    windowSpy.mockRestore();
+  });
+
+  it('isEnabledAxies when FF is disabled', () => {
+    expect(
+      isXAxisSet({ datasource: '123', viz_type: 'table' }),
+    ).not.toBeTruthy();
+    expect(
+      isXAxisSet({ datasource: '123', viz_type: 'table', x_axis: 'axis' }),
+    ).toBeTruthy();
+  });
+});

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

@@ -22,7 +22,7 @@ import {
   SqlaFormData,
 } from '@superset-ui/core';
 
-describe('disabled GENERIC_CHART_AXES', () => {
+describe('GENERIC_CHART_AXES is disabled', () => {
   let windowSpy: any;
 
   beforeAll(() => {
@@ -47,7 +47,6 @@ describe('disabled GENERIC_CHART_AXES', () => {
       time_range: '1 year ago : 2013',
       columns: ['col1'],
       metrics: ['count(*)'],
-      x_axis: 'time_column',
     };
     const query: QueryObject = {
       datasource: '5__table',
@@ -64,9 +63,54 @@ describe('disabled GENERIC_CHART_AXES', () => {
     };
     expect(normalizeTimeColumn(formData, query)).toEqual(query);
   });
+
+  it('should return converted QueryObject even though disabled GENERIC_CHART_AXES (x_axis in formData)', () => {
+    const formData: SqlaFormData = {
+      datasource: '5__table',
+      viz_type: 'table',
+      granularity: 'time_column',
+      time_grain_sqla: 'P1Y',
+      time_range: '1 year ago : 2013',
+      columns: ['col1'],
+      metrics: ['count(*)'],
+      x_axis: 'time_column',
+    };
+    const query: QueryObject = {
+      datasource: '5__table',
+      viz_type: 'table',
+      granularity: 'time_column',
+      extras: {
+        time_grain_sqla: 'P1Y',
+      },
+      time_range: '1 year ago : 2013',
+      orderby: [['count(*)', true]],
+      columns: ['time_column', 'col1'],
+      metrics: ['count(*)'],
+      is_timeseries: true,
+    };
+    expect(normalizeTimeColumn(formData, query)).toEqual({
+      datasource: '5__table',
+      viz_type: 'table',
+      granularity: 'time_column',
+      extras: {},
+      time_range: '1 year ago : 2013',
+      orderby: [['count(*)', true]],
+      columns: [
+        {
+          timeGrain: 'P1Y',
+          columnType: 'BASE_AXIS',
+          sqlExpression: 'time_column',
+          label: 'time_column',
+          expressionType: 'SQL',
+        },
+        'col1',
+      ],
+      metrics: ['count(*)'],
+    });
+  });
 });
 
-describe('enabled GENERIC_CHART_AXES', () => {
+describe('GENERIC_CHART_AXES is enabled', () => {
   let windowSpy: any;
 
   beforeAll(() => {