From 9d2c828abdd62493a94845ff2a52cdd96aa7dadb Mon Sep 17 00:00:00 2001 From: Alan Cruikshanks Date: Sun, 17 Mar 2024 23:40:04 +0000 Subject: [PATCH] Migrate legacy send bill run functionality (#771) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit https://eaflood.atlassian.net/browse/WATER-4390 We have been working on replacing the legacy SROC annual billing engine. It is slow and prone to error leading to the team having to devote resources every year to helping the billing & data team get the annual bill runs through. Like with our supplementary billing engine, we focused on the first part of the process; creating the bill run record and then generating the transactions to send to the [charging-module-api (CHA)](https://github.com/DEFRA/sroc-charging-module-api). With that done we then pass control back to the legacy code to refresh the data and mark the bill run as **READY**. But our testing demonstrated that even with this part of the process replaced annual bill runs can still prove to be problematic due to the shoddy way the previous team implemented them. For example, when we send the POST request to the legacy `/refresh` endpoint it kicks off a process to request the 'generated' billing information from the CHA. This entails sending a request for every bill in the bill run and then updating our local data with the result. You can't **SEND** a bill run until this step is complete. When you view the bill run all the values will be £0.00 and the download CSV will be working. Yet the legacy code still marks the bill run as **READY**. Users familiar with the service may know they need to leave it a while for the numbers to refresh. What they probably don't know is the CHA is getting hammered and the service is busy with this process. Ideally, they should wait for this process to complete before starting a new bill run. But there is no way to know that. So, the next bill run will take longer to complete and you increase the risk of an error. We felt this situation was ridiculous so opted to amend the [water-abstraction-service](https://github.com/DEFRA/water-abstraction-service) to solve this. (See [Stop marking bill runs as ready when they are not](https://github.com/DEFRA/water-abstraction-service/pull/2442)). It worked but inadvertently broke the legacy 'send a bill run' process (the final step in the bill run journey). When you confirm and send a bill run it tells the CHA to send the bill run to SOP. The CHA does a range of things behind the scenes one of which is to generate a transaction reference for each invoice. Again we need that information on our side but only that. This is where things go 🦇💩! The legacy service does this by calling the refresh invoices job again!! So, we're going through the whole process of sending a request for every invoice, updating values we already have including dealing with updating the transactions all over again. _All of this_ for something we can get making ***one*** request to the CHA (a request the legacy process already does before going through all the invoices again). We didn't know the legacy code reused this job. So, when we amended it to set the bill run status to **READY** at the end of the process we broke sending because in that scenario we are setting the status back from **SENT** to **READY**. We've implemented a 'hack' to get around this. But enough is enough! With this change, we take over the 'send bill run process' and resolve this properly. --- app/controllers/bill-runs.controller.js | 39 +- .../bill-runs/send-bill-run.presenter.js | 50 ++ app/routes/bill-runs.routes.js | 26 + .../bill-runs/send-bill-run.service.js | 51 ++ .../bill-runs/submit-send-bill-run.service.js | 118 +++++ app/views/bill-runs/send.njk | 77 +++ app/views/bill-runs/view.njk | 6 +- test/controllers/bill-runs.controller.test.js | 454 +++++++++--------- .../bill-runs/send-bill-run.presenter.test.js | 53 ++ .../send-bill-run.request.test.js | 129 +++++ .../bill-runs/send-bill-run.service.test.js | 60 +++ 11 files changed, 839 insertions(+), 224 deletions(-) create mode 100644 app/presenters/bill-runs/send-bill-run.presenter.js create mode 100644 app/services/bill-runs/send-bill-run.service.js create mode 100644 app/services/bill-runs/submit-send-bill-run.service.js create mode 100644 app/views/bill-runs/send.njk create mode 100644 test/presenters/bill-runs/send-bill-run.presenter.test.js create mode 100644 test/requests/charging-module/send-bill-run.request.test.js create mode 100644 test/services/bill-runs/send-bill-run.service.test.js diff --git a/app/controllers/bill-runs.controller.js b/app/controllers/bill-runs.controller.js index a4d47a345a..a3c0c1a033 100644 --- a/app/controllers/bill-runs.controller.js +++ b/app/controllers/bill-runs.controller.js @@ -11,8 +11,10 @@ const CancelBillRunService = require('../services/bill-runs/cancel-bill-run.serv const CreateBillRunValidator = require('../validators/create-bill-run.validator.js') const ReviewBillRunService = require('../services/bill-runs/two-part-tariff/review-bill-run.service.js') const ReviewLicenceService = require('../services/bill-runs/two-part-tariff/review-licence.service.js') +const SendBillRunService = require('../services/bill-runs/send-bill-run.service.js') const StartBillRunProcessService = require('../services/bill-runs/start-bill-run-process.service.js') const SubmitCancelBillRunService = require('../services/bill-runs/submit-cancel-bill-run.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') async function cancel (request, h) { @@ -55,6 +57,18 @@ async function review (request, h) { }) } +async function send (request, h) { + const { id } = request.params + + const pageData = await SendBillRunService.go(id) + + return h.view('bill-runs/send.njk', { + pageTitle: "You're about to send this bill run", + activeNavBar: 'bill-runs', + ...pageData + }) +} + async function reviewLicence (request, h) { const { id: billRunId, licenceId } = request.params @@ -72,13 +86,7 @@ async function submitCancel (request, h) { try { // NOTE: What we are awaiting here is for the SubmitCancelBillRunService to update the status of the bill run to - // `cancel'. If the bill run run is deemed small enough, we'll also wait for the cancelling to complete before - // we redirect. This avoids users always having to refresh the bill runs page to get rid of bill runs that have - // finished cancelling 1 second after the request is submitted. - // - // But for larger bill runs, especially annual, after the bill run status has been updated control will be returned - // to here and the cancel process will then happen in the background. Because of this if we didn't wrap the call - // in a try/catch and the process errored, we'd get an unhandled exception which will bring the service down! + // `cancel'. await SubmitCancelBillRunService.go(id) return h.redirect('/billing/batch/list') @@ -87,6 +95,21 @@ async function submitCancel (request, h) { } } +async function submitSend (request, h) { + const { id } = request.params + + try { + // NOTE: What we are awaiting here is for the SubmitSendBillRunService to update the status of the bill run to + // `sending'. + await SubmitSendBillRunService.go(id) + + // Redirect to the legacy processing page + return h.redirect(`/billing/batch/${id}/processing`) + } catch (error) { + return Boom.badImplementation(error.message) + } +} + async function view (request, h) { const { id } = request.params @@ -103,6 +126,8 @@ module.exports = { create, review, reviewLicence, + send, submitCancel, + submitSend, view } diff --git a/app/presenters/bill-runs/send-bill-run.presenter.js b/app/presenters/bill-runs/send-bill-run.presenter.js new file mode 100644 index 0000000000..ed3c4aa675 --- /dev/null +++ b/app/presenters/bill-runs/send-bill-run.presenter.js @@ -0,0 +1,50 @@ +'use strict' + +/** + * Formats the bill run data ready for presenting in the send bill run confirmation page + * @module SendBillRunPresenter + */ + +const { + capitalize, + formatBillRunType, + formatChargeScheme, + formatFinancialYear, + formatLongDate +} = require('../base.presenter.js') + +/** + * Prepares and processes bill run data for presentation + * + * @param {module:BillRunModel} billRun - an instance of `BillRunModel` + * + * @returns {Object} - the prepared bill run data to be passed to the send bill run confirmation page + */ +function go (billRun) { + const { + batchType, + billRunNumber, + createdAt, + id, + region, + scheme, + status, + summer, + toFinancialYearEnding + } = billRun + + return { + billRunId: id, + billRunNumber, + billRunStatus: status, + billRunType: formatBillRunType(batchType, scheme, summer), + chargeScheme: formatChargeScheme(scheme), + dateCreated: formatLongDate(createdAt), + financialYear: formatFinancialYear(toFinancialYearEnding), + region: capitalize(region.displayName) + } +} + +module.exports = { + go +} diff --git a/app/routes/bill-runs.routes.js b/app/routes/bill-runs.routes.js index b9d62a1e37..2ff40a5c21 100644 --- a/app/routes/bill-runs.routes.js +++ b/app/routes/bill-runs.routes.js @@ -96,6 +96,32 @@ const routes = [ }, description: 'Review a two-part tariff licence' } + }, + { + method: 'GET', + path: '/bill-runs/{id}/send', + handler: BillRunsController.send, + options: { + auth: { + access: { + scope: ['billing'] + } + }, + description: 'Confirm (send) a bill run' + } + }, + { + method: 'POST', + path: '/bill-runs/{id}/send', + handler: BillRunsController.submitSend, + options: { + auth: { + access: { + scope: ['billing'] + } + }, + description: 'Submit bill run (send) confirmation' + } } ] diff --git a/app/services/bill-runs/send-bill-run.service.js b/app/services/bill-runs/send-bill-run.service.js new file mode 100644 index 0000000000..e85b9b36c2 --- /dev/null +++ b/app/services/bill-runs/send-bill-run.service.js @@ -0,0 +1,51 @@ +'use strict' + +/** + * Orchestrates fetching and presenting the data needed for the send bill run confirmation page + * @module SendBillRunService + */ + +const BillRunModel = require('../../models/bill-run.model.js') +const SendBillRunPresenter = require('../../presenters/bill-runs/send-bill-run.presenter.js') + +/** + * Orchestrates fetching and presenting the data needed for the send bill run confirmation page + * + * @param {string} id - The UUID of the bill run to send + * + * @returns {Promise an object representing the `pageData` needed by the send bill run template. It contains + * details of the bill run. + */ +async function go (id) { + const billRun = await _fetchBillRun(id) + + const pageData = SendBillRunPresenter.go(billRun) + + return pageData +} + +async function _fetchBillRun (id) { + return BillRunModel.query() + .findById(id) + .select([ + 'id', + 'batchType', + 'billRunNumber', + 'createdAt', + 'scheme', + 'status', + 'summer', + 'toFinancialYearEnding' + ]) + .withGraphFetched('region') + .modifyGraph('region', (builder) => { + builder.select([ + 'id', + 'displayName' + ]) + }) +} + +module.exports = { + go +} diff --git a/app/services/bill-runs/submit-send-bill-run.service.js b/app/services/bill-runs/submit-send-bill-run.service.js new file mode 100644 index 0000000000..43cc7012fb --- /dev/null +++ b/app/services/bill-runs/submit-send-bill-run.service.js @@ -0,0 +1,118 @@ +'use strict' + +/** + * Orchestrates the sending of a bill run + * @module SubmitCancelBillRunService + */ + +const BillModel = require('../../models/bill.model.js') +const BillRunModel = require('../../models/bill-run.model.js') +const ExpandedError = require('../../errors/expanded.error.js') +const { calculateAndLogTimeTaken, timestampForPostgres } = require('../../lib/general.lib.js') +const ChargingModuleSendBillRunRequest = require('../../requests/charging-module/send-bill-run.request.js') +const ChargingModuleViewBillRunRequest = require('../../requests/charging-module/view-bill-run.request.js') + +/** + * Orchestrates the sending of a bill run + * + * After checking that the bill run has a status that can be sent (only ready bill runs can be sent) + * we set the status of the bill run to `sending`. At this point we return control to the controller so that it can + * redirect the user to the bill runs page. + * + * Meantime now in the background, we send a request to the Charging Module API to send the bill run there. Once that + * process is complete we get the updated bill run summary and extract the generated bill numbers and transaction file + * reference from it. + * + * It can take the CHA more than a second to complete the send process for larger bill runs. This is why we hand back + * control to the UI early so we don't block the user or cause a timeout. + * + * @param {string} billRunId - UUID of the bill run to be sent + * + * @returns {Promise} the promise returned is not intended to resolve to any particular value + */ +async function go (billRunId) { + const billRun = await _fetchBillRun(billRunId) + + if (billRun.status !== 'ready') { + return + } + + await _updateStatus(billRunId, 'sending') + + _sendBillRun(billRun) +} + +/** + * This service handles the POST request from the confirm sent bill run page. We _could_ have included the + * externalId as a hidden field in the form and included it in the request. This would give the impression we could + * avoid a query to the DB. + * + * But we need to ensure no one exploits the `POST /bill-runs/{id}/send` endpoint. So, we always have to fetch the bill + * run to check its status is not one that prevents us deleting it. + */ +async function _fetchBillRun (id) { + return BillRunModel.query() + .findById(id) + .select([ + 'id', + 'externalId', + 'status' + ]) +} + +async function _fetchChargingModuleBillRun (externalId) { + const result = await ChargingModuleViewBillRunRequest.send(externalId) + + if (!result.succeeded) { + const error = new ExpandedError( + 'Charging Module view bill run request failed', + { billRunExternalId: externalId, responseBody: result.response.body } + ) + + throw error + } + + return result.response.body.billRun +} + +async function _sendBillRun (billRun) { + const startTime = process.hrtime.bigint() + + const { id: billRunId, externalId } = billRun + + await ChargingModuleSendBillRunRequest.send(externalId) + + const externalBillRun = await _fetchChargingModuleBillRun(externalId) + + await _updateInvoiceData(externalBillRun) + + await _updateBillRunData(billRun, externalBillRun) + + calculateAndLogTimeTaken(startTime, 'Send bill run complete', { billRunId }) +} + +async function _updateBillRunData (billRun, externalBillRun) { + const { transactionFileReference } = externalBillRun + + return billRun.$query().patch({ status: 'sent', transactionFileReference }) +} + +async function _updateInvoiceData (externalBillRun) { + const { invoices } = externalBillRun + + for (const invoice of invoices) { + const { id, transactionReference: invoiceNumber } = invoice + + await BillModel.query().patch({ invoiceNumber }).where('externalId', id) + } +} + +async function _updateStatus (billRunId, status) { + return BillRunModel.query() + .findById(billRunId) + .patch({ status, updatedAt: timestampForPostgres() }) +} + +module.exports = { + go +} diff --git a/app/views/bill-runs/send.njk b/app/views/bill-runs/send.njk new file mode 100644 index 0000000000..20f4f67682 --- /dev/null +++ b/app/views/bill-runs/send.njk @@ -0,0 +1,77 @@ +{% extends 'layout.njk' %} +{% from "govuk/components/back-link/macro.njk" import govukBackLink %} +{% from "govuk/components/button/macro.njk" import govukButton %} +{% from "govuk/components/summary-list/macro.njk" import govukSummaryList %} + +{% from "macros/badge.njk" import statusBadge %} + +{% block breadcrumbs %} + {# Back link #} + {{ + govukBackLink({ + text: 'Back', + href: '/system/bill-runs/' + billRunId + }) + }} +{% endblock %} + +{% block content %} + {# Main heading #} +
+

+ Bill run {{ billRunNumber }}{{ pageTitle }} +

+
+ +
+
+ + {# Status badge #} +

+ {{ statusBadge(billRunStatus) }} +

+ + {# Bill run meta-data #} + {# + GOV.UK summary lists only allow us to assign attributes at the top level and not to each row. This means we + can't assign our data-test attribute using the component. Our solution is to use the html option for each row + instead of text and wrap each value in a . That way we can manually assign our data-test attribute to the + span. + #} + {{ + govukSummaryList({ + classes: 'govuk-summary-list--no-border', + attributes: { + 'data-test': 'meta-data' + }, + rows: [ + { + key: { text: "Date created", classes: "meta-data__label" }, + value: { html: '' + dateCreated + '', classes: "meta-data__value" } + }, + { + key: { text: "Region", classes: "meta-data__label" }, + value: { html: '' + region + '', classes: "meta-data__value" } + }, + { + key: { text: "Bill run type", classes: "meta-data__label" }, + value: { html: '' + billRunType + '', classes: "meta-data__value" } + }, + { + key: { text: "Charge scheme", classes: "meta-data__label" }, + value: { html: '' + chargeScheme + '', classes: "meta-data__value" } + }, + { + key: { text: "Financial year", classes: "meta-data__label" }, + value: { html: '' + financialYear + '', classes: "meta-data__value" } + } + ] + }) + }} + +
+ {{ govukButton({ text: "Send bill run" }) }} +
+
+
+{% endblock %} diff --git a/app/views/bill-runs/view.njk b/app/views/bill-runs/view.njk index 1980172dfa..f1d5a1f12d 100644 --- a/app/views/bill-runs/view.njk +++ b/app/views/bill-runs/view.njk @@ -112,15 +112,15 @@ {# Confirm and cancel buttons #} {% if billRunStatus === 'ready' %} {% set cancelBillRunLink = '/system/bill-runs/' + billRunId + '/cancel' %} - {% set confirmBillRunLink = '/billing/batch/' + billRunId + '/confirm' %} + {% set sendBillRunLink = '/system/bill-runs/' + billRunId + '/send' %}
{{ govukButton({ classes: 'govuk-!-margin-right-1', - text: "Confirm bill run", - href: confirmBillRunLink + text: "Send bill run", + href: sendBillRunLink }) }} {{ diff --git a/test/controllers/bill-runs.controller.test.js b/test/controllers/bill-runs.controller.test.js index 20ed8a894d..b98d4a35f0 100644 --- a/test/controllers/bill-runs.controller.test.js +++ b/test/controllers/bill-runs.controller.test.js @@ -13,14 +13,17 @@ const Boom = require('@hapi/boom') const CancelBillRunService = require('../../app/services/bill-runs/cancel-bill-run.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 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 SubmitCancelBillRunService = require('../../app/services/bill-runs/submit-cancel-bill-run.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') // For running our service const { init } = require('../../app/server.js') describe('Bill Runs controller', () => { + let options let server // Create server before each test @@ -39,324 +42,347 @@ describe('Bill Runs controller', () => { Sinon.restore() }) - describe('POST /bill-runs', () => { - let options - - beforeEach(() => { - options = { - method: 'POST', - url: '/bill-runs', - payload: { + describe('/bill-runs', () => { + describe('POST', () => { + beforeEach(() => { + options = _options('POST') + options.url = '/bill-runs' + options.payload = { type: 'supplementary', scheme: 'sroc', region: '07ae7f3a-2677-4102-b352-cc006828948c', user: 'test.user@defra.gov.uk' - }, - auth: { - strategy: 'session', - credentials: { scope: ['billing'] } } - } - }) - - describe('when a request is valid', () => { - const validResponse = { - id: 'f561990b-b29a-42f4-b71a-398c52339f78', - region: '07ae7f3a-2677-4102-b352-cc006828948c', - scheme: 'sroc', - batchType: 'supplementary', - status: 'processing' - } - - beforeEach(async () => { - Sinon.stub(StartBillRunProcessService, 'go').resolves(validResponse) }) - it('returns a 200 response including details of the new bill run', async () => { - const response = await server.inject(options) - const payload = JSON.parse(response.payload) - - expect(response.statusCode).to.equal(200) - expect(payload).to.equal(validResponse) - }) - }) + describe('when a request is valid', () => { + const validResponse = { + id: 'f561990b-b29a-42f4-b71a-398c52339f78', + region: '07ae7f3a-2677-4102-b352-cc006828948c', + scheme: 'sroc', + batchType: 'supplementary', + status: 'processing' + } - describe('when the request fails', () => { - describe('because the request is invalid', () => { - beforeEach(() => { - options.payload.scheme = 'INVALID' + beforeEach(async () => { + Sinon.stub(StartBillRunProcessService, 'go').resolves(validResponse) }) - it('returns an error response', async () => { + it('returns a 200 response including details of the new bill run', async () => { const response = await server.inject(options) const payload = JSON.parse(response.payload) - expect(response.statusCode).to.equal(400) - expect(payload.message).to.startWith('"scheme" must be') + expect(response.statusCode).to.equal(200) + expect(payload).to.equal(validResponse) }) }) - describe('because the bill run could not be initiated', () => { - beforeEach(async () => { - Sinon.stub(Boom, 'badImplementation').returns(new Boom.Boom('Bang', { statusCode: 500 })) - Sinon.stub(StartBillRunProcessService, 'go').rejects() + describe('when the request fails', () => { + describe('because the request is invalid', () => { + beforeEach(() => { + options.payload.scheme = 'INVALID' + }) + + it('returns an error response', async () => { + const response = await server.inject(options) + const payload = JSON.parse(response.payload) + + expect(response.statusCode).to.equal(400) + expect(payload.message).to.startWith('"scheme" must be') + }) }) - it('returns an error response', async () => { - const response = await server.inject(options) - const payload = JSON.parse(response.payload) + describe('because the bill run could not be initiated', () => { + beforeEach(async () => { + Sinon.stub(Boom, 'badImplementation').returns(new Boom.Boom('Bang', { statusCode: 500 })) + Sinon.stub(StartBillRunProcessService, 'go').rejects() + }) + + it('returns an error response', async () => { + const response = await server.inject(options) + const payload = JSON.parse(response.payload) - expect(response.statusCode).to.equal(500) - expect(payload.message).to.equal('An internal server error occurred') + expect(response.statusCode).to.equal(500) + expect(payload.message).to.equal('An internal server error occurred') + }) }) }) }) }) - describe('GET /bill-runs/{id}', () => { - let options + describe('/bill-runs/{id}', () => { + describe('GET', () => { + beforeEach(async () => { + options = _options('GET') + }) - beforeEach(async () => { - options = { - method: 'GET', - url: '/bill-runs/97db1a27-8308-4aba-b463-8a6af2558b28', - auth: { - strategy: 'session', - credentials: { scope: ['billing'] } - } - } - }) + describe('when the request succeeds', () => { + describe('and it is for a bill run with multiple bill groups', () => { + beforeEach(async () => { + Sinon.stub(ViewBillRunService, 'go').resolves(_multiGroupBillRun()) + }) - describe('when the request succeeds', () => { - describe('and it is for a bill run with multiple bill groups', () => { - beforeEach(async () => { - Sinon.stub(ViewBillRunService, 'go').resolves(_multiGroupBillRun()) + it('returns the page successfully', async () => { + const response = await server.inject(options) + + expect(response.statusCode).to.equal(200) + expect(response.payload).to.contain('2 Annual bills') + expect(response.payload).to.contain('1 water company') + expect(response.payload).to.contain('1 other abstractor') + }) }) - it('returns the page successfully', async () => { - const response = await server.inject(options) + describe('and it is for a bill run with a single bill group', () => { + beforeEach(async () => { + Sinon.stub(ViewBillRunService, 'go').resolves(_singleGroupBillRun()) + }) - expect(response.statusCode).to.equal(200) - expect(response.payload).to.contain('2 Annual bills') - expect(response.payload).to.contain('1 water company') - expect(response.payload).to.contain('1 other abstractor') + it('returns the page successfully', async () => { + const response = await server.inject(options) + + expect(response.statusCode).to.equal(200) + expect(response.payload).to.contain('1 Annual bill') + // NOTE: If we only have 1 bill group we show a single table with no caption + expect(response.payload).not.to.contain('1 water company') + expect(response.payload).not.to.contain('1 other abstractor') + }) }) }) + }) + }) - describe('and it is for a bill run with a single bill group', () => { - beforeEach(async () => { - Sinon.stub(ViewBillRunService, 'go').resolves(_singleGroupBillRun()) + describe('/bill-runs/{id}/cancel', () => { + describe('GET /bill-runs/{id}/cancel', () => { + beforeEach(async () => { + options = _options('GET', 'cancel') + }) + + describe('when a request is valid', () => { + beforeEach(() => { + Sinon.stub(CancelBillRunService, 'go').resolves({ + id: '8702b98f-ae51-475d-8fcc-e049af8b8d38', + billRunType: 'Two-part tariff', + pageTitle: "You're about to cancel this bill run" + }) }) - it('returns the page successfully', async () => { + it('returns a 200 response', async () => { const response = await server.inject(options) expect(response.statusCode).to.equal(200) - expect(response.payload).to.contain('1 Annual bill') - // NOTE: If we only have 1 bill group we show a single table with no caption - expect(response.payload).not.to.contain('1 water company') - expect(response.payload).not.to.contain('1 other abstractor') + expect(response.payload).to.contain('You're about to cancel this bill run') + expect(response.payload).to.contain('Two-part tariff') }) }) }) - }) - describe('GET /bill-runs/{id}/cancel', () => { - let options + describe('POST /bill-runs/{id}/cancel', () => { + beforeEach(() => { + options = _options('POST', 'cancel') + }) - beforeEach(async () => { - options = { - method: 'GET', - url: '/bill-runs/97db1a27-8308-4aba-b463-8a6af2558b28/cancel', - auth: { - strategy: 'session', - credentials: { scope: ['billing'] } - } - } - }) + describe('when a request is valid', () => { + beforeEach(async () => { + Sinon.stub(SubmitCancelBillRunService, 'go').resolves() + }) - describe('when a request is valid', () => { - beforeEach(() => { - Sinon.stub(CancelBillRunService, 'go').resolves({ - id: '8702b98f-ae51-475d-8fcc-e049af8b8d38', - billRunType: 'Two-part tariff', - pageTitle: "You're about to cancel this bill run" + it('redirects to the bill runs page', async () => { + const response = await server.inject(options) + + expect(response.statusCode).to.equal(302) + expect(response.headers.location).to.equal('/billing/batch/list') }) }) - it('returns a 200 response', async () => { - const response = await server.inject(options) + describe('when the request fails', () => { + describe('because the cancelling service threw an error', () => { + beforeEach(async () => { + Sinon.stub(Boom, 'badImplementation').returns(new Boom.Boom('Bang', { statusCode: 500 })) + Sinon.stub(SubmitCancelBillRunService, 'go').rejects() + }) + + it('returns the error page', async () => { + const response = await server.inject(options) - expect(response.statusCode).to.equal(200) - expect(response.payload).to.contain('You're about to cancel this bill run') - expect(response.payload).to.contain('Two-part tariff') + expect(response.statusCode).to.equal(200) + expect(response.payload).to.contain('Sorry, there is a problem with the service') + }) + }) }) }) }) - describe('POST /bill-runs/{id}/cancel', () => { - let options - - beforeEach(() => { - options = { - method: 'POST', - url: '/bill-runs/97db1a27-8308-4aba-b463-8a6af2558b28/cancel', - auth: { - strategy: 'session', - credentials: { scope: ['billing'] } - } - } - }) - - describe('when a request is valid', () => { + describe('/bill-runs/{id}/review', () => { + describe('GET', () => { beforeEach(async () => { - Sinon.stub(SubmitCancelBillRunService, 'go').resolves() + options = _options('GET', 'review') }) - it('redirects to the bill runs page', async () => { - const response = await server.inject(options) - - expect(response.statusCode).to.equal(302) - expect(response.headers.location).to.equal('/billing/batch/list') - }) - }) - - describe('when the request fails', () => { - describe('because the cancelling service threw an error', () => { - beforeEach(async () => { - Sinon.stub(Boom, 'badImplementation').returns(new Boom.Boom('Bang', { statusCode: 500 })) - Sinon.stub(SubmitCancelBillRunService, 'go').rejects() + describe('when a request is valid', () => { + beforeEach(() => { + Sinon.stub(ReviewBillRunService, 'go').resolves(_reviewBillRunData()) }) - it('returns the error page', async () => { + it('returns a 200 response', async () => { const response = await server.inject(options) expect(response.statusCode).to.equal(200) - expect(response.payload).to.contain('Sorry, there is a problem with the service') + expect(response.payload).to.contain('two-part tariff') + expect(response.payload).to.contain('Southern (Test replica)') + expect(response.payload).to.contain('Showing all 2 licences') }) }) }) - }) - describe('GET /bill-runs/{id}/review', () => { - let options + describe('POST /bill-runs/{id}/review', () => { + beforeEach(async () => { + options = _options('POST', 'review') + }) - beforeEach(async () => { - options = { - method: 'GET', - url: '/bill-runs/97db1a27-8308-4aba-b463-8a6af2558b28/review', - auth: { - strategy: 'session', - credentials: { scope: ['billing'] } - } - } - }) + describe('when a request is valid', () => { + describe('and no filters have been applied', () => { + beforeEach(() => { + Sinon.stub(ReviewBillRunService, 'go').resolves(_reviewBillRunData()) + }) + + it('returns a 200 response', async () => { + const response = await server.inject(options) + + expect(response.statusCode).to.equal(200) + expect(response.payload).to.contain('two-part tariff') + expect(response.payload).to.contain('Southern (Test replica)') + expect(response.payload).to.contain( + 'You need to review 1 licences with returns data issues. You can then continue and send the bill run.' + ) + expect(response.payload).to.contain('Showing all 2 licences') + }) + }) - describe('when a request is valid', () => { - beforeEach(() => { - Sinon.stub(ReviewBillRunService, 'go').resolves(_reviewBillRunData()) - }) + describe('and a filter has been applied', () => { + beforeEach(() => { + const reviewBillRunData = _reviewBillRunData() - it('returns a 200 response', async () => { - const response = await server.inject(options) + // edit the data to represent the filter being applied + reviewBillRunData.filterData = { openFilter: true, licenceHolder: 'big' } + reviewBillRunData.numberOfLicencesDisplayed = 1 - expect(response.statusCode).to.equal(200) - expect(response.payload).to.contain('two-part tariff') - expect(response.payload).to.contain('Southern (Test replica)') - expect(response.payload).to.contain('Showing all 2 licences') + Sinon.stub(ReviewBillRunService, 'go').resolves(reviewBillRunData) + }) + + it('returns a 200 response', async () => { + const response = await server.inject(options) + + expect(response.statusCode).to.equal(200) + expect(response.payload).to.contain('two-part tariff') + expect(response.payload).to.contain('Southern (Test replica)') + expect(response.payload).to.contain( + 'You need to review 1 licences with returns data issues. You can then continue and send the bill run.' + ) + expect(response.payload).to.contain('Showing 1 of 2 licences') + }) + }) }) }) }) - describe('POST /bill-runs/{id}/review', () => { - let options - - beforeEach(async () => { - options = { - method: 'POST', - url: '/bill-runs/97db1a27-8308-4aba-b463-8a6af2558b28/review', - auth: { - strategy: 'session', - credentials: { scope: ['billing'] } - } - } - }) + describe('/bill-runs/{id}/review/{licenceId}', () => { + describe('GET', () => { + beforeEach(async () => { + options = _options('GET', 'review/cc4bbb18-0d6a-4254-ac2c-7409de814d7e') + }) - describe('when a request is valid', () => { - describe('and no filters have been applied', () => { + describe('when a request is valid', () => { beforeEach(() => { - Sinon.stub(ReviewBillRunService, 'go').resolves(_reviewBillRunData()) + Sinon.stub(ReviewLicenceService, 'go').resolves(_licenceReviewData()) }) it('returns a 200 response', async () => { const response = await server.inject(options) expect(response.statusCode).to.equal(200) + expect(response.payload).to.contain('1/11/10/*S/0084') expect(response.payload).to.contain('two-part tariff') - expect(response.payload).to.contain('Southern (Test replica)') - expect(response.payload).to.contain( - 'You need to review 1 licences with returns data issues. You can then continue and send the bill run.' - ) - expect(response.payload).to.contain('Showing all 2 licences') }) }) + }) + }) - describe('and a filter has been applied', () => { - beforeEach(() => { - const reviewBillRunData = _reviewBillRunData() - - // edit the data to represent the filter being applied - reviewBillRunData.filterData = { openFilter: true, licenceHolder: 'big' } - reviewBillRunData.numberOfLicencesDisplayed = 1 + describe('/bill-runs/{id}/send', () => { + describe('GET', () => { + beforeEach(async () => { + options = _options('GET', 'send') + }) - Sinon.stub(ReviewBillRunService, 'go').resolves(reviewBillRunData) + describe('when a request is valid', () => { + beforeEach(() => { + Sinon.stub(SendBillRunService, 'go').resolves({ + id: '8702b98f-ae51-475d-8fcc-e049af8b8d38', + billRunType: 'Two-part tariff', + pageTitle: "You're about to send this bill run" + }) }) it('returns a 200 response', async () => { const response = await server.inject(options) expect(response.statusCode).to.equal(200) - expect(response.payload).to.contain('two-part tariff') - expect(response.payload).to.contain('Southern (Test replica)') - expect(response.payload).to.contain( - 'You need to review 1 licences with returns data issues. You can then continue and send the bill run.' - ) - expect(response.payload).to.contain('Showing 1 of 2 licences') + expect(response.payload).to.contain('You're about to send this bill run') + expect(response.payload).to.contain('Two-part tariff') }) }) }) - }) - describe('GET /bill-runs/{id}/review/{licenceId}', () => { - let options + describe('POST', () => { + beforeEach(() => { + options = _options('POST', 'send') + }) - beforeEach(async () => { - options = { - method: 'GET', - url: '/bill-runs/97db1a27-8308-4aba-b463-8a6af2558b28/review/cc4bbb18-0d6a-4254-ac2c-7409de814d7e', - auth: { - strategy: 'session', - credentials: { scope: ['billing'] } - } - } - }) + describe('when a request is valid', () => { + beforeEach(async () => { + Sinon.stub(SubmitSendBillRunService, 'go').resolves() + }) - describe('when a request is valid', () => { - beforeEach(() => { - Sinon.stub(ReviewLicenceService, 'go').resolves(_licenceReviewData()) + it('redirects to the legacy processing bill run page', async () => { + const response = await server.inject(options) + + expect(response.statusCode).to.equal(302) + expect(response.headers.location).to.equal('/billing/batch/97db1a27-8308-4aba-b463-8a6af2558b28/processing') + }) }) - it('returns a 200 response', async () => { - const response = await server.inject(options) + describe('when the request fails', () => { + describe('because the sending service threw an error', () => { + beforeEach(async () => { + Sinon.stub(Boom, 'badImplementation').returns(new Boom.Boom('Bang', { statusCode: 500 })) + Sinon.stub(SubmitSendBillRunService, 'go').rejects() + }) - expect(response.statusCode).to.equal(200) - expect(response.payload).to.contain('1/11/10/*S/0084') - expect(response.payload).to.contain('two-part tariff') + it('returns the error page', async () => { + const response = await server.inject(options) + + expect(response.statusCode).to.equal(200) + expect(response.payload).to.contain('Sorry, there is a problem with the service') + }) + }) }) }) }) }) +function _options (method, path) { + const root = '/bill-runs/97db1a27-8308-4aba-b463-8a6af2558b28' + const url = path ? `${root}/${path}` : root + + return { + method, + url, + auth: { + strategy: 'session', + credentials: { scope: ['billing'] } + } + } +} + function _licenceReviewData () { return { licenceRef: '1/11/10/*S/0084', diff --git a/test/presenters/bill-runs/send-bill-run.presenter.test.js b/test/presenters/bill-runs/send-bill-run.presenter.test.js new file mode 100644 index 0000000000..16e23a4d24 --- /dev/null +++ b/test/presenters/bill-runs/send-bill-run.presenter.test.js @@ -0,0 +1,53 @@ +'use strict' + +// Test framework dependencies +const Lab = require('@hapi/lab') +const Code = require('@hapi/code') + +const { describe, it, beforeEach } = exports.lab = Lab.script() +const { expect } = Code + +// Thing under test +const SendBillRunPresenter = require('../../../app/presenters/bill-runs/send-bill-run.presenter.js') + +describe('Send Bill Run presenter', () => { + let billRun + + describe('when provided with a populated bill run', () => { + beforeEach(() => { + billRun = _testBillRun() + }) + + it('correctly presents the data', () => { + const result = SendBillRunPresenter.go(billRun) + + expect(result).to.equal({ + billRunId: '420e948f-1992-437e-8a47-74c0066cb017', + billRunNumber: 10010, + billRunStatus: 'ready', + billRunType: 'Supplementary', + chargeScheme: 'Current', + dateCreated: '1 November 2023', + financialYear: '2023 to 2024', + region: 'Wales' + }) + }) + }) +}) + +function _testBillRun () { + return { + id: '420e948f-1992-437e-8a47-74c0066cb017', + batchType: 'supplementary', + billRunNumber: 10010, + summer: false, + scheme: 'sroc', + status: 'ready', + toFinancialYearEnding: 2024, + createdAt: new Date('2023-11-01'), + region: { + id: 'f6c4699f-9a80-419a-82e7-f785ece727e1', + displayName: 'Wales' + } + } +} diff --git a/test/requests/charging-module/send-bill-run.request.test.js b/test/requests/charging-module/send-bill-run.request.test.js new file mode 100644 index 0000000000..8efda27d41 --- /dev/null +++ b/test/requests/charging-module/send-bill-run.request.test.js @@ -0,0 +1,129 @@ +'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 ExpandedError = require('../../../app/errors/expanded.error.js') + +// Things we need to stub +const ChargingModuleRequest = require('../../../app/requests/charging-module.request.js') +const WaitForStatusRequest = require('../../../app/requests/charging-module/wait-for-status.request.js') + +// Thing under test +const SendBillRunRequest = require('../../../app/requests/charging-module/send-bill-run.request.js') + +describe('Charging Module Send Bill Run request', () => { + const billRunId = '2bbbe459-966e-4026-b5d2-2f10867bdddd' + + let chargingModuleRequestStub + + beforeEach(() => { + chargingModuleRequestStub = Sinon.stub(ChargingModuleRequest, 'patch') + }) + + afterEach(() => { + Sinon.restore() + }) + + describe('when the request can send a bill run', () => { + beforeEach(async () => { + chargingModuleRequestStub.resolves({ + succeeded: true, + response: { + info: { + gitCommit: '273604040a47e0977b0579a0fef0f09726d95e39', + dockerTag: 'ghcr.io/defra/sroc-charging-module-api:v0.19.0' + }, + statusCode: 204, + body: null + } + }) + Sinon.stub(WaitForStatusRequest, 'send').resolves({ succeeded: true, status: 'billed', attempts: 1 }) + }) + + it('returns a `true` success status', async () => { + const result = await SendBillRunRequest.send(billRunId) + + expect(result.succeeded).to.be.true() + }) + + it('returns the last status received', async () => { + const result = await SendBillRunRequest.send(billRunId) + + expect(result.status).to.equal('billed') + }) + + it('returns the number of attempts', async () => { + const result = await SendBillRunRequest.send(billRunId) + + expect(result.attempts).to.equal(1) + }) + }) + + describe('when the request cannot send a bill run', () => { + describe('because the approve request fails', () => { + beforeEach(async () => { + chargingModuleRequestStub.onFirstCall().resolves({ + succeeded: false, + response: { body: 'Boom' } + }) + }) + + it('throws an error', async () => { + const error = await expect(SendBillRunRequest.send(billRunId)).to.reject() + + expect(error).to.be.instanceOf(ExpandedError) + expect(error.message).to.equal('Charging Module approve request failed') + expect(error.billRunExternalId).to.equal(billRunId) + expect(error.responseBody).to.equal('Boom') + }) + }) + + describe('because the send request fails', () => { + beforeEach(async () => { + chargingModuleRequestStub.onFirstCall().resolves({ succeeded: true }) + chargingModuleRequestStub.onSecondCall().resolves({ + succeeded: false, + response: { body: 'Boom' } + }) + }) + + it('throws an error', async () => { + const error = await expect(SendBillRunRequest.send(billRunId)).to.reject() + + expect(error).to.be.instanceOf(ExpandedError) + expect(error.message).to.equal('Charging Module send request failed') + expect(error.billRunExternalId).to.equal(billRunId) + expect(error.responseBody).to.equal('Boom') + }) + }) + + describe('because the wait request fails', () => { + beforeEach(async () => { + chargingModuleRequestStub.onFirstCall().resolves({ succeeded: true }) + chargingModuleRequestStub.onSecondCall().resolves({ succeeded: true }) + Sinon.stub(WaitForStatusRequest, 'send').resolves({ + succeeded: false, + attempts: 100, + response: { body: 'Boom' } + }) + }) + + it('throws an error', async () => { + const error = await expect(SendBillRunRequest.send(billRunId)).to.reject() + + expect(error).to.be.instanceOf(ExpandedError) + expect(error.message).to.equal('Charging Module wait request failed') + expect(error.billRunExternalId).to.equal(billRunId) + expect(error.attempts).to.equal(100) + expect(error.responseBody).to.equal('Boom') + }) + }) + }) +}) diff --git a/test/services/bill-runs/send-bill-run.service.test.js b/test/services/bill-runs/send-bill-run.service.test.js new file mode 100644 index 0000000000..ca2cfe44b0 --- /dev/null +++ b/test/services/bill-runs/send-bill-run.service.test.js @@ -0,0 +1,60 @@ +'use strict' + +// Test framework dependencies +const Lab = require('@hapi/lab') +const Code = require('@hapi/code') + +const { describe, it, beforeEach } = exports.lab = Lab.script() +const { expect } = Code + +// Test helpers +const BillRunHelper = require('../../support/helpers/bill-run.helper.js') +const DatabaseSupport = require('../../support/database.js') +const RegionHelper = require('../../support/helpers/region.helper.js') + +// Thing under test +const SendBillRunService = require('../../../app/services/bill-runs/send-bill-run.service.js') + +describe('Send Bill Run service', () => { + let testBillRunId + + beforeEach(async () => { + await DatabaseSupport.clean() + + const { id: regionId } = await RegionHelper.add() + const billRun = await BillRunHelper.add({ + billRunNumber: 10101, + createdAt: new Date('2024-02-28'), + externalId: 'f54e53f0-37a0-400f-9f0e-bf8575c17668', + regionId, + status: 'ready' + }) + + testBillRunId = billRun.id + }) + + describe('when a bill with a matching ID exists', () => { + it('will fetch the data and format it for use in the send bill run page', async () => { + const result = await SendBillRunService.go(testBillRunId) + + expect(result).to.equal({ + billRunId: testBillRunId, + billRunNumber: 10101, + billRunStatus: 'ready', + billRunType: 'Supplementary', + chargeScheme: 'Current', + dateCreated: '28 February 2024', + financialYear: '2022 to 2023', + region: 'Avalon' + }) + }) + }) + + describe('when a bill run with a matching ID does not exist', () => { + it('throws an exception', async () => { + await expect(SendBillRunService.go('testId')) + .to + .reject() + }) + }) +})