From 07053587b16d0364524e8d5d61bead5a11971528 Mon Sep 17 00:00:00 2001 From: Edoardo Sabadelli Date: Wed, 15 Nov 2023 15:46:50 +0100 Subject: [PATCH] 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