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

Commit

Permalink
Allow metrics arrays in form data (#61)
Browse files Browse the repository at this point in the history
Metrics can take shape of an array of metrics in form data for a given metrics key. For example, `{ metrics: ['sum__num'] }`. This PR changes Metrics class to handle the array case.
  • Loading branch information
xtinec committed Dec 12, 2018
1 parent 64430a6 commit 2896242
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 17 deletions.
4 changes: 2 additions & 2 deletions packages/superset-ui-chart/src/query/FormData.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { AdhocMetric, MetricKey } from './Metric';
import { FormDataMetric, MetricKey } from './Metric';

// Type signature and utility functions for formData shared by all viz types
// It will be gradually filled out as we build out the query object
Expand All @@ -7,7 +7,7 @@ import { AdhocMetric, MetricKey } from './Metric';
// 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`).
type Metrics = Partial<Record<MetricKey, AdhocMetric | string>>;
type Metrics = Partial<Record<MetricKey, FormDataMetric | FormDataMetric[]>>;

type BaseFormData = {
datasource: string;
Expand Down
47 changes: 32 additions & 15 deletions packages/superset-ui-chart/src/query/Metric.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,11 @@ export type AdhocMetric = {
optionName?: string;
} & (AdhocMetricSimple | AdhocMetricSQL);

// Type of metrics in form data
export type FormDataMetric = string | AdhocMetric;

// Type of Metric the client provides to server after unifying various forms
// of metrics in form data
export type Metric = {
label: string;
} & Partial<AdhocMetric>;
Expand All @@ -55,22 +60,17 @@ export class Metrics {
private metrics: Metric[];

constructor(formData: FormData) {
this.metrics = Object.keys(MetricKey)
.map(key => formData[MetricKey[key as any] as MetricKey])
.filter(metric => metric)
.map(metric => {
if (typeof metric === 'string') {
return { label: metric };
this.metrics = [];
Object.keys(MetricKey).forEach(key => {
const metric = formData[MetricKey[key as keyof typeof MetricKey]];
if (metric) {
if (Array.isArray(metric)) {
metric.forEach(m => this.addMetric(m));
} else {
this.addMetric(metric);
}

// Note we further sanitize the metric label for BigQuery datasources
// TODO: move this logic to the client once client has more info on the
// the datasource
return {
...metric,
label: (metric as Metric).label || this.getDefaultLabel(metric as AdhocMetric),
};
});
}
});
}

public getMetrics() {
Expand All @@ -81,6 +81,23 @@ export class Metrics {
return this.metrics.map(m => m.label);
}

private addMetric(metric: FormDataMetric) {
if (typeof metric === 'string') {
this.metrics.push({
label: metric,
});
} else {
// Note we further sanitize the metric label for BigQuery datasources
// TODO: move this logic to the client once client has more info on the
// the datasource
const label = metric.label || this.getDefaultLabel(metric);
this.metrics.push({
...metric,
label,
});
}
}

private getDefaultLabel(metric: AdhocMetric) {
let label: string;
if (metric.expressionType === ExpressionType.SIMPLE) {
Expand Down
9 changes: 9 additions & 0 deletions packages/superset-ui-chart/test/query/Metric.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -102,4 +102,13 @@ describe('Metrics', () => {
});
expect(metrics.getLabels()[0].length).toBeLessThanOrEqual(LABEL_MAX_LENGTH);
});

it('should handle metrics arrays in form data', () => {
metrics = new Metrics({
...formData,
metrics: ['sum__num'],
});
expect(metrics.getMetrics()).toEqual([{ label: 'sum__num' }]);
expect(metrics.getLabels()).toEqual(['sum__num']);
});
});

0 comments on commit 2896242

Please sign in to comment.