Skip to content
This repository has been archived by the owner on Dec 10, 2021. It is now read-only.

feat: add control grouping functionality #485

Merged
merged 13 commits into from
May 20, 2020
4 changes: 2 additions & 2 deletions packages/superset-ui-control-utils/src/shared-controls.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ type Control = {

const groupByControl = {
type: 'SelectControl',
controlGroup: 'groupby',
queryField: 'groupby',
multi: true,
freeForm: true,
label: t('Group by'),
Expand Down Expand Up @@ -174,7 +174,7 @@ const groupByControl = {

const metrics = {
type: 'MetricsControl',
controlGroup: 'metrics',
queryField: 'metrics',
multi: true,
label: t('Metrics'),
validators: [validateNonEmpty],
Expand Down
20 changes: 10 additions & 10 deletions packages/superset-ui-query/src/buildQueryObject.ts
Original file line number Diff line number Diff line change
@@ -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';

Expand All @@ -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,
Expand All @@ -53,6 +55,4 @@ export default function buildQueryObject<T extends QueryFormData>(formData: T):
until,
...processFilters(formData),
};

return queryObject;
}
4 changes: 2 additions & 2 deletions packages/superset-ui-query/src/convertMetric.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { QueryFormDataMetric } from './types/QueryFormData';
import { QueryFormResidualDataValue } from './types/QueryFormData';
import { QueryObjectMetric } from './types/Query';
import { AdhocMetric } from './types/Metric';

Expand All @@ -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 = {
Expand Down
30 changes: 30 additions & 0 deletions packages/superset-ui-query/src/extractQueryFields.ts
Original file line number Diff line number Diff line change
@@ -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;
}
11 changes: 11 additions & 0 deletions packages/superset-ui-query/src/processGroupby.ts
Original file line number Diff line number Diff line change
@@ -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;
}
25 changes: 0 additions & 25 deletions packages/superset-ui-query/src/processMetrics.ts

This file was deleted.

13 changes: 0 additions & 13 deletions packages/superset-ui-query/src/types/Metric.ts
Original file line number Diff line number Diff line change
@@ -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 {
Expand Down
15 changes: 14 additions & 1 deletion packages/superset-ui-query/src/types/Query.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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[];
Expand Down Expand Up @@ -73,7 +78,8 @@ export type QueryObject = {

/** If set, will group by timestamp */
is_timeseries?: boolean;
} & TimeRange;
} & TimeRange &
ResidualQueryObjectData;

export interface QueryContext {
datasource: {
Expand All @@ -84,3 +90,10 @@ export interface QueryContext {
force: boolean;
queries: QueryObject[];
}

export type QueryFieldData = {
columns: QueryFormResidualDataValue[];
groupby: QueryFormResidualDataValue[];
metrics: QueryFormDataMetric[];
[key: string]: QueryFormResidualDataValue[];
};
23 changes: 12 additions & 11 deletions packages/superset-ui-query/src/types/QueryFormData.ts
Original file line number Diff line number Diff line change
@@ -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;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto on unknown

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, are the items in ResidualFormData a QueryFormDataMetric? buildGroupedControlData seems to imply that.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I was also puzzled by this. ResidualFormData (in retrospect should be QueryFormResidualData) is currently expected to contain either column names (string) or metrics (QueryFormDataMetric which is the same as string | AdhocMetric). When these are combined, the union is reduced to string | AdhocMetric which is the same as QueryFormDataMetric. I'm tempted to remove QueryFormDataMetric and just pass around QueryFormResidualData, which can later be split into QueryFormDataMetric and QueryFormDataColumn when that need arises (Down the line we'll need to add AdhocColumn which is a column with a SQL expression). I will actually do that, we'll see how it looks.

};
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
Expand Down Expand Up @@ -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 = {
Expand Down
30 changes: 27 additions & 3 deletions packages/superset-ui-query/test/buildQueryObject.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand All @@ -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',
Expand All @@ -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({
Expand Down
63 changes: 63 additions & 0 deletions packages/superset-ui-query/test/extractQueryFields.test.ts
Original file line number Diff line number Diff line change
@@ -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'],
});
});
});
12 changes: 12 additions & 0 deletions packages/superset-ui-query/test/processGroupby.test.ts
Original file line number Diff line number Diff line change
@@ -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']);
});
});
Loading