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

This commit is contained in:
Ville Brofeldt 2022-11-30 15:54:11 +02:00 committed by GitHub
parent 91d19056cf
commit b1f8fd4f64
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
13 changed files with 79 additions and 53 deletions

View File

@ -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,

View File

@ -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]],
};
}

View File

@ -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;

View File

@ -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;
}
/**

View File

@ -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,7 +152,7 @@ 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',
@ -149,7 +160,7 @@ describe('buildQueryObject', () => {
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' },

View File

@ -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',

View File

@ -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', () => {

View File

@ -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;

View File

@ -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: {},

View File

@ -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]]);
});

View File

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

View File

@ -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]]);
});

View File

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