From 71727aa98fcac34eb7d86dde2a93e40bc2a79fad Mon Sep 17 00:00:00 2001 From: Alan Cruikshanks Date: Mon, 13 May 2024 16:45:46 +0100 Subject: [PATCH] Ensure blank line b4 function return statements (#1007) https://github.com/DEFRA/water-abstraction-team/issues/115 This has been an unwritten convention we often pick up in the PRs of those new to the team. We like to see a blank line before the `return` statement in a function. We believe it helps make the code easier to follow and understand. ```javascript // Bad function rubbishNameGenerator () { const firstName = 'John' const lastName = 'Doe' return `${firstName} ${lastName}` } // Good function rubbishNameGenerator () { const firstName = 'John' const lastName = 'Doe' return `${firstName} ${lastName}` } ``` But we don't want to see it all the time! ```javascript // Bad function rubbishNameGeneratorV2 (useAlternate = false) { if (useAlternate) { return 'Jane Doe' } return 'John Doe' } // Good function rubbishNameGeneratorV2 (useAlternate = false) { if (useAlternate) { return 'Jane Doe' } return 'John Doe' } ``` We weren't sure we could make ESLint see the difference but it turns out we can! So, this change enables and adds an initial configuration for the rule [padding-line-between-statements](https://eslint.style/rules/js/padding-line-between-statements). > We expect to expand the config to cover some more unwritten conventions regarding white space in our code. But those will come separately! --- .eslintrc.js | 6 +++++- app/controllers/return-requirements.controller.js | 1 + app/presenters/bill-runs/create-bill-run-event.presenter.js | 1 + .../two-part-tariff/charge-reference-details.presenter.js | 1 + app/presenters/licences/view-licence-summary.presenter.js | 1 + app/services/bill-runs/annual/process-bill-run.service.js | 1 + .../calculate-authorised-and-billable-days.service.js | 1 + .../supplementary/pre-generate-billing-data.service.js | 1 + .../bill-runs/supplementary/process-bill-run.service.js | 1 + .../two-part-tariff/submit-review-licence.service.js | 2 ++ app/services/health/fetch-system-info.service.js | 1 + app/services/health/info.service.js | 1 + app/services/idm/fetch-user-roles-and-groups.service.js | 1 + app/services/licences/fetch-licence.service.js | 1 + .../return-requirements/check-licence-ended.service.js | 1 + app/validators/return-requirements/start-date.validator.js | 1 + 16 files changed, 21 insertions(+), 1 deletion(-) diff --git a/.eslintrc.js b/.eslintrc.js index 024e838197..72dd59b3b8 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -19,7 +19,11 @@ module.exports = { ignoreStrings: true, ignoreTemplateLiterals: true, ignoreUrls: true - }] + }], + '@stylistic/js/padding-line-between-statements': [ + 'error', + { blankLine: 'always', prev: '*', next: 'return' } + ] } } diff --git a/app/controllers/return-requirements.controller.js b/app/controllers/return-requirements.controller.js index df33836a67..ed9d058e16 100644 --- a/app/controllers/return-requirements.controller.js +++ b/app/controllers/return-requirements.controller.js @@ -210,6 +210,7 @@ async function startDate (request, h) { const { sessionId } = request.params const pageData = await StartDateService.go(sessionId) + return h.view('return-requirements/start-date.njk', { ...pageData }) diff --git a/app/presenters/bill-runs/create-bill-run-event.presenter.js b/app/presenters/bill-runs/create-bill-run-event.presenter.js index 5badbeb7ed..eb902a3b21 100644 --- a/app/presenters/bill-runs/create-bill-run-event.presenter.js +++ b/app/presenters/bill-runs/create-bill-run-event.presenter.js @@ -23,6 +23,7 @@ function go (billRun) { status, toFinancialYearEnding } = billRun + return { batch: { id, 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 1358907743..ebf8e21fde 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 @@ -98,6 +98,7 @@ function _prepareDate (startDate, endDate) { function _totalBillableReturns (reviewChargeElements) { return reviewChargeElements.reduce((total, reviewChargeElement) => { total += reviewChargeElement.amendedAllocated + return total }, 0) } diff --git a/app/presenters/licences/view-licence-summary.presenter.js b/app/presenters/licences/view-licence-summary.presenter.js index 01ab58ac0d..5c4a8fcc65 100644 --- a/app/presenters/licences/view-licence-summary.presenter.js +++ b/app/presenters/licences/view-licence-summary.presenter.js @@ -148,6 +148,7 @@ function _generateAbstractionPeriods (licenceVersions) { const formattedAbstactionPeriods = licenceVersions[0].licenceVersionPurposes.map((purpose) => { const startDate = formatAbstractionDate(purpose.abstractionPeriodStartDay, purpose.abstractionPeriodStartMonth) const endDate = formatAbstractionDate(purpose.abstractionPeriodEndDay, purpose.abstractionPeriodEndMonth) + return `${startDate} to ${endDate}` }) diff --git a/app/services/bill-runs/annual/process-bill-run.service.js b/app/services/bill-runs/annual/process-bill-run.service.js index 6980637a9a..6baceccb91 100644 --- a/app/services/bill-runs/annual/process-bill-run.service.js +++ b/app/services/bill-runs/annual/process-bill-run.service.js @@ -73,6 +73,7 @@ async function _finaliseBillRun (billRun, billRunPopulated) { // this in the UI if (!billRunPopulated) { await _updateStatus(billRun.id, 'empty') + return } diff --git a/app/services/bill-runs/calculate-authorised-and-billable-days.service.js b/app/services/bill-runs/calculate-authorised-and-billable-days.service.js index edde29994e..86ebac4c99 100644 --- a/app/services/bill-runs/calculate-authorised-and-billable-days.service.js +++ b/app/services/bill-runs/calculate-authorised-and-billable-days.service.js @@ -146,6 +146,7 @@ function _consolidateAndCalculate (referencePeriod, abstractionsPeriods) { const totalDays = consolidatedAbstractionPeriods.reduce((acc, abstractionPeriod) => { const abstractionOverlapPeriod = _calculateAbstractionOverlapPeriod(referencePeriod, abstractionPeriod) + return acc + _calculateDays(abstractionOverlapPeriod) }, 0) diff --git a/app/services/bill-runs/supplementary/pre-generate-billing-data.service.js b/app/services/bill-runs/supplementary/pre-generate-billing-data.service.js index 66209366c7..b1d6c5da82 100644 --- a/app/services/bill-runs/supplementary/pre-generate-billing-data.service.js +++ b/app/services/bill-runs/supplementary/pre-generate-billing-data.service.js @@ -100,6 +100,7 @@ function _billLicenceKey (billId, licenceId) { function _preGenerateBills (billingAccounts, billRunId, billingPeriod) { const keyedBills = billingAccounts.reduce((acc, billingAccount) => { const { id: billingAccountId, accountNumber } = billingAccount + // Note that the array of billing accounts will already have been deduped so we don't need to check whether a // bill licence already exists in the object before generating one return { diff --git a/app/services/bill-runs/supplementary/process-bill-run.service.js b/app/services/bill-runs/supplementary/process-bill-run.service.js index 30deb01fdf..28b6f6a7f2 100644 --- a/app/services/bill-runs/supplementary/process-bill-run.service.js +++ b/app/services/bill-runs/supplementary/process-bill-run.service.js @@ -90,6 +90,7 @@ async function _finaliseBillRun (billRun, accumulatedLicenceIds, resultsOfProces // this in the UI if (!isPopulated) { await _updateStatus(billRun.id, 'empty') + return } diff --git a/app/services/bill-runs/two-part-tariff/submit-review-licence.service.js b/app/services/bill-runs/two-part-tariff/submit-review-licence.service.js index 4e1dab51cc..6c3bee415e 100644 --- a/app/services/bill-runs/two-part-tariff/submit-review-licence.service.js +++ b/app/services/bill-runs/two-part-tariff/submit-review-licence.service.js @@ -39,11 +39,13 @@ function _bannerMessage (yar, parsedPayload) { if (status) { yar.flash('banner', `Licence changed to ${status}.`) + return } if (progress) { yar.flash('banner', 'This licence has been marked.') + return } diff --git a/app/services/health/fetch-system-info.service.js b/app/services/health/fetch-system-info.service.js index 1c0a1f1dda..40306be0a7 100644 --- a/app/services/health/fetch-system-info.service.js +++ b/app/services/health/fetch-system-info.service.js @@ -29,6 +29,7 @@ async function go () { async function _getCommitHash () { try { const { stdout, stderr } = await exec('git rev-parse HEAD') + return stderr ? `ERROR: ${stderr}` : stdout.replace('\n', '') } catch (error) { return `ERROR: ${error.message}` diff --git a/app/services/health/info.service.js b/app/services/health/info.service.js index 4f55f1a321..a8f8449bf0 100644 --- a/app/services/health/info.service.js +++ b/app/services/health/info.service.js @@ -134,6 +134,7 @@ async function _getRedisConnectivityData () { async function _getVirusScannerData () { try { const { stdout, stderr } = await exec('clamdscan --version') + return stderr ? `ERROR: ${stderr}` : stdout } catch (error) { return `ERROR: ${error.message}` diff --git a/app/services/idm/fetch-user-roles-and-groups.service.js b/app/services/idm/fetch-user-roles-and-groups.service.js index e7e1dfdd89..1e92b299d3 100644 --- a/app/services/idm/fetch-user-roles-and-groups.service.js +++ b/app/services/idm/fetch-user-roles-and-groups.service.js @@ -72,6 +72,7 @@ function _extractRolesFromGroups (groups) { return groups.flatMap((group) => { const { roles } = group delete group.roles + return roles }) } diff --git a/app/services/licences/fetch-licence.service.js b/app/services/licences/fetch-licence.service.js index bcc06be487..da1ad94b58 100644 --- a/app/services/licences/fetch-licence.service.js +++ b/app/services/licences/fetch-licence.service.js @@ -54,6 +54,7 @@ async function _fetchLicence (id) { ]) }) .modify('registeredToAndLicenceName') + return result } diff --git a/app/services/return-requirements/check-licence-ended.service.js b/app/services/return-requirements/check-licence-ended.service.js index fb27febac2..5110318065 100644 --- a/app/services/return-requirements/check-licence-ended.service.js +++ b/app/services/return-requirements/check-licence-ended.service.js @@ -23,6 +23,7 @@ const LicenceModel = require('../../models/licence.model.js') async function go (id) { const licence = await _fetchLicence(id) const licenceEnded = _licenceEnded(licence) + return licenceEnded } diff --git a/app/validators/return-requirements/start-date.validator.js b/app/validators/return-requirements/start-date.validator.js index 463841a57a..a0b2e0c0f4 100644 --- a/app/validators/return-requirements/start-date.validator.js +++ b/app/validators/return-requirements/start-date.validator.js @@ -36,6 +36,7 @@ function go (payload, licenceStartDate, licenceEndDate) { if (selectedOption === 'anotherStartDate') { payload.fullDate = _fullDate(payload) + return _validateAnotherStartDate(payload, licenceStartDate, licenceEndDate) }