Merge branch 'main' into check-return-details-pt1

app/controllers/notifications-setup.controller.js

@@ -24,10 +24,11 @@ async function viewReturnsPeriod(request, h) {
 
 async function viewReview(request, h) {
   const {
-    params: { sessionId }
+    params: { sessionId },
+    query: { page }
   } = request
 
-  const pageData = await ReviewService.go(sessionId)
+  const pageData = await ReviewService.go(sessionId, page)
 
   return h.view(`${basePath}/review.njk`, pageData)
 }

app/lib/return-periods.lib.js

@@ -56,11 +56,13 @@ function determineReturnsPeriods(returnCycle) {
  *
  * The result includes the start date, end date, and due date for each period.
  *
- * @param {Date} determinationDate - The date to base the calculation on. Defaults to the current date.
+ * @param {Date} date - The date to base the calculation on. Defaults to the current date.
  *
  * @returns {object} An object containing calculated dates for all return periods
  */
-function determineUpcomingReturnsPeriods(determinationDate = new Date()) {
+function determineUpcomingReturnsPeriods(date = new Date()) {
+  const determinationDate = new Date(`${date.getFullYear()}-${date.getMonth() + 1}-${date.getDate()}`)
+
   return {
     allYear: {
       startDate: _cycleStartDate(determinationDate, returnPeriodDates.allYear),
@@ -181,7 +183,7 @@ function _isDue(determinationDate, period) {
 
   const dueDate = new Date(`${year}-${periodDueMonth}-${periodDueDay}`)
 
-  return dueDate.getTime() < determinationDate.getTime()
+  return dueDate < determinationDate
 }
 
 /**

app/presenters/base.presenter.js

@@ -37,12 +37,18 @@ function generateBillRunTitle(regionName, batchType, scheme, summer) {
 /**
  * Formats an abstraction day and month into its string variant, for example, 1 and 4 becomes '1 April'
  *
+ * If either value is null or undefined, it returns null.
+ *
  * @param {number} abstractionDay
  * @param {number} abstractionMonth - Note: the index starts at 1, for example, 4 would be April
  *
- * @returns {string} The abstraction date formatted as a 'DD MMMM' string
+ * @returns {string|null} The abstraction date formatted as a 'DD MMMM' string or null if either value is not set
  */
 function formatAbstractionDate(abstractionDay, abstractionMonth) {
+  if (!abstractionDay || !abstractionMonth) {
+    return null
+  }
+
   // NOTE: Because of the unique qualities of Javascript, Year and Day are literal values, month is an index! So,
   // January is actually 0, February is 1 etc. This is why we are always deducting 1 from the months.
   const abstractionDate = new Date(1970, abstractionMonth - 1, abstractionDay)
@@ -53,17 +59,24 @@ function formatAbstractionDate(abstractionDay, abstractionMonth) {
 /**
  * Formats an abstraction period into its string variant, for example, '1 April to 31 October'
  *
+ * If the provided abstraction data is null or undefined, it returns null.
+ *
  * @param {number} startDay
  * @param {number} startMonth
  * @param {number} endDay
  * @param {number} endMonth
  *
- * @returns {string} The abstraction period formatted as a 'DD MMMM to DD MMMM' string
+ * @returns {string} The abstraction period formatted as a 'DD MMMM to DD MMMM' string, unless the abstraction period
+ * cannot be determined, in which case it returns null
  */
 function formatAbstractionPeriod(startDay, startMonth, endDay, endMonth) {
   const startDate = formatAbstractionDate(startDay, startMonth)
   const endDate = formatAbstractionDate(endDay, endMonth)
 
+  if (!startDate || !endDate) {
+    return null
+  }
+
   return `${startDate} to ${endDate}`
 }
 

app/presenters/notifications/setup/returns-period.presenter.js

@@ -50,7 +50,7 @@ function _formatReturnPeriod(returnsPeriod, savedReturnsPeriod) {
 
 function _textPrefix(returnPeriod) {
   if (returnPeriod.name === 'allYear') {
-    return 'Winter and all year'
+    return 'Winter and all year annual'
   } else if (returnPeriod.name === 'summer') {
     return 'Summer annual'
   } else {

app/presenters/notifications/setup/review.presenter.js

@@ -11,14 +11,17 @@ const { titleCase } = require('../../base.presenter.js')
 /**
  * Formats data for the `/notifications/setup/review` page
  *
- * @param recipients
+ * @param {object[]} recipients - List of recipient objects, each containing recipient details like email or name.
+ * @param {number|string} page - The currently selected page
+ * @param {object} pagination -
+ *
  * @returns {object} - The data formatted for the view template
  */
-function go(recipients) {
+function go(recipients, page, pagination) {
   return {
     defaultPageSize,
-    pageTitle: 'Send returns invitations',
-    recipients: _recipients(recipients),
+    pageTitle: _pageTitle(page, pagination),
+    recipients: _recipients(recipients, page),
     recipientsAmount: recipients.length
   }
 }
@@ -52,8 +55,28 @@ function _licences(licences) {
   return licences.split(',')
 }
 
-function _recipients(recipients) {
-  return recipients.map((recipient) => {
+function _pageTitle(page, pagination) {
+  if (pagination.numberOfPages > 1) {
+    return `Send returns invitations (page ${page} of ${pagination.numberOfPages})`
+  }
+
+  return 'Send returns invitations'
+}
+
+/**
+ * Due to the complexity of the query to get the recipients data, we handle pagination in the presenter.
+ *
+ * @private
+ */
+function _pagination(recipients, page) {
+  const pageNumber = Number(page) * defaultPageSize
+  return recipients.slice(pageNumber - defaultPageSize, pageNumber)
+}
+
+function _recipients(recipients, page) {
+  const paginatedRecipients = _pagination(recipients, page)
+
+  return paginatedRecipients.map((recipient) => {
     return {
       contact: _contact(recipient),
       licences: _licences(recipient.all_licences),

app/presenters/return-versions/view.presenter.js

@@ -5,7 +5,7 @@
  * @module ViewPresenter
  */
 
-const { formatAbstractionDate } = require('../base.presenter.js')
+const { formatAbstractionPeriod } = require('../base.presenter.js')
 const { formatLongDate } = require('../base.presenter.js')
 const { isQuarterlyReturnSubmissions } = require('../../lib/dates.lib.js')
 const { returnRequirementReasons, returnRequirementFrequencies } = require('../../lib/static-lookups.lib.js')
@@ -39,13 +39,14 @@ function go(returnVersion) {
 }
 
 function _abstractionPeriod(requirement) {
-  const { abstractionPeriodStartDay, abstractionPeriodStartMonth, abstractionPeriodEndDay, abstractionPeriodEndMonth } =
-    requirement
-
-  const startDate = formatAbstractionDate(abstractionPeriodStartDay, abstractionPeriodStartMonth)
-  const endDate = formatAbstractionDate(abstractionPeriodEndDay, abstractionPeriodEndMonth)
+  const abstractionPeriod = formatAbstractionPeriod(
+    requirement.abstractionPeriodStartDay,
+    requirement.abstractionPeriodStartMonth,
+    requirement.abstractionPeriodEndDay,
+    requirement.abstractionPeriodEndMonth
+  )
 
-  return `From ${startDate} to ${endDate}`
+  return abstractionPeriod ?? ''
 }
 
 function _agreementsExceptions(returnRequirement) {

app/services/bill-runs/match/fetch-return-logs-for-licence.service.js

@@ -19,7 +19,25 @@ const ReturnLogModel = require('../../../models/return-log.model.js')
  * `returnSubmissionLines` if they exist
  */
 async function go(licenceRef, billingPeriod) {
-  return _fetch(licenceRef, billingPeriod)
+  try {
+    return await _fetch(licenceRef, billingPeriod)
+  } catch (error) {
+    // NOTE: The try/catch was added after we found out that it is possible to set up return requirements in NALD with
+    // empty abstraction periods. This then causes the return log to also be missing abstraction period data. Our
+    // CAST() in the query then causes an error.
+    global.GlobalNotifier.omfg(
+      'Bill run process fetch return logs for licence failed',
+      { licenceRef, billingPeriod },
+      error
+    )
+    // NOTE: We rethrow the error so that the bill run will be set to 'errored'. We know this will result in us
+    // having to investigate the issue, but with this additional logging we should be able to quickly see which licence
+    // caused the error and why.
+    //
+    // If we don't rethrow the error then the bill run will complete without the return log being picked up. This would
+    // result in an incorrect bill run because the submission would not have been picked up for allocation.
+    throw error
+  }
 }
 
 async function _fetch(licenceRef, billingPeriod) {

app/services/notifications/setup/review.service.js

@@ -5,20 +5,22 @@
  * @module ReviewService
  */
 
+const DedupeRecipientsService = require('./dedupe-recipients.service.js')
+const PaginatorPresenter = require('../../../presenters/paginator.presenter.js')
 const RecipientsService = require('./fetch-recipients.service.js')
 const ReviewPresenter = require('../../../presenters/notifications/setup/review.presenter.js')
 const SessionModel = require('../../../models/session.model.js')
 const { determineUpcomingReturnPeriods } = require('../../../lib/return-periods.lib.js')
-const DedupeRecipientsService = require('./dedupe-recipients.service.js')
 
 /**
  * Orchestrates fetching and presenting the data needed for the notifications setup review page
  *
  * @param {string} sessionId - The UUID for setup ad-hoc returns notification session record
+ * @param {number|string} [page=1] - The currently selected page (if paginated)
  *
  * @returns {object} The view data for the review page
  */
-async function go(sessionId) {
+async function go(sessionId, page = 1) {
   const session = await SessionModel.query().findById(sessionId)
 
   const { returnsPeriod } = session
@@ -29,11 +31,19 @@ async function go(sessionId) {
 
   const dedupeRecipients = DedupeRecipientsService.go(recipients)
 
-  const formattedData = ReviewPresenter.go(dedupeRecipients)
+  const pagination = PaginatorPresenter.go(
+    dedupeRecipients.length,
+    Number(page),
+    `/system/notifications/setup/${sessionId}/review`
+  )
+
+  const formattedData = ReviewPresenter.go(dedupeRecipients, page, pagination)
 
   return {
     activeNavBar: 'manage',
-    ...formattedData
+    ...formattedData,
+    pagination,
+    page
   }
 }
 

app/views/notifications/setup/review.njk

@@ -3,9 +3,11 @@
 {% from "govuk/components/button/macro.njk" import govukButton %}
 {% from "govuk/components/details/macro.njk" import govukDetails %}
 {% from "govuk/components/pagination/macro.njk" import govukPagination %}
+{% from "govuk/components/pagination/macro.njk" import govukPagination %}
 {% from "govuk/components/radios/macro.njk" import govukRadios %}
 {% from "govuk/components/table/macro.njk" import govukTable %}
 {% from 'govuk/components/error-summary/macro.njk' import govukErrorSummary %}
+
 {% from "macros/new-line-array-items.njk" import newLineArrayItems %}
 
 {% block breadcrumbs %}
@@ -15,8 +17,6 @@
   }) }}
 {% endblock %}
 
-
-
 {% set rows = [] %}
 
 {%for recipient in recipients %}
@@ -47,7 +47,7 @@
 
 {% block content %}
   <div class="govuk-body">
-    <h1 class="govuk-heading-l">{{ pageTitle }}</h1>
+    <h1 class="govuk-heading-l"> {{ pageTitle }} </h1>
 
     <p> Returns invitations are ready to send.</p>
 
@@ -63,7 +63,11 @@
       }) }}
     </div>
 
-    <p>Showing {{ defaultPageSize }} of {{ recipientsAmount }}</p>
+    {% if pagination.numberOfPages > 1 %}
+      <p> Showing {{ defaultPageSize }} of {{ recipientsAmount }} recipients </p>
+    {% else %}
+      <p> Showing all {{ recipientsAmount }} recipients </p>
+    {% endif %}
 
     {{ govukTable({
       captionClasses: "govuk-table__caption--m",
@@ -85,6 +89,10 @@
       rows: rows
     }) }}
 
+    {% if pagination.numberOfPages > 1 %}
+      {{ govukPagination(pagination.component) }}
+    {% endif %}
+
     <form method="post">
       <input type="hidden" name="wrlsCrumb" value="{{ wrlsCrumb }}"/>