From 33f8b4c5b5c44ce260da69f2036434211d46b8cd Mon Sep 17 00:00:00 2001 From: Yongjie Zhao Date: Thu, 8 Jul 2021 02:38:55 +0100 Subject: [PATCH] feat: Normalize orderby clause (#1206) * feat: normalize orderby clause * fix ut --- .../superset-ui-core/src/query/index.ts | 1 + .../src/query/normalizeOrderBy.ts | 64 ++++++++ .../test/query/normalizeOrderBy.test.ts | 150 ++++++++++++++++++ .../src/MixedTimeseries/buildQuery.ts | 81 +++++----- 4 files changed, 257 insertions(+), 39 deletions(-) create mode 100644 superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-core/src/query/normalizeOrderBy.ts create mode 100644 superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-core/test/query/normalizeOrderBy.test.ts diff --git a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-core/src/query/index.ts b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-core/src/query/index.ts index 48d6f0e86ea09..2e476b26208e3 100644 --- a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-core/src/query/index.ts +++ b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-core/src/query/index.ts @@ -26,6 +26,7 @@ export { default as convertFilter } from './convertFilter'; export { default as extractTimegrain } from './extractTimegrain'; export { default as getMetricLabel } from './getMetricLabel'; export { default as DatasourceKey } from './DatasourceKey'; +export { default as normalizeOrderBy } from './normalizeOrderBy'; export * from './types/AnnotationLayer'; export * from './types/QueryFormData'; diff --git a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-core/src/query/normalizeOrderBy.ts b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-core/src/query/normalizeOrderBy.ts new file mode 100644 index 0000000000000..dca1398ef10a0 --- /dev/null +++ b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-core/src/query/normalizeOrderBy.ts @@ -0,0 +1,64 @@ +/** + * 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 isEmpty from 'lodash/isEmpty'; +import isBoolean from 'lodash/isBoolean'; + +import { QueryObject } from './types'; + +export default function normalizeOrderBy(queryObject: QueryObject): QueryObject { + if (Array.isArray(queryObject.orderby) && queryObject.orderby.length > 0) { + // ensure a valid orderby clause + const orderbyClause = queryObject.orderby[0]; + if ( + Array.isArray(orderbyClause) && + orderbyClause.length === 2 && + !isEmpty(orderbyClause[0]) && + isBoolean(orderbyClause[1]) + ) { + return queryObject; + } + } + + // ensure that remove invalid orderby clause + const cloneQueryObject = { ...queryObject }; + delete cloneQueryObject.timeseries_limit_metric; + 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) + ) { + return { + ...cloneQueryObject, + orderby: [[queryObject.timeseries_limit_metric, isAsc]], + }; + } + + if (Array.isArray(queryObject.metrics) && queryObject.metrics.length > 0) { + return { + ...cloneQueryObject, + orderby: [[queryObject.metrics[0], isAsc]], + }; + } + + return cloneQueryObject; +} diff --git a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-core/test/query/normalizeOrderBy.test.ts b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-core/test/query/normalizeOrderBy.test.ts new file mode 100644 index 0000000000000..78ebb570e5343 --- /dev/null +++ b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-core/test/query/normalizeOrderBy.test.ts @@ -0,0 +1,150 @@ +/** + * 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 { normalizeOrderBy, QueryObject } from '@superset-ui/core/src/query'; + +describe('normalizeOrderBy', () => { + it('should not change original queryObject when orderby populated', () => { + const query: QueryObject = { + datasource: '5__table', + viz_type: 'table', + time_range: '1 year ago : 2013', + orderby: [['count(*)', true]], + }; + expect(normalizeOrderBy(query)).toEqual(query); + }); + + it('has timeseries_limit_metric in queryObject', () => { + const query: QueryObject = { + datasource: '5__table', + viz_type: 'table', + time_range: '1 year ago : 2013', + metrics: ['count(*)'], + timeseries_limit_metric: { + expressionType: 'SIMPLE', + column: { + id: 1, + column_name: 'sales', + }, + aggregate: 'SUM', + }, + order_desc: true, + }; + const expectedQueryObject = normalizeOrderBy(query); + expect(expectedQueryObject).not.toHaveProperty('timeseries_limit_metric'); + expect(expectedQueryObject).not.toHaveProperty('order_desc'); + expect(expectedQueryObject).toEqual({ + datasource: '5__table', + viz_type: 'table', + time_range: '1 year ago : 2013', + metrics: ['count(*)'], + orderby: [ + [ + { + expressionType: 'SIMPLE', + column: { + id: 1, + column_name: 'sales', + }, + aggregate: 'SUM', + }, + false, + ], + ], + }); + }); + + it('has metrics in queryObject', () => { + const query: QueryObject = { + datasource: '5__table', + viz_type: 'table', + time_range: '1 year ago : 2013', + metrics: ['count(*)'], + order_desc: true, + }; + const expectedQueryObject = normalizeOrderBy(query); + expect(expectedQueryObject).not.toHaveProperty('timeseries_limit_metric'); + expect(expectedQueryObject).not.toHaveProperty('order_desc'); + expect(expectedQueryObject).toEqual({ + datasource: '5__table', + viz_type: 'table', + time_range: '1 year ago : 2013', + metrics: ['count(*)'], + orderby: [['count(*)', false]], + }); + }); + + it('should not change', () => { + const query: QueryObject = { + datasource: '5__table', + viz_type: 'table', + time_range: '1 year ago : 2013', + }; + expect(normalizeOrderBy(query)).toEqual(query); + }); + + it('remove empty orderby', () => { + const query: QueryObject = { + datasource: '5__table', + viz_type: 'table', + time_range: '1 year ago : 2013', + orderby: [], + }; + expect(normalizeOrderBy(query)).not.toHaveProperty('orderby'); + }); + + it('remove orderby with an empty array', () => { + const query: QueryObject = { + datasource: '5__table', + viz_type: 'table', + time_range: '1 year ago : 2013', + orderby: [[]], + }; + expect(normalizeOrderBy(query)).not.toHaveProperty('orderby'); + }); + + it('remove orderby with an empty metric', () => { + const query: QueryObject = { + datasource: '5__table', + viz_type: 'table', + time_range: '1 year ago : 2013', + orderby: [['', true]], + }; + expect(normalizeOrderBy(query)).not.toHaveProperty('orderby'); + }); + + it('remove orderby with an empty adhoc metric', () => { + const query: QueryObject = { + datasource: '5__table', + viz_type: 'table', + time_range: '1 year ago : 2013', + orderby: [[{}, true]], + }; + expect(normalizeOrderBy(query)).not.toHaveProperty('orderby'); + }); + + it('remove orderby with an non-boolean type', () => { + const query: QueryObject = { + datasource: '5__table', + viz_type: 'table', + time_range: '1 year ago : 2013', + orderby: [['count(*)', 'true']], + }; + expect(normalizeOrderBy(query)).not.toHaveProperty('orderby'); + }); +}); diff --git a/superset-frontend/temporary_superset_ui/superset-ui/plugins/plugin-chart-echarts/src/MixedTimeseries/buildQuery.ts b/superset-frontend/temporary_superset_ui/superset-ui/plugins/plugin-chart-echarts/src/MixedTimeseries/buildQuery.ts index 6f423975beb36..d9c636e5e10fc 100644 --- a/superset-frontend/temporary_superset_ui/superset-ui/plugins/plugin-chart-echarts/src/MixedTimeseries/buildQuery.ts +++ b/superset-frontend/temporary_superset_ui/superset-ui/plugins/plugin-chart-echarts/src/MixedTimeseries/buildQuery.ts @@ -16,7 +16,13 @@ * specific language governing permissions and limitations * under the License. */ -import { buildQueryContext, getMetricLabel, QueryFormData } from '@superset-ui/core'; +import { + buildQueryContext, + getMetricLabel, + QueryFormData, + QueryObject, + normalizeOrderBy, +} from '@superset-ui/core'; export default function buildQuery(formData: QueryFormData) { const { @@ -56,49 +62,46 @@ export default function buildQuery(formData: QueryFormData) { const queryContextA = buildQueryContext(formData1, baseQueryObject => { const metricLabels = (baseQueryObject.metrics || []).map(getMetricLabel); - return [ - { - ...baseQueryObject, - is_timeseries: true, - orderby: timeseries_limit_metric ? [[timeseries_limit_metric, !order_desc]] : [], - post_processing: [ - { - operation: 'pivot', - options: { - index: ['__timestamp'], - columns: formData1.columns || [], - // Create 'dummy' sum aggregates to assign cell values in pivot table - aggregates: Object.fromEntries( - metricLabels.map(metric => [metric, { operator: 'sum' }]), - ), - }, + const queryObjectA = { + ...baseQueryObject, + is_timeseries: true, + post_processing: [ + { + operation: 'pivot', + options: { + index: ['__timestamp'], + columns: formData1.columns || [], + // Create 'dummy' sum aggregates to assign cell values in pivot table + aggregates: Object.fromEntries( + metricLabels.map(metric => [metric, { operator: 'sum' }]), + ), }, - ], - }, - ]; + }, + ], + } as QueryObject; + return [normalizeOrderBy(queryObjectA)]; }); + const queryContextB = buildQueryContext(formData2, baseQueryObject => { const metricLabels = (baseQueryObject.metrics || []).map(getMetricLabel); - return [ - { - ...baseQueryObject, - is_timeseries: true, - orderby: timeseries_limit_metric_b ? [[timeseries_limit_metric_b, !order_desc_b]] : [], - post_processing: [ - { - operation: 'pivot', - options: { - index: ['__timestamp'], - columns: formData2.columns || [], - // Create 'dummy' sum aggregates to assign cell values in pivot table - aggregates: Object.fromEntries( - metricLabels.map(metric => [metric, { operator: 'sum' }]), - ), - }, + const queryObjectB = { + ...baseQueryObject, + is_timeseries: true, + post_processing: [ + { + operation: 'pivot', + options: { + index: ['__timestamp'], + columns: formData2.columns || [], + // Create 'dummy' sum aggregates to assign cell values in pivot table + aggregates: Object.fromEntries( + metricLabels.map(metric => [metric, { operator: 'sum' }]), + ), }, - ], - }, - ]; + }, + ], + } as QueryObject; + return [normalizeOrderBy(queryObjectB)]; }); return {