Skip to content

Commit

Permalink
fix(legacy-plugin-chart-table): time column formating (#340)
Browse files Browse the repository at this point in the history
  • Loading branch information
ktmud authored and zhaoyongjie committed Nov 26, 2021
1 parent 436b369 commit 21f82d2
Show file tree
Hide file tree
Showing 5 changed files with 49 additions and 23 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ coverage:
status:
patch:
default:
target: 50%
target: 40%
threshold: 0%
project:
default:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,9 +108,16 @@ export default function ReactDataTable(props: DataTableProps) {
/**
* Format text for cell value
*/
function cellText(key: string, format: string | undefined, val: unknown) {
function cellText(key: string, format: string | undefined, val: any) {
if (key === '__timestamp') {
return formatTimestamp(val);
let value = val;
if (typeof val === 'string') {
// force UTC time zone if is an ISO timestamp without timezone
// e.g. "2020-10-12T00:00:00"
value = val.match(/T(\d{2}:){2}\d{2}$/) ? `${val}Z` : val;
value = new Date(value);
}
return formatTimestamp(value) as string;
}
if (typeof val === 'string') {
return filterXSS(val, { stripIgnoreTag: true });
Expand All @@ -123,7 +130,7 @@ export default function ReactDataTable(props: DataTableProps) {
// default format '' will return human readable numbers (e.g. 50M, 33k)
return formatNumber(format, val as number);
}
return val;
return String(val);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import { ChartProps } from '@superset-ui/chart';
import { QueryFormDataMetric } from '@superset-ui/query';

interface DataRecord {
[key: string]: unknown;
[key: string]: any;
}

interface DataColumnMeta {
Expand All @@ -43,7 +43,7 @@ export interface DataTableProps {
includeSearch: boolean;
orderDesc: boolean;
pageLength: number;
tableTimestampFormat: string;
tableTimestampFormat?: string;
// TODO: add filters back or clean up
// filters: object;
// onAddFilter?: (key: string, value: number[]) => void;
Expand All @@ -52,6 +52,17 @@ export interface DataTableProps {
// timeseriesLimitMetric: string | object;
}

export interface TableChartFormData {
alignPn?: boolean;
colorPn?: boolean;
includeSearch?: boolean;
orderDesc?: boolean;
pageLength?: string;
metrics?: QueryFormDataMetric[];
percentMetrics?: QueryFormDataMetric[];
tableTimestampFormat?: string;
}

/**
* Consolidate list of metrics to string, identified by its unique identifier
*/
Expand All @@ -60,37 +71,38 @@ const consolidateMetricShape = (metric: QueryFormDataMetric) => {
// even thought `metric.optionName` is more unique, it's not used
// anywhere else in `queryData` and cannot be used to access `data.records`.
// The records are still keyed by `metric.label`.
return metric.label;
return metric.label || 'NOT_LABLED';
};

export default function transformProps(chartProps: ChartProps): DataTableProps {
const { height, datasource, formData, queryData } = chartProps;

const {
alignPn,
colorPn,
includeSearch,
orderDesc,
pageLength,
metrics: metrics_,
percentMetrics: percentMetrics_,
alignPn = true,
colorPn = true,
includeSearch = false,
orderDesc = false,
pageLength = 0,
metrics: metrics_ = [],
percentMetrics: percentMetrics_ = [],
tableTimestampFormat,
} = formData;
} = formData as TableChartFormData;
const { columnFormats, verboseMap } = datasource;
const { records, columns: columns_ } = queryData.data;
const {
records,
columns: columns_,
}: { records: DataRecord[]; columns: string[] } = queryData.data;
const metrics = (metrics_ ?? []).map(consolidateMetricShape);
// percent metrics always starts with a '%' sign.
const percentMetrics = (percentMetrics_ ?? [])
.map(consolidateMetricShape)
.map((x: string) => `%${x}`);
const columns = columns_.map((key: string) => {
let label = verboseMap[key] || key;

// make sure there is a " " after "%" for percent metrics
if (label[0] === '%' && label[1] !== ' ') {
label = `% ${label.slice(1)}`;
}

return {
key,
label,
Expand All @@ -108,7 +120,7 @@ export default function transformProps(chartProps: ChartProps): DataTableProps {
colorPositiveNegative: colorPn,
includeSearch,
orderDesc,
pageLength: pageLength && parseInt(pageLength, 10),
pageLength: pageLength ? parseInt(pageLength, 10) : 0,
tableTimestampFormat,
};
}
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,13 @@ describe('legacy-table', () => {
const tree = wrap.render(); // returns a CheerioWrapper with jQuery-like API
const cells = tree.find('td');
expect(tree.hasClass('superset-legacy-chart-table')).toEqual(true);
expect(cells).toHaveLength(4);
expect(cells.eq(0).text()).toEqual('Michael');
expect(cells.eq(3).attr('data-sort')).toEqual('2467');
expect(cells).toHaveLength(6);
expect(cells.eq(0).text()).toEqual('2020-01-01 12:34:56');
expect(cells.eq(1).text()).toEqual('Michael');
// number is not in `metrics` list, so it should output raw value
// (in real world Superset, this would mean the column is used in GROUP BY)
expect(cells.eq(5).text()).toEqual('2467');
expect(cells.eq(5).attr('data-sort')).toEqual('2467');
});

it('render advanced data', () => {
Expand All @@ -47,6 +51,7 @@ describe('legacy-table', () => {
expect(tree.find('th').eq(1).text()).toEqual('Sum of Num');
expect(cells.eq(2).text()).toEqual('12.346%');
expect(cells.eq(4).text()).toEqual('2.47k');
expect(cells.eq(4).attr('data-sort')).toEqual('2467');
});

it('render empty data', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,14 +59,16 @@ const basic: ChartProps = {
...basicChartProps,
queryData: {
data: {
columns: ['name', 'sum__num'],
columns: ['__timestamp', 'name', 'sum__num'],
records: [
{
__timestamp: '2020-01-01T12:34:56',
name: 'Michael',
sum__num: 2467063,
'%pct_nice': 0.123456,
},
{
__timestamp: 1585932584140,
name: 'Joe',
sum__num: 2467,
'%pct_nice': 0.00001,
Expand Down

0 comments on commit 21f82d2

Please sign in to comment.