This is an automated email from the ASF dual-hosted git repository. arivero pushed a commit to branch table-time-comparison-offset in repository https://gitbox.apache.org/repos/asf/superset.git
commit daa05f5f3006b7c6f037c3f3ed9e5bb2c6f36e9f Author: Antonio Rivero <[email protected]> AuthorDate: Mon Apr 15 18:21:48 2024 +0200 Table with Time Comparison: - Initial Table rendering with comparison columns --- .../plugin-chart-table/src/DataTable/DataTable.tsx | 3 + .../src/DataTable/hooks/useSticky.tsx | 4 +- .../plugins/plugin-chart-table/src/Styles.tsx | 7 + .../plugins/plugin-chart-table/src/TableChart.tsx | 103 ++++++++++++ .../plugins/plugin-chart-table/src/buildQuery.ts | 45 ++++-- .../plugin-chart-table/src/controlPanel.tsx | 20 ++- .../plugin-chart-table/src/transformProps.ts | 180 ++++++++++++++++++++- .../components/controls/ComparisonRangeLabel.tsx | 3 +- 8 files changed, 348 insertions(+), 17 deletions(-) diff --git a/superset-frontend/plugins/plugin-chart-table/src/DataTable/DataTable.tsx b/superset-frontend/plugins/plugin-chart-table/src/DataTable/DataTable.tsx index 67202d5a0e..5ac717b2f7 100644 --- a/superset-frontend/plugins/plugin-chart-table/src/DataTable/DataTable.tsx +++ b/superset-frontend/plugins/plugin-chart-table/src/DataTable/DataTable.tsx @@ -67,6 +67,7 @@ export interface DataTableProps<D extends object> extends TableOptions<D> { rowCount: number; wrapperRef?: MutableRefObject<HTMLDivElement>; onColumnOrderChange: () => void; + renderGroupingHeaders?: () => JSX.Element; } export interface RenderHTMLCellProps extends HTMLProps<HTMLTableCellElement> { @@ -99,6 +100,7 @@ export default typedMemo(function DataTable<D extends object>({ serverPagination, wrapperRef: userWrapperRef, onColumnOrderChange, + renderGroupingHeaders, ...moreUseTableOptions }: DataTableProps<D>): JSX.Element { const tableHooks: PluginHook<D>[] = [ @@ -251,6 +253,7 @@ export default typedMemo(function DataTable<D extends object>({ const renderTable = () => ( <table {...getTableProps({ className: tableClassName })}> <thead> + {renderGroupingHeaders ? renderGroupingHeaders() : null} {headerGroups.map(headerGroup => { const { key: headerGroupKey, ...headerGroupProps } = headerGroup.getHeaderGroupProps(); diff --git a/superset-frontend/plugins/plugin-chart-table/src/DataTable/hooks/useSticky.tsx b/superset-frontend/plugins/plugin-chart-table/src/DataTable/hooks/useSticky.tsx index 1b533c25de..8899f2bc7d 100644 --- a/superset-frontend/plugins/plugin-chart-table/src/DataTable/hooks/useSticky.tsx +++ b/superset-frontend/plugins/plugin-chart-table/src/DataTable/hooks/useSticky.tsx @@ -181,7 +181,9 @@ function StickyWrap({ } const fullTableHeight = (bodyThead.parentNode as HTMLTableElement) .clientHeight; - const ths = bodyThead.childNodes[0] + // instead of always using the first tr, we use the last one to support + // multi-level headers assuming the last one is the more detailed one + const ths = bodyThead.childNodes?.[bodyThead.childNodes?.length - 1 || 0] .childNodes as NodeListOf<HTMLTableHeaderCellElement>; const widths = Array.from(ths).map( th => th.getBoundingClientRect()?.width || th.clientWidth, diff --git a/superset-frontend/plugins/plugin-chart-table/src/Styles.tsx b/superset-frontend/plugins/plugin-chart-table/src/Styles.tsx index 9219b6f003..e394d16446 100644 --- a/superset-frontend/plugins/plugin-chart-table/src/Styles.tsx +++ b/superset-frontend/plugins/plugin-chart-table/src/Styles.tsx @@ -111,5 +111,12 @@ export default styled.div` text-align: center; padding: 1em 0.6em; } + + .right-border-only { + border-right: 2px solid ${theme.colors.grayscale.light2}; + } + table .right-border-only:last-child { + border-right: none; + } `} `; diff --git a/superset-frontend/plugins/plugin-chart-table/src/TableChart.tsx b/superset-frontend/plugins/plugin-chart-table/src/TableChart.tsx index 0f1be177a4..68bacc0aac 100644 --- a/superset-frontend/plugins/plugin-chart-table/src/TableChart.tsx +++ b/superset-frontend/plugins/plugin-chart-table/src/TableChart.tsx @@ -48,8 +48,10 @@ import { css, t, tn, + useTheme, } from '@superset-ui/core'; +import { isEmpty } from 'lodash'; import { DataColumnMeta, TableChartTransformedProps } from './types'; import DataTable, { DataTableProps, @@ -251,6 +253,7 @@ export default function TableChart<D extends DataRecord = DataRecord>( }); // keep track of whether column order changed, so that column widths can too const [columnOrderToggle, setColumnOrderToggle] = useState(false); + const theme = useTheme(); // only take relevant page size options const pageSizeOptions = useMemo(() => { @@ -415,6 +418,93 @@ export default function TableChart<D extends DataRecord = DataRecord>( } : undefined; + const comparisonLabels = [t('Main'), '#', '△', '%']; + + const getHeaderColumns = ( + columnsMeta: DataColumnMeta[], + enableTimeComparison?: boolean, + ) => { + const resultMap: Record<string, number[]> = {}; + + if (!enableTimeComparison) { + return resultMap; + } + + columnsMeta.forEach((element, index) => { + // Check if element's label is one of the comparison labels + if (comparisonLabels.includes(element.label)) { + // Extract the key portion after the space, assuming the format is always "label key" + const keyPortion = element.key.substring(element.label.length); + + // If the key portion is not in the map, initialize it with the current index + if (!resultMap[keyPortion]) { + resultMap[keyPortion] = [index]; + } else { + // Add the index to the existing array + resultMap[keyPortion].push(index); + } + } + }); + + return resultMap; + }; + + const renderGroupingHeaders = (): JSX.Element => { + // TODO: Make use of ColumnGroup to render the aditional headers + const headers: any = []; + let currentColumnIndex = 0; + + Object.entries(groupHeaderColumns || {}).forEach(([key, value]) => { + // Calculate the number of placeholder columns needed before the current header + const startPosition = value[0]; + const colSpan = value.length; + + // Add placeholder <th> for columns before this header + for (let i = currentColumnIndex; i < startPosition; i += 1) { + headers.push( + <th + key={`placeholder-${i}`} + style={{ borderBottom: 0 }} + aria-label={`Header-${i}`} + />, + ); + } + + // Add the current header <th> + headers.push( + <th key={`header-${key}`} colSpan={colSpan} style={{ borderBottom: 0 }}> + {key} + </th>, + ); + + // Update the current column index + currentColumnIndex = startPosition + colSpan; + }); + + return ( + <tr + css={css` + th { + border-right: 2px solid ${theme.colors.grayscale.light2}; + } + th:first-child { + border-left: none; + } + th:last-child { + border-right: none; + } + `} + > + {headers} + </tr> + ); + }; + + const groupHeaderColumns = useMemo( + () => getHeaderColumns(columnsMeta, true), + [columnsMeta, true], + ); + const getColumnConfigs = useCallback( (column: DataColumnMeta, i: number): ColumnWithLooseAccessor<D> => { const { @@ -462,6 +552,16 @@ export default function TableChart<D extends DataRecord = DataRecord>( className += ' dt-is-filter'; } + if (!isMetric && !isPercentMetric) { + className += ' right-border-only'; + } else if (comparisonLabels.includes(label)) { + const groupinHeader = key.substring(label.length); + const columnsUnderHeader = groupHeaderColumns[groupinHeader] || []; + if (i === columnsUnderHeader[columnsUnderHeader.length - 1]) { + className += ' right-border-only'; + } + } + return { id: String(i), // to allow duplicate column keys // must use custom accessor to allow `.` in column names @@ -742,6 +842,9 @@ export default function TableChart<D extends DataRecord = DataRecord>( selectPageSize={pageSize !== null && SelectPageSize} // not in use in Superset, but needed for unit tests sticky={sticky} + renderGroupingHeaders={ + !isEmpty(groupHeaderColumns) ? renderGroupingHeaders : undefined + } /> </Styles> ); diff --git a/superset-frontend/plugins/plugin-chart-table/src/buildQuery.ts b/superset-frontend/plugins/plugin-chart-table/src/buildQuery.ts index 75bd8c6b37..0cbc9a14f9 100644 --- a/superset-frontend/plugins/plugin-chart-table/src/buildQuery.ts +++ b/superset-frontend/plugins/plugin-chart-table/src/buildQuery.ts @@ -83,9 +83,9 @@ const buildQuery: BuildQuery<TableChartFormData> = ( return buildQueryContext(formDataCopy, baseQueryObject => { let { metrics, orderby = [], columns = [] } = baseQueryObject; let postProcessing: PostProcessingRule[] = []; - const timeOffsets = isTimeComparison(formData, baseQueryObject) - ? formData.time_compare - : []; + const timeOffsets = ensureIsArray( + isTimeComparison(formData, baseQueryObject) ? formData.time_compare : [], + ); const timeCompareGrainSqla = formData.time_comparison_grain_sqla; if (queryMode === QueryMode.Aggregate) { @@ -131,25 +131,36 @@ const buildQuery: BuildQuery<TableChartFormData> = ( postProcessing.push(timeCompareOperator(formData, baseQueryObject)); } + const temporalColumnsLookup = formData?.temporal_columns_lookup; + // Filter out the column if needed and prepare the temporal column object let temporalColumAdded = false; - columns = columns.map(col => { - if ( + let temporalColum = null; + + columns = columns.filter(col => { + const shouldBeAdded = isPhysicalColumn(col) && time_grain_sqla && - formData?.temporal_columns_lookup?.[col] - ) { - temporalColumAdded = true; - return { + temporalColumnsLookup?.[col]; + + if (shouldBeAdded && !temporalColumAdded) { + temporalColum = { timeGrain: time_grain_sqla, columnType: 'BASE_AXIS', sqlExpression: col, label: col, expressionType: 'SQL', } as AdhocColumn; + temporalColumAdded = true; + return false; // Do not include this in the output; it's added separately } - return col; + return true; }); + // So we ensure the temporal column is added first + if (temporalColum) { + columns = [temporalColum, ...columns]; + } + if (!temporalColumAdded && !isEmpty(timeOffsets)) { columns = [ { @@ -209,7 +220,18 @@ const buildQuery: BuildQuery<TableChartFormData> = ( ) { extraQueries.push({ ...queryObject, - columns: [], + columns: !isEmpty(timeOffsets) + ? [ + { + timeGrain: timeCompareGrainSqla || 'P1Y', // Group by year by default + columnType: 'BASE_AXIS', + sqlExpression: + baseQueryObject.filters?.[0]?.col.toString() || '', + label: baseQueryObject.filters?.[0]?.col.toString() || '', + expressionType: 'SQL', + } as AdhocColumn, + ] + : [], row_limit: 0, row_offset: 0, post_processing: [], @@ -230,6 +252,7 @@ const buildQuery: BuildQuery<TableChartFormData> = ( { ...queryObject }, { ...queryObject, + time_offsets: [], row_limit: 0, row_offset: 0, post_processing: [], diff --git a/superset-frontend/plugins/plugin-chart-table/src/controlPanel.tsx b/superset-frontend/plugins/plugin-chart-table/src/controlPanel.tsx index 035e3195e7..f5f28d5d38 100644 --- a/superset-frontend/plugins/plugin-chart-table/src/controlPanel.tsx +++ b/superset-frontend/plugins/plugin-chart-table/src/controlPanel.tsx @@ -47,6 +47,7 @@ import { sections, } from '@superset-ui/chart-controls'; +import { isEmpty } from 'lodash'; import { PAGE_SIZE_OPTIONS } from './consts'; function getQueryMode(controls: ControlStateMapping): QueryMode { @@ -449,6 +450,8 @@ const config: ControlPanelConfig = { description: t( "Allow end user to drag-and-drop column headers to rearrange them. Note their changes won't persist for the next time they open the chart.", ), + visibility: ({ controls }) => + isEmpty(controls?.time_compare?.value), }, }, ], @@ -535,7 +538,22 @@ const config: ControlPanelConfig = { { ...sections.timeComparisonControls, controlSetRows: [ - ...sections.timeComparisonControls.controlSetRows, + [ + { + ...(sections.timeComparisonControls.controlSetRows as any).find( + (row: any) => row[0].name === 'time_compare', + )[0], + config: { + ...(sections.timeComparisonControls.controlSetRows as any).find( + (row: any) => row[0].name === 'time_compare', + )[0].config, + multi: false, + }, + }, + ], + ...(sections.timeComparisonControls.controlSetRows as any).filter( + (row: any) => row[0].name !== 'time_compare', + ), [ { name: 'comparison_range_label', diff --git a/superset-frontend/plugins/plugin-chart-table/src/transformProps.ts b/superset-frontend/plugins/plugin-chart-table/src/transformProps.ts index 92b23307ad..2ae59b9fd6 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 { CurrencyFormatter, DataRecord, + ensureIsArray, extractTimegrain, GenericDataType, getMetricLabel, @@ -29,6 +30,7 @@ import { NumberFormats, QueryMode, smartDateFormatter, + t, TimeFormats, TimeFormatter, } from '@superset-ui/core'; @@ -37,6 +39,7 @@ import { getColorFormatters, } from '@superset-ui/chart-controls'; +import { isEmpty } from 'lodash'; import isEqualColumns from './utils/isEqualColumns'; import DateWithFormatter from './utils/DateWithFormatter'; import { @@ -81,6 +84,103 @@ const processDataRecords = memoizeOne(function processDataRecords( return data; }); +const calculateDifferences = ( + originalValue: number, + comparisonValue: number, +) => { + const valueDifference = originalValue - comparisonValue; + let percentDifferenceNum; + if (!originalValue && !comparisonValue) { + percentDifferenceNum = 0; + } else if (!originalValue || !comparisonValue) { + percentDifferenceNum = originalValue ? 1 : -1; + } else { + percentDifferenceNum = + (originalValue - comparisonValue) / Math.abs(comparisonValue); + } + return { valueDifference, percentDifferenceNum }; +}; + +const processComparisonTotals = ( + comparisonSuffix: string, + totals?: DataRecord[], +): DataRecord | undefined => { + if (!totals) { + return totals; + } + const transformedTotals: DataRecord = {}; + totals.map((totalRecord: DataRecord) => + Object.keys(totalRecord).forEach(key => { + if (totalRecord[key] !== undefined && !key.includes(comparisonSuffix)) { + transformedTotals[`Main ${key}`] = + parseInt(transformedTotals[`Main ${key}`]?.toString() || '0', 10) + + parseInt(totalRecord[key]?.toString() || '0', 10); + transformedTotals[`# ${key}`] = + parseInt(transformedTotals[`# ${key}`]?.toString() || '0', 10) + + parseInt( + totalRecord[`${key}__${comparisonSuffix}`]?.toString() || '0', + 10, + ); + const { valueDifference, percentDifferenceNum } = calculateDifferences( + transformedTotals[`Main ${key}`] as number, + transformedTotals[`# ${key}`] as number, + ); + transformedTotals[`△ ${key}`] = valueDifference; + transformedTotals[`% ${key}`] = percentDifferenceNum; + } + }), + ); + + return transformedTotals; +}; + +const processComparisonDataRecords = memoizeOne( + function processComparisonDataRecords( + originalData: DataRecord[] | undefined, + originalColumns: DataColumnMeta[], + comparisonSuffix: string, + ) { + // Transform data + return originalData?.map(originalItem => { + const transformedItem: DataRecord = {}; + originalColumns.forEach(origCol => { + if ( + (origCol.isMetric || origCol.isPercentMetric) && + !origCol.key.includes(comparisonSuffix) && + origCol.isNumeric + ) { + const originalValue = originalItem[origCol.key] || 0; + const comparisonValue = origCol.isMetric + ? originalItem?.[`${origCol.key}__${comparisonSuffix}`] || 0 + : originalItem[`%${origCol.key.slice(1)}__${comparisonSuffix}`] || + 0; + const { valueDifference, percentDifferenceNum } = + calculateDifferences( + originalValue as number, + comparisonValue as number, + ); + + transformedItem[`Main ${origCol.key}`] = originalValue; + transformedItem[`# ${origCol.key}`] = comparisonValue; + transformedItem[`△ ${origCol.key}`] = valueDifference; + transformedItem[`% ${origCol.key}`] = percentDifferenceNum; + } + }); + + Object.keys(originalItem).forEach(key => { + const isMetricOrPercentMetric = originalColumns.some( + col => col.key === key && (col.isMetric || col.isPercentMetric), + ); + if (!isMetricOrPercentMetric) { + transformedItem[key] = originalItem[key]; + } + }); + + return transformedItem; + }); + }, +); + const processColumns = memoizeOne(function processColumns( props: TableChartProps, ) { @@ -186,6 +286,56 @@ const processColumns = memoizeOne(function processColumns( ]; }, isEqualColumns); +const processComparisonColumns = ( + columns: DataColumnMeta[], + props: TableChartProps, + comparisonSuffix: string, +) => + columns + .map(col => { + const { + datasource: { columnFormats }, + rawFormData: { column_config: columnConfig = {} }, + } = props; + const config = columnConfig[col.key] || {}; + const savedFormat = columnFormats?.[col.key]; + const numberFormat = config.d3NumberFormat || savedFormat; + if (col.isNumeric && !col.key.includes(comparisonSuffix)) { + return [ + { + ...col, + label: t('Main'), + key: `${t('Main')} ${col.key}`, + }, + { + ...col, + label: `#`, + key: `# ${col.key}`, + }, + { + ...col, + label: `△`, + key: `△ ${col.key}`, + }, + { + ...col, + formatter: getNumberFormatter(numberFormat || PERCENT_3_POINT), + label: `%`, + key: `% ${col.key}`, + }, + ]; + } + if ( + !col.isMetric && + !col.isPercentMetric && + !col.key.includes(comparisonSuffix) + ) { + return [col]; + } + return []; + }) + .flat(); + /** * Automatically set page size based on number of cells. */ @@ -239,10 +389,24 @@ const transformProps = ( conditional_formatting: conditionalFormatting, allow_rearrange_columns: allowRearrangeColumns, allow_render_html: allowRenderHtml, + time_compare, } = formData; + const isUsingTimeComparison = + !isEmpty(time_compare) && queryMode === QueryMode.Aggregate; const timeGrain = extractTimegrain(formData); + const comparisonSuffix = isUsingTimeComparison + ? ensureIsArray(time_compare)[0] + : ''; const [metrics, percentMetrics, columns] = processColumns(chartProps); + let comparisonColumns: DataColumnMeta[] = []; + if (isUsingTimeComparison) { + comparisonColumns = processComparisonColumns( + columns, + chartProps, + comparisonSuffix, + ); + } let baseQuery; let countQuery; @@ -256,20 +420,30 @@ const transformProps = ( rowCount = baseQuery?.rowcount ?? 0; } const data = processDataRecords(baseQuery?.data, columns); + const comparisonData = processComparisonDataRecords( + baseQuery?.data, + columns, + comparisonSuffix, + ); const totals = showTotals && queryMode === QueryMode.Aggregate - ? totalQuery?.data[0] + ? isUsingTimeComparison + ? processComparisonTotals(comparisonSuffix, totalQuery?.data) + : totalQuery?.data[0] : undefined; const columnColorFormatters = getColorFormatters(conditionalFormatting, data) ?? defaultColorFormatters; + const passedData = isUsingTimeComparison ? comparisonData || [] : data; + const passedColumns = isUsingTimeComparison ? comparisonColumns : columns; + return { height, width, isRawRecords: queryMode === QueryMode.Raw, - data, + data: passedData, totals, - columns, + columns: passedColumns, serverPagination, metrics, percentMetrics, diff --git a/superset-frontend/src/explore/components/controls/ComparisonRangeLabel.tsx b/superset-frontend/src/explore/components/controls/ComparisonRangeLabel.tsx index b056388107..1d34c6e129 100644 --- a/superset-frontend/src/explore/components/controls/ComparisonRangeLabel.tsx +++ b/superset-frontend/src/explore/components/controls/ComparisonRangeLabel.tsx @@ -23,6 +23,7 @@ import { isEmpty, isEqual } from 'lodash'; import { BinaryAdhocFilter, css, + ensureIsArray, fetchTimeRange, SimpleAdhocFilter, t, @@ -65,7 +66,7 @@ export const ComparisonRangeLabel = ({ fetchTimeRange( filter.comparator, filter.subject, - multi ? shifts : shifts.slice(0, 1), + multi ? shifts : ensureIsArray(shifts).slice(0, 1), ), ); Promise.all(promises).then(res => {
