Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: compute subtotals/totals for boolean types (DHIS2-9155) #1696

Merged
merged 4 commits into from
Aug 27, 2024
Merged

fix: compute subtotals/totals for boolean types (DHIS2-9155) #1696

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
ed4fa5d
fix: compute subtotals/totals for boolean types (DHIS2-9155)
edoardo Aug 21, 2024
373c225
fix: use 2 decimals as default
edoardo Aug 26, 2024
484ffed
test: adjust test for 2 decimals
edoardo Aug 27, 2024
60b890b
Merge remote-tracking branch 'origin/master' into fix/boolean-type-to…
edoardo Aug 27, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 10 additions & 10 deletions src/modules/__tests__/renderValue.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,21 +25,21 @@ const tests = [
// Numbers
{
value: 1000.5,
expected: '1 000.5',
expected: '1 000.50',
valueType: VALUE_TYPE_NUMBER,
round: true,
dgs: DGS_SPACE,
},
{
value: 33777889.55,
expected: '33,777,889.5',
value: 33777889.555,
expected: '33,777,889.55',
valueType: VALUE_TYPE_NUMBER,
round: true,
dgs: DGS_COMMA,
},
{
value: 33777889.556,
expected: '33 777 889.6',
expected: '33 777 889.56',
valueType: VALUE_TYPE_NUMBER,
round: true,
dgs: DGS_SPACE,
Expand All @@ -53,7 +53,7 @@ const tests = [
},
{
value: 33777889.56,
expected: '33777889.6',
expected: '33777889.56',
valueType: VALUE_TYPE_NUMBER,
round: true,
dgs: DGS_NONE,
Expand All @@ -74,7 +74,7 @@ const tests = [
},
{
value: 1.101,
expected: '1.1',
expected: '1.10',
valueType: VALUE_TYPE_NUMBER,
round: true,
dgs: DGS_SPACE,
Expand Down Expand Up @@ -135,16 +135,16 @@ const tests = [
dgs: DGS_SPACE,
},
{
value: -0.0234,
expected: '-2.3%',
value: -0.02345,
expected: '-2.34%',
valueType: VALUE_TYPE_NUMBER,
numberType: NUMBER_TYPE_ROW_PERCENTAGE,
round: true,
dgs: DGS_SPACE,
},
{
value: -0.0234,
expected: '-2.34%',
value: -0.02345,
expected: '-2.345%',
valueType: VALUE_TYPE_NUMBER,
numberType: NUMBER_TYPE_ROW_PERCENTAGE,
round: false,
Expand Down
17 changes: 15 additions & 2 deletions src/modules/pivotTable/PivotTableEngine.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,12 @@ import {
} from '../dataTypes.js'
import { DIMENSION_ID_ORGUNIT } from '../predefinedDimensions.js'
import { renderValue } from '../renderValue.js'
import { VALUE_TYPE_NUMBER, VALUE_TYPE_TEXT } from '../valueTypes.js'
import {
VALUE_TYPE_NUMBER,
VALUE_TYPE_TEXT,
isBooleanValueType,
isNumericValueType,
} from '../valueTypes.js'
import { AdaptiveClippingController } from './AdaptiveClippingController.js'
import { addToTotalIfNumber } from './addToTotalIfNumber.js'
import { parseValue } from './parseValue.js'
Expand Down Expand Up @@ -744,7 +749,15 @@ export class PivotTableEngine {
totalCell.valueType = currentValueType
}

if (dxDimension?.valueType === VALUE_TYPE_NUMBER) {
// compute subtotals and totals for all numeric and boolean value types
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had to keep the part above with the prev/current comparison because valueType can be of types other than numeric and boolean.
In that case, the total cell would not get the type, but always N/A if we simply check for numeric/boolean for assigning the last evaluated valueType in the loop and fallback to N/A otherwise.

// in that case, force value type of subtotal and total cells to NUMBER to format them correctly
// (see DHIS2-9155)
if (
isNumericValueType(dxDimension?.valueType) ||
isBooleanValueType(dxDimension?.valueType)
) {
totalCell.valueType = VALUE_TYPE_NUMBER

dataFields.forEach((field) => {
const headerIndex = this.dimensionLookup.dataHeaders[field]
const value = parseValue(dataRow[headerIndex])
Expand Down
2 changes: 1 addition & 1 deletion src/modules/renderValue.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ const toFixedPrecisionString = (value, skipRounding) => {
return value
}

const precision = skipRounding ? 10 : value > -1 && value < 1 ? 2 : 1
const precision = skipRounding ? 10 : 2

return value.toFixed(precision)
}
Expand Down
Loading