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

Commit

Permalink
feat(query): remove redundant metric label truncation (#492)
Browse files Browse the repository at this point in the history
* feat: remove redundant metric label truncation

* fix lint
  • Loading branch information
villebro committed May 15, 2020
1 parent 7b608f0 commit 5b42bc1
Show file tree
Hide file tree
Showing 2 changed files with 2 additions and 23 deletions.
15 changes: 2 additions & 13 deletions packages/superset-ui-query/src/convertMetric.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,19 +2,11 @@ import { QueryFormDataMetric } from './types/QueryFormData';
import { QueryObjectMetric } from './types/Query';
import { AdhocMetric } from './types/Metric';

export const LABEL_MAX_LENGTH = 43;

function getDefaultLabel(metric: AdhocMetric) {
let label: string;
if (metric.expressionType === 'SIMPLE') {
label = `${metric.aggregate}(${metric.column.columnName})`;
} else {
label = metric.sqlExpression;
return `${metric.aggregate}(${metric.column.columnName})`;
}

return label.length <= LABEL_MAX_LENGTH
? label
: `${label.slice(0, Math.max(0, LABEL_MAX_LENGTH - 3))}...`;
return metric.sqlExpression;
}

export default function convertMetric(metric: QueryFormDataMetric): QueryObjectMetric {
Expand All @@ -24,9 +16,6 @@ export default function convertMetric(metric: QueryFormDataMetric): QueryObjectM
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 ?? getDefaultLabel(metric);
formattedMetric = {
...metric,
Expand Down
10 changes: 0 additions & 10 deletions packages/superset-ui-query/test/convertMetric.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import { ColumnType, convertMetric } from '../src';
import { LABEL_MAX_LENGTH } from '../src/convertMetric';

describe('convertMetric', () => {
it('should handle string metric name', () => {
Expand Down Expand Up @@ -55,13 +54,4 @@ describe('convertMetric', () => {
sqlExpression: 'COUNT(sum_girls)',
});
});

it('should truncate labels if they are too long', () => {
expect(
convertMetric({
expressionType: 'SQL',
sqlExpression: 'COUNT(verrrrrrrrry_loooooooooooooooooooooong_string)',
}).label.length,
).toBeLessThanOrEqual(LABEL_MAX_LENGTH);
});
});

1 comment on commit 5b42bc1

@vercel
Copy link

@vercel vercel bot commented on 5b42bc1 May 15, 2020

Choose a reason for hiding this comment

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

Please sign in to comment.