addressing PR comments

apache · Nov 29, 2018 · a4597af · a4597af
1 parent c11e9c8
commit a4597af
Show file tree

Hide file tree

Showing 5 changed files with 33 additions and 9 deletions.
diff --git a/superset/assets/package.json b/superset/assets/package.json
@@ -15,8 +15,8 @@
     "dev-server": "webpack-dev-server --mode=development --progress",
     "prod": "node --max_old_space_size=4096 webpack --mode=production --colors --progress",
     "build": "NODE_ENV=production webpack --mode=production --colors --progress",
-    "lint": "eslint --ignore-path=.eslintignore --ext .js,.jsx . && tslint -c tslint.json ./src/**/*.ts{,x}",
-    "lint-fix": "eslint --fix --ignore-path=.eslintignore --ext .js,.jsx . && tslint -c tslint.json --fix ./src/**/*.ts{,x}",
+    "lint": "eslint --ignore-path=.eslintignore --ext .js,.jsx . && tslint -c tslint.json ./{src,spec}/**/*.ts{,x}",
+    "lint-fix": "eslint --fix --ignore-path=.eslintignore --ext .js,.jsx . && tslint -c tslint.json --fix ./{src,spec}/**/*.ts{,x}",
     "sync-backend": "babel-node --presets env src/syncBackend.js",
     "cypress": "cypress",
     "cypress-debug": "cypress open --config watchForFileChanges=true"

diff --git a/superset/assets/spec/javascripts/superset-ui/Metric.test.ts b/superset/assets/spec/javascripts/superset-ui/Metric.test.ts
@@ -1,4 +1,7 @@
-import { AdhocMetric, Aggregate, ExpressionType, Metrics } from 'src/query/Metric';
+import { ColumnType } from 'src/query/Column';
+import {
+  AdhocMetric, Aggregate, ExpressionType, LABEL_MAX_LENGTH, Metrics,
+} from 'src/query/Metric';
 
 describe('Metrics', () => {
   let metrics: Metrics;
@@ -22,7 +25,7 @@ describe('Metrics', () => {
       column: {
         columnName: 'sum_girls',
         id: 5,
-        type: 'BIGINT',
+        type: ColumnType.BIGINT,
       },
       expressionType: ExpressionType.SIMPLE,
     };
@@ -35,7 +38,7 @@ describe('Metrics', () => {
       column: {
         columnName: 'sum_girls',
         id: 5,
-        type: 'BIGINT',
+        type: ColumnType.BIGINT,
       },
       expressionType: 'SIMPLE',
       label: 'AVG(sum_girls)',
@@ -87,6 +90,6 @@ describe('Metrics', () => {
       ...formData,
       metric: adhocMetric,
     });
-    expect(metrics.getLabels()).toEqual(['COUNT(verrrrrrrrry_loooooooooooooooooooo...']);
+    expect(metrics.getLabels()[0].length).toBeLessThanOrEqual(LABEL_MAX_LENGTH);
   });
 });
diff --git a/superset/assets/spec/javascripts/superset-ui/buildQueryObject.test.ts b/superset/assets/spec/javascripts/superset-ui/buildQueryObject.test.ts
@@ -8,7 +8,7 @@ describe('queryObjectBuilder', () => {
     expect(query.granularity).toEqual('ds');
   });
 
-  it('should build granularity for sql alchemy datasources', () => {
+  it('should build granularity for sql druid datasources', () => {
     query = build({datasource: '5__druid', granularity: 'ds'});
     expect(query.granularity).toEqual('ds');
   });

diff --git a/superset/assets/src/query/Column.ts b/superset/assets/src/query/Column.ts
@@ -1,6 +1,24 @@
+export enum ColumnType {
+  DOUBLE = 'DOUBLE',
+  FLOAT = 'FLOAT',
+  INT = 'INT',
+  BIGINT = 'BIGINT',
+  LONG = 'LONG',
+  REAL = 'REAL',
+  NUMERIC = 'NUMERIC',
+  DECIMAL = 'DECIMAL',
+  MONEY = 'MONEY',
+  DATE = 'DATE',
+  TIME = 'TIME',
+  DATETIME = 'DATETIME',
+  VARCHAR = 'VARCHAR',
+  STRING = 'STRING',
+  CHAR = 'CHAR',
+}
+
 // TODO: fill out additional fields of the Column interface
 export default interface Column {
   id: number;
-  type: string;
+  type: ColumnType;
   columnName: string;
 }
diff --git a/superset/assets/src/query/Metric.ts b/superset/assets/src/query/Metric.ts
@@ -1,6 +1,8 @@
 import Column from './Column';
 import FormData from './FormData';
 
+export const LABEL_MAX_LENGTH = 43;
+
 export enum MetricKey {
   METRIC = 'metric',
   METRICS = 'metrics',
@@ -90,6 +92,7 @@ export class Metrics {
     } else {
       label = metric.sqlExpression;
     }
-    return label.length < 43 ? label : `${label.substring(0, 40)}...`;
+    return label.length <= LABEL_MAX_LENGTH ? label :
+      `${label.substring(0, LABEL_MAX_LENGTH - 3)}...`;
   }
 }