From 72dd6dcb4d9b90d2648903a6712d7700c712b3d3 Mon Sep 17 00:00:00 2001 From: Edoardo Sabadelli Date: Fri, 18 Aug 2023 09:25:24 +0200 Subject: [PATCH] fix: avoid undefined in totals DHIS2-14511 (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 46f8cc0fa..815a5b8d4 100644 --- a/src/modules/pivotTable/PivotTableEngine.js +++ b/src/modules/pivotTable/PivotTableEngine.js @@ -9,6 +9,7 @@ import { DIMENSION_ID_ORGUNIT } from '../predefinedDimensions.js' import { renderValue } from '../renderValue.js' import { VALUE_TYPE_NUMBER, VALUE_TYPE_TEXT } from '../valueTypes.js' import { AdaptiveClippingController } from './AdaptiveClippingController.js' +import { addToTotalIfNumber } from './addToTotalIfNumber.js' import { parseValue } from './parseValue.js' import { AGGREGATE_TYPE_NA, @@ -705,9 +706,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 @@ -724,10 +726,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) { @@ -742,10 +744,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] + ) }) } @@ -761,10 +763,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] + ) }) } } @@ -780,10 +782,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) { @@ -798,10 +800,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] + ) }) } @@ -817,10 +819,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