From ed2f3d79c3fda98020ee3e27614aaa04df72afb9 Mon Sep 17 00:00:00 2001 From: Alan Cruikshanks Date: Wed, 1 Nov 2023 16:55:35 +0000 Subject: [PATCH] Refactor money formatters in base presenter (#491) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit https://eaflood.atlassian.net/browse/WATER-4070 https://eaflood.atlassian.net/browse/WATER-4155 https://eaflood.atlassian.net/browse/WATER-4132 https://eaflood.atlassian.net/browse/WATER-4156 We are working on replacing the legacy bill view page because it crashes for large bills. As we are doing this work we are creating some new presenters and have found all rely on taking a value stored in the DB as pence, for example `12776805` and formatting it as money `£127,768.05`. This means each presenter is going through the same steps - make the value positive (we don't show a sign but instead will add ` credit` after the formatted number) - convert it to pounds - format it with `£` and commas for large numbers We're doing this enough it is time to move the logic to our base presenter. But another money formatter? We already have 2! When we checked `formatNumberAsMoney()` is only used by one mock-data endpoint which never asks for the currency symbol. `formatNumberAsMoneyWithCommas()` was added to deal with the final step we listed above in the new presenters we wish to add. So, rather than add _another_ formatter we are refactoring the 2 we have - `formatNumberAsMoney()` becomes `formatPounds()` it will just take a value in pence and return it as pounds which deals with the mock-data endpoint's needs - `formatNumberAsMoneyWithCommas()` becomes `formatMoney()` which will cover the 3 steps listed above --- app/presenters/base.presenter.js | 36 +++++++++-------- .../data/mock-bill-run.presenter.js | 26 ++++++------ test/presenters/base.presenter.test.js | 40 ++++++++----------- 3 files changed, 50 insertions(+), 52 deletions(-) diff --git a/app/presenters/base.presenter.js b/app/presenters/base.presenter.js index 11ac87160b..659e35b7c3 100644 --- a/app/presenters/base.presenter.js +++ b/app/presenters/base.presenter.js @@ -116,33 +116,37 @@ function formatLongDateTime (date) { } /** - * Formats a number which represents a value in pounds as a money string, for example, 1149 as '1149.00' + * Formats a value in pence as a money string with commas, for example, 12776805 as '£127,768.05' + * + * As this is intended for showing values in the UI if the value is a negative it will be made a positive before then + * being formatted. This is because the UI shows credits as '£127,768.05 credit' rather than '£-127,768.05'. + * + * > Credit to https://stackoverflow.com/a/32154217/6117745 for showing numbers with commas * * @param {Number} value The value to display as currency. Assumed to be in pounds * @param {Boolean} includeSymbol Whether to add the £ symbol to the start of the returned string * - * @returns {string} The value formatted as a money string with optional currency symbol + * @returns {string} The value formatted as a money string with commas with optional currency symbol */ -function formatNumberAsMoney (value, includeSymbol = false) { - const symbol = includeSymbol ? '£' : '' +function formatMoney (valueInPence) { + // Even though we store signed values (which you should never do!) we don't display them in the UI. Argh!!! + const positiveValueInPence = Math.abs(valueInPence) + const positiveValueInPounds = convertPenceToPounds(positiveValueInPence) - return `${symbol}${value.toFixed(2)}` + return `£${positiveValueInPounds.toLocaleString('en-GB', { minimumFractionDigits: 2, maximumFractionDigits: 2 })}` } /** - * Formats a number which represents a value in pounds as a money string with commas, for example, 2889 as '2,889.00' + * Formats a number, which represents a value in pence to pounds, for example, 12776805 as '127768.05' * - * > Credit to https://stackoverflow.com/a/32154217/6117745 + * @param {*} valueInPence The value to be formatted to pounds * - * @param {Number} value The value to display as currency. Assumed to be in pounds - * @param {Boolean} includeSymbol Whether to add the £ symbol to the start of the returned string - * - * @returns {string} The value formatted as a money string with commas with optional currency symbol + * @returns {string} The value converted to pounds and formatted to two decimal places */ -function formatNumberAsMoneyWithCommas (value, includeSymbol = false) { - const symbol = includeSymbol ? '£' : '' +function formatPounds (valueInPence) { + const valueInPounds = convertPenceToPounds(valueInPence) - return `${symbol}${value.toLocaleString('en-GB', { minimumFractionDigits: 2 })}` + return valueInPounds.toFixed(2) } /** @@ -167,7 +171,7 @@ module.exports = { formatChargingModuleDate, formatLongDate, formatLongDateTime, - formatNumberAsMoney, - formatNumberAsMoneyWithCommas, + formatMoney, + formatPounds, leftPadZeroes } diff --git a/app/presenters/data/mock-bill-run.presenter.js b/app/presenters/data/mock-bill-run.presenter.js index ddbf4c7d7e..f1d1d0d71e 100644 --- a/app/presenters/data/mock-bill-run.presenter.js +++ b/app/presenters/data/mock-bill-run.presenter.js @@ -5,7 +5,7 @@ * @module MockBillRunPresenter */ -const { convertPenceToPounds, formatAbstractionPeriod, formatLongDate, formatNumberAsMoney } = require('../base.presenter.js') +const { formatAbstractionPeriod, formatLongDate, formatPounds } = require('../base.presenter.js') function go (billRun) { const { @@ -31,7 +31,7 @@ function go (billRun) { transactionFile, billRunNumber, financialYear: `${fromFinancialYearEnding} to ${toFinancialYearEnding}`, - debit: formatNumberAsMoney(convertPenceToPounds(netTotal)), + debit: formatPounds(netTotal), bills: _formatBills(bills) } } @@ -42,8 +42,8 @@ function _formatAdditionalCharges (transaction) { const { grossValuesCalculated, isWaterCompanyCharge, supportedSourceName } = transaction if (supportedSourceName) { - const formattedSupportedSourceCharge = formatNumberAsMoney(grossValuesCalculated.supportedSourceCharge, true) - formattedData.push(`Supported source ${supportedSourceName} (${formattedSupportedSourceCharge})`) + const formattedSupportedSourceCharge = formatPounds(grossValuesCalculated.supportedSourceCharge * 100, true) + formattedData.push(`Supported source ${supportedSourceName} (£${formattedSupportedSourceCharge})`) } if (isWaterCompanyCharge) { @@ -110,9 +110,9 @@ function _formatBills (bills) { accountAddress, contact, isWaterCompany: billLicences[0].licence.isWaterUndertaker, - credit: formatNumberAsMoney(convertPenceToPounds(creditNoteValue)), - debit: formatNumberAsMoney(convertPenceToPounds(invoiceValue)), - netTotal: formatNumberAsMoney(convertPenceToPounds(netAmount)), + credit: formatPounds(creditNoteValue), + debit: formatPounds(invoiceValue), + netTotal: formatPounds(netAmount), licences: _formatBillLicences(billLicences) } }) @@ -135,9 +135,9 @@ function _formatBillLicences (billLicences) { licence, licenceStartDate: billLicence.licence.startDate, licenceHolder, - credit: formatNumberAsMoney(convertPenceToPounds(credit)), - debit: formatNumberAsMoney(convertPenceToPounds(debit)), - netTotal: formatNumberAsMoney(convertPenceToPounds(netTotal)), + credit: formatPounds(credit), + debit: formatPounds(debit), + netTotal: formatPounds(netTotal), transactions: _formatTransactions(transactions) } }) @@ -167,10 +167,10 @@ function _formatTransactions (transactions) { billableDays, authorisedDays, chargeQuantity, - credit: isCredit ? formatNumberAsMoney(convertPenceToPounds(netAmount)) : '0.00', - debit: isCredit ? '0.00' : formatNumberAsMoney(convertPenceToPounds(netAmount)), + credit: isCredit ? formatPounds(netAmount) : '0.00', + debit: isCredit ? '0.00' : formatPounds(netAmount), chargePeriod: `${formatLongDate(startDate)} to ${formatLongDate(endDate)}`, - chargeRefNumber: `${chargeCategoryCode} (${formatNumberAsMoney(grossValuesCalculated.baselineCharge, true)})`, + chargeRefNumber: `${chargeCategoryCode} (£${formatPounds(grossValuesCalculated.baselineCharge * 100, true)})`, chargeDescription, addCharges: _formatAdditionalCharges(transaction), adjustments: _formatAdjustments(chargeReference), diff --git a/test/presenters/base.presenter.test.js b/test/presenters/base.presenter.test.js index 74fb20b3dd..598434e14f 100644 --- a/test/presenters/base.presenter.test.js +++ b/test/presenters/base.presenter.test.js @@ -166,40 +166,34 @@ describe('Base presenter', () => { }) }) - describe('#formatNumberAsMoney()', () => { - const valueInPounds = 1149.5 + describe('#formatPounds()', () => { + const valueInPence = 114950 - describe('when no £ symbol is requested', () => { - it('correctly returns the value as a money string with no symbol, for example, 1149.50', async () => { - const result = BasePresenter.formatNumberAsMoney(valueInPounds) + it('correctly returns the value as pounds, for example, 1149.50', async () => { + const result = BasePresenter.formatPounds(valueInPence) - expect(result).to.equal('1149.50') - }) + expect(result).to.equal('1149.50') }) + }) - describe('when the £ symbol is requested', () => { - it('correctly returns the value as a money string with a symbol, for example, £1149.50', async () => { - const result = BasePresenter.formatNumberAsMoney(valueInPounds, true) + describe('#formatMoney()', () => { + let valueInPence - expect(result).to.equal('£1149.50') + describe('when the value in pence is positive', () => { + beforeEach(() => { + valueInPence = 114950 }) - }) - }) - - describe('#formatNumberAsMoneyWithCommas()', () => { - const valueInPounds = 1149.5 - describe('when no £ symbol is requested', () => { - it('correctly returns the value as a money string with commas and no symbol, for example, 1,149.50', async () => { - const result = BasePresenter.formatNumberAsMoneyWithCommas(valueInPounds) + it('correctly returns the value as a money string with commas and a symbol, for example, £1,149.50', async () => { + const result = BasePresenter.formatMoney(valueInPence) - expect(result).to.equal('1,149.50') + expect(result).to.equal('£1,149.50') }) }) - describe('when the £ symbol is requested', () => { - it('correctly returns the value as a money string with commas and a symbol, for example, £1,149.50', async () => { - const result = BasePresenter.formatNumberAsMoneyWithCommas(valueInPounds, true) + describe('when the value in pence is negative', () => { + it('correctly returns the value as a positive money string with commas and a symbol, for example, £1,149.50', async () => { + const result = BasePresenter.formatMoney(valueInPence, true) expect(result).to.equal('£1,149.50') })