Skip to content

Commit

Permalink
fix(plugin-chart-table): formatting non-numeric metrics (apache#663)
Browse files Browse the repository at this point in the history
  • Loading branch information
ktmud authored and zhaoyongjie committed Nov 24, 2021
1 parent 244eebb commit 8acc2ab
Show file tree
Hide file tree
Showing 5 changed files with 79 additions and 829 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import memoizeOne from 'memoize-one';
import { withKnobs, number, boolean } from '@storybook/addon-knobs';
import { SuperChart } from '@superset-ui/chart';
import TableChartPlugin, { TableChartProps } from '@superset-ui/plugin-chart-table';
import { basicData, birthNames } from './testData';
import { basicFormData, basicData, birthNames } from './testData';
import { withResizableChartDemo } from '../../../shared/components/ResizableChartDemo';

export default {
Expand Down Expand Up @@ -71,19 +71,7 @@ export const basic = ({ width, height }) => (
width={width}
height={height}
queryData={{ data: basicData }}
formData={{
alignPn: false,
colorPn: false,
includeSearch: false,
metrics: ['sum__num'],
orderDesc: true,
pageLength: 0,
percentMetrics: null,
showCellBars: true,
tableFilter: false,
tableTimestampFormat: '%Y-%m-%d %H:%M:%S',
timeseriesLimitMetric: null,
}}
formData={basicFormData}
/>
);
basic.story = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,48 +21,71 @@ import { TableChartProps } from '@superset-ui/plugin-chart-table';
import birthNamesJson from './birthNames.json';

export const birthNames = (birthNamesJson as unknown) as TableChartProps;
export const basicFormData = {
alignPn: false,
colorPn: false,
includeSearch: false,
metrics: ['sum__num', 'MAX(ds)'],
orderDesc: true,
pageLength: 0,
percentMetrics: null,
showCellBars: true,
tableFilter: false,
tableTimestampFormat: 'smart_date',
timeseriesLimitMetric: null,
};
export const basicData = {
columns: ['name', 'sum__num'],
columns: ['name', 'sum__num', 'MAX(ds)'],
records: [
{
name: 'Michael',
sum__num: 2467063,
'MAX(ds)': '2008-01-01T00:00:00',
},
{
name: 'Christopher',
sum__num: 1725265,
'MAX(ds)': '2008-01-01T00:00:00',
},
{
name: 'David',
sum__num: 1570516,
'MAX(ds)': '2008-01-01T00:00:00',
},
{
name: 'James',
sum__num: 1506025,
'MAX(ds)': '2008-01-01T00:00:00',
},
{
name: 'John',
sum__num: 1426074,
'MAX(ds)': '2008-01-01T00:00:00',
},
{
name: 'Matthew',
sum__num: 1355803,
'MAX(ds)': '2008-01-01T00:00:00',
},
{
name: 'Robert',
sum__num: 1314800,
'MAX(ds)': '2008-01-01T00:00:00',
},
{
name: 'Daniel',
sum__num: 1159354,
'MAX(ds)': '2008-01-01T00:00:00',
},
{
name: 'Joseph',
sum__num: 1114098,
'MAX(ds)': '2008-01-01T00:00:00',
},
{
name: 'William',
sum__num: 1113701,
'MAX(ds)': '2008-01-01T00:00:00',
},
],
};
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
* under the License.
*/
import memoizeOne from 'memoize-one';
import { DataRecord } from '@superset-ui/chart';
import { DataRecord, DataRecordValue } from '@superset-ui/chart';
import { QueryFormDataMetric } from '@superset-ui/query';
import { getNumberFormatter, NumberFormats } from '@superset-ui/number-format';
import {
Expand All @@ -31,6 +31,7 @@ import { TableChartProps, TableChartTransformedProps, DataType, DataColumnMeta }

const { PERCENT_3_POINT } = NumberFormats;
const TIME_COLUMN = '__timestamp';
const toString = (x: DataRecordValue) => String(x);

/**
* Consolidate list of metrics to string, identified by its unique identifier
Expand All @@ -47,25 +48,37 @@ function isTimeColumn(key: string) {
return key === TIME_COLUMN;
}

const REGEXP_DATETIME = /^\d{4}-[01]\d-[03]\d/;
const REGEXP_TIMESTAMP_NO_TIMEZONE = /T(\d{2}:){2}\d{2}$/;
function isTimeType(key: string, data: DataRecord[] = []) {
return isTimeColumn(key) || data.some(x => x[key] instanceof Date);
return (
isTimeColumn(key) ||
data.some(x => {
const value = x[key];
return value instanceof Date || (typeof value === 'string' && REGEXP_DATETIME.test(value));
})
);
}

function isNumeric(key: string, data: DataRecord[] = []) {
return data.every(x => x[key] === null || x[key] === undefined || typeof x[key] === 'number');
}

const processDataRecords = memoizeOne(function processDataRecords(data: DataRecord[] | undefined) {
if (!data || !data[0] || !(TIME_COLUMN in data[0])) {
return data || [];
}
return data.map(x => {
const time = x[TIME_COLUMN];
return {
...x,
[TIME_COLUMN]:
typeof time === 'string' && REGEXP_TIMESTAMP_NO_TIMEZONE.test(time)
? // force UTC time for timestamps without a timezone
`${time}Z`
: time,
};
const datum: typeof x = {};
Object.entries(x).forEach(([key, value]) => {
// force UTC time for all timestamps without a timezone
if (typeof value === 'string' && REGEXP_TIMESTAMP_NO_TIMEZONE.test(value)) {
datum[key] = `${value}Z`;
} else {
datum[key] = value;
}
});
return datum;
});
});

Expand Down Expand Up @@ -108,24 +121,34 @@ const processColumns = memoizeOne(function processColumns(props: TableChartProps
// add a " " after "%" for percent metric labels
label = `% ${label.slice(1)}`;
}
// percent metrics have a default format
// fallback to column level formats defined in datasource
const format = columnFormats?.[key];
const isTime = isTimeType(key, records);
const isMetric = metricsSet.has(key);
// for the purpose of presentation, only numeric values are treated as metrics
const isMetric = metricsSet.has(key) && isNumeric(key, records);
const isPercentMetric = percentMetricsSet.has(key);
let dataType = DataType.Number; // TODO: get this from data source
let formatter;
if (isTime) {
// Use ganularity for "Adaptive Formatting" (smart_date)
const timeFormat = format || tableTimestampFormat;
formatter =
timeFormat === smartDateFormatter.id && isTimeColumn(key)
? getTimeFormatterForGranularity(granularity)
: getTimeFormatter(timeFormat);
formatter = getTimeFormatter(timeFormat);
if (timeFormat === smartDateFormatter.id) {
if (isTimeColumn(key)) {
formatter = getTimeFormatterForGranularity(granularity);
} else if (format) {
formatter = getTimeFormatter(format);
} else {
// return the identity string when datasource level formatter is not set
// and table timestamp format is set to Adaptive formatting
formatter = toString;
}
}
dataType = DataType.DateTime;
} else if (isMetric) {
formatter = getNumberFormatter(format);
} else if (isPercentMetric) {
// percent metrics have a default format
formatter = getNumberFormatter(format || PERCENT_3_POINT);
} else {
dataType = DataType.String;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
* under the License.
*/
import { QueryFormDataMetric } from '@superset-ui/query';
import { ChartProps, DataRecord, DataRecordFilters } from '@superset-ui/chart';
import { ChartProps, DataRecord, DataRecordValue, DataRecordFilters } from '@superset-ui/chart';
import { TimeFormatter, TimeGranularity } from '@superset-ui/time-format';
import { NumberFormatter } from '@superset-ui/number-format';

Expand All @@ -27,13 +27,15 @@ export enum DataType {
DateTime = 'datetime',
}

export type CustomFormatter = (value: DataRecordValue) => string;

export interface DataColumnMeta {
// `key` is what is called `label` in the input props
key: string;
// `label` is verbose column name used for rendering
label: string;
dataType: DataType;
formatter?: TimeFormatter | NumberFormatter;
formatter?: TimeFormatter | NumberFormatter | CustomFormatter;
}

export interface TableChartData<D extends DataRecord = DataRecord> {
Expand Down

0 comments on commit 8acc2ab

Please sign in to comment.