From 4e4148a40c0339e8cedbe3540c12b3feb68a0b5f Mon Sep 17 00:00:00 2001 From: Alan Cruikshanks Date: Tue, 23 Apr 2024 08:47:30 +0100 Subject: [PATCH 1/5] Refactor review licence POST pattern https://eaflood.atlassian.net/browse/WATER-4447 We recently added [Hapi yar session manager to the project](https://github.com/DEFRA/water-abstraction-system/pull/926). This was specifically to support flash notification banners (ones that appear on first GET request but then disappear). The Review licence screen has a number of these already. Ones for when you toggle the progress and and others for toggling the status. These have been achieved by breaking the [post-redirect-get pattern](https://www.geeksforgeeks.org/post-redirect-get-prg-design-pattern/) and responding directly from the POST request. As a first step to switching to using **yar** we need to refactor `ReviewLicenceService` into two; `ReviewLicenceService` and `SubmitReviewLicenceService`. This means it will also follow the pattern we've been using in other areas of the project. This change is that first step. We'll switch to **yar** in the next one. From 026ae78029917c4ca078a4992f88f8c4c160881a Mon Sep 17 00:00:00 2001 From: Alan Cruikshanks Date: Tue, 23 Apr 2024 14:11:36 +0100 Subject: [PATCH 2/5] Split POST and GET into separate handlers We always had the 2 routes obviously, but now they call separate handlers rather than the same one. That way they can focus on doing one thing and calling their own service. --- app/controllers/bill-runs.controller.js | 31 ++++++++++++++++++------- app/routes/bill-runs.routes.js | 2 +- 2 files changed, 23 insertions(+), 10 deletions(-) diff --git a/app/controllers/bill-runs.controller.js b/app/controllers/bill-runs.controller.js index e71ecc1b27..7a4425a7f3 100644 --- a/app/controllers/bill-runs.controller.js +++ b/app/controllers/bill-runs.controller.js @@ -99,7 +99,7 @@ async function review (request, h) { async function reviewLicence (request, h) { const { id: billRunId, licenceId } = request.params - const pageData = await ReviewLicenceService.go(billRunId, licenceId, request.payload) + const pageData = await ReviewLicenceService.go(billRunId, licenceId) return h.view('bill-runs/review-licence.njk', { pageTitle: `Licence ${pageData.licence.licenceRef}`, @@ -120,6 +120,18 @@ async function send (request, h) { }) } +async function submitAmendedBillableReturns (request, h) { + const { id: billRunId, licenceId, reviewChargeElementId } = request.params + + const pageData = await SubmitAmendedBillableReturnsService.go(billRunId, licenceId, reviewChargeElementId, request.payload) + + if (pageData.error) { + return h.view('bill-runs/amend-billable-returns.njk', pageData) + } + + return h.redirect(`/system/bill-runs/${billRunId}/review/${licenceId}/match-details/${reviewChargeElementId}`) +} + async function submitCancel (request, h) { const { id } = request.params @@ -134,16 +146,16 @@ async function submitCancel (request, h) { } } -async function submitAmendedBillableReturns (request, h) { - const { id: billRunId, licenceId, reviewChargeElementId } = request.params - - const pageData = await SubmitAmendedBillableReturnsService.go(billRunId, licenceId, reviewChargeElementId, request.payload) +async function submitReviewLicence (request, h) { + const { id: billRunId, licenceId } = request.params - if (pageData.error) { - return h.view('bill-runs/amend-billable-returns.njk', pageData) - } + const pageData = await ReviewLicenceService.go(billRunId, licenceId, request.payload) - return h.redirect(`/system/bill-runs/${billRunId}/review/${licenceId}/match-details/${reviewChargeElementId}`) + return h.view('bill-runs/review-licence.njk', { + pageTitle: `Licence ${pageData.licence.licenceRef}`, + activeNavBar: 'bill-runs', + ...pageData + }) } async function submitSend (request, h) { @@ -183,6 +195,7 @@ module.exports = { send, submitAmendedBillableReturns, submitCancel, + submitReviewLicence, submitSend, view } diff --git a/app/routes/bill-runs.routes.js b/app/routes/bill-runs.routes.js index 2dec573663..cfbee82ee5 100644 --- a/app/routes/bill-runs.routes.js +++ b/app/routes/bill-runs.routes.js @@ -113,7 +113,7 @@ const routes = [ { method: 'POST', path: '/bill-runs/{id}/review/{licenceId}', - handler: BillRunsController.reviewLicence, + handler: BillRunsController.submitReviewLicence, options: { auth: { access: { From 8c5acab23a3ff777ac389d6b676d6e49cd059c90 Mon Sep 17 00:00:00 2001 From: Alan Cruikshanks Date: Tue, 23 Apr 2024 18:08:23 +0100 Subject: [PATCH 3/5] Refactor ReviewLicenceService for GET only We've split off the functionality that gets used when a POST request is made into `SubmitReviewLicenceService`. So, now this service only needs to handle when a GET request is made. On the tests, it was previously understandable that `ReviewLicencePresenter` was stubbed. There is _a lot_ of test data needed to make it work otherwise. But stubbing it is the exception. In all our other tests we've not needed to. We think this is a 'smell' that both the fetch service and the presenter are doing too much. Having to cater for this in the tests is a handy way of highlighting this. It is beyond the scope of this PR to revisit the service and the presenter. So, for now we limit the change to drop the stubbing and take the data hit. But we'll endeavour to revisit these modules in the future. --- .../two-part-tariff/review-licence.service.js | 44 +-- .../review-licence.service.test.js | 312 +++++++++++------- 2 files changed, 198 insertions(+), 158 deletions(-) diff --git a/app/services/bill-runs/two-part-tariff/review-licence.service.js b/app/services/bill-runs/two-part-tariff/review-licence.service.js index 26978985a1..5445e99702 100644 --- a/app/services/bill-runs/two-part-tariff/review-licence.service.js +++ b/app/services/bill-runs/two-part-tariff/review-licence.service.js @@ -6,7 +6,6 @@ */ const FetchReviewLicenceResultsService = require('./fetch-review-licence-results.service.js') -const ReviewLicenceModel = require('../../../models/review-licence.model.js') const ReviewLicencePresenter = require('../../../presenters/bill-runs/two-part-tariff/review-licence.presenter.js') /** @@ -14,53 +13,18 @@ const ReviewLicencePresenter = require('../../../presenters/bill-runs/two-part-t * * @param {module:BillRunModel} billRunId The UUID for the bill run * @param {module:LicenceModel} licenceId The UUID of the licence that is being reviewed - * @param {Object} payload The `request.payload` containing the `marKProgress` data. This is only passed to the service - * when there is a POST request, which only occurs when the 'Mark progress' button is clicked. * - * @returns {Object} an object representing the 'pageData' needed to review the individual licence. It contains the - * licence, bill run, matched and unmatched returns and the licence charge data + * @returns {Promise} the 'pageData' needed for the review licence page. It contains the licence, bill run, + * matched and unmatched returns and the licence charge data */ -async function go (billRunId, licenceId, payload) { - const licenceStatus = payload?.licenceStatus - const markProgress = payload?.marKProgress - - if (payload) { - await _processPayload(billRunId, licenceId, licenceStatus, markProgress) - } - +async function go (billRunId, licenceId) { const { billRun, licence } = await FetchReviewLicenceResultsService.go(billRunId, licenceId) - const pageData = ReviewLicencePresenter.go(billRun, licence, licenceStatus, markProgress) + const pageData = ReviewLicencePresenter.go(billRun, licence) return pageData } -async function _processPayload (billRunId, licenceId, licenceStatus, markProgress) { - if (licenceStatus === 'ready' || licenceStatus === 'review') { - await _updateStatus(billRunId, licenceId, licenceStatus) - } - - if (markProgress === 'mark' || markProgress === 'unmark') { - await _updateProgress(billRunId, licenceId, markProgress) - } -} - -async function _updateProgress (billRunId, licenceId, marKProgress) { - const progress = marKProgress === 'mark' - - await ReviewLicenceModel.query() - .patch({ progress }) - .where('billRunId', billRunId) - .andWhere('licenceId', licenceId) -} - -async function _updateStatus (billRunId, licenceId, licenceStatus) { - await ReviewLicenceModel.query() - .patch({ status: licenceStatus }) - .where('billRunId', billRunId) - .andWhere('licenceId', licenceId) -} - module.exports = { go } diff --git a/test/services/bill-runs/two-part-tariff/review-licence.service.test.js b/test/services/bill-runs/two-part-tariff/review-licence.service.test.js index 17dbf80795..ffb57aaf7c 100644 --- a/test/services/bill-runs/two-part-tariff/review-licence.service.test.js +++ b/test/services/bill-runs/two-part-tariff/review-licence.service.test.js @@ -8,138 +8,214 @@ const Sinon = require('sinon') const { describe, it, beforeEach, afterEach } = exports.lab = Lab.script() const { expect } = Code -// Test helpers -const DatabaseSupport = require('../../../support/database.js') -const ReviewLicenceHelper = require('../../../support/helpers/review-licence.helper.js') -const ReviewLicenceModel = require('../../../../app/models/review-licence.model.js') - // Things we need to stub const FetchReviewLicenceResultsService = require('../../../../app/services/bill-runs/two-part-tariff/fetch-review-licence-results.service.js') -const ReviewLicencePresenter = require('../../../../app/presenters/bill-runs/two-part-tariff/review-licence.presenter.js') // Thing under test const ReviewLicenceService = require('../../../../app/services/bill-runs/two-part-tariff/review-licence.service.js') describe('Review Licence Service', () => { - beforeEach(async () => { - await DatabaseSupport.clean() + const billRunId = '2c80bd22-a005-4cf4-a2a2-73812a9861de' + const licenceId = '082f528e-4ae4-4f41-ba64-b740a0a210ff' - Sinon.stub(FetchReviewLicenceResultsService, 'go').resolves({ - billRun: 'bill run data', - licence: [{ licenceRef: '7/34/10/*S/0084' }] - }) - Sinon.stub(ReviewLicencePresenter, 'go').resolves('page data') + beforeEach(async () => { + Sinon.stub(FetchReviewLicenceResultsService, 'go').resolves(_fetchReviewLicenceResults()) }) afterEach(() => { Sinon.restore() }) - describe('when there is data to process', () => { - const billRunId = '2c80bd22-a005-4cf4-a2a2-73812a9861de' - const licenceId = '082f528e-4ae4-4f41-ba64-b740a0a210ff' - const payload = undefined - - it('will fetch the bill run data and return it once formatted by the presenter', async () => { - const result = await ReviewLicenceService.go(billRunId, licenceId, payload) - - expect(result).to.equal('page data') - }) - }) - - describe('when there is data to process after the user clicking the "Confirm licence is ready" button', () => { - const payload = { licenceStatus: 'ready' } - let billRunId - let licenceId - - beforeEach(async () => { - const reviewLicence = await ReviewLicenceHelper.add({ status: 'review' }) - - billRunId = reviewLicence.billRunId - licenceId = reviewLicence.licenceId - }) - - it('will set `status` to `ready` in the database and return the page data', async () => { - const result = await ReviewLicenceService.go(billRunId, licenceId, payload) - const reviewLicenceQuery = await ReviewLicenceModel.query() - .where('billRunId', billRunId) - .andWhere('licenceId', licenceId) - .first() - - expect(reviewLicenceQuery.status).to.equal('ready') - expect(result).to.equal('page data') - }) - }) - - describe('when there is data to process after the user clicking the "Put licence into Review" button', () => { - const payload = { licenceStatus: 'review' } - let billRunId - let licenceId - - beforeEach(async () => { - const reviewLicence = await ReviewLicenceHelper.add({ status: 'ready' }) - - billRunId = reviewLicence.billRunId - licenceId = reviewLicence.licenceId - }) - - it('will set `status` to `review` in the database and return the page data', async () => { - const result = await ReviewLicenceService.go(billRunId, licenceId, payload) - const reviewLicenceQuery = await ReviewLicenceModel.query() - .where('billRunId', billRunId) - .andWhere('licenceId', licenceId) - .first() - - expect(reviewLicenceQuery.status).to.equal('review') - expect(result).to.equal('page data') - }) - }) - - describe('when there is data to process after the user clicking the "Mark progress" button', () => { - const payload = { marKProgress: 'mark' } - let billRunId - let licenceId - - beforeEach(async () => { - const reviewLicence = await ReviewLicenceHelper.add({ progress: false }) - - billRunId = reviewLicence.billRunId - licenceId = reviewLicence.licenceId - }) - - it('will set `progress` to true in the database and return the page data', async () => { - const result = await ReviewLicenceService.go(billRunId, licenceId, payload) - const reviewLicenceQuery = await ReviewLicenceModel.query() - .where('billRunId', billRunId) - .andWhere('licenceId', licenceId) - .first() - - expect(reviewLicenceQuery.progress).to.be.true() - expect(result).to.equal('page data') - }) - }) - - describe('when there is data to process after the user clicking the "Remove progress mark" button', () => { - const payload = { marKProgress: 'unmark' } - let billRunId - let licenceId - - beforeEach(async () => { - const reviewLicence = await ReviewLicenceHelper.add({ progress: true }) - - billRunId = reviewLicence.billRunId - licenceId = reviewLicence.licenceId - }) - - it('will set `progress` to false in the database and return the page data', async () => { - const result = await ReviewLicenceService.go(billRunId, licenceId, payload) - const reviewLicenceQuery = await ReviewLicenceModel.query() - .where('billRunId', billRunId) - .andWhere('licenceId', licenceId) - .first() - - expect(reviewLicenceQuery.progress).to.be.false() - expect(result).to.equal('page data') + describe('when called', () => { + it('returns page data for the view', async () => { + const result = await ReviewLicenceService.go(billRunId, licenceId) + + // NOTE: The service just regurgitates what the ReviewLicencePresenter returns. So, we don't diligently check + // each property of the result because we know this will have been covered by the ReviewLicencePresenter + expect(result.billRunId).to.equal('6620135b-0ecf-4fd4-924e-371f950c0526') + expect(result.region).to.equal('Anglian') + expect(result.licence.licenceId).to.equal('786f0d83-eaf7-43c3-9de5-ec59e3de05ee') + expect(result.licenceUpdatedMessage).to.be.null() + expect(result.matchedReturns[0].returnId).to.equal('v1:1:01/60/28/3437:17061181:2022-04-01:2023-03-31') + expect(result.unmatchedReturns[0].returnId).to.equal('v2:1:01/60/28/3437:17061181:2022-04-01:2023-03-31') + expect(result.chargeData[0].financialYear).to.equal('2022 to 2023') + expect(result.chargeData[0].licenceHolderName).to.equal('Licence Holder Ltd') + expect(result.chargeData[0].billingAccountDetails.billingAccountId).to.equal('a17ae69b-8074-4d27-80bf-074f4c79a05a') }) }) }) + +function _fetchReviewLicenceResults () { + return { + billRun: { + id: '6620135b-0ecf-4fd4-924e-371f950c0526', + fromFinancialYearEnding: 2023, + toFinancialYearEnding: 2023, + region: { + displayName: 'Anglian' + } + }, + licence: [{ + id: '5aa8e752-1a5c-4b01-9112-d92a543b70d1', + billRunId: '82772a06-c8ce-45f7-8504-dd20ea8824e4', + licenceId: '786f0d83-eaf7-43c3-9de5-ec59e3de05ee', + licenceRef: '01/49/80/4608', + licenceHolder: 'Licence Holder Ltd', + issues: '', + status: 'ready', + progress: false, + hasReviewStatus: false, + reviewReturns: [{ + id: '2264f443-5c16-4ca9-8522-f63e2d4e38be', + reviewLicenceId: '78a99c1c-26d3-4163-ab58-084cd78594ab', + returnId: 'v1:1:01/60/28/3437:17061181:2022-04-01:2023-03-31', + returnReference: '10031343', + quantity: 0, + allocated: 0, + underQuery: false, + returnStatus: 'completed', + nilReturn: false, + abstractionOutsidePeriod: false, + receivedDate: new Date('2022-06-03'), + dueDate: new Date('2022-06-03'), + purposes: [{ + tertiary: { + description: 'Site description' + } + }], + description: 'Lands at Mosshayne Farm, Exeter & Broadclyst', + startDate: new Date(' 2022-04-01'), + endDate: new Date('2022-05-06'), + issues: '', + reviewChargeElements: [{ + id: 'e840f418-ca6b-4d96-9f36-bf684c78590f', + reviewChargeReferenceId: '7759e0f9-5763-4b94-8d45-0621aea3edc1', + chargeElementId: 'b1cd4f98-ad96-4901-9e21-4432f032492a', + allocated: 0, + chargeDatesOverlap: false, + issues: '', + status: 'ready' + }] + }, + { + id: '4864f643-5c16-5ca9-8512-f63e1d4e58be', + reviewLicenceId: '78a99c1c-26d3-4163-ab58-084cd78594ab', + returnId: 'v2:1:01/60/28/3437:17061181:2022-04-01:2023-03-31', + returnReference: '10031343', + quantity: 0, + allocated: 0, + underQuery: false, + returnStatus: 'completed', + nilReturn: false, + abstractionOutsidePeriod: false, + receivedDate: new Date('2022-06-03'), + dueDate: new Date('2022-06-03'), + purposes: [{ + tertiary: { + description: 'Site description' + } + }], + description: 'Lands at Mosshayne Farm, Exeter & Broadclyst', + startDate: new Date(' 2022-04-01'), + endDate: new Date('2022-05-06'), + issues: '', + reviewChargeElements: [] + }], + reviewChargeVersions: [{ + id: '3de5634a-da26-4241-87e9-7248a4b83a69', + reviewLicenceId: 'd9e78306-bf65-4020-b279-5ae471cea4e6', + chargeVersionId: 'd103bb54-1819-4e77-b3d9-bc8913454e06', + changeReason: 'Strategic review of charges (SRoC)', + chargePeriodStartDate: new Date('2022-04-01'), + chargePeriodEndDate: new Date('2022-06-05'), + reviewChargeReferences: [{ + id: 'b2af5935-4b65-4dce-9f75-9073798f6375', + reviewChargeVersionId: 'bd16e7b0-c2a3-4258-b873-b965fd74cdf5', + chargeReferenceId: '82ce8695-5841-41b0-a1e7-d016407adad4', + aggregate: 1, + createdAt: new Date('2024-03-18'), + updatedAt: new Date('2024-03-18'), + chargeReference: { + chargeCategoryId: 'f100dc23-c6a7-4efa-af4f-80618260b32e', + chargeCategory: { + reference: '4.6.7', + shortDescription: 'High loss, non-tidal, greater than 15 up to and including 50 ML/yr' + } + }, + reviewChargeElements: [{ + id: '8bc0cd32-400e-4a45-9dd7-fbce3d486031', + reviewChargeReferenceId: '2210bb45-1efc-4e69-85cb-c8cc6e75c4fd', + chargeElementId: 'b1001716-cfb4-43c6-91f0-1863f4529223', + allocated: 0, + chargeDatesOverlap: false, + issues: '', + status: 'ready', + chargeElement: { + description: 'Trickle Irrigation - Direct', + abstractionPeriodStartDay: 1, + abstractionPeriodStartMonth: 4, + abstractionPeriodEndDay: 31, + abstractionPeriodEndMonth: 3, + authorisedAnnualQuantity: 200 + }, + reviewReturns: [{ + id: '2264f443-5c16-4ca9-8522-f63e2d4e38be', + reviewLicenceId: '78a99c1c-26d3-4163-ab58-084cd78594ab', + returnId: 'v1:1:01/60/28/3437:17061181:2022-04-01:2023-03-31', + returnReference: '10031343', + quantity: 0, + allocated: 0, + underQuery: false, + returnStatus: 'completed', + nilReturn: false, + abstractionOutsidePeriod: false, + receivedDate: new Date('2022-06-03'), + dueDate: new Date('2022-06-03'), + purposes: {}, + description: 'Lands at Mosshayne Farm, Exeter & Broadclyst', + startDate: new Date(' 2022-04-01'), + endDate: new Date('2022-05-06'), + issues: '' + }] + }] + }], + chargeVersion: { + billingAccountId: '67d7cacb-5d10-4a08-b7f8-e6ce98cbf4c8' + }, + billingAccountDetails: { + id: 'a17ae69b-8074-4d27-80bf-074f4c79a05a', + accountNumber: 'E88896464A', + company: { + id: 'e44491db-2b33-4473-9c3a-b57aceabb6e8', + name: 'Furland Farm', + type: 'organisation' + }, + billingAccountAddresses: [ + { + id: 'eb5cb54a-0b51-4e4a-8472-dab993eb6157', + billingAccountId: 'a17ae69b-8074-4d27-80bf-074f4c79a05a', + addressId: 'cc32fefd-7f3e-4581-b437-78a3fae66d4b', + startDate: new Date('2016-05-20'), + endDate: null, + companyId: null, + contactId: null, + company: null, + contact: null, + address: { + id: 'cc32fefd-7f3e-4581-b437-78a3fae66d4b', + address1: 'Furland Farm', + address2: 'Furland', + address3: null, + address4: null, + address5: 'Crewkerne', + address6: 'Somerset', + postcode: 'TA18 7TT', + country: 'England' + } + } + ] + } + }] + }] + } +} From f64954ba276e5fca22ec598a477bc2d88840daf6 Mon Sep 17 00:00:00 2001 From: Alan Cruikshanks Date: Tue, 23 Apr 2024 18:22:02 +0100 Subject: [PATCH 4/5] Create SubmitReviewLicenceService for POST The keen eyed will see this is an exact copy of `ReviewLicenceService` before we refactored it! The scope of this PR is to split the GET and POST request handling to get us in a position we can re-write how the POST is handled to use [yar](https://github.com/DEFRA/water-abstraction-system/pull/926) and fix it to support post-redirect-get again. So, we not only just copy the code across, we also copy the test as it was. We promise we'll be updating it in a subsequent change! ;-) --- .../submit-review-licence.service.js | 66 ++++++++ .../submit-review-licence.service.test.js | 145 ++++++++++++++++++ 2 files changed, 211 insertions(+) create mode 100644 app/services/bill-runs/two-part-tariff/submit-review-licence.service.js create mode 100644 test/services/bill-runs/two-part-tariff/submit-review-licence.service.test.js 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 new file mode 100644 index 0000000000..26978985a1 --- /dev/null +++ b/app/services/bill-runs/two-part-tariff/submit-review-licence.service.js @@ -0,0 +1,66 @@ +'use strict' + +/** + * Orchestrates fetching and presenting the data needed for the licence review page in a two-part tariff bill run + * @module ReviewLicenceService + */ + +const FetchReviewLicenceResultsService = require('./fetch-review-licence-results.service.js') +const ReviewLicenceModel = require('../../../models/review-licence.model.js') +const ReviewLicencePresenter = require('../../../presenters/bill-runs/two-part-tariff/review-licence.presenter.js') + +/** + * Orchestrated fetching and presenting the data needed for the licence review page + * + * @param {module:BillRunModel} billRunId The UUID for the bill run + * @param {module:LicenceModel} licenceId The UUID of the licence that is being reviewed + * @param {Object} payload The `request.payload` containing the `marKProgress` data. This is only passed to the service + * when there is a POST request, which only occurs when the 'Mark progress' button is clicked. + * + * @returns {Object} an object representing the 'pageData' needed to review the individual licence. It contains the + * licence, bill run, matched and unmatched returns and the licence charge data + */ +async function go (billRunId, licenceId, payload) { + const licenceStatus = payload?.licenceStatus + const markProgress = payload?.marKProgress + + if (payload) { + await _processPayload(billRunId, licenceId, licenceStatus, markProgress) + } + + const { billRun, licence } = await FetchReviewLicenceResultsService.go(billRunId, licenceId) + + const pageData = ReviewLicencePresenter.go(billRun, licence, licenceStatus, markProgress) + + return pageData +} + +async function _processPayload (billRunId, licenceId, licenceStatus, markProgress) { + if (licenceStatus === 'ready' || licenceStatus === 'review') { + await _updateStatus(billRunId, licenceId, licenceStatus) + } + + if (markProgress === 'mark' || markProgress === 'unmark') { + await _updateProgress(billRunId, licenceId, markProgress) + } +} + +async function _updateProgress (billRunId, licenceId, marKProgress) { + const progress = marKProgress === 'mark' + + await ReviewLicenceModel.query() + .patch({ progress }) + .where('billRunId', billRunId) + .andWhere('licenceId', licenceId) +} + +async function _updateStatus (billRunId, licenceId, licenceStatus) { + await ReviewLicenceModel.query() + .patch({ status: licenceStatus }) + .where('billRunId', billRunId) + .andWhere('licenceId', licenceId) +} + +module.exports = { + go +} diff --git a/test/services/bill-runs/two-part-tariff/submit-review-licence.service.test.js b/test/services/bill-runs/two-part-tariff/submit-review-licence.service.test.js new file mode 100644 index 0000000000..ab61874baa --- /dev/null +++ b/test/services/bill-runs/two-part-tariff/submit-review-licence.service.test.js @@ -0,0 +1,145 @@ +'use strict' + +// Test framework dependencies +const Lab = require('@hapi/lab') +const Code = require('@hapi/code') +const Sinon = require('sinon') + +const { describe, it, beforeEach, afterEach } = exports.lab = Lab.script() +const { expect } = Code + +// Test helpers +const DatabaseSupport = require('../../../support/database.js') +const ReviewLicenceHelper = require('../../../support/helpers/review-licence.helper.js') +const ReviewLicenceModel = require('../../../../app/models/review-licence.model.js') + +// Things we need to stub +const FetchReviewLicenceResultsService = require('../../../../app/services/bill-runs/two-part-tariff/fetch-review-licence-results.service.js') +const ReviewLicencePresenter = require('../../../../app/presenters/bill-runs/two-part-tariff/review-licence.presenter.js') + +// Thing under test +const SubmitReviewLicenceService = require('../../../../app/services/bill-runs/two-part-tariff/submit-review-licence.service.js') + +describe('Submit Review Licence Service', () => { + beforeEach(async () => { + await DatabaseSupport.clean() + + Sinon.stub(FetchReviewLicenceResultsService, 'go').resolves({ + billRun: 'bill run data', + licence: [{ licenceRef: '7/34/10/*S/0084' }] + }) + Sinon.stub(ReviewLicencePresenter, 'go').resolves('page data') + }) + + afterEach(() => { + Sinon.restore() + }) + + describe('when there is data to process', () => { + const billRunId = '2c80bd22-a005-4cf4-a2a2-73812a9861de' + const licenceId = '082f528e-4ae4-4f41-ba64-b740a0a210ff' + const payload = undefined + + it('will fetch the bill run data and return it once formatted by the presenter', async () => { + const result = await SubmitReviewLicenceService.go(billRunId, licenceId, payload) + + expect(result).to.equal('page data') + }) + }) + + describe('when there is data to process after the user clicking the "Confirm licence is ready" button', () => { + const payload = { licenceStatus: 'ready' } + let billRunId + let licenceId + + beforeEach(async () => { + const reviewLicence = await ReviewLicenceHelper.add({ status: 'review' }) + + billRunId = reviewLicence.billRunId + licenceId = reviewLicence.licenceId + }) + + it('will set `status` to `ready` in the database and return the page data', async () => { + const result = await SubmitReviewLicenceService.go(billRunId, licenceId, payload) + const reviewLicenceQuery = await ReviewLicenceModel.query() + .where('billRunId', billRunId) + .andWhere('licenceId', licenceId) + .first() + + expect(reviewLicenceQuery.status).to.equal('ready') + expect(result).to.equal('page data') + }) + }) + + describe('when there is data to process after the user clicking the "Put licence into Review" button', () => { + const payload = { licenceStatus: 'review' } + let billRunId + let licenceId + + beforeEach(async () => { + const reviewLicence = await ReviewLicenceHelper.add({ status: 'ready' }) + + billRunId = reviewLicence.billRunId + licenceId = reviewLicence.licenceId + }) + + it('will set `status` to `review` in the database and return the page data', async () => { + const result = await SubmitReviewLicenceService.go(billRunId, licenceId, payload) + const reviewLicenceQuery = await ReviewLicenceModel.query() + .where('billRunId', billRunId) + .andWhere('licenceId', licenceId) + .first() + + expect(reviewLicenceQuery.status).to.equal('review') + expect(result).to.equal('page data') + }) + }) + + describe('when there is data to process after the user clicking the "Mark progress" button', () => { + const payload = { marKProgress: 'mark' } + let billRunId + let licenceId + + beforeEach(async () => { + const reviewLicence = await ReviewLicenceHelper.add({ progress: false }) + + billRunId = reviewLicence.billRunId + licenceId = reviewLicence.licenceId + }) + + it('will set `progress` to true in the database and return the page data', async () => { + const result = await SubmitReviewLicenceService.go(billRunId, licenceId, payload) + const reviewLicenceQuery = await ReviewLicenceModel.query() + .where('billRunId', billRunId) + .andWhere('licenceId', licenceId) + .first() + + expect(reviewLicenceQuery.progress).to.be.true() + expect(result).to.equal('page data') + }) + }) + + describe('when there is data to process after the user clicking the "Remove progress mark" button', () => { + const payload = { marKProgress: 'unmark' } + let billRunId + let licenceId + + beforeEach(async () => { + const reviewLicence = await ReviewLicenceHelper.add({ progress: true }) + + billRunId = reviewLicence.billRunId + licenceId = reviewLicence.licenceId + }) + + it('will set `progress` to false in the database and return the page data', async () => { + const result = await SubmitReviewLicenceService.go(billRunId, licenceId, payload) + const reviewLicenceQuery = await ReviewLicenceModel.query() + .where('billRunId', billRunId) + .andWhere('licenceId', licenceId) + .first() + + expect(reviewLicenceQuery.progress).to.be.false() + expect(result).to.equal('page data') + }) + }) +}) From dd2b57c84f9151913588c0629948cd86dffe5c18 Mon Sep 17 00:00:00 2001 From: Alan Cruikshanks Date: Tue, 23 Apr 2024 18:26:14 +0100 Subject: [PATCH 5/5] Update controller to use the new Submit service --- app/controllers/bill-runs.controller.js | 3 ++- test/controllers/bill-runs.controller.test.js | 5 +++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/app/controllers/bill-runs.controller.js b/app/controllers/bill-runs.controller.js index 7a4425a7f3..47494df1f5 100644 --- a/app/controllers/bill-runs.controller.js +++ b/app/controllers/bill-runs.controller.js @@ -18,6 +18,7 @@ const SendBillRunService = require('../services/bill-runs/send-bill-run.service. const StartBillRunProcessService = require('../services/bill-runs/start-bill-run-process.service.js') const SubmitAmendedBillableReturnsService = require('..//services/bill-runs/two-part-tariff/submit-amended-billable-returns.service.js') const SubmitCancelBillRunService = require('../services/bill-runs/submit-cancel-bill-run.service.js') +const SubmitReviewLicenceService = require('../services/bill-runs/two-part-tariff/submit-review-licence.service.js') const SubmitSendBillRunService = require('../services/bill-runs/submit-send-bill-run.service.js') const ViewBillRunService = require('../services/bill-runs/view-bill-run.service.js') @@ -149,7 +150,7 @@ async function submitCancel (request, h) { async function submitReviewLicence (request, h) { const { id: billRunId, licenceId } = request.params - const pageData = await ReviewLicenceService.go(billRunId, licenceId, request.payload) + const pageData = await SubmitReviewLicenceService.go(billRunId, licenceId, request.payload) return h.view('bill-runs/review-licence.njk', { pageTitle: `Licence ${pageData.licence.licenceRef}`, diff --git a/test/controllers/bill-runs.controller.test.js b/test/controllers/bill-runs.controller.test.js index e5d87d0efa..ad9e277a2c 100644 --- a/test/controllers/bill-runs.controller.test.js +++ b/test/controllers/bill-runs.controller.test.js @@ -14,12 +14,13 @@ const Boom = require('@hapi/boom') const CancelBillRunService = require('../../app/services/bill-runs/cancel-bill-run.service.js') const IndexBillRunsService = require('../../app/services/bill-runs/index-bill-runs.service.js') const MatchDetailsService = require('../../app/services/bill-runs/two-part-tariff/match-details.service.js') -const ReviewLicenceService = require('../../app/services/bill-runs/two-part-tariff/review-licence.service.js') const ReviewBillRunService = require('../../app/services/bill-runs/two-part-tariff/review-bill-run.service.js') +const ReviewLicenceService = require('../../app/services/bill-runs/two-part-tariff/review-licence.service.js') const SendBillRunService = require('../../app/services/bill-runs/send-bill-run.service.js') const StartBillRunProcessService = require('../../app/services/bill-runs/start-bill-run-process.service.js') const SubmitAmendedBillableReturnsService = require('../../app/services/bill-runs/two-part-tariff/submit-amended-billable-returns.service.js') const SubmitCancelBillRunService = require('../../app/services/bill-runs/submit-cancel-bill-run.service.js') +const SubmitReviewLicenceService = require('../../app/services/bill-runs/two-part-tariff/submit-review-licence.service.js') const SubmitSendBillRunService = require('../../app/services/bill-runs/submit-send-bill-run.service.js') const ViewBillRunService = require('../../app/services/bill-runs/view-bill-run.service.js') @@ -373,7 +374,7 @@ describe('Bill Runs controller', () => { describe('when a request is valid', () => { beforeEach(() => { - Sinon.stub(ReviewLicenceService, 'go').resolves(_licenceReviewData()) + Sinon.stub(SubmitReviewLicenceService, 'go').resolves(_licenceReviewData()) }) it('returns a 200 response', async () => {