fix: avoid undefined in totals DHIS2-14511 (#1552)

(cherry picked from commit d93bc3e)
dhis2 · Oct 5, 2023 · 6b7214f · 6b7214f
1 parent d130385
commit 6b7214f
Show file tree

Hide file tree

Showing 3 changed files with 118 additions and 28 deletions.
diff --git a/src/modules/pivotTable/PivotTableEngine.js b/src/modules/pivotTable/PivotTableEngine.js
@@ -1,7 +1,8 @@
 import times from 'lodash/times'
 import { DIMENSION_ID_ORGUNIT } from '../predefinedDimensions'
 import { AdaptiveClippingController } from './AdaptiveClippingController.js'
-import { parseValue } from './parseValue'
+import { addToTotalIfNumber } from './addToTotalIfNumber.js'
+import { parseValue } from './parseValue.js'
 import {
     AGGREGATE_TYPE_NA,
     AGGREGATE_TYPE_AVERAGE,
@@ -692,9 +693,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
@@ -711,10 +713,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) {
@@ -729,10 +731,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]
+                    )
                 })
             }
 
@@ -748,10 +750,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]
+                    )
                 })
             }
         }
@@ -767,10 +769,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) {
@@ -785,10 +787,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]
+                    )
                 })
             }
 
@@ -804,10 +806,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