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

fix(legacy-plugin-chart-table): time column formatting #340

Merged
merged 1 commit into from
Apr 3, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion codecov.yml
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
13 changes: 10 additions & 3 deletions plugins/table/src/ReactDataTable.tsx
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
42 changes: 27 additions & 15 deletions plugins/table/src/transformProps.ts
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,
};
}
11 changes: 8 additions & 3 deletions plugins/table/test/ReactDataTable.test.tsx
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
4 changes: 3 additions & 1 deletion plugins/table/test/testData.ts
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