diff --git a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-control-utils/src/shared-controls.tsx b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-control-utils/src/shared-controls.tsx index a2e38fde7618..ff87fea744db 100644 --- a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-control-utils/src/shared-controls.tsx +++ b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-control-utils/src/shared-controls.tsx @@ -144,7 +144,7 @@ type Control = { const groupByControl = { type: 'SelectControl', - controlGroup: 'groupby', + queryField: 'groupby', multi: true, freeForm: true, label: t('Group by'), @@ -174,7 +174,7 @@ const groupByControl = { const metrics = { type: 'MetricsControl', - controlGroup: 'metrics', + queryField: 'metrics', multi: true, label: t('Metrics'), validators: [validateNonEmpty], diff --git a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-query/src/buildQueryObject.ts b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-query/src/buildQueryObject.ts index 9585ffc1e1fb..1a71c39d2d7c 100644 --- a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-query/src/buildQueryObject.ts +++ b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-query/src/buildQueryObject.ts @@ -1,10 +1,11 @@ /* eslint-disable camelcase */ import { QueryObject } from './types/Query'; -import { QueryFormData, isSqlaFormData } from './types/QueryFormData'; +import { isSqlaFormData, QueryFormData } from './types/QueryFormData'; +import processGroupby from './processGroupby'; import convertMetric from './convertMetric'; import processFilters from './processFilters'; -import processMetrics from './processMetrics'; import processExtras from './processExtras'; +import extractQueryFields from './extractQueryFields'; export const DTTM_ALIAS = '__timestamp'; @@ -24,23 +25,24 @@ export default function buildQueryObject(formData: T): time_range, since, until, - columns = [], - groupby = [], order_desc, row_limit, limit, timeseries_limit_metric, + queryFields, + ...residualFormData } = formData; - const groupbySet = new Set([...columns, ...groupby]); const numericRowLimit = Number(row_limit); + const { metrics, groupby, columns } = extractQueryFields(residualFormData, queryFields); + const groupbySet = new Set([...columns, ...groupby]); - const queryObject: QueryObject = { + return { extras: processExtras(formData), granularity: processGranularity(formData), - groupby: Array.from(groupbySet), + groupby: processGroupby(Array.from(groupbySet)), is_timeseries: groupbySet.has(DTTM_ALIAS), - metrics: processMetrics(formData), + metrics: metrics.map(convertMetric), order_desc: typeof order_desc === 'undefined' ? true : order_desc, orderby: [], row_limit: row_limit == null || Number.isNaN(numericRowLimit) ? undefined : numericRowLimit, @@ -53,6 +55,4 @@ export default function buildQueryObject(formData: T): until, ...processFilters(formData), }; - - return queryObject; } diff --git a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-query/src/convertMetric.ts b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-query/src/convertMetric.ts index 078c99fd2d45..076f00e791f6 100644 --- a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-query/src/convertMetric.ts +++ b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-query/src/convertMetric.ts @@ -1,4 +1,4 @@ -import { QueryFormDataMetric } from './types/QueryFormData'; +import { QueryFormResidualDataValue } from './types/QueryFormData'; import { QueryObjectMetric } from './types/Query'; import { AdhocMetric } from './types/Metric'; @@ -9,7 +9,7 @@ function getDefaultLabel(metric: AdhocMetric) { return metric.sqlExpression; } -export default function convertMetric(metric: QueryFormDataMetric): QueryObjectMetric { +export default function convertMetric(metric: QueryFormResidualDataValue): QueryObjectMetric { let formattedMetric; if (typeof metric === 'string') { formattedMetric = { diff --git a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-query/src/extractQueryFields.ts b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-query/src/extractQueryFields.ts new file mode 100644 index 000000000000..511f3b7578ca --- /dev/null +++ b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-query/src/extractQueryFields.ts @@ -0,0 +1,30 @@ +import { QueryFields, QueryFormResidualData } from './types/QueryFormData'; +import { QueryFieldData } from './types/Query'; + +export default function extractQueryFields( + residualFormData: QueryFormResidualData, + queryFields?: QueryFields, +) { + const queryFieldAliases: QueryFields = { + /** These are predefined for backward compatibility */ + metric: 'metrics', + percent_metrics: 'metrics', + metric_2: 'metrics', + secondary_metric: 'metrics', + x: 'metrics', + y: 'metrics', + size: 'metrics', + ...queryFields, + }; + const finalQueryFields: QueryFieldData = { + columns: [], + groupby: [], + metrics: [], + }; + Object.entries(residualFormData).forEach(entry => { + const [key, residualValue] = entry; + const normalizedKey = queryFieldAliases[key] || key; + finalQueryFields[normalizedKey] = (finalQueryFields[normalizedKey] || []).concat(residualValue); + }); + return finalQueryFields; +} diff --git a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-query/src/processGroupby.ts b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-query/src/processGroupby.ts new file mode 100644 index 000000000000..19ad008206ee --- /dev/null +++ b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-query/src/processGroupby.ts @@ -0,0 +1,11 @@ +import { QueryFormResidualDataValue } from './types/QueryFormData'; + +export default function processGroupby(groupby: QueryFormResidualDataValue[]): string[] { + const groupbyList: string[] = []; + groupby.forEach(value => { + if (typeof value === 'string') { + groupbyList.push(value); + } + }); + return groupbyList; +} diff --git a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-query/src/processMetrics.ts b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-query/src/processMetrics.ts deleted file mode 100644 index aa07ee468751..000000000000 --- a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-query/src/processMetrics.ts +++ /dev/null @@ -1,25 +0,0 @@ -import { QueryFormData } from './types/QueryFormData'; -import { QueryObjectMetric } from './types/Query'; -import { MetricKey } from './types/Metric'; -import convertMetric from './convertMetric'; - -export default function processMetrics(formData: QueryFormData) { - // Use Array to maintain insertion order - // for metrics that are order sensitive - const metrics: QueryObjectMetric[] = []; - - Object.keys(MetricKey).forEach(key => { - const metric = formData[MetricKey[key as keyof typeof MetricKey]]; - if (metric) { - if (Array.isArray(metric)) { - metric.forEach(m => { - metrics.push(convertMetric(m)); - }); - } else { - metrics.push(convertMetric(metric)); - } - } - }); - - return metrics; -} diff --git a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-query/src/types/Metric.ts b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-query/src/types/Metric.ts index df6fb28e6129..e382129cd5c2 100644 --- a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-query/src/types/Metric.ts +++ b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-query/src/types/Metric.ts @@ -1,18 +1,5 @@ import { Column } from './Column'; -// Note that the values of MetricKeys are lower_snake_case because they're -// used as keys of form data jsons. -export enum MetricKey { - METRIC = 'metric', - METRICS = 'metrics', - PERCENT_METRICS = 'percent_metrics', - RIGHT_AXIS_METRIC = 'metric_2', - SECONDARY_METRIC = 'secondary_metric', - X = 'x', - Y = 'y', - SIZE = 'size', -} - export type Aggregate = 'AVG' | 'COUNT' | 'COUNT_DISTINCT' | 'MAX' | 'MIN' | 'SUM'; interface AdhocMetricSimple { diff --git a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-query/src/types/Query.ts b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-query/src/types/Query.ts index dddabccd1073..fde0291573f8 100644 --- a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-query/src/types/Query.ts +++ b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-query/src/types/Query.ts @@ -3,6 +3,7 @@ import { DatasourceType } from './Datasource'; import { AdhocMetric } from './Metric'; import { BinaryOperator, SetOperator, UnaryOperator } from './Operator'; import { TimeRange } from './Time'; +import { QueryFormDataMetric, QueryFormResidualDataValue } from './QueryFormData'; export type QueryObjectFilterClause = { col: string; @@ -40,6 +41,10 @@ export type QueryObjectExtras = Partial<{ where?: string; }>; +export type ResidualQueryObjectData = { + [key: string]: unknown; +}; + export type QueryObject = { /** Columns to group by */ groupby?: string[]; @@ -73,7 +78,8 @@ export type QueryObject = { /** If set, will group by timestamp */ is_timeseries?: boolean; -} & TimeRange; +} & TimeRange & + ResidualQueryObjectData; export interface QueryContext { datasource: { @@ -84,3 +90,10 @@ export interface QueryContext { force: boolean; queries: QueryObject[]; } + +export type QueryFieldData = { + columns: QueryFormResidualDataValue[]; + groupby: QueryFormResidualDataValue[]; + metrics: QueryFormDataMetric[]; + [key: string]: QueryFormResidualDataValue[]; +}; diff --git a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-query/src/types/QueryFormData.ts b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-query/src/types/QueryFormData.ts index 88de3dc5097c..388a1127ea4c 100644 --- a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-query/src/types/QueryFormData.ts +++ b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-query/src/types/QueryFormData.ts @@ -1,18 +1,18 @@ /* eslint camelcase: 0 */ // FormData uses snake_cased keys. -import { MetricKey, AdhocMetric } from './Metric'; +import { AdhocMetric } from './Metric'; import { TimeRange } from './Time'; import { AdhocFilter } from './Filter'; export type QueryFormDataMetric = string | AdhocMetric; - -// Define mapped type separately to work around a limitation of TypeScript -// https://github.com/Microsoft/TypeScript/issues/13573 -// The Metrics in formData is either a string or a proper metric. It will be -// unified into a proper Metric type during buildQuery (see `/query/Metrics.ts`). -export type QueryFormDataMetrics = Partial< - Record ->; +export type QueryFormResidualDataValue = string | AdhocMetric; +export type QueryFormResidualData = { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + [key: string]: any; +}; +export type QueryFields = { + [key: string]: string; +}; // Type signature for formData shared by all viz types // It will be gradually filled out as we build out the query object @@ -44,11 +44,12 @@ export type BaseFormData = { /** limit number of row in the results */ row_limit?: string | number | null; /** The metric used to order timeseries for limiting */ - timeseries_limit_metric?: QueryFormDataMetric; + timeseries_limit_metric?: QueryFormResidualDataValue; /** Force refresh */ force?: boolean; + queryFields?: QueryFields; } & TimeRange & - QueryFormDataMetrics; + QueryFormResidualData; // FormData is either sqla-based or druid-based export type SqlaFormData = { diff --git a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-query/test/buildQueryObject.test.ts b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-query/test/buildQueryObject.test.ts index e901b4040398..42aed0b3a444 100644 --- a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-query/test/buildQueryObject.test.ts +++ b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-query/test/buildQueryObject.test.ts @@ -3,7 +3,7 @@ import { buildQueryObject, QueryObject } from '../src'; describe('buildQueryObject', () => { let query: QueryObject; - it('should build granularity for sql alchemy datasources', () => { + it('should build granularity for sqlalchemy datasources', () => { query = buildQueryObject({ datasource: '5__table', granularity_sqla: 'ds', @@ -12,7 +12,7 @@ describe('buildQueryObject', () => { expect(query.granularity).toEqual('ds'); }); - it('should build granularity for sql druid datasources', () => { + it('should build granularity for druid datasources', () => { query = buildQueryObject({ datasource: '5__druid', granularity: 'ds', @@ -21,16 +21,40 @@ describe('buildQueryObject', () => { expect(query.granularity).toEqual('ds'); }); - it('should build metrics', () => { + it('should build metrics based on default queryFields', () => { query = buildQueryObject({ datasource: '5__table', granularity_sqla: 'ds', viz_type: 'table', metric: 'sum__num', + secondary_metric: 'avg__num', + }); + expect(query.metrics).toEqual([{ label: 'sum__num' }, { label: 'avg__num' }]); + }); + + it('should group custom metric control', () => { + query = buildQueryObject({ + datasource: '5__table', + granularity_sqla: 'ds', + viz_type: 'table', + my_custom_metric_control: 'sum__num', + queryFields: { my_custom_metric_control: 'metrics' }, }); expect(query.metrics).toEqual([{ label: 'sum__num' }]); }); + it('should group custom metric control with predefined metrics', () => { + query = buildQueryObject({ + datasource: '5__table', + granularity_sqla: 'ds', + viz_type: 'table', + metrics: ['sum__num'], + my_custom_metric_control: 'avg__num', + queryFields: { my_custom_metric_control: 'metrics' }, + }); + expect(query.metrics).toEqual([{ label: 'sum__num' }, { label: 'avg__num' }]); + }); + it('should build limit', () => { const limit = 2; query = buildQueryObject({ diff --git a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-query/test/extractQueryFields.test.ts b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-query/test/extractQueryFields.test.ts new file mode 100644 index 000000000000..8500766280c7 --- /dev/null +++ b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-query/test/extractQueryFields.test.ts @@ -0,0 +1,63 @@ +import extractQueryFields from '../src/extractQueryFields'; + +describe('extractQueryFields', () => { + it('should return default object', () => { + expect(extractQueryFields({})).toEqual({ + columns: [], + groupby: [], + metrics: [], + }); + }); + + it('should group default metric controls to metrics', () => { + expect(extractQueryFields({ metric: 'my_metric' }).metrics).toEqual(['my_metric']); + }); + + it('should group custom metrics with default metrics', () => { + expect( + extractQueryFields( + { metric: 'metric_1', my_custom_metric: 'metric_2' }, + { my_custom_metric: 'metrics' }, + ).metrics, + ).toEqual(['metric_1', 'metric_2']); + }); + + it('should extract columns', () => { + expect(extractQueryFields({ columns: 'col_1' })).toEqual({ + columns: ['col_1'], + groupby: [], + metrics: [], + }); + }); + + it('should extract groupby', () => { + expect(extractQueryFields({ groupby: 'col_1' })).toEqual({ + columns: [], + groupby: ['col_1'], + metrics: [], + }); + }); + + it('should extract custom groupby', () => { + expect( + extractQueryFields({ series: 'col_1', metric: 'metric_1' }, { series: 'groupby' }), + ).toEqual({ + columns: [], + groupby: ['col_1'], + metrics: ['metric_1'], + }); + }); + + it('should merge custom groupby with default group', () => { + expect( + extractQueryFields( + { groupby: 'col_1', series: 'col_2', metric: 'metric_1' }, + { series: 'groupby' }, + ), + ).toEqual({ + columns: [], + groupby: ['col_1', 'col_2'], + metrics: ['metric_1'], + }); + }); +}); diff --git a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-query/test/processGroupby.test.ts b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-query/test/processGroupby.test.ts new file mode 100644 index 000000000000..e2adef546d8d --- /dev/null +++ b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-query/test/processGroupby.test.ts @@ -0,0 +1,12 @@ +import processGroupby from '../src/processGroupby'; + +describe('processGroupby', () => { + it('should handle array of strings', () => { + expect(processGroupby(['foo', 'bar'])).toEqual(['foo', 'bar']); + }); + + it('should exclude non-string values', () => { + // @ts-ignore, change to @ts-expect-error when updated to TypeScript>=3.9 + expect(processGroupby(['bar', 1, undefined, null, 'foo'])).toEqual(['bar', 'foo']); + }); +}); diff --git a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-query/test/processMetrics.test.ts b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-query/test/processMetrics.test.ts deleted file mode 100644 index 7536d815833d..000000000000 --- a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-query/test/processMetrics.test.ts +++ /dev/null @@ -1,66 +0,0 @@ -import { ColumnType } from '../src'; -import processMetrics from '../src/processMetrics'; - -describe('processMetrics', () => { - const formData = { - datasource: '5__table', - granularity_sqla: 'ds', - viz_type: 'word_cloud', - }; - - it('should handle single metric', () => { - const metrics = processMetrics({ - ...formData, - metric: 'sum__num', - }); - expect(metrics).toEqual([{ label: 'sum__num' }]); - }); - - it('should handle an array of metrics', () => { - const metrics = processMetrics({ - ...formData, - metrics: ['sum__num'], - }); - expect(metrics).toEqual([{ label: 'sum__num' }]); - }); - - it('should handle multiple types of metrics', () => { - const metrics = processMetrics({ - ...formData, - metrics: [ - 'sum__num', - { - aggregate: 'AVG', - column: { - columnName: 'sum_girls', - id: 5, - type: ColumnType.BIGINT, - }, - expressionType: 'SIMPLE', - }, - { - expressionType: 'SQL', - sqlExpression: 'COUNT(sum_girls)', - }, - ], - }); - expect(metrics).toEqual([ - { label: 'sum__num' }, - { - aggregate: 'AVG', - column: { - columnName: 'sum_girls', - id: 5, - type: ColumnType.BIGINT, - }, - expressionType: 'SIMPLE', - label: 'AVG(sum_girls)', - }, - { - expressionType: 'SQL', - label: 'COUNT(sum_girls)', - sqlExpression: 'COUNT(sum_girls)', - }, - ]); - }); -}); diff --git a/superset-frontend/temporary_superset_ui/superset-ui/plugins/plugin-chart-word-cloud/src/plugin/buildQuery.ts b/superset-frontend/temporary_superset_ui/superset-ui/plugins/plugin-chart-word-cloud/src/plugin/buildQuery.ts index 2a3a713213f3..e31c39d732a8 100644 --- a/superset-frontend/temporary_superset_ui/superset-ui/plugins/plugin-chart-word-cloud/src/plugin/buildQuery.ts +++ b/superset-frontend/temporary_superset_ui/superset-ui/plugins/plugin-chart-word-cloud/src/plugin/buildQuery.ts @@ -6,7 +6,6 @@ export default function buildQuery(formData: WordCloudFormData) { return buildQueryContext(formData, baseQueryObject => [ { ...baseQueryObject, - groupby: [formData.series], }, ]); } diff --git a/superset-frontend/temporary_superset_ui/superset-ui/plugins/plugin-chart-word-cloud/test/plugin/buildQuery.test.ts b/superset-frontend/temporary_superset_ui/superset-ui/plugins/plugin-chart-word-cloud/test/plugin/buildQuery.test.ts index ecc8013183f7..9213fc01f8b4 100644 --- a/superset-frontend/temporary_superset_ui/superset-ui/plugins/plugin-chart-word-cloud/test/plugin/buildQuery.test.ts +++ b/superset-frontend/temporary_superset_ui/superset-ui/plugins/plugin-chart-word-cloud/test/plugin/buildQuery.test.ts @@ -7,6 +7,7 @@ describe('WordCloud buildQuery', () => { granularity_sqla: 'ds', series: 'foo', viz_type: 'word_cloud', + queryFields: { series: 'groupby' }, }; it('should build groupby with series in form data', () => {