Refactor money formatters in base presenter (#491)

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

@@ -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
 }

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),

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')
       })