chore(superset-ui): remove deprecated fields from QueryObject

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

@@ -22,18 +22,18 @@ import {
   AdhocFilter,
   QueryObject,
   QueryObjectFilterClause,
-  isPhysicalColumn,
-  isAdhocColumn,
+  isQueryFormMetric,
 } from './types';
 import {
   QueryFieldAliases,
-  QueryFormColumn,
+  QueryFormMetric,
   QueryFormData,
 } from './types/QueryFormData';
 import processFilters from './processFilters';
 import extractExtras from './extractExtras';
 import extractQueryFields from './extractQueryFields';
 import { overrideExtraFormData } from './processExtraFormData';
+import { isDefined } from '../utils';
 
 /**
  * Build the common segments of all query objects (e.g. the granularity field derived from
@@ -94,9 +94,9 @@ export default function buildQueryObject<T extends QueryFormData>(
     ...extras,
     ...filterFormData,
   });
-  const normalizeSeriesLimitMetric = (column: QueryFormColumn | undefined) => {
-    if (isAdhocColumn(column) || isPhysicalColumn(column)) {
-      return column;
+  const normalizeSeriesLimitMetric = (metric: QueryFormMetric | undefined) => {
+    if (isQueryFormMetric(metric)) {
+      return metric;
     }
     return undefined;
   };
@@ -123,10 +123,11 @@ export default function buildQueryObject<T extends QueryFormData>(
         ? undefined
         : numericRowOffset,
     series_columns,
-    series_limit,
-    series_limit_metric: normalizeSeriesLimitMetric(series_limit_metric),
-    timeseries_limit: limit ? Number(limit) : 0,
-    timeseries_limit_metric: timeseries_limit_metric || undefined,
+    series_limit: series_limit ?? (isDefined(limit) ? Number(limit) : 0),
+    series_limit_metric:
+      normalizeSeriesLimitMetric(series_limit_metric) ??
+      timeseries_limit_metric ??
+      undefined,
     order_desc: typeof order_desc === 'undefined' ? true : order_desc,
     url_params: url_params || undefined,
     custom_params,

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

@@ -39,20 +39,20 @@ export default function normalizeOrderBy(
 
   // ensure that remove invalid orderby clause
   const cloneQueryObject = { ...queryObject };
-  delete cloneQueryObject.timeseries_limit_metric;
+  delete cloneQueryObject.series_limit_metric;
   delete cloneQueryObject.legacy_order_by;
   delete cloneQueryObject.order_desc;
   delete cloneQueryObject.orderby;
 
   const isAsc = !queryObject.order_desc;
   if (
-    queryObject.timeseries_limit_metric !== undefined &&
-    queryObject.timeseries_limit_metric !== null &&
-    !isEmpty(queryObject.timeseries_limit_metric)
+    queryObject.series_limit_metric !== undefined &&
+    queryObject.series_limit_metric !== null &&
+    !isEmpty(queryObject.series_limit_metric)
   ) {
     return {
       ...cloneQueryObject,
-      orderby: [[queryObject.timeseries_limit_metric, isAsc]],
+      orderby: [[queryObject.series_limit_metric, isAsc]],
     };
   }
 

superset-frontend/packages/superset-ui-core/src/query/types/Query.ts

@@ -136,12 +136,6 @@ export interface QueryObject
   /** The size of bucket by which to group timeseries data (forthcoming) */
   time_grain?: string;
 
-  /** Maximum number of timeseries */
-  timeseries_limit?: number;
-
-  /** The metric used to sort the returned result. */
-  timeseries_limit_metric?: Maybe<QueryFormMetric>;
-
   /** Direction to ordered by */
   order_desc?: boolean;
 

superset-frontend/packages/superset-ui-core/src/query/types/QueryFormData.ts

@@ -166,13 +166,15 @@ export interface BaseFormData extends TimeRange, FormDataResidual {
   extra_form_data?: ExtraFormData;
   /** order descending */
   order_desc?: boolean;
-  /** limit number of time series */
+  /** limit number of time series
+   *  deprecated - use series_limit instead */
   limit?: number;
   /** limit number of row in the results */
   row_limit?: string | number | null;
   /** row offset for server side pagination */
   row_offset?: string | number | null;
-  /** The metric used to order timeseries for limiting */
+  /** The metric used to order timeseries for limiting
+   *  deprecated - use series_limit_metric instead */
   timeseries_limit_metric?: QueryFormMetric;
   /** Force refresh */
   force?: boolean;
@@ -184,7 +186,7 @@ export interface BaseFormData extends TimeRange, FormDataResidual {
   /** limit number of series */
   series_columns?: QueryFormColumn[];
   series_limit?: number;
-  series_limit_metric?: QueryFormColumn;
+  series_limit_metric?: QueryFormMetric;
 }
 
 /**

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

@@ -119,15 +119,26 @@ describe('buildQueryObject', () => {
     expect(query.metrics).toEqual(['sum__num', 'avg__num']);
   });
 
-  it('should build limit', () => {
-    const limit = 2;
+  it('should build series_limit from legacy control', () => {
+    const series_limit = 2;
     query = buildQueryObject({
       datasource: '5__table',
       granularity_sqla: 'ds',
       viz_type: 'table',
-      limit,
+      limit: series_limit,
     });
-    expect(query.timeseries_limit).toEqual(limit);
+    expect(query.series_limit).toEqual(series_limit);
+  });
+
+  it('should build series_limit', () => {
+    const series_limit = 2;
+    query = buildQueryObject({
+      datasource: '5__table',
+      granularity_sqla: 'ds',
+      viz_type: 'table',
+      series_limit,
+    });
+    expect(query.series_limit).toEqual(series_limit);
   });
 
   it('should build order_desc', () => {
@@ -141,15 +152,15 @@ describe('buildQueryObject', () => {
     expect(query.order_desc).toEqual(orderDesc);
   });
 
-  it('should build timeseries_limit_metric', () => {
+  it('should build series_limit_metric from legacy control', () => {
     const metric = 'country';
     query = buildQueryObject({
       datasource: '5__table',
       granularity_sqla: 'ds',
       viz_type: 'table',
       timeseries_limit_metric: metric,
     });
-    expect(query.timeseries_limit_metric).toEqual(metric);
+    expect(query.series_limit_metric).toEqual(metric);
   });
 
   it('should build series_limit_metric', () => {
@@ -291,6 +302,26 @@ describe('buildQueryObject', () => {
     ).toBeUndefined();
   });
 
+  it('should populate granularity', () => {
+    const granularity = 'ds';
+    query = buildQueryObject({
+      datasource: '5__table',
+      granularity,
+      viz_type: 'table',
+    });
+    expect(query.granularity).toEqual(granularity);
+  });
+
+  it('should populate granularity from legacy field', () => {
+    const granularity = 'ds';
+    query = buildQueryObject({
+      datasource: '5__table',
+      granularity_sqla: granularity,
+      viz_type: 'table',
+    });
+    expect(query.granularity).toEqual(granularity);
+  });
+
   it('should populate custom_params', () => {
     const customParams: JsonObject = {
       customObject: { id: 137, name: 'C-137' },

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

@@ -29,13 +29,13 @@ describe('normalizeOrderBy', () => {
     expect(normalizeOrderBy(query)).toEqual(query);
   });
 
-  it('has timeseries_limit_metric in queryObject', () => {
+  it('has series_limit_metric in queryObject', () => {
     const query: QueryObject = {
       datasource: '5__table',
       viz_type: 'table',
       time_range: '1 year ago : 2013',
       metrics: ['count(*)'],
-      timeseries_limit_metric: {
+      series_limit_metric: {
         expressionType: 'SIMPLE',
         column: {
           id: 1,
@@ -46,7 +46,7 @@ describe('normalizeOrderBy', () => {
       order_desc: true,
     };
     const expectedQueryObject = normalizeOrderBy(query);
-    expect(expectedQueryObject).not.toHaveProperty('timeseries_limit_metric');
+    expect(expectedQueryObject).not.toHaveProperty('series_limit_metric');
     expect(expectedQueryObject).not.toHaveProperty('order_desc');
     expect(expectedQueryObject).toEqual({
       datasource: '5__table',
@@ -118,7 +118,7 @@ describe('normalizeOrderBy', () => {
       order_desc: true,
     };
     const expectedQueryObject = normalizeOrderBy(query);
-    expect(expectedQueryObject).not.toHaveProperty('timeseries_limit_metric');
+    expect(expectedQueryObject).not.toHaveProperty('series_limit_metric');
     expect(expectedQueryObject).not.toHaveProperty('order_desc');
     expect(expectedQueryObject).toEqual({
       datasource: '5__table',

superset-frontend/packages/superset-ui-core/test/query/types/Metric.test.ts

@@ -43,8 +43,10 @@ const adhocMetricSQL = {
   sqlExpression: 'count(*)',
 };
 
+const savedMetric = 'count(*)';
+
 test('isSavedMetric returns true', () => {
-  expect(isSavedMetric('count(*)')).toEqual(true);
+  expect(isSavedMetric(savedMetric)).toEqual(true);
 });
 
 test('isSavedMetric returns false', () => {
@@ -76,7 +78,7 @@ test('isAdhocMetricSQL returns false', () => {
 test('isQueryFormMetric returns true', () => {
   expect(isQueryFormMetric(adhocMetricSQL)).toEqual(true);
   expect(isQueryFormMetric(adhocMetricSimple)).toEqual(true);
-  expect(isQueryFormMetric('count(*)')).toEqual(true);
+  expect(isQueryFormMetric(savedMetric)).toEqual(true);
 });
 
 test('isQueryFormMetric returns false', () => {

superset-frontend/plugins/plugin-chart-echarts/src/Radar/buildQuery.ts

@@ -23,8 +23,8 @@ import {
 } from '@superset-ui/core';
 
 export default function buildQuery(formData: QueryFormData) {
-  const { timeseries_limit_metric } = formData;
-  const sortByMetric = ensureIsArray(timeseries_limit_metric)[0];
+  const { series_limit_metric } = formData;
+  const sortByMetric = ensureIsArray(series_limit_metric)[0];
 
   return buildQueryContext(formData, baseQueryObject => {
     let { metrics, orderby = [] } = baseQueryObject;

superset-frontend/plugins/plugin-chart-echarts/test/MixedTimeseries/buildQuery.test.ts

@@ -107,9 +107,8 @@ test('should compile query object A', () => {
     row_limit: 10,
     row_offset: undefined,
     series_columns: ['foo'],
-    series_limit: undefined,
+    series_limit: 5,
     series_limit_metric: undefined,
-    timeseries_limit: 5,
     url_params: {},
     custom_params: {},
     custom_form_data: {},
@@ -167,9 +166,8 @@ test('should compile query object B', () => {
     row_limit: 100,
     row_offset: undefined,
     series_columns: [],
-    series_limit: undefined,
+    series_limit: 0,
     series_limit_metric: undefined,
-    timeseries_limit: 0,
     url_params: {},
     custom_params: {},
     custom_form_data: {},
@@ -313,9 +311,8 @@ test('should convert a queryObject with x-axis although FF is disabled', () => {
     row_limit: 10,
     row_offset: undefined,
     series_columns: ['foo'],
-    series_limit: undefined,
+    series_limit: 5,
     series_limit_metric: undefined,
-    timeseries_limit: 5,
     url_params: {},
     custom_params: {},
     custom_form_data: {},

superset-frontend/plugins/plugin-chart-echarts/test/Timeseries/buildQuery.test.ts

@@ -41,7 +41,7 @@ describe('Timeseries buildQuery', () => {
     });
     const [query] = queryContext.queries;
     expect(query.metrics).toEqual(['bar', 'baz']);
-    expect(query.timeseries_limit_metric).toEqual('bar');
+    expect(query.series_limit_metric).toEqual('bar');
     expect(query.order_desc).toEqual(true);
     expect(query.orderby).toEqual([['bar', false]]);
   });
@@ -55,7 +55,7 @@ describe('Timeseries buildQuery', () => {
     });
     const [query] = queryContext.queries;
     expect(query.metrics).toEqual(['bar', 'baz']);
-    expect(query.timeseries_limit_metric).toEqual('bar');
+    expect(query.series_limit_metric).toEqual('bar');
     expect(query.order_desc).toEqual(true);
     expect(query.orderby).toEqual([['foo', true]]);
   });

superset-frontend/src/filters/components/Range/buildQuery.ts

@@ -44,7 +44,6 @@ export default function buildQuery(formData: QueryFormData) {
     {
       ...baseQueryObject,
       columns: [],
-      groupby: [],
       metrics: [
         {
           aggregate: 'MIN',

superset-frontend/src/filters/components/Select/buildQuery.test.ts

@@ -41,7 +41,7 @@ describe('Select buildQuery', () => {
     const queryContext = buildQuery(formData);
     expect(queryContext.queries.length).toEqual(1);
     const [query] = queryContext.queries;
-    expect(query.groupby).toEqual(['my_col']);
+    expect(query.columns).toEqual(['my_col']);
     expect(query.filters).toEqual([]);
     expect(query.metrics).toEqual([]);
     expect(query.orderby).toEqual([]);
@@ -55,7 +55,7 @@ describe('Select buildQuery', () => {
     });
     expect(queryContext.queries.length).toEqual(1);
     const [query] = queryContext.queries;
-    expect(query.groupby).toEqual(['my_col']);
+    expect(query.columns).toEqual(['my_col']);
     expect(query.metrics).toEqual(['my_metric']);
     expect(query.orderby).toEqual([['my_metric', false]]);
   });
@@ -68,7 +68,7 @@ describe('Select buildQuery', () => {
     });
     expect(queryContext.queries.length).toEqual(1);
     const [query] = queryContext.queries;
-    expect(query.groupby).toEqual(['my_col']);
+    expect(query.columns).toEqual(['my_col']);
     expect(query.metrics).toEqual(['my_metric']);
     expect(query.orderby).toEqual([['my_metric', true]]);
   });
@@ -80,7 +80,7 @@ describe('Select buildQuery', () => {
     });
     expect(queryContext.queries.length).toEqual(1);
     const [query] = queryContext.queries;
-    expect(query.groupby).toEqual(['my_col']);
+    expect(query.columns).toEqual(['my_col']);
     expect(query.metrics).toEqual([]);
     expect(query.orderby).toEqual([['my_col', true]]);
   });
@@ -92,7 +92,7 @@ describe('Select buildQuery', () => {
     });
     expect(queryContext.queries.length).toEqual(1);
     const [query] = queryContext.queries;
-    expect(query.groupby).toEqual(['my_col']);
+    expect(query.columns).toEqual(['my_col']);
     expect(query.metrics).toEqual([]);
     expect(query.orderby).toEqual([['my_col', false]]);
   });

superset-frontend/src/filters/components/Select/buildQuery.ts

@@ -63,7 +63,7 @@ const buildQuery: BuildQuery<PluginFilterSelectQueryFormData> = (
     const query: QueryObject[] = [
       {
         ...baseQueryObject,
-        groupby: columns,
+        columns,
         metrics: sortMetric ? [sortMetric] : [],
         filters: filters.concat(extraFilters),
         orderby: