From 21eb3015ad1f0c62d52e10ea326cc250ee4aaa31 Mon Sep 17 00:00:00 2001 From: Alan Cruikshanks Date: Wed, 1 Nov 2023 10:11:55 +0000 Subject: [PATCH 1/2] Add money formatter to base presenter 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 were 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. From 787c7536d092459d545a299633cafd12109b60e1 Mon Sep 17 00:00:00 2001 From: Alan Cruikshanks Date: Wed, 1 Nov 2023 16:48:13 +0000 Subject: [PATCH 2/2] Implement changes --- 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') })