diff --git a/app/controllers/notifications-setup.controller.js b/app/controllers/notifications-setup.controller.js index 0ac8f1debd..8733a68794 100644 --- a/app/controllers/notifications-setup.controller.js +++ b/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) } diff --git a/app/lib/return-periods.lib.js b/app/lib/return-periods.lib.js index d4738304ad..f45d4f2bdd 100644 --- a/app/lib/return-periods.lib.js +++ b/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 } /** diff --git a/app/presenters/base.presenter.js b/app/presenters/base.presenter.js index 61e8de5b85..942571bd3c 100644 --- a/app/presenters/base.presenter.js +++ b/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}` } diff --git a/app/presenters/notifications/setup/returns-period.presenter.js b/app/presenters/notifications/setup/returns-period.presenter.js index 49ef39cef2..b219068a40 100644 --- a/app/presenters/notifications/setup/returns-period.presenter.js +++ b/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 { diff --git a/app/presenters/notifications/setup/review.presenter.js b/app/presenters/notifications/setup/review.presenter.js index 3c9870a997..c8e2fb33b2 100644 --- a/app/presenters/notifications/setup/review.presenter.js +++ b/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), diff --git a/app/presenters/return-versions/view.presenter.js b/app/presenters/return-versions/view.presenter.js index cf223b6145..62ff459832 100644 --- a/app/presenters/return-versions/view.presenter.js +++ b/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) { diff --git a/app/services/bill-runs/match/fetch-return-logs-for-licence.service.js b/app/services/bill-runs/match/fetch-return-logs-for-licence.service.js index 4a527f21a5..4abc683482 100644 --- a/app/services/bill-runs/match/fetch-return-logs-for-licence.service.js +++ b/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) { diff --git a/app/services/notifications/setup/review.service.js b/app/services/notifications/setup/review.service.js index 51c597c602..4a69e16fc3 100644 --- a/app/services/notifications/setup/review.service.js +++ b/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 } } diff --git a/app/views/notifications/setup/review.njk b/app/views/notifications/setup/review.njk index 4c12397900..512f602273 100644 --- a/app/views/notifications/setup/review.njk +++ b/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 %}
Returns invitations are ready to send.
@@ -63,7 +63,11 @@ }) }}Showing {{ defaultPageSize }} of {{ recipientsAmount }}
+ {% if pagination.numberOfPages > 1 %} +Showing {{ defaultPageSize }} of {{ recipientsAmount }} recipients
+ {% else %} +Showing all {{ recipientsAmount }} recipients
+ {% endif %} {{ govukTable({ captionClasses: "govuk-table__caption--m", @@ -85,6 +89,10 @@ rows: rows }) }} + {% if pagination.numberOfPages > 1 %} + {{ govukPagination(pagination.component) }} + {% endif %} +