Skip to content

Commit

Permalink
feat(table): enable table filter and better typing (apache#344)
Browse files Browse the repository at this point in the history
  • Loading branch information
ktmud authored and zhaoyongjie committed Nov 25, 2021
1 parent 9b59fb5 commit 1bd8664
Show file tree
Hide file tree
Showing 23 changed files with 283 additions and 144 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@
"@types/enzyme": "^3.10.3",
"@types/jest": "^25.1.1",
"@types/jsdom": "^12.2.4",
"@types/react-test-renderer": "^16.9.0",
"@types/react-test-renderer": "^16.9.2",
"enzyme": "^3.10.0",
"enzyme-adapter-react-16": "^1.15.1",
"enzyme-to-json": "^3.4.3",
Expand All @@ -73,9 +73,9 @@
"jest-mock-console": "^1.0.0",
"lerna": "^3.15.0",
"lint-staged": "^10.0.3",
"react": "^16.9.0",
"react-dom": "^16.9.0",
"react-test-renderer": "^16.9.0"
"react": "^16.13.1",
"react-dom": "^16.13.1",
"react-test-renderer": "^16.13.1"
},
"engines": {
"node": ">=10.10.0",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,12 @@
"access": "public"
},
"dependencies": {
"@types/react": "^16.7.17",
"@types/react": "^16.9.34",
"@vx/responsive": "^0.0.195",
"csstype": "^2.6.4"
},
"peerDependencies": {
"@superset-ui/core": "^0.12.0",
"react": "^16.7.17"
"react": "^16.13.1"
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
"access": "public"
},
"dependencies": {
"@types/react": "^16.7.17",
"@types/react": "^16.9.34",
"@types/react-loadable": "^5.4.2",
"@vx/responsive": "^0.0.195",
"prop-types": "^15.6.2",
Expand All @@ -45,6 +45,6 @@
"@superset-ui/core": "^0.12.0",
"@superset-ui/dimension": "^0.12.0",
"@superset-ui/query": "^0.12.0",
"react": "^16.7.17"
"react": "^16.13.1"
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import {
import { QueryFormData, Datasource } from '@superset-ui/query';
import getChartBuildQueryRegistry from '../registries/ChartBuildQueryRegistrySingleton';
import getChartMetadataRegistry from '../registries/ChartMetadataRegistrySingleton';
import { QueryData } from '../models/ChartProps';
import { QueryData } from '../types/QueryResponse';
import { AnnotationLayerMetadata } from '../types/Annotation';
import { PlainObject } from '../types/Base';

Expand Down Expand Up @@ -94,7 +94,10 @@ export default class ChartClient {
},
...options,
} as RequestConfig)
.then(response => response.json as Json);
.then(response => {
// let's assume response.json always has the shape of QueryData
return response.json as QueryData;
});
}

return Promise.reject(new Error(`Unknown chart type: ${visType}`));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import React, { ReactNode } from 'react';
import { SupersetClientInterface, RequestConfig } from '@superset-ui/connection';
import { QueryFormData, Datasource } from '@superset-ui/query';
import ChartClient, { SliceIdAndOrFormData } from '../clients/ChartClient';
import { QueryData } from '../models/ChartProps';
import { QueryData } from '../types/QueryResponse';

interface Payload {
formData: Partial<QueryFormData>;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,3 +16,4 @@ export { default as getChartTransformPropsRegistry } from './registries/ChartTra
export { default as ChartDataProvider } from './components/ChartDataProvider';

export * from './types/TransformFunction';
export * from './types/QueryResponse';
Original file line number Diff line number Diff line change
@@ -1,30 +1,32 @@
import { createSelector } from 'reselect';
import { convertKeysToCamelCase } from '@superset-ui/core';
import { Datasource } from '@superset-ui/query';
import { HandlerFunction, PlainObject } from '../types/Base';
import { QueryData, DataRecordFilters } from '../types/QueryResponse';

// TODO: more specific typing for these fields of ChartProps
type AnnotationData = PlainObject;
type CamelCaseDatasource = PlainObject;
type SnakeCaseDatasource = PlainObject;
type CamelCaseFormData = PlainObject;
type SnakeCaseFormData = PlainObject;
export type QueryData = PlainObject;
/** Initial values for the visualizations, currently used by only filter_box and table */
type InitialValues = PlainObject;
type RawFormData = CamelCaseFormData | SnakeCaseFormData;

type ChartPropsSelector = (c: ChartPropsConfig) => ChartProps;

/** Optional field for event handlers, renderers */
type Hooks = {
/** handle adding filters */
onAddFilter?: HandlerFunction;
/** handle errors */
/**
* sync active filters between chart and dashboard, "add" actually
* also handles "change" and "remove".
*/
onAddFilter?: (newFilters: DataRecordFilters, merge?: boolean) => void;
/** handle errors */
onError?: HandlerFunction;
/** use the vis as control to update state */
setControlValue?: HandlerFunction;
/** handle tooltip */
setTooltip?: HandlerFunction;
[key: string]: any;
};
} & PlainObject;

/**
* Preferred format for ChartProps config
Expand All @@ -37,9 +39,9 @@ export interface ChartPropsConfig {
* Formerly called "filters", which was misleading because it is actually
* initial values of the filter_box and table vis
*/
initialValues?: InitialValues;
initialValues?: DataRecordFilters;
/** Main configuration of the chart */
formData?: SnakeCaseFormData;
formData?: RawFormData;
/** Chart height */
height?: number;
/** Programmatic overrides such as event handlers, renderers */
Expand All @@ -53,22 +55,20 @@ export interface ChartPropsConfig {
const DEFAULT_WIDTH = 800;
const DEFAULT_HEIGHT = 600;

export default class ChartProps<
FormDataType extends CamelCaseFormData | SnakeCaseFormData = CamelCaseFormData
> {
export default class ChartProps {
static createSelector: () => ChartPropsSelector;

annotationData: AnnotationData;

datasource: CamelCaseDatasource;
datasource: Datasource;

rawDatasource: SnakeCaseDatasource;

initialValues: InitialValues;
initialValues: DataRecordFilters;

formData: CamelCaseFormData;

rawFormData: SnakeCaseFormData | CamelCaseFormData;
rawFormData: RawFormData;

height: number;

Expand All @@ -82,7 +82,7 @@ export default class ChartProps<
const {
annotationData = {},
datasource = {},
formData = {} as FormDataType,
formData = {},
hooks = {},
initialValues = {},
queryData = {},
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
/**
* Types for query response
*/
import { PlainObject } from './Base';

export type DataRecordValue = number | string | boolean | Date | null;

export interface DataRecord {
[key: string]: DataRecordValue;
}

// data record value filters from FilterBox
export interface DataRecordFilters {
[key: string]: DataRecordValue[];
}

// the response json from query API
export type QueryData = PlainObject;
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,10 @@ const RAW_FORM_DATA = {
};

const RAW_DATASOURCE = {
another_field: 2,
column_formats: { test: '%s' },
};

const QUERY_DATA = {};
const QUERY_DATA = { data: {} };

describe('ChartProps', () => {
it('exists', () => {
Expand All @@ -33,7 +33,7 @@ describe('ChartProps', () => {
queryData: QUERY_DATA,
});
expect(props.formData.someField).toEqual(1);
expect(props.datasource.anotherField).toEqual(2);
expect(props.datasource.columnFormats).toEqual(RAW_DATASOURCE.column_formats);
expect(props.rawFormData).toEqual(RAW_FORM_DATA);
expect(props.rawDatasource).toEqual(RAW_DATASOURCE);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@
"core-js": "3.6.5",
"gh-pages": "^2.2.0",
"jquery": "^3.4.1",
"react": "^16.6.0",
"react": "^16.13.1",
"storybook-addon-jsx": "^7.1.0"
},
"devDependencies": {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@
"fetch_values_predicate": null,
"template_params": null
},
"initialValues": {},
"activeFilters": {},
"formData": {
"datasource": "3__table",
"viz_type": "table",
Expand Down Expand Up @@ -300,7 +300,7 @@
"table_timestamp_format": "%Y-%m-%d %H:%M:%S",
"page_length": 0,
"include_search": true,
"table_filter": false,
"table_filter": true,
"align_pn": false,
"color_pn": true
},
Expand Down Expand Up @@ -419,7 +419,7 @@
"table_timestamp_format": "%Y-%m-%d %H:%M:%S",
"page_length": 0,
"include_search": true,
"table_filter": false,
"table_filter": true,
"align_pn": false,
"color_pn": true,
"where": "",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,15 @@ export enum DatasourceType {
export interface Datasource {
id: number;
name: string;
description?: string;
type: DatasourceType;
columns: Column[];
metrics: QueryObjectMetric[];
description?: string;
// key is column names (labels)
columnFormats?: {
[key: string]: string;
};
verboseMap?: {
[key: string]: string;
};
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ export type QueryObjectFilterClause = {

export type QueryObjectMetric = {
label: string;
metric_name?: string;
d3format?: string;
} & Partial<AdhocMetric>;

export type QueryObjectExtras = Partial<{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,25 +19,56 @@
import * as color from 'd3-color';
import { getNumberFormatter, NumberFormats } from '@superset-ui/number-format';
import { ChartProps } from '@superset-ui/chart';
import getTimeFormatterForGranularity from '../utils/getTimeFormatterForGranularity';
import getTimeFormatterForGranularity, {
TimeGranularity,
} from '../utils/getTimeFormatterForGranularity';

const TIME_COLUMN = '__timestamp';
const formatPercentChange = getNumberFormatter(NumberFormats.PERCENT_SIGNED_1_POINT);

export interface DatasourceMetric {
label: string;
// eslint-disable-next-line camelcase
metric_name?: string;
d3format?: string;
}

// we trust both the x (time) and y (big number) to be numeric
type BigNumberDatum = {
[TIME_COLUMN]: number;
export interface BigNumberDatum {
[key: string]: number | null;
}

export type BigNumberFormData = {
colorPicker?: {
r: number;
g: number;
b: number;
};
metric?:
| {
label: string;
}
| string;
compareLag?: string | number;
yAxisFormat?: string;
timeGrainSqla?: TimeGranularity;
};

export default function transformProps(chartProps: ChartProps) {
const { width, height, formData, queryData } = chartProps;
export type BignumberChartProps = ChartProps & {
formData: BigNumberFormData;
queryData: ChartProps['queryData'] & {
data?: BigNumberDatum[];
};
};

export default function transformProps(chartProps: BignumberChartProps) {
const { width, height, queryData, formData } = chartProps;
const {
colorPicker,
compareLag: compareLagInput,
compareLag: compareLag_,
compareSuffix = '',
headerFontSize,
metric,
metric = 'value',
showTrendLine,
startYAxisAtZero,
subheader = '',
Expand All @@ -47,9 +78,9 @@ export default function transformProps(chartProps: ChartProps) {
timeRangeFixed = false,
} = formData;
let { yAxisFormat } = formData;
const { data, from_dttm: fromDatetime, to_dttm: toDatetime } = queryData;
const metricName = metric?.label ? metric.label : metric;
const compareLag = Number(compareLagInput) || 0;
const { data = [], from_dttm: fromDatetime, to_dttm: toDatetime } = queryData;
const metricName = typeof metric === 'string' ? metric : metric.label;
const compareLag = Number(compareLag_) || 0;
const supportTrendLine = vizType === 'big_number';
const supportAndShowTrendLine = supportTrendLine && showTrendLine;
let formattedSubheader = subheader;
Expand All @@ -68,7 +99,8 @@ export default function transformProps(chartProps: ChartProps) {
if (data.length > 0) {
const sortedData = (data as BigNumberDatum[])
.map(d => ({ x: d[TIME_COLUMN], y: d[metricName] }))
.sort((a, b) => b.x - a.x); // sort in time descending order
// sort in time descending order
.sort((a, b) => (a.x !== null && b.x !== null ? b.x - a.x : 0));

bigNumber = sortedData[0].y;
if (bigNumber === null) {
Expand Down Expand Up @@ -103,14 +135,11 @@ export default function transformProps(chartProps: ChartProps) {
}

if (!yAxisFormat && chartProps.datasource && chartProps.datasource.metrics) {
chartProps.datasource.metrics.forEach(
// eslint-disable-next-line camelcase
(metricEntry: { metric_name?: string; d3format: string }) => {
if (metricEntry.metric_name === metric && metricEntry.d3format) {
yAxisFormat = metricEntry.d3format;
}
},
);
chartProps.datasource.metrics.forEach((metricEntry: DatasourceMetric) => {
if (metricEntry.metric_name === metric && metricEntry.d3format) {
yAxisFormat = metricEntry.d3format;
}
});
}

const formatNumber = getNumberFormatter(yAxisFormat);
Expand Down

0 comments on commit 1bd8664

Please sign in to comment.