Tidy supplementary billing code - second pass

app/lib/legacy-request.lib.js

@@ -76,7 +76,7 @@ const services = {
  * @returns {Object} result.response The legacy service's response if successful or the error response if not.
  */
 async function get (serviceName, path, apiRequest = true) {
-  return await _sendRequest(RequestLib.get, serviceName, path, apiRequest)
+  return _sendRequest(RequestLib.get, serviceName, path, apiRequest)
 }
 
 /**
@@ -93,7 +93,7 @@ async function get (serviceName, path, apiRequest = true) {
  * @returns {Object} result.response The legacy service's response if successful or the error response if not.
  */
 async function post (serviceName, path, apiRequest = true, body = {}) {
-  return await _sendRequest(RequestLib.post, serviceName, path, apiRequest, body)
+  return _sendRequest(RequestLib.post, serviceName, path, apiRequest, body)
 }
 
 /**

app/lib/request.lib.js

@@ -31,15 +31,15 @@ const requestConfig = require('../../config/request.config.js')
  * @returns {Object} The result of the request; whether it succeeded and the response or error returned
  */
 async function get (url, additionalOptions = {}) {
-  return await _sendRequest('get', url, additionalOptions)
+  return _sendRequest('get', url, additionalOptions)
 }
 
 async function patch (url, additionalOptions = {}) {
-  return await _sendRequest('patch', url, additionalOptions)
+  return _sendRequest('patch', url, additionalOptions)
 }
 
 async function post (url, additionalOptions = {}) {
-  return await _sendRequest('post', url, additionalOptions)
+  return _sendRequest('post', url, additionalOptions)
 }
 
 async function _sendRequest (method, url, additionalOptions) {

app/services/plugins/filter-routes.service.js

@@ -42,7 +42,7 @@ function _protectedEnvironment (environment) {
 }
 
 function _filteredRoutes (routes) {
-  return routes.filter(route => !route?.options?.app?.excludeFromProd)
+  return routes.filter((route) => !route?.options?.app?.excludeFromProd)
 }
 
 module.exports = {

app/services/supplementary-billing/calculate-authorised-and-billable-days.service.js

@@ -61,10 +61,10 @@ function go (chargePeriod, billingPeriod, chargeElement) {
   const authorisedAbstractionPeriods = []
   const billableAbstractionPeriods = []
 
-  for (const chargePurpose of chargePurposes) {
+  chargePurposes.forEach((chargePurpose) => {
     authorisedAbstractionPeriods.push(..._abstractionPeriods(billingPeriod, chargePurpose))
     billableAbstractionPeriods.push(..._abstractionPeriods(chargePeriod, chargePurpose))
-  }
+  })
 
   return {
     authorisedDays: _consolidateAndCalculate(billingPeriod, authorisedAbstractionPeriods),
@@ -207,13 +207,12 @@ function _calculateAbstractionOverlapPeriod (referencePeriod, abstractionPeriod)
 function _consolidateAndCalculate (referencePeriod, abstractionsPeriods) {
   const consolidatedAbstractionPeriods = ConsolidateDateRangesService.go(abstractionsPeriods)
 
-  let days = 0
-  for (const abstractionPeriod of consolidatedAbstractionPeriods) {
+  const totalDays = consolidatedAbstractionPeriods.reduce((acc, abstractionPeriod) => {
     const abstractionOverlapPeriod = _calculateAbstractionOverlapPeriod(referencePeriod, abstractionPeriod)
-    days += _calculateDays(abstractionOverlapPeriod)
-  }
+    return acc + _calculateDays(abstractionOverlapPeriod)
+  }, 0)
 
-  return days
+  return totalDays
 }
 
 /**

app/services/supplementary-billing/determine-charge-period.service.js

@@ -33,19 +33,16 @@ function go (chargeVersion, financialYearEnding) {
     chargeVersion.licence.startDate
   )
 
-  // If the end date is null then use the financial year end date instead as Math.min() will count nulls as 0
-  const chargeVersionEndDate = chargeVersion.endDate || financialYearEndDate
-  const licenceExpiredDate = chargeVersion.licence.expiredDate || financialYearEndDate
-  const licencelapsedDate = chargeVersion.licence.lapsedDate || financialYearEndDate
-  const licencerevokedDate = chargeVersion.licence.revokedDate || financialYearEndDate
-
-  const earliestEndDateTimestamp = Math.min(
+  // We use .filter() to remove any null timestamps, as Math.min() assumes a value of `0` for these
+  const endDateTimestamps = [
     financialYearEndDate,
-    chargeVersionEndDate,
-    licenceExpiredDate,
-    licencelapsedDate,
-    licencerevokedDate
-  )
+    chargeVersion.endDate,
+    chargeVersion.licence.expiredDate,
+    chargeVersion.licence.lapsedDate,
+    chargeVersion.licence.revokedDate
+  ].filter((timestamp) => timestamp)
+
+  const earliestEndDateTimestamp = Math.min(...endDateTimestamps)
 
   return {
     startDate: new Date(latestStartDateTimestamp),

app/services/supplementary-billing/fetch-previous-billing-transactions.service.js

@@ -39,15 +39,15 @@ function _cleanse (billingTransactions) {
   const credits = billingTransactions.filter((transaction) => transaction.isCredit)
   const debits = billingTransactions.filter((transaction) => !transaction.isCredit)
 
-  for (const credit of credits) {
+  credits.forEach((credit) => {
     const debitIndex = debits.findIndex((debit) => {
       return debit.billableDays === credit.billableDays && debit.chargeType === credit.chargeType
     })
 
     if (debitIndex > -1) {
       debits.splice(debitIndex, 1)
     }
-  }
+  })
 
   return debits
 }

app/services/supplementary-billing/process-billing-batch.service.js

@@ -58,30 +58,23 @@ async function go (billingBatch, billingPeriod) {
         billingPeriod
       )
 
-      // We need to deal with the very first iteration when currentBillingData is all nulls! So, we check both there is
-      // a billingInvoiceLicence and that its ID is different
-      if (
-        currentBillingData.billingInvoiceLicence &&
-        currentBillingData.billingInvoiceLicence.billingInvoiceLicenceId !== billingInvoiceLicence.billingInvoiceLicenceId
-      ) {
+      // If we've moved on to the next invoice licence then we need to finalise the previous one before we can continue
+      if (_thisIsADifferentBillingInvoiceLicence(currentBillingData, billingInvoiceLicence)) {
         await _finaliseCurrentInvoiceLicence(currentBillingData, billingPeriod, billingBatch)
         currentBillingData.calculatedTransactions = []
       }
 
-      currentBillingData.licence = chargeVersion.licence
-      currentBillingData.billingInvoice = billingInvoice
-      currentBillingData.billingInvoiceLicence = billingInvoiceLicence
-
-      // If the charge version has a status of 'current' (APPROVED) then it is likely to be something we have never
-      // billed previously so we need to calculate a debit line.
-      // Else the charge version has been 'superseded' (REPLACED). So, we won't be adding a new debit line to the bill
-      // for it. But we still need to process it to understand what, if anything, needs to be credited back or if our
-      // calculated debit line has already been billed.
-      if (chargeVersion.status === 'current') {
-        const calculatedTransactions = _generateCalculatedTransactions(billingPeriod, chargeVersion, billingBatchId, billingInvoiceLicence)
-        currentBillingData.calculatedTransactions.push(...calculatedTransactions)
-      }
+      _updateCurrentBillingData(currentBillingData, chargeVersion, billingInvoice, billingInvoiceLicence)
+
+      _generateTransactionsIfStatusIsCurrent(
+        chargeVersion,
+        billingPeriod,
+        billingBatchId,
+        billingInvoiceLicence,
+        currentBillingData
+      )
     }
+
     await _finaliseCurrentInvoiceLicence(currentBillingData, billingPeriod, billingBatch)
 
     await _finaliseBillingBatch(billingBatch, chargeVersions, currentBillingData.isEmpty)
@@ -93,6 +86,39 @@ async function go (billingBatch, billingPeriod) {
   }
 }
 
+function _generateTransactionsIfStatusIsCurrent (chargeVersion, billingPeriod, billingBatchId, billingInvoiceLicence, currentBillingData) {
+  // If the charge version status isn't 'current' then we don't need to add any new debit lines to the bill
+  if (chargeVersion.status !== 'current') {
+    return
+  }
+
+  // Otherwise, it's likely to be something we have never billed previously so we need to calculate debit line(s)
+  const calculatedTransactions = _generateCalculatedTransactions(
+    billingPeriod,
+    chargeVersion,
+    billingBatchId,
+    billingInvoiceLicence
+  )
+  currentBillingData.calculatedTransactions.push(...calculatedTransactions)
+}
+
+function _updateCurrentBillingData (currentBillingData, chargeVersion, billingInvoice, billingInvoiceLicence) {
+  currentBillingData.licence = chargeVersion.licence
+  currentBillingData.billingInvoice = billingInvoice
+  currentBillingData.billingInvoiceLicence = billingInvoiceLicence
+}
+
+function _thisIsADifferentBillingInvoiceLicence (currentBillingData, billingInvoiceLicence) {
+  // If we don't yet have a billing invoice licence (which will be the case the first time we iterate over the charge
+  // versions) then simply return false straight away
+  if (!currentBillingData.billingInvoiceLicence) {
+    return false
+  }
+
+  // Otherwise we want to return true if the previous and current licence ids don't match, and false if they do match
+  return currentBillingData.billingInvoiceLicence.billingInvoiceLicenceId !== billingInvoiceLicence.billingInvoiceLicenceId
+}
+
 /**
   * Log the time taken to process the billing batch
   *
@@ -164,7 +190,11 @@ async function _fetchChargeVersions (billingBatch, billingPeriod) {
     // generating bill runs and reviewing if there is anything to bill. For now, whilst our knowledge of the process
     // is low we are focusing on just the current financial year, and intending to ship a working version for just it.
     // This is why we are only passing through the first billing period; we know there is only one!
-    return await FetchChargeVersionsService.go(billingBatch.regionId, billingPeriod)
+    const chargeVersions = await FetchChargeVersionsService.go(billingBatch.regionId, billingPeriod)
+
+    // We don't just `return FetchChargeVersionsService.go()` as we need to call HandleErroredBillingBatchService if it
+    // fails
+    return chargeVersions
   } catch (error) {
     HandleErroredBillingBatchService.go(
       billingBatch.billingBatchId,
@@ -252,18 +282,16 @@ function _generateCalculatedTransactions (billingPeriod, chargeVersion, billingB
     const isNewLicence = DetermineMinimumChargeService.go(chargeVersion, financialYearEnding)
     const isWaterUndertaker = chargeVersion.licence.isWaterUndertaker
 
-    const transactions = []
-    for (const chargeElement of chargeVersion.chargeElements) {
-      const result = GenerateBillingTransactionsService.go(
+    // We use flatMap as GenerateBillingTransactionsService returns an array of transactions
+    const transactions = chargeVersion.chargeElements.flatMap((chargeElement) => {
+      return GenerateBillingTransactionsService.go(
         chargeElement,
         billingPeriod,
         chargePeriod,
         isNewLicence,
         isWaterUndertaker
       )
-      result.billingInvoiceLicenceId = billingInvoiceLicence.billingInvoiceLicenceId
-      transactions.push(...result)
-    }
+    })
 
     return transactions
   } catch (error) {

app/services/supplementary-billing/process-billing-transactions.service.js

@@ -112,11 +112,11 @@ function _cleanseTransactions (calculatedTransactions, reverseTransactions) {
   // "cancelling pair" with it. If not then add the unpaired calculated transaction to our array of cleansed transaction
   // lines. Note that `reverseTransactions` will be mutated to remove any reverse transactions which form a cancelling
   // pair.
-  for (const calculatedTransactionLine of calculatedTransactions) {
+  calculatedTransactions.forEach((calculatedTransactionLine) => {
     if (!_cancelCalculatedTransaction(calculatedTransactionLine, reverseTransactions)) {
       cleansedTransactionLines.push(calculatedTransactionLine)
     }
-  }
+  })
 
   // Add the remaining reverse transactions (ie. those which didn't form a cancelling pair)
   cleansedTransactionLines.push(...reverseTransactions)