Revert "[Chart]Unify Metric format (#63)" (#65)

This reverts commit a97f666.
apache · Nov 26, 2021 · 908f969 · 908f969
1 parent 9c8c0a9
commit 908f969
Show file tree

Hide file tree

Showing 3 changed files with 19 additions and 38 deletions.
diff --git a/...frontend/temporary_superset_ui/superset-ui/packages/superset-ui-chart/src/query/Metric.ts b/...frontend/temporary_superset_ui/superset-ui/packages/superset-ui-chart/src/query/Metric.ts
@@ -26,37 +26,34 @@ export enum Aggregate {
 }
 
 export enum ExpressionType {
-  BUILTIN = 'BUILTIN',
   SIMPLE = 'SIMPLE',
   SQL = 'SQL',
 }
 
-interface SimpleMetric {
+interface AdhocMetricSimple {
   expressionType: ExpressionType.SIMPLE;
   column: Column;
   aggregate: Aggregate;
-  label?: string;
-  optionName?: string;
 }
 
-interface SQLMetric {
+interface AdhocMetricSQL {
   expressionType: ExpressionType.SQL;
   sqlExpression: string;
-  label?: string;
-  optionName?: string;
 }
 
-interface BuiltInMetric {
-  expressionType: ExpressionType.BUILTIN;
-  label: string;
-}
+export type AdhocMetric = {
+  label?: string;
+  optionName?: string;
+} & (AdhocMetricSimple | AdhocMetricSQL);
 
 // Type of metrics in form data
-export type FormDataMetric = string | SQLMetric | SimpleMetric;
+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 = BuiltInMetric | SQLMetric | SimpleMetric;
+export type Metric = {
+  label: string;
+} & Partial<AdhocMetric>;
 
 export class Metrics {
   // Use Array to maintain insertion order for metrics that are order sensitive
@@ -87,7 +84,6 @@ export class Metrics {
   private addMetric(metric: FormDataMetric) {
     if (typeof metric === 'string') {
       this.metrics.push({
-        expressionType: ExpressionType.BUILTIN,
         label: metric,
       });
     } else {
@@ -102,7 +98,7 @@ export class Metrics {
     }
   }
 
-  private getDefaultLabel(metric: SQLMetric | SimpleMetric) {
+  private getDefaultLabel(metric: AdhocMetric) {
     let label: string;
     if (metric.expressionType === ExpressionType.SIMPLE) {
       label = `${metric.aggregate}(${metric.column.columnName})`;

diff --git a/...nd/temporary_superset_ui/superset-ui/packages/superset-ui-chart/test/query/Metric.test.ts b/...nd/temporary_superset_ui/superset-ui/packages/superset-ui-chart/test/query/Metric.test.ts
@@ -1,6 +1,6 @@
 import { ColumnType } from '../../src/query/Column';
 import {
-  FormDataMetric,
+  AdhocMetric,
   Aggregate,
   ExpressionType,
   LABEL_MAX_LENGTH,
@@ -20,17 +20,12 @@ describe('Metrics', () => {
       ...formData,
       metric: 'sum__num',
     });
-    expect(metrics.getMetrics()).toEqual([
-      {
-        label: 'sum__num',
-        expressionType: 'BUILTIN',
-      },
-    ]);
+    expect(metrics.getMetrics()).toEqual([{ label: 'sum__num' }]);
     expect(metrics.getLabels()).toEqual(['sum__num']);
   });
 
   it('should build metrics for simple adhoc metrics', () => {
-    const adhocMetric: FormDataMetric = {
+    const adhocMetric: AdhocMetric = {
       aggregate: Aggregate.AVG,
       column: {
         columnName: 'sum_girls',
@@ -59,7 +54,7 @@ describe('Metrics', () => {
   });
 
   it('should build metrics for SQL adhoc metrics', () => {
-    const adhocMetric: FormDataMetric = {
+    const adhocMetric: AdhocMetric = {
       expressionType: ExpressionType.SQL,
       sqlExpression: 'COUNT(sum_girls)',
     };
@@ -78,7 +73,7 @@ describe('Metrics', () => {
   });
 
   it('should build metrics for adhoc metrics with custom labels', () => {
-    const adhocMetric: FormDataMetric = {
+    const adhocMetric: AdhocMetric = {
       expressionType: ExpressionType.SQL,
       label: 'foo',
       sqlExpression: 'COUNT(sum_girls)',
@@ -98,7 +93,7 @@ describe('Metrics', () => {
   });
 
   it('should truncate labels if they are too long', () => {
-    const adhocMetric: FormDataMetric = {
+    const adhocMetric: AdhocMetric = {
       expressionType: ExpressionType.SQL,
       sqlExpression: 'COUNT(verrrrrrrrry_loooooooooooooooooooooong_string)',
     };
@@ -114,12 +109,7 @@ describe('Metrics', () => {
       ...formData,
       metrics: ['sum__num'],
     });
-    expect(metrics.getMetrics()).toEqual([
-      {
-        label: 'sum__num',
-        expressionType: 'BUILTIN',
-      },
-    ]);
+    expect(metrics.getMetrics()).toEqual([{ label: 'sum__num' }]);
     expect(metrics.getLabels()).toEqual(['sum__num']);
   });
 });
diff --git a/...ry_superset_ui/superset-ui/packages/superset-ui-chart/test/query/buildQueryObject.test.ts b/...ry_superset_ui/superset-ui/packages/superset-ui-chart/test/query/buildQueryObject.test.ts
@@ -28,11 +28,6 @@ describe('queryObjectBuilder', () => {
       viz_type: 'table',
       metric: 'sum__num',
     });
-    expect(query.metrics).toEqual([
-      {
-        label: 'sum__num',
-        expressionType: 'BUILTIN',
-      },
-    ]);
+    expect(query.metrics).toEqual([{ label: 'sum__num' }]);
   });
 });