From dac69e20922ac06b21267502fc9cf18b61de15cc Mon Sep 17 00:00:00 2001 From: Ross Mabbett <92495987+rtexelm@users.noreply.github.com> Date: Thu, 25 Jul 2024 18:43:56 -0400 Subject: [PATCH] feat(explorer): Add configs and formatting to discrete comparison columns (#29553) --- .../plugin-chart-table/src/controlPanel.tsx | 49 ++++++- .../plugin-chart-table/src/transformProps.ts | 82 ++++++++++- .../test/TableChart.test.tsx | 110 +++++++++++++++ .../plugin-chart-table/test/testData.ts | 128 ++++++++++++++++++ .../ColumnConfigControl.tsx | 13 +- 5 files changed, 365 insertions(+), 17 deletions(-) diff --git a/superset-frontend/plugins/plugin-chart-table/src/controlPanel.tsx b/superset-frontend/plugins/plugin-chart-table/src/controlPanel.tsx index 91e2c5143..74809a6e0 100644 --- a/superset-frontend/plugins/plugin-chart-table/src/controlPanel.tsx +++ b/superset-frontend/plugins/plugin-chart-table/src/controlPanel.tsx @@ -18,7 +18,6 @@ * under the License. */ import { - ChartDataResponseResult, ensureIsArray, GenericDataType, isAdhocColumn, @@ -145,6 +144,21 @@ const percentMetricsControl: typeof sharedControls.metrics = { validators: [], }; +/** + * Generate comparison column names for a given column. + */ +const generateComparisonColumns = (colname: string) => [ + `${t('Main')} ${colname}`, + `# ${colname}`, + `△ ${colname}`, + `% ${colname}`, +]; +/** + * Generate column types for the comparison columns. + */ +const generateComparisonColumnTypes = (count: number) => + Array(count).fill(GenericDataType.Numeric); + const processComparisonColumns = (columns: any[], suffix: string) => columns .map(col => { @@ -470,10 +484,37 @@ const config: ControlPanelConfig = { return true; }, mapStateToProps(explore, _, chart) { + const timeComparisonStatus = + !!explore?.controls?.time_compare?.value; + + const { colnames: _colnames, coltypes: _coltypes } = + chart?.queriesResponse?.[0] ?? {}; + let colnames: string[] = _colnames || []; + let coltypes: GenericDataType[] = _coltypes || []; + + if (timeComparisonStatus) { + /** + * Replace numeric columns with sets of comparison columns. + */ + const updatedColnames: string[] = []; + const updatedColtypes: GenericDataType[] = []; + colnames.forEach((colname, index) => { + if (coltypes[index] === GenericDataType.Numeric) { + updatedColnames.push( + ...generateComparisonColumns(colname), + ); + updatedColtypes.push(...generateComparisonColumnTypes(4)); + } else { + updatedColnames.push(colname); + updatedColtypes.push(coltypes[index]); + } + }); + + colnames = updatedColnames; + coltypes = updatedColtypes; + } return { - queryResponse: chart?.queriesResponse?.[0] as - | ChartDataResponseResult - | undefined, + columnsPropsObject: { colnames, coltypes }, }; }, }, diff --git a/superset-frontend/plugins/plugin-chart-table/src/transformProps.ts b/superset-frontend/plugins/plugin-chart-table/src/transformProps.ts index 66bf6b68e..8c02d8293 100644 --- a/superset-frontend/plugins/plugin-chart-table/src/transformProps.ts +++ b/superset-frontend/plugins/plugin-chart-table/src/transformProps.ts @@ -20,6 +20,7 @@ import memoizeOne from 'memoize-one'; import { ComparisonType, CurrencyFormatter, + Currency, DataRecord, ensureIsArray, extractTimegrain, @@ -53,6 +54,7 @@ import { DataColumnMeta, TableChartProps, TableChartTransformedProps, + TableColumnConfig, } from './types'; const { PERCENT_3_POINT } = NumberFormats; @@ -293,6 +295,48 @@ const processColumns = memoizeOne(function processColumns( ]; }, isEqualColumns); +const getComparisonColConfig = ( + label: string, + parentColKey: string, + columnConfig: Record, +) => { + const comparisonKey = `${label} ${parentColKey}`; + const comparisonColConfig = columnConfig[comparisonKey] || {}; + return comparisonColConfig; +}; + +const getComparisonColFormatter = ( + label: string, + parentCol: DataColumnMeta, + columnConfig: Record, + savedFormat: string | undefined, + savedCurrency: Currency | undefined, +) => { + const currentColConfig = getComparisonColConfig( + label, + parentCol.key, + columnConfig, + ); + const hasCurrency = currentColConfig.currencyFormat?.symbol; + const currentColNumberFormat = + // fallback to parent's number format if not set + currentColConfig.d3NumberFormat || parentCol.config?.d3NumberFormat; + let { formatter } = parentCol; + if (label === '%') { + formatter = getNumberFormatter(currentColNumberFormat || PERCENT_3_POINT); + } else if (currentColNumberFormat || hasCurrency) { + const currency = currentColConfig.currencyFormat || savedCurrency; + const numberFormat = currentColNumberFormat || savedFormat; + formatter = currency + ? new CurrencyFormatter({ + d3Format: numberFormat, + currency, + }) + : getNumberFormatter(numberFormat); + } + return formatter; +}; + const processComparisonColumns = ( columns: DataColumnMeta[], props: TableChartProps, @@ -301,12 +345,11 @@ const processComparisonColumns = ( columns .map(col => { const { - datasource: { columnFormats }, + datasource: { columnFormats, currencyFormats }, rawFormData: { column_config: columnConfig = {} }, } = props; - const config = columnConfig[col.key] || {}; const savedFormat = columnFormats?.[col.key]; - const numberFormat = config.d3NumberFormat || savedFormat; + const savedCurrency = currencyFormats?.[col.key]; if ( (col.isMetric || col.isPercentMetric) && !col.key.includes(comparisonSuffix) && @@ -317,22 +360,53 @@ const processComparisonColumns = ( ...col, label: t('Main'), key: `${t('Main')} ${col.key}`, + config: getComparisonColConfig(t('Main'), col.key, columnConfig), + formatter: getComparisonColFormatter( + t('Main'), + col, + columnConfig, + savedFormat, + savedCurrency, + ), }, { ...col, label: `#`, key: `# ${col.key}`, + config: getComparisonColConfig(`#`, col.key, columnConfig), + formatter: getComparisonColFormatter( + `#`, + col, + columnConfig, + savedFormat, + savedCurrency, + ), }, { ...col, label: `△`, key: `△ ${col.key}`, + config: getComparisonColConfig(`△`, col.key, columnConfig), + formatter: getComparisonColFormatter( + `△`, + col, + columnConfig, + savedFormat, + savedCurrency, + ), }, { ...col, - formatter: getNumberFormatter(numberFormat || PERCENT_3_POINT), label: `%`, key: `% ${col.key}`, + config: getComparisonColConfig(`%`, col.key, columnConfig), + formatter: getComparisonColFormatter( + `%`, + col, + columnConfig, + savedFormat, + savedCurrency, + ), }, ]; } diff --git a/superset-frontend/plugins/plugin-chart-table/test/TableChart.test.tsx b/superset-frontend/plugins/plugin-chart-table/test/TableChart.test.tsx index 978616a0a..df62ba4c1 100644 --- a/superset-frontend/plugins/plugin-chart-table/test/TableChart.test.tsx +++ b/superset-frontend/plugins/plugin-chart-table/test/TableChart.test.tsx @@ -65,6 +65,116 @@ describe('plugin-chart-table', () => { expect(String(parsedDate)).toBe('2020-01-01 12:34:56'); expect(parsedDate.getTime()).toBe(1577882096000); }); + it('should process comparison columns when time_compare and comparison_type are set', () => { + const transformedProps = transformProps(testData.comparison); + + // Check if comparison columns are processed + const comparisonColumns = transformedProps.columns.filter( + col => + col.label === 'Main' || + col.label === '#' || + col.label === '△' || + col.label === '%', + ); + + expect(comparisonColumns.length).toBeGreaterThan(0); + expect(comparisonColumns.some(col => col.label === 'Main')).toBe(true); + expect(comparisonColumns.some(col => col.label === '#')).toBe(true); + expect(comparisonColumns.some(col => col.label === '△')).toBe(true); + expect(comparisonColumns.some(col => col.label === '%')).toBe(true); + }); + + it('should not process comparison columns when time_compare is empty', () => { + const propsWithoutTimeCompare = { + ...testData.comparison, + rawFormData: { + ...testData.comparison.rawFormData, + time_compare: [], + }, + }; + + const transformedProps = transformProps(propsWithoutTimeCompare); + + // Check if comparison columns are not processed + const comparisonColumns = transformedProps.columns.filter( + col => + col.label === 'Main' || + col.label === '#' || + col.label === '△' || + col.label === '%', + ); + + expect(comparisonColumns.length).toBe(0); + }); + + it('should correctly apply column configuration for comparison columns', () => { + const transformedProps = transformProps(testData.comparisonWithConfig); + + const comparisonColumns = transformedProps.columns.filter( + col => + col.key.startsWith('Main') || + col.key.startsWith('#') || + col.key.startsWith('△') || + col.key.startsWith('%'), + ); + + expect(comparisonColumns).toHaveLength(4); + + const mainMetricConfig = comparisonColumns.find( + col => col.key === 'Main metric_1', + ); + expect(mainMetricConfig).toBeDefined(); + expect(mainMetricConfig?.config).toEqual({ d3NumberFormat: '.2f' }); + + const hashMetricConfig = comparisonColumns.find( + col => col.key === '# metric_1', + ); + expect(hashMetricConfig).toBeDefined(); + expect(hashMetricConfig?.config).toEqual({ d3NumberFormat: '.1f' }); + + const deltaMetricConfig = comparisonColumns.find( + col => col.key === '△ metric_1', + ); + expect(deltaMetricConfig).toBeDefined(); + expect(deltaMetricConfig?.config).toEqual({ d3NumberFormat: '.0f' }); + + const percentMetricConfig = comparisonColumns.find( + col => col.key === '% metric_1', + ); + expect(percentMetricConfig).toBeDefined(); + expect(percentMetricConfig?.config).toEqual({ d3NumberFormat: '.3f' }); + }); + + it('should correctly format comparison columns using getComparisonColFormatter', () => { + const transformedProps = transformProps(testData.comparisonWithConfig); + const comparisonColumns = transformedProps.columns.filter( + col => + col.key.startsWith('Main') || + col.key.startsWith('#') || + col.key.startsWith('△') || + col.key.startsWith('%'), + ); + + const formattedMainMetric = comparisonColumns + .find(col => col.key === 'Main metric_1') + ?.formatter?.(12345.678); + expect(formattedMainMetric).toBe('12345.68'); + + const formattedHashMetric = comparisonColumns + .find(col => col.key === '# metric_1') + ?.formatter?.(12345.678); + expect(formattedHashMetric).toBe('12345.7'); + + const formattedDeltaMetric = comparisonColumns + .find(col => col.key === '△ metric_1') + ?.formatter?.(12345.678); + expect(formattedDeltaMetric).toBe('12346'); + + const formattedPercentMetric = comparisonColumns + .find(col => col.key === '% metric_1') + ?.formatter?.(0.123456); + expect(formattedPercentMetric).toBe('0.123'); + }); }); describe('TableChart', () => { diff --git a/superset-frontend/plugins/plugin-chart-table/test/testData.ts b/superset-frontend/plugins/plugin-chart-table/test/testData.ts index 6c76e865e..5993e335a 100644 --- a/superset-frontend/plugins/plugin-chart-table/test/testData.ts +++ b/superset-frontend/plugins/plugin-chart-table/test/testData.ts @@ -23,6 +23,7 @@ import { GenericDataType, QueryMode, supersetTheme, + ComparisonType, } from '@superset-ui/core'; import { TableChartProps, TableChartFormData } from '../src/types'; @@ -175,6 +176,131 @@ const advanced: TableChartProps = { ], }; +const comparison: TableChartProps = { + ...basic, + rawFormData: { + ...basicFormData, + table_timestamp_format: 'smart_date', + metrics: ['metric_1', 'metric_2'], + percent_metrics: ['percent_metric_1'], + column_config: {}, + align_pn: true, + color_pn: true, + show_cell_bars: true, + include_search: true, + page_length: 10, + server_pagination: false, + order_desc: false, + query_mode: QueryMode.Aggregate, + show_totals: true, + conditional_formatting: [], + allow_rearrange_columns: true, + allow_render_html: false, + time_compare: ['P1D'], + comparison_color_enabled: true, + comparison_color_scheme: 'Green', + comparison_type: ComparisonType.Values, + }, + queriesData: [ + { + ...basicQueryResult, + data: [ + { + metric_1: 100, + metric_2: 200, + percent_metric_1: 0.5, + date: '2023-01-01', + }, + { + metric_1: 110, + metric_2: 210, + percent_metric_1: 0.55, + date: '2023-01-02', + }, + ], + colnames: ['metric_1', 'metric_2', 'percent_metric_1', 'date'], + coltypes: [ + GenericDataType.Numeric, + GenericDataType.Numeric, + GenericDataType.Numeric, + GenericDataType.Temporal, + ], + }, + { + ...basicQueryResult, + data: [ + { + metric_1: 10, + metric_2: 20, + percent_metric_1: 0.05, + date: '2023-01-01', + }, + { + metric_1: 11, + metric_2: 21, + percent_metric_1: 0.055, + date: '2023-01-02', + }, + ], + }, + ], + filterState: { filters: {} }, + ownState: {}, + hooks: { + onAddFilter: jest.fn(), + setDataMask: jest.fn(), + onContextMenu: jest.fn(), + }, + emitCrossFilters: true, +}; + +const comparisonWithConfig: TableChartProps = { + ...comparison, + height: 400, + width: 400, + rawFormData: { + ...comparison.rawFormData, + table_timestamp_format: 'smart_date', + metrics: ['metric_1'], + percent_metrics: ['percent_metric_1'], + column_config: { + 'Main metric_1': { d3NumberFormat: '.2f' }, + '# metric_1': { d3NumberFormat: '.1f' }, + '△ metric_1': { d3NumberFormat: '.0f' }, + '% metric_1': { d3NumberFormat: '.3f' }, + }, + time_compare: ['1 year ago'], + comparison_color_enabled: true, + comparison_type: ComparisonType.Values, + }, + datasource: { + ...comparison.datasource, + columnFormats: { metric_1: '.2f' }, + currencyFormats: {}, + verboseMap: { metric_1: 'Metric 1' }, + }, + queriesData: [ + { + ...basicQueryResult, + data: [{ metric_1: 100, 'metric_1__1 year ago': 80 }], + colnames: ['metric_1', 'metric_1__1 year ago'], + coltypes: [GenericDataType.Numeric, GenericDataType.Numeric], + }, + { + ...basicQueryResult, + data: [{ rowcount: 1 }], + }, + ], + filterState: { filters: {} }, + ownState: {}, + hooks: { + onAddFilter: jest.fn(), + setDataMask: jest.fn(), + onContextMenu: jest.fn(), + }, + emitCrossFilters: false, +}; + const raw = { ...advanced, rawFormData: { @@ -226,6 +352,8 @@ export default { basic, advanced, advancedWithCurrency, + comparison, + comparisonWithConfig, empty, raw, }; diff --git a/superset-frontend/src/explore/components/controls/ColumnConfigControl/ColumnConfigControl.tsx b/superset-frontend/src/explore/components/controls/ColumnConfigControl/ColumnConfigControl.tsx index 7df23dee3..2715bd786 100644 --- a/superset-frontend/src/explore/components/controls/ColumnConfigControl/ColumnConfigControl.tsx +++ b/superset-frontend/src/explore/components/controls/ColumnConfigControl/ColumnConfigControl.tsx @@ -17,12 +17,7 @@ * under the License. */ import { useMemo, useState } from 'react'; -import { - ChartDataResponseResult, - useTheme, - t, - GenericDataType, -} from '@superset-ui/core'; +import { useTheme, t, GenericDataType } from '@superset-ui/core'; import { COLUMN_NAME_ALIASES, @@ -39,7 +34,7 @@ import ControlHeader from '../../ControlHeader'; export type ColumnConfigControlProps = ControlComponentProps> & { - queryResponse?: ChartDataResponseResult; + columnsPropsObject?: { colnames: string[]; coltypes: GenericDataType[] }; configFormLayout?: ColumnConfigFormLayout; appliedColumnNames?: string[]; width?: number | string; @@ -55,7 +50,7 @@ const MAX_NUM_COLS = 10; * Add per-column config to queried results. */ export default function ColumnConfigControl({ - queryResponse, + columnsPropsObject, appliedColumnNames = [], value, onChange, @@ -64,7 +59,7 @@ export default function ColumnConfigControl({ height, ...props }: ColumnConfigControlProps) { - const { colnames: _colnames, coltypes: _coltypes } = queryResponse || {}; + const { colnames: _colnames, coltypes: _coltypes } = columnsPropsObject || {}; let colnames: string[] = []; let coltypes: GenericDataType[] = []; if (appliedColumnNames.length === 0) {