diff --git a/superset-frontend/packages/superset-ui-chart-controls/src/operators/pivotOperator.ts b/superset-frontend/packages/superset-ui-chart-controls/src/operators/pivotOperator.ts index efd4f50a8bd0d..bd929dd7742ea 100644 --- a/superset-frontend/packages/superset-ui-chart-controls/src/operators/pivotOperator.ts +++ b/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 = ( formData, queryObject, ) => { const metricLabels = ensureIsArray(queryObject.metrics).map(getMetricLabel); - const xAxis = getAxis(formData); + const xAxis = getXAxis(formData); if (xAxis && metricLabels.length) { return { diff --git a/superset-frontend/packages/superset-ui-chart-controls/src/operators/prophetOperator.ts b/superset-frontend/packages/superset-ui-chart-controls/src/operators/prophetOperator.ts index 5d8d7feea9345..da651ba12c5fd 100644 --- a/superset-frontend/packages/superset-ui-chart-controls/src/operators/prophetOperator.ts +++ b/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 = ( formData, queryObject, ) => { - const xAxis = getAxis(formData); + const xAxis = getXAxis(formData); if (formData.forecastEnabled && xAxis) { return { operation: 'prophet', diff --git a/superset-frontend/packages/superset-ui-chart-controls/src/operators/renameOperator.ts b/superset-frontend/packages/superset-ui-chart-controls/src/operators/renameOperator.ts index 84cbbce8c5fdb..c51e588cf3142 100644 --- a/superset-frontend/packages/superset-ui-chart-controls/src/operators/renameOperator.ts +++ b/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 = ( ) => { 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 = ( if ( metrics.length === 1 && columns.length > 0 && - (xAxis || queryObject.is_timeseries) && + xAxis && !( // todo: we should provide an approach to handle derived metrics ( diff --git a/superset-frontend/packages/superset-ui-chart-controls/src/operators/timeComparePivotOperator.ts b/superset-frontend/packages/superset-ui-chart-controls/src/operators/timeComparePivotOperator.ts index e1310a4e3aa32..d4ecbf9b62e73 100644 --- a/superset-frontend/packages/superset-ui-chart-controls/src/operators/timeComparePivotOperator.ts +++ b/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 = (formData, queryObject) => { const metricOffsetMap = getMetricOffsetsMap(formData, queryObject); - const xAxis = getAxis(formData); + const xAxis = getXAxis(formData); if (isTimeComparison(formData, queryObject) && xAxis) { const aggregates = Object.fromEntries( diff --git a/superset-frontend/packages/superset-ui-chart-controls/src/operators/utils/index.ts b/superset-frontend/packages/superset-ui-chart-controls/src/operators/utils/index.ts index 809ebe06ed499..8d65ca1e590fb 100644 --- a/superset-frontend/packages/superset-ui-chart-controls/src/operators/utils/index.ts +++ b/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'; diff --git a/superset-frontend/packages/superset-ui-chart-controls/test/operators/pivotOperator.test.ts b/superset-frontend/packages/superset-ui-chart-controls/test/operators/pivotOperator.test.ts index c6c7f905d20ec..f29c4c568c1c4 100644 --- a/superset-frontend/packages/superset-ui-chart-controls/test/operators/pivotOperator.test.ts +++ b/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', }, }, { diff --git a/superset-frontend/packages/superset-ui-chart-controls/test/operators/prophetOperator.test.ts b/superset-frontend/packages/superset-ui-chart-controls/test/operators/prophetOperator.test.ts index 2f3c63ddf86fb..9613584f8e7b6 100644 --- a/superset-frontend/packages/superset-ui-chart-controls/test/operators/prophetOperator.test.ts +++ b/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', diff --git a/superset-frontend/packages/superset-ui-chart-controls/test/operators/renameOperator.test.ts b/superset-frontend/packages/superset-ui-chart-controls/test/operators/renameOperator.test.ts index 26bbe9e3695cc..3aed86401eca4 100644 --- a/superset-frontend/packages/superset-ui-chart-controls/test/operators/renameOperator.test.ts +++ b/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, ), diff --git a/superset-frontend/packages/superset-ui-chart-controls/test/operators/timeComparePivotOperator.test.ts b/superset-frontend/packages/superset-ui-chart-controls/test/operators/timeComparePivotOperator.test.ts index bf46940d713ec..f95050cbbe990 100644 --- a/superset-frontend/packages/superset-ui-chart-controls/test/operators/timeComparePivotOperator.test.ts +++ b/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, diff --git a/superset-frontend/packages/superset-ui-core/src/query/buildQueryContext.ts b/superset-frontend/packages/superset-ui-core/src/query/buildQueryContext.ts index 4ab69ab40e8b6..c5b2b3780b9aa 100644 --- a/superset-frontend/packages/superset-ui-core/src/query/buildQueryContext.ts +++ b/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 { diff --git a/superset-frontend/packages/superset-ui-chart-controls/src/operators/utils/getAxis.ts b/superset-frontend/packages/superset-ui-core/src/query/getXAxis.ts similarity index 79% rename from superset-frontend/packages/superset-ui-chart-controls/src/operators/utils/getAxis.ts rename to superset-frontend/packages/superset-ui-core/src/query/getXAxis.ts index d47089504e578..2413ef5166af4 100644 --- a/superset-frontend/packages/superset-ui-chart-controls/src/operators/utils/getAxis.ts +++ b/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; }; diff --git a/superset-frontend/packages/superset-ui-core/src/query/index.ts b/superset-frontend/packages/superset-ui-core/src/query/index.ts index a539267f9d56b..c3d9bc1bd83e0 100644 --- a/superset-frontend/packages/superset-ui-core/src/query/index.ts +++ b/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'; diff --git a/superset-frontend/packages/superset-ui-core/src/query/normalizeTimeColumn.ts b/superset-frontend/packages/superset-ui-core/src/query/normalizeTimeColumn.ts index 9879e9fe0a512..f7ea0d0e60061 100644 --- a/superset-frontend/packages/superset-ui-core/src/query/normalizeTimeColumn.ts +++ b/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; } diff --git a/superset-frontend/packages/superset-ui-core/test/query/buildQueryContext.test.ts b/superset-frontend/packages/superset-ui-core/test/query/buildQueryContext.test.ts index 3cfbb4e3bbd1c..4a9e71a6dd89e 100644 --- a/superset-frontend/packages/superset-ui-core/test/query/buildQueryContext.test.ts +++ b/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', }, () => [{}], ); diff --git a/superset-frontend/packages/superset-ui-core/test/query/getAxis.test.ts b/superset-frontend/packages/superset-ui-core/test/query/getAxis.test.ts new file mode 100644 index 0000000000000..6db1c150eaf33 --- /dev/null +++ b/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(); + }); +}); diff --git a/superset-frontend/packages/superset-ui-core/test/query/normalizeTimeColumn.test.ts b/superset-frontend/packages/superset-ui-core/test/query/normalizeTimeColumn.test.ts index 1466d10619835..22189b90551c1 100644 --- a/superset-frontend/packages/superset-ui-core/test/query/normalizeTimeColumn.test.ts +++ b/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(() => { diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/buildQuery.ts b/superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/buildQuery.ts index 4bd4df0bcc26e..d22772c0709f4 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/buildQuery.ts +++ b/superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/buildQuery.ts @@ -18,12 +18,13 @@ */ import { buildQueryContext, - DTTM_ALIAS, ensureIsArray, normalizeOrderBy, PostProcessingPivot, QueryFormData, QueryObject, + getXAxis, + isXAxisSet, } from '@superset-ui/core'; import { pivotOperator, @@ -41,11 +42,8 @@ import { } from '../utils/formDataSuffix'; export default function buildQuery(formData: QueryFormData) { - const { x_axis: index } = formData; - const is_timeseries = index === DTTM_ALIAS || !index; const baseFormData = { ...formData, - is_timeseries, }; const formData1 = removeFormDataSuffix(baseFormData, '_b'); @@ -55,9 +53,12 @@ export default function buildQuery(formData: QueryFormData) { buildQueryContext(fd, baseQueryObject => { const queryObject = { ...baseQueryObject, - columns: [...ensureIsArray(index), ...ensureIsArray(fd.groupby)], + columns: [ + ...(isXAxisSet(formData) ? ensureIsArray(getXAxis(formData)) : []), + ...ensureIsArray(fd.groupby), + ], series_columns: fd.groupby, - is_timeseries, + ...(isXAxisSet(formData) ? {} : { is_timeseries: true }), }; const pivotOperatorInRuntime: PostProcessingPivot = isTimeComparison( @@ -68,8 +69,6 @@ export default function buildQuery(formData: QueryFormData) { : pivotOperator(fd, { ...queryObject, columns: fd.groupby, - index, - is_timeseries, }); const tmpQueryObject = { @@ -83,7 +82,6 @@ export default function buildQuery(formData: QueryFormData) { renameOperator(fd, { ...queryObject, columns: fd.groupby, - is_timeseries, }), flattenOperator(fd, queryObject), ], diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/transformProps.ts b/superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/transformProps.ts index 1300f66201ff6..e6e12b498d5f4 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/transformProps.ts +++ b/superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/transformProps.ts @@ -20,16 +20,16 @@ import { AnnotationLayer, CategoricalColorNamespace, - DTTM_ALIAS, GenericDataType, - getColumnLabel, getNumberFormatter, isEventAnnotationLayer, isFormulaAnnotationLayer, isIntervalAnnotationLayer, isTimeseriesAnnotationLayer, + QueryFormData, TimeseriesChartDataResponseResult, TimeseriesDataRecord, + getXAxis, } from '@superset-ui/core'; import { EChartsCoreOption, SeriesOption } from 'echarts'; import { @@ -152,8 +152,7 @@ export default function transformProps( const colorScale = CategoricalColorNamespace.getScale(colorScheme as string); - const xAxisCol = - verboseMap[xAxisOrig] || getColumnLabel(xAxisOrig || DTTM_ALIAS); + const xAxisCol = getXAxis(chartProps.rawFormData as QueryFormData) as string; const rebasedDataA = rebaseForecastDatum(data1, verboseMap); const rawSeriesA = extractSeries(rebasedDataA, { diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/buildQuery.ts b/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/buildQuery.ts index 085635209ac20..3e563d88338d6 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/buildQuery.ts +++ b/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/buildQuery.ts @@ -18,11 +18,12 @@ */ import { buildQueryContext, - DTTM_ALIAS, ensureIsArray, normalizeOrderBy, PostProcessingPivot, QueryFormData, + getXAxis, + isXAxisSet, } from '@superset-ui/core'; import { rollingWindowOperator, @@ -38,8 +39,7 @@ import { } from '@superset-ui/chart-controls'; export default function buildQuery(formData: QueryFormData) { - const { x_axis, groupby } = formData; - const is_timeseries = x_axis === DTTM_ALIAS || !x_axis; + const { groupby } = formData; return buildQueryContext(formData, baseQueryObject => { /* the `pivotOperatorInRuntime` determines how to pivot the dataframe returned from the raw query. 1. If it's a time compared query, there will return a pivoted dataframe that append time compared metrics. for instance: @@ -66,18 +66,17 @@ export default function buildQuery(formData: QueryFormData) { baseQueryObject, ) ? timeComparePivotOperator(formData, baseQueryObject) - : pivotOperator(formData, { - ...baseQueryObject, - index: x_axis, - is_timeseries, - }); + : pivotOperator(formData, baseQueryObject); return [ { ...baseQueryObject, - columns: [...ensureIsArray(x_axis), ...ensureIsArray(groupby)], + columns: [ + ...(isXAxisSet(formData) ? ensureIsArray(getXAxis(formData)) : []), + ...ensureIsArray(groupby), + ], series_columns: groupby, - is_timeseries, + ...(isXAxisSet(formData) ? {} : { is_timeseries: true }), // todo: move `normalizeOrderBy to extractQueryFields` orderby: normalizeOrderBy(baseQueryObject).orderby, time_offsets: isTimeComparison(formData, baseQueryObject) @@ -92,10 +91,7 @@ export default function buildQuery(formData: QueryFormData) { rollingWindowOperator(formData, baseQueryObject), timeCompareOperator(formData, baseQueryObject), resampleOperator(formData, baseQueryObject), - renameOperator(formData, { - ...baseQueryObject, - is_timeseries, - }), + renameOperator(formData, baseQueryObject), contributionOperator(formData, baseQueryObject), flattenOperator(formData, baseQueryObject), // todo: move prophet before flatten diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts b/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts index 5d49b657e847c..177b14a35ebb2 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts +++ b/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts @@ -28,9 +28,9 @@ import { isTimeseriesAnnotationLayer, TimeseriesChartDataResponseResult, t, - DTTM_ALIAS, + getXAxis, } from '@superset-ui/core'; -import { getAxis, isDerivedSeries } from '@superset-ui/chart-controls'; +import { isDerivedSeries } from '@superset-ui/chart-controls'; import { EChartsCoreOption, SeriesOption } from 'echarts'; import { ZRLineType } from 'echarts/types/src/util/types'; import { @@ -148,9 +148,7 @@ 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] || getAxis(chartProps.rawFormData) || DTTM_ALIAS; + const xAxisCol = getXAxis(chartProps.rawFormData) as string; const isHorizontal = orientation === OrientationType.horizontal; const { totalStackedValues, thresholdValues } = extractDataTotalValues( rebasedData, diff --git a/superset-frontend/plugins/plugin-chart-echarts/test/MixedTimeseries/buildQuery.test.ts b/superset-frontend/plugins/plugin-chart-echarts/test/MixedTimeseries/buildQuery.test.ts index d8eec19ea6d3f..25936767630e2 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/test/MixedTimeseries/buildQuery.test.ts +++ b/superset-frontend/plugins/plugin-chart-echarts/test/MixedTimeseries/buildQuery.test.ts @@ -267,7 +267,22 @@ test('should compile AA in query B', () => { }); }); -test('should compile query objects with x-axis', () => { +test('should convert a queryObject with x-axis although FF is disabled', () => { + let windowSpy: any; + + beforeAll(() => { + // @ts-ignore + windowSpy = jest.spyOn(window, 'window', 'get').mockImplementation(() => ({ + featureFlags: { + GENERIC_CHART_AXES: false, + }, + })); + }); + + afterAll(() => { + windowSpy.mockRestore(); + }); + const { queries } = buildQuery({ ...formDataMixedChart, x_axis: 'my_index', @@ -280,11 +295,19 @@ test('should compile query objects with x-axis', () => { filters: [], extras: { having: '', - time_grain_sqla: 'P1W', where: "(foo in ('a', 'b'))", }, applied_time_extras: {}, - columns: ['my_index', 'foo'], + columns: [ + { + columnType: 'BASE_AXIS', + expressionType: 'SQL', + label: 'my_index', + sqlExpression: 'my_index', + timeGrain: 'P1W', + }, + 'foo', + ], metrics: ['sum(sales)'], annotation_layers: [], row_limit: 10, @@ -296,7 +319,6 @@ test('should compile query objects with x-axis', () => { url_params: {}, custom_params: {}, custom_form_data: {}, - is_timeseries: false, time_offsets: [], post_processing: [ { @@ -332,8 +354,16 @@ test('should compile query objects with x-axis', () => { // check the main props on the second query expect(queries[1]).toEqual( expect.objectContaining({ - is_timeseries: false, - columns: ['my_index'], + columns: [ + { + columnType: 'BASE_AXIS', + expressionType: 'SQL', + label: 'my_index', + sqlExpression: 'my_index', + timeGrain: 'P1W', + }, + ], + granularity: 'ds', series_columns: [], metrics: ['count'], post_processing: [ @@ -357,3 +387,90 @@ test('should compile query objects with x-axis', () => { }), ); }); + +test("shouldn't convert a queryObject with axis although FF is enabled", () => { + let windowSpy: any; + + beforeAll(() => { + // @ts-ignore + windowSpy = jest.spyOn(window, 'window', 'get').mockImplementation(() => ({ + featureFlags: { + GENERIC_CHART_AXES: true, + }, + })); + }); + + afterAll(() => { + windowSpy.mockRestore(); + }); + + const { queries } = buildQuery(formDataMixedChart); + expect(queries[0]).toEqual( + expect.objectContaining({ + granularity: 'ds', + columns: ['foo'], + series_columns: ['foo'], + metrics: ['sum(sales)'], + is_timeseries: true, + extras: { + time_grain_sqla: 'P1W', + having: '', + where: "(foo in ('a', 'b'))", + }, + post_processing: [ + { + operation: 'pivot', + options: { + aggregates: { + 'sum(sales)': { + operator: 'mean', + }, + }, + columns: ['foo'], + drop_missing_columns: false, + index: ['__timestamp'], + }, + }, + { + operation: 'rename', + options: { columns: { 'sum(sales)': null }, inplace: true, level: 0 }, + }, + { + operation: 'flatten', + }, + ], + }), + ); + expect(queries[1]).toEqual( + expect.objectContaining({ + granularity: 'ds', + columns: [], + series_columns: [], + metrics: ['count'], + is_timeseries: true, + extras: { + time_grain_sqla: 'P1W', + having: '', + where: "(name in ('c', 'd'))", + }, + post_processing: [ + { + operation: 'pivot', + options: { + aggregates: { + count: { + operator: 'mean', + }, + }, + columns: [], + drop_missing_columns: false, + index: ['__timestamp'], + }, + }, + { + operation: 'flatten', + }, + ], + }), + ); +}); diff --git a/superset-frontend/plugins/plugin-chart-echarts/test/Timeseries/buildQuery.test.ts b/superset-frontend/plugins/plugin-chart-echarts/test/Timeseries/buildQuery.test.ts index 7c0787dc0241d..50432a136af8e 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/test/Timeseries/buildQuery.test.ts +++ b/superset-frontend/plugins/plugin-chart-echarts/test/Timeseries/buildQuery.test.ts @@ -16,6 +16,7 @@ * specific language governing permissions and limitations * under the License. */ +import { SqlaFormData } from '@superset-ui/core'; import buildQuery from '../../src/Timeseries/buildQuery'; describe('Timeseries buildQuery', () => { @@ -59,3 +60,181 @@ describe('Timeseries buildQuery', () => { expect(query.orderby).toEqual([['foo', true]]); }); }); + +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(); + }); + + const formData: SqlaFormData = { + datasource: '5__table', + viz_type: 'table', + granularity_sqla: 'time_column', + time_grain_sqla: 'P1Y', + time_range: '1 year ago : 2013', + groupby: ['col1'], + metrics: ['count(*)'], + }; + + it("shouldn't convert queryObject", () => { + const { queries } = buildQuery(formData); + expect(queries[0]).toEqual( + expect.objectContaining({ + granularity: 'time_column', + time_range: '1 year ago : 2013', + extras: { time_grain_sqla: 'P1Y', having: '', where: '' }, + columns: ['col1'], + series_columns: ['col1'], + metrics: ['count(*)'], + is_timeseries: true, + post_processing: [ + { + operation: 'pivot', + options: { + aggregates: { 'count(*)': { operator: 'mean' } }, + columns: ['col1'], + drop_missing_columns: true, + index: ['__timestamp'], + }, + }, + { operation: 'flatten' }, + ], + }), + ); + }); + + it('should convert queryObject', () => { + const { queries } = buildQuery({ ...formData, x_axis: 'time_column' }); + expect(queries[0]).toEqual( + expect.objectContaining({ + granularity: 'time_column', + time_range: '1 year ago : 2013', + extras: { having: '', where: '' }, + columns: [ + { + columnType: 'BASE_AXIS', + expressionType: 'SQL', + label: 'time_column', + sqlExpression: 'time_column', + timeGrain: 'P1Y', + }, + 'col1', + ], + series_columns: ['col1'], + metrics: ['count(*)'], + post_processing: [ + { + operation: 'pivot', + options: { + aggregates: { 'count(*)': { operator: 'mean' } }, + columns: ['col1'], + drop_missing_columns: true, + index: ['time_column'], + }, + }, + { operation: 'flatten' }, + ], + }), + ); + }); +}); + +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(); + }); + + const formData: SqlaFormData = { + datasource: '5__table', + viz_type: 'table', + granularity_sqla: 'time_column', + time_grain_sqla: 'P1Y', + time_range: '1 year ago : 2013', + groupby: ['col1'], + metrics: ['count(*)'], + }; + + it("shouldn't convert queryObject", () => { + const { queries } = buildQuery(formData); + expect(queries[0]).toEqual( + expect.objectContaining({ + granularity: 'time_column', + time_range: '1 year ago : 2013', + extras: { time_grain_sqla: 'P1Y', having: '', where: '' }, + columns: ['col1'], + series_columns: ['col1'], + metrics: ['count(*)'], + is_timeseries: true, + post_processing: [ + { + operation: 'pivot', + options: { + aggregates: { 'count(*)': { operator: 'mean' } }, + columns: ['col1'], + drop_missing_columns: true, + index: ['__timestamp'], + }, + }, + { operation: 'flatten' }, + ], + }), + ); + }); + + it('should convert queryObject', () => { + const { queries } = buildQuery({ ...formData, x_axis: 'time_column' }); + expect(queries[0]).toEqual( + expect.objectContaining({ + granularity: 'time_column', + time_range: '1 year ago : 2013', + extras: { having: '', where: '' }, + columns: [ + { + columnType: 'BASE_AXIS', + expressionType: 'SQL', + label: 'time_column', + sqlExpression: 'time_column', + timeGrain: 'P1Y', + }, + 'col1', + ], + series_columns: ['col1'], + metrics: ['count(*)'], + post_processing: [ + { + operation: 'pivot', + options: { + aggregates: { 'count(*)': { operator: 'mean' } }, + columns: ['col1'], + drop_missing_columns: true, + index: ['time_column'], + }, + }, + { operation: 'flatten' }, + ], + }), + ); + }); +});