From f2c1fd1807af875e0af0f49bc2027862863ade35 Mon Sep 17 00:00:00 2001 From: Edoardo Sabadelli Date: Thu, 5 Oct 2023 10:29:10 +0200 Subject: [PATCH] fix: avoid undefined in totals DHIS2-14511 (#1552) (#1585) (cherry picked from commit d93bc3e35c560aac7e64b3db8196032201686d3a) --- src/modules/pivotTable/PivotTableEngine.js | 56 +++++++------ .../__tests__/addToTotalIfNumber.js | 84 +++++++++++++++++++ src/modules/pivotTable/addToTotalIfNumber.js | 4 + 3 files changed, 117 insertions(+), 27 deletions(-) create mode 100644 src/modules/pivotTable/__tests__/addToTotalIfNumber.js create mode 100644 src/modules/pivotTable/addToTotalIfNumber.js diff --git a/src/modules/pivotTable/PivotTableEngine.js b/src/modules/pivotTable/PivotTableEngine.js index a3f0b61ce..fc3d91384 100644 --- a/src/modules/pivotTable/PivotTableEngine.js +++ b/src/modules/pivotTable/PivotTableEngine.js @@ -1,6 +1,7 @@ import times from 'lodash/times' import { DIMENSION_ID_ORGUNIT } from '../predefinedDimensions.js' import { AdaptiveClippingController } from './AdaptiveClippingController.js' +import { addToTotalIfNumber } from './addToTotalIfNumber.js' import { parseValue } from './parseValue.js' import { AGGREGATE_TYPE_NA, @@ -703,9 +704,10 @@ export class PivotTableEngine { dataFields.forEach((field) => { const headerIndex = this.dimensionLookup.dataHeaders[field] const value = parseValue(dataRow[headerIndex]) - if (value && !isNaN(value)) { - totalCell[field] = (totalCell[field] || 0) + value - } + totalCell[field] = addToTotalIfNumber( + value, + totalCell[field] + ) }) } totalCell.count += 1 @@ -722,10 +724,10 @@ export class PivotTableEngine { dataFields.forEach((field) => { const headerIndex = this.dimensionLookup.dataHeaders[field] const value = parseValue(dataRow[headerIndex]) - if (value && !isNaN(value)) { - percentageTotal[field] = - (percentageTotal[field] || 0) + value - } + percentageTotal[field] = addToTotalIfNumber( + value, + percentageTotal[field] + ) }) if (totals.columnSubtotal) { @@ -740,10 +742,10 @@ export class PivotTableEngine { dataFields.forEach((field) => { const headerIndex = this.dimensionLookup.dataHeaders[field] const value = parseValue(dataRow[headerIndex]) - if (value && !isNaN(value)) { - percentageTotal[field] = - (percentageTotal[field] || 0) + value - } + percentageTotal[field] = addToTotalIfNumber( + value, + percentageTotal[field] + ) }) } @@ -759,10 +761,10 @@ export class PivotTableEngine { dataFields.forEach((field) => { const headerIndex = this.dimensionLookup.dataHeaders[field] const value = parseValue(dataRow[headerIndex]) - if (value && !isNaN(value)) { - percentageTotal[field] = - (percentageTotal[field] || 0) + value - } + percentageTotal[field] = addToTotalIfNumber( + value, + percentageTotal[field] + ) }) } } @@ -778,10 +780,10 @@ export class PivotTableEngine { dataFields.forEach((field) => { const headerIndex = this.dimensionLookup.dataHeaders[field] const value = parseValue(dataRow[headerIndex]) - if (value && !isNaN(value)) { - percentageTotal[field] = - (percentageTotal[field] || 0) + value - } + percentageTotal[field] = addToTotalIfNumber( + value, + percentageTotal[field] + ) }) if (totals.rowSubtotal) { @@ -796,10 +798,10 @@ export class PivotTableEngine { dataFields.forEach((field) => { const headerIndex = this.dimensionLookup.dataHeaders[field] const value = parseValue(dataRow[headerIndex]) - if (value && !isNaN(value)) { - percentageTotal[field] = - (percentageTotal[field] || 0) + value - } + percentageTotal[field] = addToTotalIfNumber( + value, + percentageTotal[field] + ) }) } @@ -815,10 +817,10 @@ export class PivotTableEngine { dataFields.forEach((field) => { const headerIndex = this.dimensionLookup.dataHeaders[field] const value = parseValue(dataRow[headerIndex]) - if (value && !isNaN(value)) { - percentageTotal[field] = - (percentageTotal[field] || 0) + value - } + percentageTotal[field] = addToTotalIfNumber( + value, + percentageTotal[field] + ) }) } } diff --git a/src/modules/pivotTable/__tests__/addToTotalIfNumber.js b/src/modules/pivotTable/__tests__/addToTotalIfNumber.js new file mode 100644 index 000000000..577f6e778 --- /dev/null +++ b/src/modules/pivotTable/__tests__/addToTotalIfNumber.js @@ -0,0 +1,84 @@ +import { addToTotalIfNumber } from '../addToTotalIfNumber.js' + +const tests = [ + { + testName: 'negative value', + value: -1, + total: undefined, + expected: -1, + }, + { + testName: 'zero value', + value: 0, + total: undefined, + expected: 0, + }, + { + testName: 'positive value', + value: 1, + total: undefined, + expected: 1, + }, + { + testName: 'null value', + value: null, + total: undefined, + expected: undefined, + }, + { + testName: 'undefined value', + value: undefined, + total: undefined, + expected: undefined, + }, + { + testName: 'string value', + value: 'string', + total: undefined, + expected: undefined, + }, + { + testName: 'negative value with existing total', + value: -1, + total: 100, + expected: 99, + }, + { + testName: 'zero value with existing total', + value: 0, + total: 100, + expected: 100, + }, + { + testName: 'positive value with existing total', + value: 1, + total: 100, + expected: 101, + }, + { + testName: 'null value with existing total', + value: null, + total: 100, + expected: 100, + }, + { + testName: 'undefined value with existing total', + value: undefined, + total: 100, + expected: 100, + }, + { + testName: 'string value with existing total', + value: 'string', + total: 100, + expected: 100, + }, +] + +describe('addToTotalIfNumber', () => { + tests.forEach((t) => { + it(t.testName, () => { + expect(addToTotalIfNumber(t.value, t.total)).toEqual(t.expected) + }) + }) +}) diff --git a/src/modules/pivotTable/addToTotalIfNumber.js b/src/modules/pivotTable/addToTotalIfNumber.js new file mode 100644 index 000000000..372d29b1b --- /dev/null +++ b/src/modules/pivotTable/addToTotalIfNumber.js @@ -0,0 +1,4 @@ +export const addToTotalIfNumber = (value, total) => + typeof value === 'number' && Number.isFinite(value) + ? (total ?? 0) + value + : total