Fix outstanding JSDoc issues (#1492)

DEFRA/water-abstraction-team#125

In [Lint JSDOC's](#1269), we added linting of our JSDoc comments. If we are going to do it, we should do it correctly and consistently!

There were quite a few, so since then, we have cleaned up certain areas of the codebase while configuring the linter to only warn us of issues.

With this final change, we deal with the remaining issues so we can switch the linter to error when it encounters an issue.

.eslintrc.js

@@ -44,23 +44,23 @@ module.exports = {
     'arrow-body-style': ['error', 'always'],
     'import/extensions': ['error', 'always'],
     strict: ['error', 'global'],
-    'jsdoc/require-jsdoc': ['warn', {
+    'jsdoc/require-jsdoc': ['error', {
       publicOnly: true
     }],
-    'jsdoc/require-description': 'warn',
-    'jsdoc/require-param': ['warn', {
+    'jsdoc/require-description': 'error',
+    'jsdoc/require-param': ['error', {
       exemptedBy: ['private']
     }],
-    'jsdoc/require-returns': ['warn', {
+    'jsdoc/require-returns': ['error', {
       publicOnly: true
     }],
-    'jsdoc/check-tag-names': 'warn',
-    'jsdoc/check-alignment': 'warn',
+    'jsdoc/check-tag-names': 'error',
+    'jsdoc/check-alignment': 'error',
     'jsdoc/newline-after-description': 'off', // does not work with 'use strict'
-    'jsdoc/check-indentation': 'warn',
-    'jsdoc/lines-before-block': 'warn',
-    'jsdoc/check-types': 'warn',
-    'jsdoc/require-hyphen-before-param-description': 'warn'
+    'jsdoc/check-indentation': 'error',
+    'jsdoc/lines-before-block': 'error',
+    'jsdoc/check-types': 'error',
+    'jsdoc/require-hyphen-before-param-description': 'error'
   },
   settings: {
     jsdoc: {

app/errors/expanded.error.js

@@ -1,6 +1,7 @@
 'use strict'
 
 class ExpandedError extends Error {
+  // eslint-disable-next-line jsdoc/lines-before-block
   /**
    * Custom error that allows additional data properties to be assigned to the error instance
    *

app/lib/request-notifier.lib.js

@@ -20,6 +20,7 @@ const BaseNotifierLib = require('./base-notifier.lib.js')
  * related log entries and Errbit notifications by using the ID.
  */
 class RequestNotifierLib extends BaseNotifierLib {
+  // eslint-disable-next-line jsdoc/lines-before-block
   /**
    * Instantiate a new instance
    *

app/models/base.model.js

@@ -15,6 +15,7 @@ const { db } = require('../../db/db.js')
 Model.knex(db)
 
 class BaseModel extends Model {
+  // eslint-disable-next-line jsdoc/lines-before-block
   /**
    * Array of paths to search for models used in relationships
    *
@@ -32,6 +33,8 @@ class BaseModel extends Model {
    * ```
    *
    * We don't want to do this in every model so set it in the `BaseModel` as Objection recommends.
+   *
+   * @returns {string[]} An array of paths
    */
   static get modelPaths () {
     return [__dirname]

app/models/licence-entity.model.js

@@ -23,10 +23,10 @@ const BaseModel = require('./base.model.js')
  * 'things'. So, you'll find 4 types if you look at the raw data.
  *
  * - regime (only one record has this type and comes from the original team being pressured to create a tactical CRM for
- *   all regimes, not just water abstraction)
+ * all regimes, not just water abstraction)
  * - company (this has been replaced by `crm_v2.companies`)
  * - individual (this has been replaced by `crm_v2.companies`, except for registered users which is the only reason we
- *   need to add this model)
+ * need to add this model)
  * - delete_me (no idea! But only one record has this type so it can be ignored)
  */
 class LicenceEntityModel extends BaseModel {

app/plugins/yar.plugin.js

@@ -30,22 +30,20 @@
  * - A licence review is currently not 'in progress'
  * - The user clicks the 'Mark progress' button. This sends a POST request
  * - The server updates the licence review to 'in progress' and redirects to the licence review page (as per the
- *   {@link https://www.geeksforgeeks.org/post-redirect-get-prg-design-pattern/ | PRG pattern})
+ * {@link https://www.geeksforgeeks.org/post-redirect-get-prg-design-pattern/ | PRG pattern})
  * - The redirect causes a GET request which results in the server getting the updated record
  * - The licence review is displayed to the user
  *
  * In this situation we want to display a banner to the user confirming a change was just made. The problem is there is
  * nothing to tell the GET request that this is so. We could do the following
  *
- * - Don't redirect but respond directly from the POST
- *   - This breaks PRG which you'll see if you hit refresh. The browser will resend your POST request which might have
- *     unintended consequences
- * - Set a flag in the DB
- *   - You then have to add logic to the GET request to update the record and remove the flag. It is also storing
- *     something in the DB that we only care about from a UI/behaviour context
- * - Add a query string param
- *   - This can be seen by the user and feels a bit 'ick'. Also, it could be abused. There is nothing to stop users
- *     adding the flag themselves to force the banner to appear. It also won't clear when the browser is refreshed.
+ * - **Don't redirect but respond directly from the POST** - This breaks PRG which you'll see if you hit refresh. The
+ * browser will resend your POST request which might have unintended consequences
+ * - **Set a flag in the DB** - You then have to add logic to the GET request to update the record and remove the flag.
+ * It is also storing something in the DB that we only care about from a UI/behaviour context
+ * - **Add a query string param** - This can be seen by the user and feels a bit 'ick'. Also, it could be abused. There
+ * is nothing to stop users adding the flag themselves to force the banner to appear. It also won't clear when the
+ * browser is refreshed.
  *
  * We're not the first to encounter this problem. Hence why session cookies and **yar** exist. **yar** even has a
  * feature specific to this use case.

app/presenters/bill-runs/create-bill-run-event.presenter.js

@@ -5,6 +5,13 @@
  * @module CreateBillRunEventPresenter
  */
 
+/**
+ * Formats a `BillRunModel` into the metadata content needed for a bill run's 'event' record
+ *
+ * @param {module:BillRunModel} billRun - the bill run to format
+ *
+ * @returns {object} - the formatted content
+ */
 function go (billRun) {
   const {
     batchType,

app/presenters/bill-runs/view-bill-run.presenter.js

@@ -15,6 +15,14 @@ const {
   titleCase
 } = require('../base.presenter.js')
 
+/**
+ * Formats bill run data ready for presenting in the view bill run page
+ *
+ * @param {module:BillRunModel} billRun - an instance of `BillRunModel`
+ * @param {object[]} billSummaries - summary data of bills connected to the bill run
+ *
+ * @returns {object} - the prepared bill run data to be passed to the view bill run page
+ */
 function go (billRun, billSummaries) {
   const {
     batchType,

app/presenters/bill-runs/view-bill-summaries.presenter.js

@@ -7,6 +7,15 @@
 
 const { formatMoney } = require('../base.presenter.js')
 
+/**
+ * Formats summary data of bills connected to a bill run for the bill run summary page
+ *
+ * @param {object[]} billSummaries - An array of bill summary objects.
+ *
+ * @returns {object[]} An array of bill groups. Each group contains a type, a caption, and an array of bills. If there
+ * are water company bills, they are grouped under 'water-companies'. If there are other abstractor bills, they are
+ * grouped under 'other-abstractors'.
+ */
 function go (billSummaries) {
   const waterCompanies = _waterCompanies(billSummaries)
   const otherAbstractors = _otherAbstractors(billSummaries)

app/presenters/bills/view-licence-summaries.presenter.js

@@ -7,6 +7,13 @@
 
 const { formatMoney } = require('../base.presenter.js')
 
+/**
+ * Formats summary data of licences connected to a bill for the multi-licence bill page
+ *
+ * @param {object[]} licenceSummaries - an array of licence summaries
+ *
+ * @returns {object} a formatted representation of the bill's licence summaries
+ */
 function go (licenceSummaries) {
   const billLicences = _billLicences(licenceSummaries)
 

app/presenters/import/legacy/address.presenter.js

@@ -10,7 +10,7 @@
  *
  * @param {ImportLegacyAddressType} address - the legacy NALD address
  * @param {string} dataSource
-
+ *
  * @returns {object} the NALD company data transformed into the WRLS format for an address
  * ready for validation and persisting
  */

app/presenters/licences/customer-contacts.presenter.js

@@ -14,7 +14,6 @@ const ContactModel = require('../../models/contact.model.js')
  * template
  *
  * @returns {object} The data formatted for the view template
- * @returns {object} The data formatted for the view template
  */
 function go (customerContacts) {
   return {

app/presenters/licences/licence-contacts.presenter.js

@@ -8,6 +8,8 @@
 /**
  * Formats data for the `/licences/{id}/contact-details` view licence contact details page
  *
+ * @param {object[]} contacts - The results from `FetchLicenceContactsService` to be formatted for the view
+ *
  * @returns {object} The data formatted for the view template
  */
 function go (contacts) {

app/presenters/licences/view-licence-bills.presenter.js

@@ -10,6 +10,8 @@ const { formatBillRunType, formatLongDate, formatMoney } = require('../base.pres
 /**
  * Formats data for the `/licences/{id}/bills` view licence bill page
  *
+ * @param {object[]} bills - The licence's bills
+ *
  * @returns {object} The data formatted for the view template
  */
 function go (bills) {

app/presenters/paginator.presenter.js

@@ -21,7 +21,7 @@ const COMPLEX_END_PAGINATOR = 'end'
  * The pagination component is the thing seen at the bottom of pages which have to display dynamic results, for example,
  * search results or all bill runs. For example,
  *
- *  `<- Previous 1 ... 6 [7] 8 ... 42 Next ->`
+ * `<- Previous 1 ... 6 [7] 8 ... 42 Next ->`
  *
  * The first step is to take the number of records and divide them by our page size config (defaults to 25) to determine
  * how many pages are needed. If only 1 page is required (`numberOfRecords` is less then or equal to 25) no pagination
@@ -58,7 +58,7 @@ const COMPLEX_END_PAGINATOR = 'end'
  * This applies for any scenario where the number of pages is 7 or less. In this case we simply iterate from 1 up to the
  * number of pages creating a page item for each one. No ellipses are used.
  *
- *  `[1] 2 3 4 5 6 7 Next -->`
+ * `[1] 2 3 4 5 6 7 Next -->`
  *
  * ### Complex
  *

app/requests/base.request.js

@@ -107,10 +107,52 @@ async function get (url, additionalOptions = {}) {
   return _sendRequest('get', url, additionalOptions)
 }
 
+/**
+ * Make a PATCH request to the specified URL
+ *
+ * Use when you need to make a PATCH request. It returns a result tuple:
+ *
+ * ```javascript
+ * {
+ *  succeeded: true,
+ *  response: {} // The full response from Got
+ * }
+ * ```
+ *
+ * Any 2xx or 3xx will be flagged as succeeded. Anything else and `succeeded:` will be false. As long as the other
+ * service responds, `response:` will be the full response Got returns. In the event of a network error `response:`
+ * will be a Got error instance.
+ *
+ * @param {string} url - The full URL that you wish to connect to
+ * @param {object} additionalOptions - Append to or replace the options passed to Got when making the request
+ *
+ * @returns {Promise<object>} The result of the request; whether it succeeded and the response or error returned
+ */
 async function patch (url, additionalOptions = {}) {
   return _sendRequest('patch', url, additionalOptions)
 }
 
+/**
+ * Make a POST request to the specified URL
+ *
+ * Use when you need to make a POST request. It returns a result tuple:
+ *
+ * ```javascript
+ * {
+ *  succeeded: true,
+ *  response: {} // The full response from Got
+ * }
+ * ```
+ *
+ * Any 2xx or 3xx will be flagged as succeeded. Anything else and `succeeded:` will be false. As long as the other
+ * service responds, `response:` will be the full response Got returns. In the event of a network error `response:`
+ * will be a Got error instance.
+ *
+ * @param {string} url - The full URL that you wish to connect to
+ * @param {object} additionalOptions - Append to or replace the options passed to Got when making the request
+ *
+ * @returns {Promise<object>} The result of the request; whether it succeeded and the response or error returned
+ */
 async function post (url, additionalOptions = {}) {
   return _sendRequest('post', url, additionalOptions)
 }

app/requests/charging-module/reissue-bill.request.js

@@ -19,7 +19,7 @@ const ChargingModuleRequest = require('../charging-module.request.js')
  * @param {string} billId - UUID of the Charging Module the bill to be reissued
  *
  * @returns {Promise<object>} The result of the request; whether it succeeded and the response or error returned
-*/
+ */
 async function send (billRunId, billId) {
   const path = `v3/wrls/bill-runs/${billRunId}/invoices/${billId}/rebill`
 

app/requests/charging-module/view-bill.request.js

@@ -17,7 +17,7 @@ const ChargingModuleRequest = require('../charging-module.request.js')
  * @param {string} billId - UUID of the Charging Module bill to view
  *
  * @returns {Promise<object>} The result of the request; whether it succeeded and the response or error returned
-*/
+ */
 async function send (billRunId, billId) {
   const path = `v3/wrls/bill-runs/${billRunId}/invoices/${billId}`
 

app/requests/legacy/create-bill-run.request.js

@@ -12,7 +12,7 @@ const LegacyRequest = require('../legacy.request.js')
  *
  * @param {string} batchType - the type of bill run the legacy service needs to create. We only expect this to be
  * sending `supplementary` and `two_part_tariff` as annual is now handled by us.
- * @param {string} regionID - UUID of the region the bill run is for
+ * @param {string} regionId - UUID of the region the bill run is for
  * @param {number} financialYearEnding - The end year for the financial year to be generated
  * @param {module:UserModel} user - Instance representing the user that originated the request
  * @param {boolean} [summer] - Only relates to two-part tariff. In PRESROC 2PT bill runs were split by summer or winter

app/services/bill-runs/calculate-authorised-and-billable-days.service.js

@@ -45,14 +45,14 @@ const ONE_DAY_IN_MILLISECONDS = 24 * 60 * 60 * 1000
  * problem is what this service tackles.
  *
  * @param {{startDate: Date, endDate: Date}} chargePeriod - Charge period is determined as the overlap between a charge
- *  version's start and end dates, and the billing period's (financial year) start and end dates. So, when the charge
- *  version and billing period are compared the charge period's start date is the latest of the two, and the end date is
- *  the earliest of their end dates
+ * version's start and end dates, and the billing period's (financial year) start and end dates. So, when the charge
+ * version and billing period are compared the charge period's start date is the latest of the two, and the end date is
+ * the earliest of their end dates
  * @param {{startDate: Date, endDate: Date}} billingPeriod - The period a bill run is being calculated for. Currently,
- *  this always equates to a financial year, for example, 2022-04-01 to 2023-03-31
+ * this always equates to a financial year, for example, 2022-04-01 to 2023-03-31
  * @param {module:ChargeReferenceModel} chargeReference - A charge version can have multiple charge references, though
- *  each will have a different reference, for example, 4.1.10. Each reference can have multiple charge elements and it's
- *  these that hold the abstraction period data
+ * each will have a different reference, for example, 4.1.10. Each reference can have multiple charge elements and it's
+ * these that hold the abstraction period data
  *
  * @returns {object} An object containing an `authorisedDays` and `billableDays` property
  */

app/services/bill-runs/create-bill-run.service.js

@@ -16,7 +16,7 @@ const BillRunModel = require('../../models/bill-run.model.js')
  * @param {string} [options.batchType=supplementary] - The type of bill run to create. Defaults to 'supplementary'
  * @param {string} [options.scheme=sroc] - The applicable charging scheme. Defaults to 'sroc'
  * @param {string} [options.source=wrls] - Where the bill run originated from. Records imported from NALD have the
- *  source 'nald'. Those created in the service use 'wrls'. Defaults to 'wrls'
+ * source 'nald'. Those created in the service use 'wrls'. Defaults to 'wrls'
  * @param {string} [options.externalId=null] - The id of the bill run as created in the Charging Module
  * @param {string} [options.status=queued] - The status that the bill run should be created with
  * @param {number} [options.errorCode=null] - Numeric error code