From d93bc3e35c560aac7e64b3db8196032201686d3a Mon Sep 17 00:00:00 2001 From: Edoardo Sabadelli Date: Fri, 18 Aug 2023 09:25:24 +0200 Subject: [PATCH 1/4] fix: avoid undefined in totals DHIS2-14511 (#1552) --- 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 From 82f3626a8fb9789e77e6bfdc7d97d7141e0c5d30 Mon Sep 17 00:00:00 2001 From: "@dhis2-bot" Date: Fri, 18 Aug 2023 07:30:25 +0000 Subject: [PATCH 2/4] chore(release): cut 26.0.16 [skip ci] ## [26.0.16](https://github.com/dhis2/analytics/compare/v26.0.15...v26.0.16) (2023-08-18) ### Bug Fixes * avoid undefined in totals DHIS2-14511 ([#1552](https://github.com/dhis2/analytics/issues/1552)) ([d93bc3e](https://github.com/dhis2/analytics/commit/d93bc3e35c560aac7e64b3db8196032201686d3a)) --- CHANGELOG.md | 7 +++++++ package.json | 2 +- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index af3c225f4..1484780c0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,10 @@ +## [26.0.16](https://github.com/dhis2/analytics/compare/v26.0.15...v26.0.16) (2023-08-18) + + +### Bug Fixes + +* avoid undefined in totals DHIS2-14511 ([#1552](https://github.com/dhis2/analytics/issues/1552)) ([d93bc3e](https://github.com/dhis2/analytics/commit/d93bc3e35c560aac7e64b3db8196032201686d3a)) + ## [26.0.15](https://github.com/dhis2/analytics/compare/v26.0.14...v26.0.15) (2023-07-27) diff --git a/package.json b/package.json index 5400ba96a..5e045ed10 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@dhis2/analytics", - "version": "26.0.15", + "version": "26.0.16", "main": "./build/cjs/index.js", "module": "./build/es/index.js", "exports": { From c740e320be568cce4430afaab688dd52ced83fa3 Mon Sep 17 00:00:00 2001 From: Edoardo Sabadelli Date: Mon, 28 Aug 2023 14:09:26 +0200 Subject: [PATCH 3/4] fix: use correct aggregation type if numberType undefined DHIS2-15698 (#1564) numberType can be missing in AO that use the default value for it. The default value is VALUE and should not cause the total aggregation type to be overridden. Co-authored-by: Martin --- src/modules/pivotTable/PivotTableEngine.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/modules/pivotTable/PivotTableEngine.js b/src/modules/pivotTable/PivotTableEngine.js index 815a5b8d4..ddea5c485 100644 --- a/src/modules/pivotTable/PivotTableEngine.js +++ b/src/modules/pivotTable/PivotTableEngine.js @@ -835,7 +835,10 @@ export class PivotTableEngine { if (totalCell && totalCell.count) { totalCell.value = applyTotalAggregationType( totalCell, - this.visualization.numberType !== NUMBER_TYPE_VALUE && + // DHIS2-15698: do not override total aggregation type when numberType option is not present + // (numberType option default is VALUE) + this.visualization.numberType && + this.visualization.numberType !== NUMBER_TYPE_VALUE && AGGREGATE_TYPE_SUM ) this.adaptiveClippingController.add( From 6d1bcc9c83b336bf655d34c7455ed9347124ce7c Mon Sep 17 00:00:00 2001 From: "@dhis2-bot" Date: Mon, 28 Aug 2023 12:14:16 +0000 Subject: [PATCH 4/4] chore(release): cut 26.0.17 [skip ci] ## [26.0.17](https://github.com/dhis2/analytics/compare/v26.0.16...v26.0.17) (2023-08-28) ### Bug Fixes * use correct aggregation type if numberType undefined DHIS2-15698 ([#1564](https://github.com/dhis2/analytics/issues/1564)) ([c740e32](https://github.com/dhis2/analytics/commit/c740e320be568cce4430afaab688dd52ced83fa3)) --- CHANGELOG.md | 7 +++++++ package.json | 2 +- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1484780c0..69b7b35c7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,10 @@ +## [26.0.17](https://github.com/dhis2/analytics/compare/v26.0.16...v26.0.17) (2023-08-28) + + +### Bug Fixes + +* use correct aggregation type if numberType undefined DHIS2-15698 ([#1564](https://github.com/dhis2/analytics/issues/1564)) ([c740e32](https://github.com/dhis2/analytics/commit/c740e320be568cce4430afaab688dd52ced83fa3)) + ## [26.0.16](https://github.com/dhis2/analytics/compare/v26.0.15...v26.0.16) (2023-08-18) diff --git a/package.json b/package.json index 5e045ed10..eef1e94f0 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@dhis2/analytics", - "version": "26.0.16", + "version": "26.0.17", "main": "./build/cjs/index.js", "module": "./build/es/index.js", "exports": {