feat: add control grouping functionality (#485)

* feat: add control grouping functionality

* implement controlGroups on WordCloud

* fix tests

* break up into smaller packages and add unit tests

* fix test

* address comments

* rename controlGroup to queryField

* move import

* fix word cloud test

* fix a few remaining references to control groups

* Rename funcs

* Simplify extractQueryFields

* fix

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

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<T extends QueryFormData>(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<T extends QueryFormData>(formData: T):
     until,
     ...processFilters(formData),
   };
-
-  return queryObject;
 }

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 = {

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;
+}

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;
+}

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 {

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[];
+};

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<MetricKey, QueryFormDataMetric | QueryFormDataMetric[]>
->;
+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 = {

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({

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'],
+    });
+  });
+});

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