fix: avoid undefined in totals DHIS2-14511

(cherry picked from commit d93bc3e)
dhis2 · Oct 12, 2023 · 72dd6dc · 72dd6dc
1 parent 16ef1f6
commit 72dd6dc
Show file tree

Hide file tree

Showing 3 changed files with 117 additions and 27 deletions.
diff --git 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
@@ -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
@@ -0,0 +1,4 @@
+export const addToTotalIfNumber = (value, total) =>
+    typeof value === 'number' && Number.isFinite(value)
+        ? (total ?? 0) + value
+        : total