From 3f1d319949870f4c13dd72e134a82d9e02ec4bb5 Mon Sep 17 00:00:00 2001 From: Jason Claxton Date: Tue, 4 Jun 2024 14:41:01 +0100 Subject: [PATCH 1/7] Resolve values with too many decimal places https://eaflood.atlassian.net/browse/WATER-4489 We have spotted an issue while working on two-part tariff where some of the decimal values didn't quite look as we would have expected them to. After some investigation we found that this is a common issue with JavaScript when adding decimals together. A decent explanation of this issue can be found here https://ellenaua.medium.com/floating-point-errors-in-javascript-node-js-21aadd897bf8 To resolve this problem we are importing a new package that will enable us to perform mathematical functions on decimals without any issues. Several potential packages were evaluated for suitability including `decimal.js` & `bignumber.js` but in the end we have settled on `big.js` as it fits our needs the best and is well used at nearly 2 million downloads a week. In this PR the package is going to be imported and used on any existing calculations that involve decimals. From 2b201674a813d22059057815dcf82896a796754d Mon Sep 17 00:00:00 2001 From: Jason Claxton Date: Tue, 4 Jun 2024 14:54:36 +0100 Subject: [PATCH 2/7] Install `big.js` package --- package-lock.json | 18 ++++++++++++++++++ package.json | 1 + 2 files changed, 19 insertions(+) diff --git a/package-lock.json b/package-lock.json index 7dc4e18dfc..4bd169e581 100644 --- a/package-lock.json +++ b/package-lock.json @@ -21,6 +21,7 @@ "@joi/date": "^2.1.0", "@smithy/node-http-handler": "^2.0.4", "bcryptjs": "^2.4.3", + "big.js": "^6.2.1", "blipp": "^4.0.2", "dotenv": "^16.3.1", "got": "^12.5.3", @@ -4486,6 +4487,18 @@ "resolved": "https://registry.npmjs.org/bcryptjs/-/bcryptjs-2.4.3.tgz", "integrity": "sha512-V/Hy/X9Vt7f3BbPJEi8BdVFMByHi+jNXrYkW3huaybV/kQ0KJg0Y6PkEMbn+zeT+i+SiKZ/HMqJGIIt4LZDqNQ==" }, + "node_modules/big.js": { + "version": "6.2.1", + "resolved": "https://registry.npmjs.org/big.js/-/big.js-6.2.1.tgz", + "integrity": "sha512-bCtHMwL9LeDIozFn+oNhhFoq+yQ3BNdnsLSASUxLciOb1vgvpHsIO1dsENiGMgbb4SkP5TrzWzRiLddn8ahVOQ==", + "engines": { + "node": "*" + }, + "funding": { + "type": "opencollective", + "url": "https://opencollective.com/bigjs" + } + }, "node_modules/binary-extensions": { "version": "2.2.0", "resolved": "https://registry.npmjs.org/binary-extensions/-/binary-extensions-2.2.0.tgz", @@ -13720,6 +13733,11 @@ "resolved": "https://registry.npmjs.org/bcryptjs/-/bcryptjs-2.4.3.tgz", "integrity": "sha512-V/Hy/X9Vt7f3BbPJEi8BdVFMByHi+jNXrYkW3huaybV/kQ0KJg0Y6PkEMbn+zeT+i+SiKZ/HMqJGIIt4LZDqNQ==" }, + "big.js": { + "version": "6.2.1", + "resolved": "https://registry.npmjs.org/big.js/-/big.js-6.2.1.tgz", + "integrity": "sha512-bCtHMwL9LeDIozFn+oNhhFoq+yQ3BNdnsLSASUxLciOb1vgvpHsIO1dsENiGMgbb4SkP5TrzWzRiLddn8ahVOQ==" + }, "binary-extensions": { "version": "2.2.0", "resolved": "https://registry.npmjs.org/binary-extensions/-/binary-extensions-2.2.0.tgz", diff --git a/package.json b/package.json index 1404b9ba53..465e3ecc47 100644 --- a/package.json +++ b/package.json @@ -41,6 +41,7 @@ "@joi/date": "^2.1.0", "@smithy/node-http-handler": "^2.0.4", "bcryptjs": "^2.4.3", + "big.js": "^6.2.1", "blipp": "^4.0.2", "dotenv": "^16.3.1", "got": "^12.5.3", From 5b6e89dcb84e3e47385289299e7d5a408af920e0 Mon Sep 17 00:00:00 2001 From: Jason Claxton Date: Tue, 4 Jun 2024 15:41:16 +0100 Subject: [PATCH 3/7] Add `BIG` to allocate returns service --- ...ocate-returns-to-charge-element.service.js | 40 +++++++++++++------ 1 file changed, 27 insertions(+), 13 deletions(-) diff --git a/app/services/bill-runs/two-part-tariff/allocate-returns-to-charge-element.service.js b/app/services/bill-runs/two-part-tariff/allocate-returns-to-charge-element.service.js index 127eb0c7b0..5cafabad38 100644 --- a/app/services/bill-runs/two-part-tariff/allocate-returns-to-charge-element.service.js +++ b/app/services/bill-runs/two-part-tariff/allocate-returns-to-charge-element.service.js @@ -5,6 +5,8 @@ * @module AllocateReturnsToChargeElementService */ +const Big = require('big.js') + const { periodsOverlap } = require('../../../lib/general.lib.js') /** @@ -34,8 +36,12 @@ function go (chargeElement, matchingReturns, chargePeriod, chargeReference) { function _allocateReturns (chargeElement, matchedReturn, chargePeriod, chargeReference, i, matchedLines) { matchedLines.forEach((matchedLine) => { - const chargeElementRemainingAllocation = chargeElement.authorisedAnnualQuantity - chargeElement.allocatedQuantity - const chargeReferenceRemainingAllocation = chargeReference.volume - chargeReference.allocatedQuantity + const chargeElementRemainingAllocation = Big(chargeElement.authorisedAnnualQuantity) + .minus(chargeElement.allocatedQuantity) + .toNumber() + const chargeReferenceRemainingAllocation = Big(chargeReference.volume) + .minus(chargeReference.allocatedQuantity) + .toNumber() const remainingAllocation = Math.min(chargeElementRemainingAllocation, chargeReferenceRemainingAllocation) @@ -56,33 +62,41 @@ function _allocateReturns (chargeElement, matchedReturn, chargePeriod, chargeRef chargeElement.chargeDatesOverlap = _chargeDatesOverlap(matchedLine, chargePeriod) } - chargeElement.allocatedQuantity += qtyToAllocate - chargeElement.returnLogs[i].allocatedQuantity += qtyToAllocate - matchedLine.unallocated -= qtyToAllocate - matchedReturn.allocatedQuantity += qtyToAllocate - chargeReference.allocatedQuantity += qtyToAllocate + chargeElement.allocatedQuantity = Big(chargeElement.allocatedQuantity).plus(qtyToAllocate).toNumber() + chargeElement.returnLogs[i].allocatedQuantity = Big(chargeElement.returnLogs[i].allocatedQuantity) + .plus(qtyToAllocate) + .toNumber() + matchedLine.unallocated = Big(matchedLine.unallocated).minus(qtyToAllocate).toNumber() + matchedReturn.allocatedQuantity = Big(matchedReturn.allocatedQuantity).plus(qtyToAllocate).toNumber() + chargeReference.allocatedQuantity = Big(chargeReference.allocatedQuantity).plus(qtyToAllocate).toNumber() } }) } function _allocateDueReturns (chargeElement, matchedReturn, chargeReference, i) { - const chargeElementRemainingAllocation = chargeElement.authorisedAnnualQuantity - chargeElement.allocatedQuantity + const chargeElementRemainingAllocation = Big(chargeElement.authorisedAnnualQuantity) + .minus(chargeElement.allocatedQuantity) + .toNumber() if (chargeElementRemainingAllocation > 0) { let qtyToAllocate = chargeElementRemainingAllocation // If the unallocated volume on the element is greater than that remaining on the reference the `qtyToAllocate` will // be set to whatever volume remains unallocated on the reference - const chargeReferenceRemainingAllocation = chargeReference.volume - chargeReference.allocatedQuantity + const chargeReferenceRemainingAllocation = Big(chargeReference.volume) + .minus(chargeReference.allocatedQuantity) + .toNumber() if (chargeElementRemainingAllocation > chargeReferenceRemainingAllocation) { qtyToAllocate = chargeReferenceRemainingAllocation } - chargeElement.allocatedQuantity += qtyToAllocate - chargeElement.returnLogs[i].allocatedQuantity += qtyToAllocate - matchedReturn.allocatedQuantity += qtyToAllocate - chargeReference.allocatedQuantity += qtyToAllocate + chargeElement.allocatedQuantity = Big(chargeElement.allocatedQuantity).plus(qtyToAllocate).toNumber() + chargeElement.returnLogs[i].allocatedQuantity = Big(chargeElement.returnLogs[i].allocatedQuantity) + .plus(qtyToAllocate) + .toNumber() + matchedReturn.allocatedQuantity = Big(matchedReturn.allocatedQuantity).plus(qtyToAllocate).toNumber() + chargeReference.allocatedQuantity = Big(chargeReference.allocatedQuantity).plus(qtyToAllocate).toNumber() } } From 9538dd1d5b93ecf4124e4a913b05efc9509a312b Mon Sep 17 00:00:00 2001 From: Jason Claxton Date: Tue, 4 Jun 2024 15:57:15 +0100 Subject: [PATCH 4/7] Add `Big` to prepare return logs service --- .../two-part-tariff/prepare-return-logs.service.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/app/services/bill-runs/two-part-tariff/prepare-return-logs.service.js b/app/services/bill-runs/two-part-tariff/prepare-return-logs.service.js index 902a276ae3..37c93c433c 100644 --- a/app/services/bill-runs/two-part-tariff/prepare-return-logs.service.js +++ b/app/services/bill-runs/two-part-tariff/prepare-return-logs.service.js @@ -5,6 +5,8 @@ * @module PrepareReturnLogsService */ +const Big = require('big.js') + const DetermineAbstractionPeriodService = require('../determine-abstraction-periods.service.js') const FetchReturnLogsForLicenceService = require('./fetch-return-logs-for-licence.service.js') const { periodsOverlap } = require('../../../lib/general.lib.js') @@ -53,8 +55,8 @@ function _prepReturnsForMatching (returnLogs, billingPeriod) { if (!abstractionOutsidePeriod) { abstractionOutsidePeriod = _abstractionOutsidePeriod(abstractionPeriods, returnSubmissionLine) } - returnSubmissionLine.unallocated = returnSubmissionLine.quantity / 1000 - quantity += returnSubmissionLine.unallocated + returnSubmissionLine.unallocated = Big(returnSubmissionLine.quantity).div(1000).toNumber() + quantity = Big(quantity).plus(returnSubmissionLine.unallocated).toNumber() }) returnLog.nilReturn = returnSubmissions[0]?.nilReturn ?? false From b0c84191a3c1849c4770504cbf216376f57d84c3 Mon Sep 17 00:00:00 2001 From: Jason Claxton Date: Tue, 4 Jun 2024 16:14:28 +0100 Subject: [PATCH 5/7] Add `Big` to charge reference details presenter --- .../two-part-tariff/charge-reference-details.presenter.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/app/presenters/bill-runs/two-part-tariff/charge-reference-details.presenter.js b/app/presenters/bill-runs/two-part-tariff/charge-reference-details.presenter.js index 2a02eb0bf9..dadf1643dc 100644 --- a/app/presenters/bill-runs/two-part-tariff/charge-reference-details.presenter.js +++ b/app/presenters/bill-runs/two-part-tariff/charge-reference-details.presenter.js @@ -5,6 +5,8 @@ * @module ChargeReferenceDetailsPresenter */ +const Big = require('big.js') + const { formatLongDate, formatFinancialYear } = require('../../base.presenter.js') /** @@ -113,7 +115,7 @@ function _prepareDate (startDate, endDate) { function _totalBillableReturns (reviewChargeElements) { return reviewChargeElements.reduce((total, reviewChargeElement) => { - total += reviewChargeElement.amendedAllocated + total = Big(total).plus(reviewChargeElement.amendedAllocated).toNumber() return total }, 0) From 91c1f530a7072b37bd0452f6445e49b69f7aa014 Mon Sep 17 00:00:00 2001 From: Jason Claxton Date: Tue, 4 Jun 2024 16:17:42 +0100 Subject: [PATCH 6/7] Add `Big` to review licence presenter --- .../bill-runs/two-part-tariff/review-licence.presenter.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/app/presenters/bill-runs/two-part-tariff/review-licence.presenter.js b/app/presenters/bill-runs/two-part-tariff/review-licence.presenter.js index e8b36d0379..d403d9bc61 100644 --- a/app/presenters/bill-runs/two-part-tariff/review-licence.presenter.js +++ b/app/presenters/bill-runs/two-part-tariff/review-licence.presenter.js @@ -5,6 +5,8 @@ * @module ReviewLicencePresenter */ +const Big = require('big.js') + const DetermineAbstractionPeriodService = require('../../../services/bill-runs/determine-abstraction-periods.service.js') const { formatLongDate } = require('../../base.presenter.js') @@ -277,7 +279,7 @@ function _totalBillableReturns (reviewChargeReference) { let totalBillableReturns = 0 reviewChargeReference.reviewChargeElements.forEach((reviewChargeElement) => { - totalBillableReturns += reviewChargeElement.amendedAllocated + totalBillableReturns = Big(totalBillableReturns).plus(reviewChargeElement.amendedAllocated).toNumber() }) return `${totalBillableReturns} ML / ${reviewChargeReference.amendedAuthorisedVolume} ML` From dc4caef5f1db6358b7eb7259d574de1f3105cf48 Mon Sep 17 00:00:00 2001 From: Jason Claxton Date: Tue, 4 Jun 2024 16:40:43 +0100 Subject: [PATCH 7/7] Fix review licence service tests --- .../bill-runs/two-part-tariff/review-licence.service.test.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/services/bill-runs/two-part-tariff/review-licence.service.test.js b/test/services/bill-runs/two-part-tariff/review-licence.service.test.js index a2c8627c4d..59d5818bf9 100644 --- a/test/services/bill-runs/two-part-tariff/review-licence.service.test.js +++ b/test/services/bill-runs/two-part-tariff/review-licence.service.test.js @@ -97,6 +97,7 @@ function _fetchReviewLicenceResults () { reviewChargeReferenceId: '7759e0f9-5763-4b94-8d45-0621aea3edc1', chargeElementId: 'b1cd4f98-ad96-4901-9e21-4432f032492a', allocated: 0, + amendedAllocated: 0, chargeDatesOverlap: false, issues: '', status: 'ready' @@ -152,6 +153,7 @@ function _fetchReviewLicenceResults () { reviewChargeReferenceId: '2210bb45-1efc-4e69-85cb-c8cc6e75c4fd', chargeElementId: 'b1001716-cfb4-43c6-91f0-1863f4529223', allocated: 0, + amendedAllocated: 0, chargeDatesOverlap: false, issues: '', status: 'ready',