From 3bd073d586babfbad736b9a8ccc4c928213c4220 Mon Sep 17 00:00:00 2001 From: Edoardo Sabadelli Date: Fri, 25 Aug 2023 16:10:17 +0200 Subject: [PATCH 01/10] feat: implement cumulative values DHIS2-5497 --- src/__demo__/PivotTable.stories.js | 40 +++++++++++++++++ src/modules/pivotTable/PivotTableEngine.js | 50 +++++++++++++++++++++- 2 files changed, 89 insertions(+), 1 deletion(-) diff --git a/src/__demo__/PivotTable.stories.js b/src/__demo__/PivotTable.stories.js index e06782146..93ca49ae9 100644 --- a/src/__demo__/PivotTable.stories.js +++ b/src/__demo__/PivotTable.stories.js @@ -784,6 +784,26 @@ storiesOf('PivotTable', module).add( } ) +storiesOf('PivotTable', module).add( + 'cumulative + empty columns (weekly) - shown', + (_, { pivotTableOptions }) => { + const visualization = { + ...weeklyColumnsVisualization, + ...pivotTableOptions, + hideEmptyColumns: false, + cumulativeValues: true, + } + return ( +
+ +
+ ) + } +) + storiesOf('PivotTable', module).add( 'empty columns (weekly) - hidden', (_, { pivotTableOptions }) => { @@ -803,6 +823,26 @@ storiesOf('PivotTable', module).add( } ) +storiesOf('PivotTable', module).add( + 'cumulative + empty columns (weekly) - hidden', + (_, { pivotTableOptions }) => { + const visualization = { + ...weeklyColumnsVisualization, + ...pivotTableOptions, + hideEmptyColumns: true, + cumulativeValues: true, + } + return ( +
+ +
+ ) + } +) + storiesOf('PivotTable', module).add( 'empty columns + assigned cats (shown)', (_, { pivotTableOptions }) => { diff --git a/src/modules/pivotTable/PivotTableEngine.js b/src/modules/pivotTable/PivotTableEngine.js index d59559d2b..5b0480cb1 100644 --- a/src/modules/pivotTable/PivotTableEngine.js +++ b/src/modules/pivotTable/PivotTableEngine.js @@ -55,6 +55,7 @@ const defaultOptions = { showColumnSubtotals: false, fixColumnHeaders: false, fixRowHeaders: false, + cumulativeValues: false, } const defaultVisualizationProps = { @@ -268,6 +269,7 @@ export class PivotTableEngine { data = [] rowMap = [] columnMap = [] + accumulators = { rows: {} } constructor(visualization, data, legendSets) { this.visualization = Object.assign( @@ -306,6 +308,7 @@ export class PivotTableEngine { fixRowHeaders: this.dimensionLookup.rows.length ? visualization.fixRowHeaders : false, + cumulativeValues: visualization.cumulativeValues, } this.adaptiveClippingController = new AdaptiveClippingController(this) @@ -397,6 +400,10 @@ export class PivotTableEngine { } } + getCumulative({ row, column }) { + return this.accumulators.rows[row][column] + } + get({ row, column }) { const mappedRow = this.rowMap[row], mappedColumn = this.columnMap[column] @@ -407,7 +414,27 @@ export class PivotTableEngine { return undefined } - return this.getRaw({ row: mappedRow, column: mappedColumn }) + const value = this.getRaw({ row: mappedRow, column: mappedColumn }) + + // XXX cannot be done directly in getRaw because of the resetAccumulators function + if (this.options.cumulativeValues) { + // some duplicated code here, see if there's a better way + const dxDimension = this.getRawCellDxDimension({ row, column }) + + // XXX this doesn't make much sense, it has to be numeric for accumulation + value.valueType = dxDimension?.valueType || VALUE_TYPE_TEXT + value.empty = false + value.renderedValue = renderValue( + this.getCumulative({ + row: mappedRow, + column: mappedColumn, + }), + value.valueType, + this.visualization + ) + } + + return value } getRawCellType({ row, column }) { @@ -967,6 +994,25 @@ export class PivotTableEngine { : times(this.dataWidth, (n) => n) } + resetAccumulators() { + if (this.options.cumulativeValues) { + this.rowMap.forEach((row) => { + this.accumulators.rows[row] = {} + this.columnMap.reduce((acc, column) => { + const value = this.getRaw({ row, column }) + + acc += value.empty ? 0 : value.rawValue + + this.accumulators.rows[row][column] = acc + + return acc + }, 0) + }) + } else { + this.accumulators = { rows: {} } + } + } + get cellPadding() { switch (this.visualization.displayDensity) { case DISPLAY_DENSITY_OPTION_COMPACT: @@ -1072,6 +1118,8 @@ export class PivotTableEngine { this.resetRowMap() this.resetColumnMap() + this.resetAccumulators() + this.height = this.rowMap.length this.width = this.columnMap.length From 9a825a0d781aaa8eef0deee0d06afc60da17775f Mon Sep 17 00:00:00 2001 From: Edoardo Sabadelli Date: Thu, 9 Nov 2023 15:52:30 +0100 Subject: [PATCH 02/10] fix: render NUMBER type cells for cumulative values This solves formatting issues, like right alignment, caused by the default type being TEXT. --- src/modules/pivotTable/PivotTableEngine.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/modules/pivotTable/PivotTableEngine.js b/src/modules/pivotTable/PivotTableEngine.js index 5b0480cb1..5783c3d66 100644 --- a/src/modules/pivotTable/PivotTableEngine.js +++ b/src/modules/pivotTable/PivotTableEngine.js @@ -422,7 +422,7 @@ export class PivotTableEngine { const dxDimension = this.getRawCellDxDimension({ row, column }) // XXX this doesn't make much sense, it has to be numeric for accumulation - value.valueType = dxDimension?.valueType || VALUE_TYPE_TEXT + value.valueType = dxDimension?.valueType || VALUE_TYPE_NUMBER value.empty = false value.renderedValue = renderValue( this.getCumulative({ From 0a879c4597bc0add226b634ac7a46f2e05d2c051 Mon Sep 17 00:00:00 2001 From: Edoardo Sabadelli Date: Thu, 9 Nov 2023 16:02:05 +0100 Subject: [PATCH 03/10] fix: do not accumulate TEXT values --- src/modules/pivotTable/PivotTableEngine.js | 40 +++++++++++++--------- 1 file changed, 24 insertions(+), 16 deletions(-) diff --git a/src/modules/pivotTable/PivotTableEngine.js b/src/modules/pivotTable/PivotTableEngine.js index 5783c3d66..f30dea784 100644 --- a/src/modules/pivotTable/PivotTableEngine.js +++ b/src/modules/pivotTable/PivotTableEngine.js @@ -418,20 +418,24 @@ export class PivotTableEngine { // XXX cannot be done directly in getRaw because of the resetAccumulators function if (this.options.cumulativeValues) { - // some duplicated code here, see if there's a better way - const dxDimension = this.getRawCellDxDimension({ row, column }) - - // XXX this doesn't make much sense, it has to be numeric for accumulation - value.valueType = dxDimension?.valueType || VALUE_TYPE_NUMBER - value.empty = false - value.renderedValue = renderValue( - this.getCumulative({ - row: mappedRow, - column: mappedColumn, - }), - value.valueType, - this.visualization - ) + const cumulativeValue = this.getCumulative({ + row: mappedRow, + column: mappedColumn, + }) + + if (cumulativeValue) { + // force to NUMBER for accumulated values + value.valueType = + value.valueType === undefined || value.valueType === null + ? VALUE_TYPE_NUMBER + : value.valueType + value.empty = false + value.renderedValue = renderValue( + cumulativeValue, + value.valueType, + this.visualization + ) + } } return value @@ -1001,9 +1005,13 @@ export class PivotTableEngine { this.columnMap.reduce((acc, column) => { const value = this.getRaw({ row, column }) - acc += value.empty ? 0 : value.rawValue + // only accumulate numeric values + // accumulating text values does not make sense + if (value.valueType === VALUE_TYPE_NUMBER) { + acc += value.empty ? 0 : value.rawValue - this.accumulators.rows[row][column] = acc + this.accumulators.rows[row][column] = acc + } return acc }, 0) From f27664a2fb4cea4f010ee6e01a7f2d9647aad5fa Mon Sep 17 00:00:00 2001 From: Edoardo Sabadelli Date: Fri, 10 Nov 2023 12:50:05 +0100 Subject: [PATCH 04/10] fix: pass valueType also for empty cells This is needed for the cumulative values feature, so empty cells can be set to 0 or to the accumulated value. --- src/modules/pivotTable/PivotTableEngine.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/modules/pivotTable/PivotTableEngine.js b/src/modules/pivotTable/PivotTableEngine.js index f30dea784..e30428f01 100644 --- a/src/modules/pivotTable/PivotTableEngine.js +++ b/src/modules/pivotTable/PivotTableEngine.js @@ -336,6 +336,7 @@ export class PivotTableEngine { getRaw({ row, column }) { const cellType = this.getRawCellType({ row, column }) const dxDimension = this.getRawCellDxDimension({ row, column }) + const valueType = dxDimension?.valueType || VALUE_TYPE_TEXT const headers = [ ...this.getRawRowHeader(row), @@ -353,6 +354,7 @@ export class PivotTableEngine { return { cellType, empty: true, + valueType, ouId, peId, } @@ -365,7 +367,6 @@ export class PivotTableEngine { ? dataRow[this.dimensionLookup.dataHeaders.value] : dataRow.value let renderedValue = rawValue - const valueType = dxDimension?.valueType || VALUE_TYPE_TEXT if (valueType === VALUE_TYPE_NUMBER) { rawValue = parseValue(rawValue) From 45dbcc1204c4afa30f672d5dc4dec8f4cb19b0f0 Mon Sep 17 00:00:00 2001 From: Edoardo Sabadelli Date: Fri, 10 Nov 2023 12:51:05 +0100 Subject: [PATCH 05/10] fix: show 0 for emtpy cells with no accumulated value --- src/modules/pivotTable/PivotTableEngine.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/modules/pivotTable/PivotTableEngine.js b/src/modules/pivotTable/PivotTableEngine.js index e30428f01..1ac23d294 100644 --- a/src/modules/pivotTable/PivotTableEngine.js +++ b/src/modules/pivotTable/PivotTableEngine.js @@ -424,7 +424,7 @@ export class PivotTableEngine { column: mappedColumn, }) - if (cumulativeValue) { + if (cumulativeValue !== undefined && cumulativeValue !== null) { // force to NUMBER for accumulated values value.valueType = value.valueType === undefined || value.valueType === null From d861a32b2f3c0b3240c54bd2cd73d7c6844de4d3 Mon Sep 17 00:00:00 2001 From: Edoardo Sabadelli Date: Fri, 10 Nov 2023 15:57:04 +0100 Subject: [PATCH 06/10] fix: pass dxDimension object also for empty cells Needed for the cumulative values feature. --- src/modules/pivotTable/PivotTableEngine.js | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/modules/pivotTable/PivotTableEngine.js b/src/modules/pivotTable/PivotTableEngine.js index 1ac23d294..abef32923 100644 --- a/src/modules/pivotTable/PivotTableEngine.js +++ b/src/modules/pivotTable/PivotTableEngine.js @@ -520,10 +520,8 @@ export class PivotTableEngine { return undefined } const cellValue = this.data[row][column] - if (!cellValue) { - return undefined - } - if (!Array.isArray(cellValue)) { + + if (cellValue && !Array.isArray(cellValue)) { // This is a total cell return { valueType: cellValue.valueType, @@ -559,6 +557,11 @@ export class PivotTableEngine { } } + // Empty cell + // The cell still needs to get the valueType to render correctly 0 and cumulative values + // + // OR + // // Data is in Filter // TODO : This assumes the server ignores text types, we should confirm this is the case return { From 8ac95eeff32125a776d151ed305a014b23c83911 Mon Sep 17 00:00:00 2001 From: Edoardo Sabadelli Date: Fri, 10 Nov 2023 15:59:27 +0100 Subject: [PATCH 07/10] fix: store rendered values in accumulators --- src/modules/pivotTable/PivotTableEngine.js | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/src/modules/pivotTable/PivotTableEngine.js b/src/modules/pivotTable/PivotTableEngine.js index abef32923..20305f671 100644 --- a/src/modules/pivotTable/PivotTableEngine.js +++ b/src/modules/pivotTable/PivotTableEngine.js @@ -417,7 +417,7 @@ export class PivotTableEngine { const value = this.getRaw({ row: mappedRow, column: mappedColumn }) - // XXX cannot be done directly in getRaw because of the resetAccumulators function + // NB: cannot be done directly in getRaw because of the resetAccumulators function if (this.options.cumulativeValues) { const cumulativeValue = this.getCumulative({ row: mappedRow, @@ -431,11 +431,7 @@ export class PivotTableEngine { ? VALUE_TYPE_NUMBER : value.valueType value.empty = false - value.renderedValue = renderValue( - cumulativeValue, - value.valueType, - this.visualization - ) + value.renderedValue = cumulativeValue } } @@ -1014,7 +1010,13 @@ export class PivotTableEngine { if (value.valueType === VALUE_TYPE_NUMBER) { acc += value.empty ? 0 : value.rawValue - this.accumulators.rows[row][column] = acc + const renderedValue = renderValue( + acc, + value.valueType, + this.visualization + ) + + this.accumulators.rows[row][column] = renderedValue } return acc From 44fa7e6ab0cdfdfd8277a8e31f56154745b60fea Mon Sep 17 00:00:00 2001 From: Edoardo Sabadelli Date: Fri, 10 Nov 2023 15:59:48 +0100 Subject: [PATCH 08/10] fix: override cell size when using cumulative values This is to accommodate the accumulated value which might need a wider cell. The cell size is computed based on the rendered value. --- src/modules/pivotTable/PivotTableEngine.js | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/modules/pivotTable/PivotTableEngine.js b/src/modules/pivotTable/PivotTableEngine.js index 20305f671..0640aeed4 100644 --- a/src/modules/pivotTable/PivotTableEngine.js +++ b/src/modules/pivotTable/PivotTableEngine.js @@ -1017,6 +1017,13 @@ export class PivotTableEngine { ) this.accumulators.rows[row][column] = renderedValue + + // override cell sizes based on their new content + // this works for non empty cells where the new value can require a wider cell + this.adaptiveClippingController.add( + { row, column }, + renderedValue + ) } return acc From 9a03421193d9bc7c19abb61945bcf85a512a1b2a Mon Sep 17 00:00:00 2001 From: Edoardo Sabadelli Date: Mon, 13 Nov 2023 14:17:25 +0100 Subject: [PATCH 09/10] fix: add styles for title in disabled sections --- src/components/Options/VisualizationOptions.js | 2 ++ src/components/Options/styles/VisualizationOptions.style.js | 6 ++++++ 2 files changed, 8 insertions(+) diff --git a/src/components/Options/VisualizationOptions.js b/src/components/Options/VisualizationOptions.js index 3bb3e3174..c8eb10e72 100644 --- a/src/components/Options/VisualizationOptions.js +++ b/src/components/Options/VisualizationOptions.js @@ -18,6 +18,7 @@ import { modalContent, tabSection, tabSectionTitle, + tabSectionTitleDisabled, tabSectionTitleMargin, tabSectionOption, tabSectionOptionItem, @@ -95,6 +96,7 @@ const VisualizationOptions = ({ {tabContent.styles} {tabSection.styles} {tabSectionTitle.styles} + {tabSectionTitleDisabled.styles} {tabSectionTitleMargin.styles} {tabSectionOption.styles} {tabSectionOptionItem.styles} diff --git a/src/components/Options/styles/VisualizationOptions.style.js b/src/components/Options/styles/VisualizationOptions.style.js index bcd74f517..61c87803f 100644 --- a/src/components/Options/styles/VisualizationOptions.style.js +++ b/src/components/Options/styles/VisualizationOptions.style.js @@ -51,6 +51,12 @@ export const tabSectionTitle = css.resolve` } ` +export const tabSectionTitleDisabled = css.resolve` + span { + color: ${colors.grey600}; + } +` + export const tabSectionTitleMargin = css.resolve` span { margin-top: ${spacers.dp8}; From 07053587b16d0364524e8d5d61bead5a11971528 Mon Sep 17 00:00:00 2001 From: Edoardo Sabadelli Date: Wed, 15 Nov 2023 15:46:50 +0100 Subject: [PATCH 10/10] fix: fix sorting with cumulative values DHIS2-16156 Refactored and moved some code around to allow getRaw() to return cumulative values, thus fixing the sorting which uses getRaw() to sort the rows/columns. --- src/modules/pivotTable/PivotTableEngine.js | 186 +++++++++++---------- 1 file changed, 95 insertions(+), 91 deletions(-) diff --git a/src/modules/pivotTable/PivotTableEngine.js b/src/modules/pivotTable/PivotTableEngine.js index 0640aeed4..b5788fee1 100644 --- a/src/modules/pivotTable/PivotTableEngine.js +++ b/src/modules/pivotTable/PivotTableEngine.js @@ -350,55 +350,75 @@ export class PivotTableEngine { header?.dimensionItemType === DIMENSION_TYPE_ORGANISATION_UNIT )?.uid - if (!this.data[row] || !this.data[row][column]) { - return { - cellType, - empty: true, - valueType, - ouId, - peId, - } + const rawCell = { + cellType, + valueType, + ouId, + peId, } - const dataRow = this.data[row][column] - - let rawValue = - cellType === CELL_TYPE_VALUE - ? dataRow[this.dimensionLookup.dataHeaders.value] - : dataRow.value - let renderedValue = rawValue - - if (valueType === VALUE_TYPE_NUMBER) { - rawValue = parseValue(rawValue) - switch (this.visualization.numberType) { - case NUMBER_TYPE_ROW_PERCENTAGE: - renderedValue = rawValue / this.percentageTotals[row].value - break - case NUMBER_TYPE_COLUMN_PERCENTAGE: - renderedValue = - rawValue / this.percentageTotals[column].value - break - default: - break + if (!this.data[row] || !this.data[row][column]) { + rawCell.empty = true + } else { + const dataRow = this.data[row][column] + + let rawValue = + cellType === CELL_TYPE_VALUE + ? dataRow[this.dimensionLookup.dataHeaders.value] + : dataRow.value + let renderedValue = rawValue + + if (valueType === VALUE_TYPE_NUMBER) { + rawValue = parseValue(rawValue) + switch (this.visualization.numberType) { + case NUMBER_TYPE_ROW_PERCENTAGE: + renderedValue = + rawValue / this.percentageTotals[row].value + break + case NUMBER_TYPE_COLUMN_PERCENTAGE: + renderedValue = + rawValue / this.percentageTotals[column].value + break + default: + break + } } + + renderedValue = renderValue( + renderedValue, + valueType, + this.visualization + ) + + rawCell.dxDimension = dxDimension + rawCell.empty = false + rawCell.rawValue = rawValue + rawCell.renderedValue = renderedValue } - renderedValue = renderValue( - renderedValue, - valueType, - this.visualization - ) + if (this.options.cumulativeValues) { + const cumulativeValue = this.getCumulative({ + row, + column, + }) - return { - cellType, - empty: false, - valueType, - rawValue, - renderedValue, - dxDimension, - ouId, - peId, + if (cumulativeValue !== undefined && cumulativeValue !== null) { + // force to NUMBER for accumulated values + rawCell.valueType = + valueType === undefined || valueType === null + ? VALUE_TYPE_NUMBER + : valueType + rawCell.empty = false + rawCell.rawValue = cumulativeValue + rawCell.renderedValue = renderValue( + cumulativeValue, + valueType, + this.visualization + ) + } } + + return rawCell } getCumulative({ row, column }) { @@ -415,27 +435,7 @@ export class PivotTableEngine { return undefined } - const value = this.getRaw({ row: mappedRow, column: mappedColumn }) - - // NB: cannot be done directly in getRaw because of the resetAccumulators function - if (this.options.cumulativeValues) { - const cumulativeValue = this.getCumulative({ - row: mappedRow, - column: mappedColumn, - }) - - if (cumulativeValue !== undefined && cumulativeValue !== null) { - // force to NUMBER for accumulated values - value.valueType = - value.valueType === undefined || value.valueType === null - ? VALUE_TYPE_NUMBER - : value.valueType - value.empty = false - value.renderedValue = cumulativeValue - } - } - - return value + return this.getRaw({ row: mappedRow, column: mappedColumn }) } getRawCellType({ row, column }) { @@ -570,7 +570,7 @@ export class PivotTableEngine { return !this.data[row] || this.data[row].length === 0 } columnIsEmpty(column) { - return !this.adaptiveClippingController.columns.sizes[column] + return !this.rowMap.some((row) => this.data[row][column]) } getRawColumnHeader(column) { @@ -1003,27 +1003,30 @@ export class PivotTableEngine { this.rowMap.forEach((row) => { this.accumulators.rows[row] = {} this.columnMap.reduce((acc, column) => { - const value = this.getRaw({ row, column }) + const cellType = this.getRawCellType({ row, column }) + const dxDimension = this.getRawCellDxDimension({ + row, + column, + }) + const valueType = dxDimension?.valueType || VALUE_TYPE_TEXT // only accumulate numeric values // accumulating text values does not make sense - if (value.valueType === VALUE_TYPE_NUMBER) { - acc += value.empty ? 0 : value.rawValue - - const renderedValue = renderValue( - acc, - value.valueType, - this.visualization - ) - - this.accumulators.rows[row][column] = renderedValue - - // override cell sizes based on their new content - // this works for non empty cells where the new value can require a wider cell - this.adaptiveClippingController.add( - { row, column }, - renderedValue - ) + if (valueType === VALUE_TYPE_NUMBER) { + if (this.data[row] && this.data[row][column]) { + const dataRow = this.data[row][column] + + const rawValue = + cellType === CELL_TYPE_VALUE + ? dataRow[ + this.dimensionLookup.dataHeaders.value + ] + : dataRow.value + + acc += parseValue(rawValue) + } + + this.accumulators.rows[row][column] = acc } return acc @@ -1126,21 +1129,22 @@ export class PivotTableEngine { this.finalizeTotals() - this.rawData.rows.forEach((dataRow) => { - const pos = lookup(dataRow, this.dimensionLookup, this) - if (pos) { + this.resetRowMap() + this.resetColumnMap() + + this.resetAccumulators() + + this.rowMap.forEach((row) => { + this.columnMap.forEach((column) => { + const pos = { row, column } + this.adaptiveClippingController.add( pos, this.getRaw(pos).renderedValue ) - } + }) }) - this.resetRowMap() - this.resetColumnMap() - - this.resetAccumulators() - this.height = this.rowMap.length this.width = this.columnMap.length