From 63614fc50ad5c880bbf755d8989cd07cd339843e Mon Sep 17 00:00:00 2001 From: Ville Brofeldt Date: Wed, 13 May 2020 12:23:42 +0300 Subject: [PATCH 01/13] feat: add control grouping functionality --- .../superset-ui-query/src/buildQueryObject.ts | 47 ++++++++++++++--- .../superset-ui-query/src/processMetrics.ts | 22 ++------ .../superset-ui-query/src/types/Metric.ts | 13 ----- packages/superset-ui-query/src/types/Query.ts | 8 ++- .../src/types/QueryFormData.ts | 19 ++++--- .../test/buildQueryObject.test.ts | 27 +++++++++- .../test/processMetrics.test.ts | 52 ++++++------------- 7 files changed, 101 insertions(+), 87 deletions(-) diff --git a/packages/superset-ui-query/src/buildQueryObject.ts b/packages/superset-ui-query/src/buildQueryObject.ts index 9585ffc1e1..c77b1c17a1 100644 --- a/packages/superset-ui-query/src/buildQueryObject.ts +++ b/packages/superset-ui-query/src/buildQueryObject.ts @@ -1,6 +1,6 @@ /* eslint-disable camelcase */ import { QueryObject } from './types/Query'; -import { QueryFormData, isSqlaFormData } from './types/QueryFormData'; +import { QueryFormData, isSqlaFormData, QueryFormDataMetric } from './types/QueryFormData'; import convertMetric from './convertMetric'; import processFilters from './processFilters'; import processMetrics from './processMetrics'; @@ -24,23 +24,58 @@ export default function buildQueryObject(formData: T): time_range, since, until, - columns = [], - groupby = [], order_desc, row_limit, limit, timeseries_limit_metric, + controlGroups = { + /** These are predefined for backward compatibility */ + metric: 'metrics', + percent_metrics: 'metrics', + metric_2: 'metrics', + secondary_metric: 'metrics', + x: 'metrics', + y: 'metrics', + size: 'metrics', + }, + ...residualFormData } = formData; + type GroupedControlData = { + columns: string[]; + groupby: string[]; + metrics: QueryFormDataMetric[]; + }; + type ResidualGroupedControlData = { + [key: string]: QueryFormDataMetric[]; + }; + const groupedControls: GroupedControlData & ResidualGroupedControlData = { + columns: [], + groupby: [], + metrics: [], + }; + + Object.entries(residualFormData).forEach(entry => { + const [key, residualValue] = entry; + if (Object.prototype.hasOwnProperty.call(controlGroups, key)) { + const controlGroup = controlGroups[key]; + const controlValue = residualFormData[key]; + groupedControls[controlGroup] = (groupedControls[controlGroup] || []).concat(controlValue); + } else { + // eslint-disable-next-line @typescript-eslint/no-unsafe-call + groupedControls[key] = (groupedControls[key] || []).concat(residualValue); + } + }); + const { metrics, groupby, columns } = groupedControls; const groupbySet = new Set([...columns, ...groupby]); const numericRowLimit = Number(row_limit); - const queryObject: QueryObject = { + return { extras: processExtras(formData), granularity: processGranularity(formData), groupby: Array.from(groupbySet), is_timeseries: groupbySet.has(DTTM_ALIAS), - metrics: processMetrics(formData), + metrics: processMetrics(metrics), order_desc: typeof order_desc === 'undefined' ? true : order_desc, orderby: [], row_limit: row_limit == null || Number.isNaN(numericRowLimit) ? undefined : numericRowLimit, @@ -53,6 +88,4 @@ export default function buildQueryObject(formData: T): until, ...processFilters(formData), }; - - return queryObject; } diff --git a/packages/superset-ui-query/src/processMetrics.ts b/packages/superset-ui-query/src/processMetrics.ts index aa07ee4687..b75b08fc3d 100644 --- a/packages/superset-ui-query/src/processMetrics.ts +++ b/packages/superset-ui-query/src/processMetrics.ts @@ -1,25 +1,11 @@ -import { QueryFormData } from './types/QueryFormData'; +import { QueryFormDataMetric } 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 +export default function processMetrics(rawMetrics: QueryFormDataMetric[]) { 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)); - } - } + Object.values(rawMetrics).forEach(metric => { + metrics.push(convertMetric(metric)); }); - return metrics; } diff --git a/packages/superset-ui-query/src/types/Metric.ts b/packages/superset-ui-query/src/types/Metric.ts index df6fb28e61..e382129cd5 100644 --- a/packages/superset-ui-query/src/types/Metric.ts +++ b/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/packages/superset-ui-query/src/types/Query.ts b/packages/superset-ui-query/src/types/Query.ts index dddabccd10..a9fb1216a8 100644 --- a/packages/superset-ui-query/src/types/Query.ts +++ b/packages/superset-ui-query/src/types/Query.ts @@ -40,6 +40,11 @@ export type QueryObjectExtras = Partial<{ where?: string; }>; +export type ResidualQueryObjectData = { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + [key: string]: any; +}; + 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: { diff --git a/packages/superset-ui-query/src/types/QueryFormData.ts b/packages/superset-ui-query/src/types/QueryFormData.ts index 88de3dc509..c4dac4b640 100644 --- a/packages/superset-ui-query/src/types/QueryFormData.ts +++ b/packages/superset-ui-query/src/types/QueryFormData.ts @@ -1,18 +1,14 @@ /* 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 ResidualFormData = { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + [key: string]: any; +}; // Type signature for formData shared by all viz types // It will be gradually filled out as we build out the query object @@ -47,8 +43,11 @@ export type BaseFormData = { timeseries_limit_metric?: QueryFormDataMetric; /** Force refresh */ force?: boolean; + controlGroups?: { + [key in string]: string; + }; } & TimeRange & - QueryFormDataMetrics; + ResidualFormData; // FormData is either sqla-based or druid-based export type SqlaFormData = { diff --git a/packages/superset-ui-query/test/buildQueryObject.test.ts b/packages/superset-ui-query/test/buildQueryObject.test.ts index e901b40403..99c7d82360 100644 --- a/packages/superset-ui-query/test/buildQueryObject.test.ts +++ b/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', @@ -72,6 +72,29 @@ describe('buildQueryObject', () => { row_limit: null, }; + it('should group custom metric control', () => { + query = buildQueryObject({ + datasource: '5__table', + granularity_sqla: 'ds', + viz_type: 'table', + my_custom_metric_control: 'sum__num', + controlGroups: { 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', + controlGroups: { my_custom_metric_control: 'metrics' }, + }); + expect(query.metrics).toEqual([{ label: 'sum__num' }, { label: 'avg__num' }]); + }); + // undefined query = buildQueryObject({ ...baseQuery }); expect(query.row_limit).toBeUndefined(); diff --git a/packages/superset-ui-query/test/processMetrics.test.ts b/packages/superset-ui-query/test/processMetrics.test.ts index 7536d81583..ddb0a5625a 100644 --- a/packages/superset-ui-query/test/processMetrics.test.ts +++ b/packages/superset-ui-query/test/processMetrics.test.ts @@ -2,48 +2,28 @@ 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'], - }); + const metrics = processMetrics(['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)', + const metrics = processMetrics([ + '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' }, { From ef65ac30d18180d70da5484985727146d5b64e6b Mon Sep 17 00:00:00 2001 From: Ville Brofeldt Date: Wed, 13 May 2020 12:33:16 +0300 Subject: [PATCH 02/13] implement controlGroups on WordCloud --- plugins/plugin-chart-word-cloud/src/plugin/buildQuery.ts | 1 - plugins/plugin-chart-word-cloud/test/plugin/buildQuery.test.ts | 1 + 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/plugin-chart-word-cloud/src/plugin/buildQuery.ts b/plugins/plugin-chart-word-cloud/src/plugin/buildQuery.ts index 2a3a713213..e31c39d732 100644 --- a/plugins/plugin-chart-word-cloud/src/plugin/buildQuery.ts +++ b/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/plugins/plugin-chart-word-cloud/test/plugin/buildQuery.test.ts b/plugins/plugin-chart-word-cloud/test/plugin/buildQuery.test.ts index ecc8013183..5bbf51adb3 100644 --- a/plugins/plugin-chart-word-cloud/test/plugin/buildQuery.test.ts +++ b/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', + controlGroups: { series: 'groupby' }, }; it('should build groupby with series in form data', () => { From af9f20d704d9762d1ae70feaae1e7ae27a308dce Mon Sep 17 00:00:00 2001 From: Ville Brofeldt Date: Wed, 13 May 2020 13:06:08 +0300 Subject: [PATCH 03/13] fix tests --- .../test/buildQueryObject.test.ts | 49 ++++++++++--------- 1 file changed, 25 insertions(+), 24 deletions(-) diff --git a/packages/superset-ui-query/test/buildQueryObject.test.ts b/packages/superset-ui-query/test/buildQueryObject.test.ts index 99c7d82360..9b694dbda6 100644 --- a/packages/superset-ui-query/test/buildQueryObject.test.ts +++ b/packages/superset-ui-query/test/buildQueryObject.test.ts @@ -21,16 +21,40 @@ describe('buildQueryObject', () => { expect(query.granularity).toEqual('ds'); }); - it('should build metrics', () => { + it('should build metrics based on default controlGroups', () => { 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', + controlGroups: { 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', + controlGroups: { my_custom_metric_control: 'metrics' }, + }); + expect(query.metrics).toEqual([{ label: 'sum__num' }, { label: 'avg__num' }]); + }); + it('should build limit', () => { const limit = 2; query = buildQueryObject({ @@ -72,29 +96,6 @@ describe('buildQueryObject', () => { row_limit: null, }; - it('should group custom metric control', () => { - query = buildQueryObject({ - datasource: '5__table', - granularity_sqla: 'ds', - viz_type: 'table', - my_custom_metric_control: 'sum__num', - controlGroups: { 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', - controlGroups: { my_custom_metric_control: 'metrics' }, - }); - expect(query.metrics).toEqual([{ label: 'sum__num' }, { label: 'avg__num' }]); - }); - // undefined query = buildQueryObject({ ...baseQuery }); expect(query.row_limit).toBeUndefined(); From 0b2020c1b6f3144c927e4284adc7595081b6ee7d Mon Sep 17 00:00:00 2001 From: Ville Brofeldt Date: Wed, 13 May 2020 15:23:47 +0300 Subject: [PATCH 04/13] break up into smaller packages and add unit tests --- .../src/buildGroupedControls.ts | 38 +++++++++++ .../superset-ui-query/src/buildQueryObject.ts | 40 ++---------- .../src/types/ControlData.ts | 10 +++ .../src/types/QueryFormData.ts | 7 ++- .../test/buildGroupedControls.test.ts | 63 +++++++++++++++++++ 5 files changed, 119 insertions(+), 39 deletions(-) create mode 100644 packages/superset-ui-query/src/buildGroupedControls.ts create mode 100644 packages/superset-ui-query/src/types/ControlData.ts create mode 100644 packages/superset-ui-query/test/buildGroupedControls.test.ts diff --git a/packages/superset-ui-query/src/buildGroupedControls.ts b/packages/superset-ui-query/src/buildGroupedControls.ts new file mode 100644 index 0000000000..8f9dc07be2 --- /dev/null +++ b/packages/superset-ui-query/src/buildGroupedControls.ts @@ -0,0 +1,38 @@ +import { ControlGroups, ResidualFormData } from './types/QueryFormData'; +import { GroupedControlData, ResidualGroupedControlData } from './types/ControlData'; + +export default function buildGroupedControlData( + residualFormData: ResidualFormData, + controlGroups?: ControlGroups, +): GroupedControlData & ResidualGroupedControlData { + const defaultedControlGroups: ControlGroups = { + /** These are predefined for backward compatibility */ + metric: 'metrics', + percent_metrics: 'metrics', + metric_2: 'metrics', + secondary_metric: 'metrics', + x: 'metrics', + y: 'metrics', + size: 'metrics', + ...controlGroups, + }; + const groupedControls: GroupedControlData & ResidualGroupedControlData = { + columns: [], + groupby: [], + metrics: [], + }; + + Object.entries(residualFormData).forEach(entry => { + const [key, residualValue] = entry; + if (Object.prototype.hasOwnProperty.call(defaultedControlGroups, key)) { + const controlGroup: string = defaultedControlGroups[key]; + const controlValue = residualFormData[key]; + // eslint-disable-next-line @typescript-eslint/no-unsafe-call + groupedControls[controlGroup] = (groupedControls[controlGroup] || []).concat(controlValue); + } else { + // eslint-disable-next-line @typescript-eslint/no-unsafe-call + groupedControls[key] = (groupedControls[key] || []).concat(residualValue); + } + }); + return groupedControls; +} diff --git a/packages/superset-ui-query/src/buildQueryObject.ts b/packages/superset-ui-query/src/buildQueryObject.ts index c77b1c17a1..0186b3870e 100644 --- a/packages/superset-ui-query/src/buildQueryObject.ts +++ b/packages/superset-ui-query/src/buildQueryObject.ts @@ -1,10 +1,11 @@ /* eslint-disable camelcase */ import { QueryObject } from './types/Query'; -import { QueryFormData, isSqlaFormData, QueryFormDataMetric } from './types/QueryFormData'; +import { isSqlaFormData, QueryFormData } from './types/QueryFormData'; import convertMetric from './convertMetric'; import processFilters from './processFilters'; import processMetrics from './processMetrics'; import processExtras from './processExtras'; +import buildGroupedControlData from './buildGroupedControls'; export const DTTM_ALIAS = '__timestamp'; @@ -28,44 +29,11 @@ export default function buildQueryObject(formData: T): row_limit, limit, timeseries_limit_metric, - controlGroups = { - /** These are predefined for backward compatibility */ - metric: 'metrics', - percent_metrics: 'metrics', - metric_2: 'metrics', - secondary_metric: 'metrics', - x: 'metrics', - y: 'metrics', - size: 'metrics', - }, + controlGroups, ...residualFormData } = formData; - type GroupedControlData = { - columns: string[]; - groupby: string[]; - metrics: QueryFormDataMetric[]; - }; - type ResidualGroupedControlData = { - [key: string]: QueryFormDataMetric[]; - }; - const groupedControls: GroupedControlData & ResidualGroupedControlData = { - columns: [], - groupby: [], - metrics: [], - }; - - Object.entries(residualFormData).forEach(entry => { - const [key, residualValue] = entry; - if (Object.prototype.hasOwnProperty.call(controlGroups, key)) { - const controlGroup = controlGroups[key]; - const controlValue = residualFormData[key]; - groupedControls[controlGroup] = (groupedControls[controlGroup] || []).concat(controlValue); - } else { - // eslint-disable-next-line @typescript-eslint/no-unsafe-call - groupedControls[key] = (groupedControls[key] || []).concat(residualValue); - } - }); + const groupedControls = buildGroupedControlData(residualFormData, controlGroups); const { metrics, groupby, columns } = groupedControls; const groupbySet = new Set([...columns, ...groupby]); const numericRowLimit = Number(row_limit); diff --git a/packages/superset-ui-query/src/types/ControlData.ts b/packages/superset-ui-query/src/types/ControlData.ts new file mode 100644 index 0000000000..b3a1529d2f --- /dev/null +++ b/packages/superset-ui-query/src/types/ControlData.ts @@ -0,0 +1,10 @@ +import { QueryFormDataMetric } from './QueryFormData'; + +export type GroupedControlData = { + columns: string[]; + groupby: string[]; + metrics: QueryFormDataMetric[]; +}; +export type ResidualGroupedControlData = { + [key: string]: QueryFormDataMetric[]; +}; diff --git a/packages/superset-ui-query/src/types/QueryFormData.ts b/packages/superset-ui-query/src/types/QueryFormData.ts index c4dac4b640..3403d922cd 100644 --- a/packages/superset-ui-query/src/types/QueryFormData.ts +++ b/packages/superset-ui-query/src/types/QueryFormData.ts @@ -9,6 +9,9 @@ export type ResidualFormData = { // eslint-disable-next-line @typescript-eslint/no-explicit-any [key: string]: any; }; +export type ControlGroups = { + [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 @@ -43,9 +46,7 @@ export type BaseFormData = { timeseries_limit_metric?: QueryFormDataMetric; /** Force refresh */ force?: boolean; - controlGroups?: { - [key in string]: string; - }; + controlGroups?: ControlGroups; } & TimeRange & ResidualFormData; diff --git a/packages/superset-ui-query/test/buildGroupedControls.test.ts b/packages/superset-ui-query/test/buildGroupedControls.test.ts new file mode 100644 index 0000000000..317b0ef3a9 --- /dev/null +++ b/packages/superset-ui-query/test/buildGroupedControls.test.ts @@ -0,0 +1,63 @@ +import buildGroupedControls from '../src/buildGroupedControls'; + +describe('buildGroupedControls', () => { + it('should return default object', () => { + expect(buildGroupedControls({})).toEqual({ + columns: [], + groupby: [], + metrics: [], + }); + }); + + it('should group default metric controls to metrics', () => { + expect(buildGroupedControls({ metric: 'my_metric' }).metric).toEqual(['my_metric']); + }); + + it('should group custom metrics with default metrics', () => { + expect( + buildGroupedControls( + { metric: 'metric_1', my_custom_metric: 'metric_2' }, + { my_custom_metric: 'metrics' }, + ).metrics, + ).toEqual(['metric_1', 'metric_2']); + }); + + it('should extract columns', () => { + expect(buildGroupedControls({ columns: 'col_1' })).toEqual({ + columns: ['col_1'], + groupby: [], + metrics: [], + }); + }); + + it('should extract groupby', () => { + expect(buildGroupedControls({ groupby: 'col_1' })).toEqual({ + columns: [], + groupby: ['col_1'], + metrics: [], + }); + }); + + it('should extract custom groupby', () => { + expect( + buildGroupedControls({ 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( + buildGroupedControls( + { groupby: 'col_1', series: 'col_2', metric: 'metric_1' }, + { series: 'groupby' }, + ), + ).toEqual({ + columns: [], + groupby: ['col_1', 'col_2'], + metrics: ['metric_1'], + }); + }); +}); From c6f1424c7501facca0a6b650e1f9e2026b6a6669 Mon Sep 17 00:00:00 2001 From: Ville Brofeldt Date: Wed, 13 May 2020 15:40:02 +0300 Subject: [PATCH 05/13] fix test --- packages/superset-ui-query/test/buildGroupedControls.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/superset-ui-query/test/buildGroupedControls.test.ts b/packages/superset-ui-query/test/buildGroupedControls.test.ts index 317b0ef3a9..36313c3383 100644 --- a/packages/superset-ui-query/test/buildGroupedControls.test.ts +++ b/packages/superset-ui-query/test/buildGroupedControls.test.ts @@ -10,7 +10,7 @@ describe('buildGroupedControls', () => { }); it('should group default metric controls to metrics', () => { - expect(buildGroupedControls({ metric: 'my_metric' }).metric).toEqual(['my_metric']); + expect(buildGroupedControls({ metric: 'my_metric' }).metrics).toEqual(['my_metric']); }); it('should group custom metrics with default metrics', () => { From 1e42e85c00cc14e56819d691d275200127c37219 Mon Sep 17 00:00:00 2001 From: Ville Brofeldt Date: Fri, 15 May 2020 13:53:01 +0300 Subject: [PATCH 06/13] address comments --- .../src/buildGroupedControls.ts | 24 ++++------ .../superset-ui-query/src/buildQueryObject.ts | 11 ++--- .../superset-ui-query/src/convertMetric.ts | 4 +- .../superset-ui-query/src/processGroupby.ts | 11 +++++ .../superset-ui-query/src/processMetrics.ts | 11 ----- .../src/types/ControlData.ts | 10 ++-- packages/superset-ui-query/src/types/Query.ts | 2 +- .../src/types/QueryFormData.ts | 7 +-- .../test/processGroupby.test.ts | 12 +++++ .../test/processMetrics.test.ts | 46 ------------------- 10 files changed, 48 insertions(+), 90 deletions(-) create mode 100644 packages/superset-ui-query/src/processGroupby.ts delete mode 100644 packages/superset-ui-query/src/processMetrics.ts create mode 100644 packages/superset-ui-query/test/processGroupby.test.ts delete mode 100644 packages/superset-ui-query/test/processMetrics.test.ts diff --git a/packages/superset-ui-query/src/buildGroupedControls.ts b/packages/superset-ui-query/src/buildGroupedControls.ts index 8f9dc07be2..c84efa9bae 100644 --- a/packages/superset-ui-query/src/buildGroupedControls.ts +++ b/packages/superset-ui-query/src/buildGroupedControls.ts @@ -1,10 +1,10 @@ -import { ControlGroups, ResidualFormData } from './types/QueryFormData'; -import { GroupedControlData, ResidualGroupedControlData } from './types/ControlData'; +import { ControlGroups, QueryFormResidualData } from './types/QueryFormData'; +import { GroupedControlData } from './types/ControlData'; export default function buildGroupedControlData( - residualFormData: ResidualFormData, + residualFormData: QueryFormResidualData, controlGroups?: ControlGroups, -): GroupedControlData & ResidualGroupedControlData { +) { const defaultedControlGroups: ControlGroups = { /** These are predefined for backward compatibility */ metric: 'metrics', @@ -16,23 +16,17 @@ export default function buildGroupedControlData( size: 'metrics', ...controlGroups, }; - const groupedControls: GroupedControlData & ResidualGroupedControlData = { + const groupedControls: GroupedControlData = { columns: [], groupby: [], metrics: [], }; - Object.entries(residualFormData).forEach(entry => { const [key, residualValue] = entry; - if (Object.prototype.hasOwnProperty.call(defaultedControlGroups, key)) { - const controlGroup: string = defaultedControlGroups[key]; - const controlValue = residualFormData[key]; - // eslint-disable-next-line @typescript-eslint/no-unsafe-call - groupedControls[controlGroup] = (groupedControls[controlGroup] || []).concat(controlValue); - } else { - // eslint-disable-next-line @typescript-eslint/no-unsafe-call - groupedControls[key] = (groupedControls[key] || []).concat(residualValue); - } + const normalizedKey = Object.prototype.hasOwnProperty.call(defaultedControlGroups, key) + ? defaultedControlGroups[key] + : key; + groupedControls[normalizedKey] = (groupedControls[normalizedKey] || []).concat(residualValue); }); return groupedControls; } diff --git a/packages/superset-ui-query/src/buildQueryObject.ts b/packages/superset-ui-query/src/buildQueryObject.ts index 0186b3870e..56a990c409 100644 --- a/packages/superset-ui-query/src/buildQueryObject.ts +++ b/packages/superset-ui-query/src/buildQueryObject.ts @@ -1,9 +1,9 @@ /* eslint-disable camelcase */ import { QueryObject } from './types/Query'; 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 buildGroupedControlData from './buildGroupedControls'; @@ -33,17 +33,16 @@ export default function buildQueryObject(formData: T): ...residualFormData } = formData; - const groupedControls = buildGroupedControlData(residualFormData, controlGroups); - const { metrics, groupby, columns } = groupedControls; - const groupbySet = new Set([...columns, ...groupby]); const numericRowLimit = Number(row_limit); + const { metrics, groupby, columns } = buildGroupedControlData(residualFormData, controlGroups); + const groupbySet = new Set([...columns, ...groupby]); return { extras: processExtras(formData), granularity: processGranularity(formData), - groupby: Array.from(groupbySet), + groupby: processGroupby(Array.from(groupbySet)), is_timeseries: groupbySet.has(DTTM_ALIAS), - metrics: processMetrics(metrics), + metrics: metrics.map(convertMetric), order_desc: typeof order_desc === 'undefined' ? true : order_desc, orderby: [], row_limit: row_limit == null || Number.isNaN(numericRowLimit) ? undefined : numericRowLimit, diff --git a/packages/superset-ui-query/src/convertMetric.ts b/packages/superset-ui-query/src/convertMetric.ts index 0b197ad547..f25eb88c5b 100644 --- a/packages/superset-ui-query/src/convertMetric.ts +++ b/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'; @@ -17,7 +17,7 @@ function getDefaultLabel(metric: AdhocMetric) { : `${label.slice(0, Math.max(0, LABEL_MAX_LENGTH - 3))}...`; } -export default function convertMetric(metric: QueryFormDataMetric): QueryObjectMetric { +export default function convertMetric(metric: QueryFormResidualDataValue): QueryObjectMetric { let formattedMetric; if (typeof metric === 'string') { formattedMetric = { diff --git a/packages/superset-ui-query/src/processGroupby.ts b/packages/superset-ui-query/src/processGroupby.ts new file mode 100644 index 0000000000..19ad008206 --- /dev/null +++ b/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/packages/superset-ui-query/src/processMetrics.ts b/packages/superset-ui-query/src/processMetrics.ts deleted file mode 100644 index b75b08fc3d..0000000000 --- a/packages/superset-ui-query/src/processMetrics.ts +++ /dev/null @@ -1,11 +0,0 @@ -import { QueryFormDataMetric } from './types/QueryFormData'; -import { QueryObjectMetric } from './types/Query'; -import convertMetric from './convertMetric'; - -export default function processMetrics(rawMetrics: QueryFormDataMetric[]) { - const metrics: QueryObjectMetric[] = []; - Object.values(rawMetrics).forEach(metric => { - metrics.push(convertMetric(metric)); - }); - return metrics; -} diff --git a/packages/superset-ui-query/src/types/ControlData.ts b/packages/superset-ui-query/src/types/ControlData.ts index b3a1529d2f..ad6739221e 100644 --- a/packages/superset-ui-query/src/types/ControlData.ts +++ b/packages/superset-ui-query/src/types/ControlData.ts @@ -1,10 +1,8 @@ -import { QueryFormDataMetric } from './QueryFormData'; +import { QueryFormDataMetric, QueryFormResidualDataValue } from './QueryFormData'; export type GroupedControlData = { - columns: string[]; - groupby: string[]; + columns: QueryFormResidualDataValue[]; + groupby: QueryFormResidualDataValue[]; metrics: QueryFormDataMetric[]; -}; -export type ResidualGroupedControlData = { - [key: string]: QueryFormDataMetric[]; + [key: string]: QueryFormResidualDataValue[]; }; diff --git a/packages/superset-ui-query/src/types/Query.ts b/packages/superset-ui-query/src/types/Query.ts index a9fb1216a8..cf68cdbf46 100644 --- a/packages/superset-ui-query/src/types/Query.ts +++ b/packages/superset-ui-query/src/types/Query.ts @@ -42,7 +42,7 @@ export type QueryObjectExtras = Partial<{ export type ResidualQueryObjectData = { // eslint-disable-next-line @typescript-eslint/no-explicit-any - [key: string]: any; + [key: string]: unknown; }; export type QueryObject = { diff --git a/packages/superset-ui-query/src/types/QueryFormData.ts b/packages/superset-ui-query/src/types/QueryFormData.ts index 3403d922cd..4e6b7d5515 100644 --- a/packages/superset-ui-query/src/types/QueryFormData.ts +++ b/packages/superset-ui-query/src/types/QueryFormData.ts @@ -5,7 +5,8 @@ import { TimeRange } from './Time'; import { AdhocFilter } from './Filter'; export type QueryFormDataMetric = string | AdhocMetric; -export type ResidualFormData = { +export type QueryFormResidualDataValue = string | AdhocMetric; +export type QueryFormResidualData = { // eslint-disable-next-line @typescript-eslint/no-explicit-any [key: string]: any; }; @@ -43,12 +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; controlGroups?: ControlGroups; } & TimeRange & - ResidualFormData; + QueryFormResidualData; // FormData is either sqla-based or druid-based export type SqlaFormData = { diff --git a/packages/superset-ui-query/test/processGroupby.test.ts b/packages/superset-ui-query/test/processGroupby.test.ts new file mode 100644 index 0000000000..e2adef546d --- /dev/null +++ b/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/packages/superset-ui-query/test/processMetrics.test.ts b/packages/superset-ui-query/test/processMetrics.test.ts deleted file mode 100644 index ddb0a5625a..0000000000 --- a/packages/superset-ui-query/test/processMetrics.test.ts +++ /dev/null @@ -1,46 +0,0 @@ -import { ColumnType } from '../src'; -import processMetrics from '../src/processMetrics'; - -describe('processMetrics', () => { - it('should handle an array of metrics', () => { - const metrics = processMetrics(['sum__num']); - expect(metrics).toEqual([{ label: 'sum__num' }]); - }); - - it('should handle multiple types of metrics', () => { - const metrics = processMetrics([ - '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)', - }, - ]); - }); -}); From 7d9672c848464c2f404effba9b00aedae26c9d37 Mon Sep 17 00:00:00 2001 From: Ville Brofeldt Date: Fri, 15 May 2020 15:27:18 +0300 Subject: [PATCH 07/13] rename controlGroup to queryField --- .../superset-ui-query/src/buildQueryObject.ts | 6 +++--- ...roupedControls.ts => extractQueryFields.ts} | 18 +++++++++--------- .../superset-ui-query/src/types/ControlData.ts | 8 -------- packages/superset-ui-query/src/types/Query.ts | 10 +++++++++- .../src/types/QueryFormData.ts | 4 ++-- .../test/buildQueryObject.test.ts | 6 +++--- ...rols.test.ts => extractQueryFields.test.ts} | 18 +++++++++--------- 7 files changed, 35 insertions(+), 35 deletions(-) rename packages/superset-ui-query/src/{buildGroupedControls.ts => extractQueryFields.ts} (55%) delete mode 100644 packages/superset-ui-query/src/types/ControlData.ts rename packages/superset-ui-query/test/{buildGroupedControls.test.ts => extractQueryFields.test.ts} (68%) diff --git a/packages/superset-ui-query/src/buildQueryObject.ts b/packages/superset-ui-query/src/buildQueryObject.ts index 56a990c409..1a71c39d2d 100644 --- a/packages/superset-ui-query/src/buildQueryObject.ts +++ b/packages/superset-ui-query/src/buildQueryObject.ts @@ -5,7 +5,7 @@ import processGroupby from './processGroupby'; import convertMetric from './convertMetric'; import processFilters from './processFilters'; import processExtras from './processExtras'; -import buildGroupedControlData from './buildGroupedControls'; +import extractQueryFields from './extractQueryFields'; export const DTTM_ALIAS = '__timestamp'; @@ -29,12 +29,12 @@ export default function buildQueryObject(formData: T): row_limit, limit, timeseries_limit_metric, - controlGroups, + queryFields, ...residualFormData } = formData; const numericRowLimit = Number(row_limit); - const { metrics, groupby, columns } = buildGroupedControlData(residualFormData, controlGroups); + const { metrics, groupby, columns } = extractQueryFields(residualFormData, queryFields); const groupbySet = new Set([...columns, ...groupby]); return { diff --git a/packages/superset-ui-query/src/buildGroupedControls.ts b/packages/superset-ui-query/src/extractQueryFields.ts similarity index 55% rename from packages/superset-ui-query/src/buildGroupedControls.ts rename to packages/superset-ui-query/src/extractQueryFields.ts index c84efa9bae..63e84b63ec 100644 --- a/packages/superset-ui-query/src/buildGroupedControls.ts +++ b/packages/superset-ui-query/src/extractQueryFields.ts @@ -1,11 +1,11 @@ -import { ControlGroups, QueryFormResidualData } from './types/QueryFormData'; -import { GroupedControlData } from './types/ControlData'; +import { QueryFields, QueryFormResidualData } from './types/QueryFormData'; +import { QueryFieldData } from './types/Query'; -export default function buildGroupedControlData( +export default function buildQueryFieldData( residualFormData: QueryFormResidualData, - controlGroups?: ControlGroups, + queryFields?: QueryFields, ) { - const defaultedControlGroups: ControlGroups = { + const queryFieldAliases: QueryFields = { /** These are predefined for backward compatibility */ metric: 'metrics', percent_metrics: 'metrics', @@ -14,17 +14,17 @@ export default function buildGroupedControlData( x: 'metrics', y: 'metrics', size: 'metrics', - ...controlGroups, + ...queryFields, }; - const groupedControls: GroupedControlData = { + const groupedControls: QueryFieldData = { columns: [], groupby: [], metrics: [], }; Object.entries(residualFormData).forEach(entry => { const [key, residualValue] = entry; - const normalizedKey = Object.prototype.hasOwnProperty.call(defaultedControlGroups, key) - ? defaultedControlGroups[key] + const normalizedKey = Object.prototype.hasOwnProperty.call(queryFieldAliases, key) + ? queryFieldAliases[key] : key; groupedControls[normalizedKey] = (groupedControls[normalizedKey] || []).concat(residualValue); }); diff --git a/packages/superset-ui-query/src/types/ControlData.ts b/packages/superset-ui-query/src/types/ControlData.ts deleted file mode 100644 index ad6739221e..0000000000 --- a/packages/superset-ui-query/src/types/ControlData.ts +++ /dev/null @@ -1,8 +0,0 @@ -import { QueryFormDataMetric, QueryFormResidualDataValue } from './QueryFormData'; - -export type GroupedControlData = { - columns: QueryFormResidualDataValue[]; - groupby: QueryFormResidualDataValue[]; - metrics: QueryFormDataMetric[]; - [key: string]: QueryFormResidualDataValue[]; -}; diff --git a/packages/superset-ui-query/src/types/Query.ts b/packages/superset-ui-query/src/types/Query.ts index cf68cdbf46..cb88d4390c 100644 --- a/packages/superset-ui-query/src/types/Query.ts +++ b/packages/superset-ui-query/src/types/Query.ts @@ -41,7 +41,6 @@ export type QueryObjectExtras = Partial<{ }>; export type ResidualQueryObjectData = { - // eslint-disable-next-line @typescript-eslint/no-explicit-any [key: string]: unknown; }; @@ -90,3 +89,12 @@ export interface QueryContext { force: boolean; queries: QueryObject[]; } + +import { QueryFormDataMetric, QueryFormResidualDataValue } from './QueryFormData'; + +export type QueryFieldData = { + columns: QueryFormResidualDataValue[]; + groupby: QueryFormResidualDataValue[]; + metrics: QueryFormDataMetric[]; + [key: string]: QueryFormResidualDataValue[]; +}; diff --git a/packages/superset-ui-query/src/types/QueryFormData.ts b/packages/superset-ui-query/src/types/QueryFormData.ts index 4e6b7d5515..388a1127ea 100644 --- a/packages/superset-ui-query/src/types/QueryFormData.ts +++ b/packages/superset-ui-query/src/types/QueryFormData.ts @@ -10,7 +10,7 @@ export type QueryFormResidualData = { // eslint-disable-next-line @typescript-eslint/no-explicit-any [key: string]: any; }; -export type ControlGroups = { +export type QueryFields = { [key: string]: string; }; @@ -47,7 +47,7 @@ export type BaseFormData = { timeseries_limit_metric?: QueryFormResidualDataValue; /** Force refresh */ force?: boolean; - controlGroups?: ControlGroups; + queryFields?: QueryFields; } & TimeRange & QueryFormResidualData; diff --git a/packages/superset-ui-query/test/buildQueryObject.test.ts b/packages/superset-ui-query/test/buildQueryObject.test.ts index 9b694dbda6..42aed0b3a4 100644 --- a/packages/superset-ui-query/test/buildQueryObject.test.ts +++ b/packages/superset-ui-query/test/buildQueryObject.test.ts @@ -21,7 +21,7 @@ describe('buildQueryObject', () => { expect(query.granularity).toEqual('ds'); }); - it('should build metrics based on default controlGroups', () => { + it('should build metrics based on default queryFields', () => { query = buildQueryObject({ datasource: '5__table', granularity_sqla: 'ds', @@ -38,7 +38,7 @@ describe('buildQueryObject', () => { granularity_sqla: 'ds', viz_type: 'table', my_custom_metric_control: 'sum__num', - controlGroups: { my_custom_metric_control: 'metrics' }, + queryFields: { my_custom_metric_control: 'metrics' }, }); expect(query.metrics).toEqual([{ label: 'sum__num' }]); }); @@ -50,7 +50,7 @@ describe('buildQueryObject', () => { viz_type: 'table', metrics: ['sum__num'], my_custom_metric_control: 'avg__num', - controlGroups: { my_custom_metric_control: 'metrics' }, + queryFields: { my_custom_metric_control: 'metrics' }, }); expect(query.metrics).toEqual([{ label: 'sum__num' }, { label: 'avg__num' }]); }); diff --git a/packages/superset-ui-query/test/buildGroupedControls.test.ts b/packages/superset-ui-query/test/extractQueryFields.test.ts similarity index 68% rename from packages/superset-ui-query/test/buildGroupedControls.test.ts rename to packages/superset-ui-query/test/extractQueryFields.test.ts index 36313c3383..72e0c3358e 100644 --- a/packages/superset-ui-query/test/buildGroupedControls.test.ts +++ b/packages/superset-ui-query/test/extractQueryFields.test.ts @@ -1,8 +1,8 @@ -import buildGroupedControls from '../src/buildGroupedControls'; +import buildQueryFields from '../src/extractQueryFields'; -describe('buildGroupedControls', () => { +describe('buildQueryFields', () => { it('should return default object', () => { - expect(buildGroupedControls({})).toEqual({ + expect(buildQueryFields({})).toEqual({ columns: [], groupby: [], metrics: [], @@ -10,12 +10,12 @@ describe('buildGroupedControls', () => { }); it('should group default metric controls to metrics', () => { - expect(buildGroupedControls({ metric: 'my_metric' }).metrics).toEqual(['my_metric']); + expect(buildQueryFields({ metric: 'my_metric' }).metrics).toEqual(['my_metric']); }); it('should group custom metrics with default metrics', () => { expect( - buildGroupedControls( + buildQueryFields( { metric: 'metric_1', my_custom_metric: 'metric_2' }, { my_custom_metric: 'metrics' }, ).metrics, @@ -23,7 +23,7 @@ describe('buildGroupedControls', () => { }); it('should extract columns', () => { - expect(buildGroupedControls({ columns: 'col_1' })).toEqual({ + expect(buildQueryFields({ columns: 'col_1' })).toEqual({ columns: ['col_1'], groupby: [], metrics: [], @@ -31,7 +31,7 @@ describe('buildGroupedControls', () => { }); it('should extract groupby', () => { - expect(buildGroupedControls({ groupby: 'col_1' })).toEqual({ + expect(buildQueryFields({ groupby: 'col_1' })).toEqual({ columns: [], groupby: ['col_1'], metrics: [], @@ -40,7 +40,7 @@ describe('buildGroupedControls', () => { it('should extract custom groupby', () => { expect( - buildGroupedControls({ series: 'col_1', metric: 'metric_1' }, { series: 'groupby' }), + buildQueryFields({ series: 'col_1', metric: 'metric_1' }, { series: 'groupby' }), ).toEqual({ columns: [], groupby: ['col_1'], @@ -50,7 +50,7 @@ describe('buildGroupedControls', () => { it('should merge custom groupby with default group', () => { expect( - buildGroupedControls( + buildQueryFields( { groupby: 'col_1', series: 'col_2', metric: 'metric_1' }, { series: 'groupby' }, ), From e6cff17b67a517c728b8d35ced6ed100ee4e5136 Mon Sep 17 00:00:00 2001 From: Ville Brofeldt Date: Fri, 15 May 2020 16:03:09 +0300 Subject: [PATCH 08/13] move import --- packages/superset-ui-query/src/types/Query.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/superset-ui-query/src/types/Query.ts b/packages/superset-ui-query/src/types/Query.ts index cb88d4390c..fde0291573 100644 --- a/packages/superset-ui-query/src/types/Query.ts +++ b/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; @@ -90,8 +91,6 @@ export interface QueryContext { queries: QueryObject[]; } -import { QueryFormDataMetric, QueryFormResidualDataValue } from './QueryFormData'; - export type QueryFieldData = { columns: QueryFormResidualDataValue[]; groupby: QueryFormResidualDataValue[]; From e42de0036dfc5b14aa9fd40365cf6c023c6ffb00 Mon Sep 17 00:00:00 2001 From: Ville Brofeldt Date: Fri, 15 May 2020 17:30:10 +0300 Subject: [PATCH 09/13] fix word cloud test --- plugins/plugin-chart-word-cloud/test/plugin/buildQuery.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/plugin-chart-word-cloud/test/plugin/buildQuery.test.ts b/plugins/plugin-chart-word-cloud/test/plugin/buildQuery.test.ts index 5bbf51adb3..9213fc01f8 100644 --- a/plugins/plugin-chart-word-cloud/test/plugin/buildQuery.test.ts +++ b/plugins/plugin-chart-word-cloud/test/plugin/buildQuery.test.ts @@ -7,7 +7,7 @@ describe('WordCloud buildQuery', () => { granularity_sqla: 'ds', series: 'foo', viz_type: 'word_cloud', - controlGroups: { series: 'groupby' }, + queryFields: { series: 'groupby' }, }; it('should build groupby with series in form data', () => { From 900fb66d81e74a5659a0a33b778d3c809fc48eeb Mon Sep 17 00:00:00 2001 From: Ville Brofeldt Date: Fri, 15 May 2020 20:06:04 +0300 Subject: [PATCH 10/13] fix a few remaining references to control groups --- packages/superset-ui-control-utils/src/shared-controls.tsx | 4 ++-- packages/superset-ui-query/src/extractQueryFields.ts | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/superset-ui-control-utils/src/shared-controls.tsx b/packages/superset-ui-control-utils/src/shared-controls.tsx index a2e38fde76..ff87fea744 100644 --- a/packages/superset-ui-control-utils/src/shared-controls.tsx +++ b/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/packages/superset-ui-query/src/extractQueryFields.ts b/packages/superset-ui-query/src/extractQueryFields.ts index 63e84b63ec..033ca0c2ac 100644 --- a/packages/superset-ui-query/src/extractQueryFields.ts +++ b/packages/superset-ui-query/src/extractQueryFields.ts @@ -16,7 +16,7 @@ export default function buildQueryFieldData( size: 'metrics', ...queryFields, }; - const groupedControls: QueryFieldData = { + const finalQueryFields: QueryFieldData = { columns: [], groupby: [], metrics: [], @@ -26,7 +26,7 @@ export default function buildQueryFieldData( const normalizedKey = Object.prototype.hasOwnProperty.call(queryFieldAliases, key) ? queryFieldAliases[key] : key; - groupedControls[normalizedKey] = (groupedControls[normalizedKey] || []).concat(residualValue); + finalQueryFields[normalizedKey] = (finalQueryFields[normalizedKey] || []).concat(residualValue); }); - return groupedControls; + return finalQueryFields; } From d3bd3c80020ab32f50c315a4ebb9b9819f4e9914 Mon Sep 17 00:00:00 2001 From: Ville Brofeldt Date: Wed, 20 May 2020 09:18:23 +0300 Subject: [PATCH 11/13] Rename funcs --- .../src/extractQueryFields.ts | 2 +- .../test/extractQueryFields.test.ts | 18 +++++++++--------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/packages/superset-ui-query/src/extractQueryFields.ts b/packages/superset-ui-query/src/extractQueryFields.ts index 033ca0c2ac..e2fe36e53b 100644 --- a/packages/superset-ui-query/src/extractQueryFields.ts +++ b/packages/superset-ui-query/src/extractQueryFields.ts @@ -1,7 +1,7 @@ import { QueryFields, QueryFormResidualData } from './types/QueryFormData'; import { QueryFieldData } from './types/Query'; -export default function buildQueryFieldData( +export default function extractQueryFields( residualFormData: QueryFormResidualData, queryFields?: QueryFields, ) { diff --git a/packages/superset-ui-query/test/extractQueryFields.test.ts b/packages/superset-ui-query/test/extractQueryFields.test.ts index 72e0c3358e..8500766280 100644 --- a/packages/superset-ui-query/test/extractQueryFields.test.ts +++ b/packages/superset-ui-query/test/extractQueryFields.test.ts @@ -1,8 +1,8 @@ -import buildQueryFields from '../src/extractQueryFields'; +import extractQueryFields from '../src/extractQueryFields'; -describe('buildQueryFields', () => { +describe('extractQueryFields', () => { it('should return default object', () => { - expect(buildQueryFields({})).toEqual({ + expect(extractQueryFields({})).toEqual({ columns: [], groupby: [], metrics: [], @@ -10,12 +10,12 @@ describe('buildQueryFields', () => { }); it('should group default metric controls to metrics', () => { - expect(buildQueryFields({ metric: 'my_metric' }).metrics).toEqual(['my_metric']); + expect(extractQueryFields({ metric: 'my_metric' }).metrics).toEqual(['my_metric']); }); it('should group custom metrics with default metrics', () => { expect( - buildQueryFields( + extractQueryFields( { metric: 'metric_1', my_custom_metric: 'metric_2' }, { my_custom_metric: 'metrics' }, ).metrics, @@ -23,7 +23,7 @@ describe('buildQueryFields', () => { }); it('should extract columns', () => { - expect(buildQueryFields({ columns: 'col_1' })).toEqual({ + expect(extractQueryFields({ columns: 'col_1' })).toEqual({ columns: ['col_1'], groupby: [], metrics: [], @@ -31,7 +31,7 @@ describe('buildQueryFields', () => { }); it('should extract groupby', () => { - expect(buildQueryFields({ groupby: 'col_1' })).toEqual({ + expect(extractQueryFields({ groupby: 'col_1' })).toEqual({ columns: [], groupby: ['col_1'], metrics: [], @@ -40,7 +40,7 @@ describe('buildQueryFields', () => { it('should extract custom groupby', () => { expect( - buildQueryFields({ series: 'col_1', metric: 'metric_1' }, { series: 'groupby' }), + extractQueryFields({ series: 'col_1', metric: 'metric_1' }, { series: 'groupby' }), ).toEqual({ columns: [], groupby: ['col_1'], @@ -50,7 +50,7 @@ describe('buildQueryFields', () => { it('should merge custom groupby with default group', () => { expect( - buildQueryFields( + extractQueryFields( { groupby: 'col_1', series: 'col_2', metric: 'metric_1' }, { series: 'groupby' }, ), From 9e52a4c5ed0c1828a8fff92992136f724fbba355 Mon Sep 17 00:00:00 2001 From: Ville Brofeldt Date: Wed, 20 May 2020 19:49:27 +0300 Subject: [PATCH 12/13] Simplify extractQueryFields --- packages/superset-ui-query/src/extractQueryFields.ts | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/packages/superset-ui-query/src/extractQueryFields.ts b/packages/superset-ui-query/src/extractQueryFields.ts index e2fe36e53b..038e8da151 100644 --- a/packages/superset-ui-query/src/extractQueryFields.ts +++ b/packages/superset-ui-query/src/extractQueryFields.ts @@ -23,10 +23,8 @@ export default function extractQueryFields( }; Object.entries(residualFormData).forEach(entry => { const [key, residualValue] = entry; - const normalizedKey = Object.prototype.hasOwnProperty.call(queryFieldAliases, key) - ? queryFieldAliases[key] - : key; - finalQueryFields[normalizedKey] = (finalQueryFields[normalizedKey] || []).concat(residualValue); + const normalizedKey = queryFieldAliases[key] || key; + finalQueryFields[normalizedKey] = [...(finalQueryFields[normalizedKey] || []), residualValue]; }); return finalQueryFields; } From 511e3e7544154f79e74131b61e19a41957efcb83 Mon Sep 17 00:00:00 2001 From: Ville Brofeldt Date: Wed, 20 May 2020 20:19:43 +0300 Subject: [PATCH 13/13] fix --- packages/superset-ui-query/src/extractQueryFields.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/superset-ui-query/src/extractQueryFields.ts b/packages/superset-ui-query/src/extractQueryFields.ts index 038e8da151..511f3b7578 100644 --- a/packages/superset-ui-query/src/extractQueryFields.ts +++ b/packages/superset-ui-query/src/extractQueryFields.ts @@ -24,7 +24,7 @@ export default function extractQueryFields( Object.entries(residualFormData).forEach(entry => { const [key, residualValue] = entry; const normalizedKey = queryFieldAliases[key] || key; - finalQueryFields[normalizedKey] = [...(finalQueryFields[normalizedKey] || []), residualValue]; + finalQueryFields[normalizedKey] = (finalQueryFields[normalizedKey] || []).concat(residualValue); }); return finalQueryFields; }