From a52cc7065b7ce6221f2cb64f2b718a87c5d8ec05 Mon Sep 17 00:00:00 2001 From: jonathangoulding Date: Tue, 7 May 2024 10:12:34 +0100 Subject: [PATCH 01/17] Add View Licence Bills page https://eaflood.atlassian.net/browse/WATER-4316 The existing service handling view licence is slow because it loads all the data for the tabs in one render. Work has been done previously to refactor the summary page to load only the summary information. This change will introduce a bills controller, service and presenter to handle the view licence bills page. This will share the same view as the summary page and load the same 'common data' established in [previous work](#957). --- app/controllers/licences.controller.js | 16 +++++- .../licences/view-licence-bills.presenter.js | 21 +++++++ .../licences/view-licence.presenter.js | 3 +- app/routes/licence.routes.js | 8 +++ .../licences/view-licence-bills.service.js | 31 +++++++++++ app/views/licences/tabs/bills.njk | 9 +++ app/views/licences/view.njk | 3 + test/controllers/licences.controller.test.js | 37 +++++++++++++ .../view-licence-bills-presenter.test.js | 33 +++++++++++ .../licences/view-licence-presenter.test.js | 1 + .../view-licence-bills.service.test.js | 55 +++++++++++++++++++ .../licences/view-licence.service.test.js | 1 + 12 files changed, 214 insertions(+), 4 deletions(-) create mode 100644 app/presenters/licences/view-licence-bills.presenter.js create mode 100644 app/services/licences/view-licence-bills.service.js create mode 100644 app/views/licences/tabs/bills.njk create mode 100644 test/presenters/licences/view-licence-bills-presenter.test.js create mode 100644 test/services/licences/view-licence-bills.service.test.js diff --git a/app/controllers/licences.controller.js b/app/controllers/licences.controller.js index 821e7b8f1d..3278f85c32 100644 --- a/app/controllers/licences.controller.js +++ b/app/controllers/licences.controller.js @@ -6,6 +6,7 @@ */ const InitiateReturnRequirementSessionService = require('../services/return-requirements/initiate-return-requirement-session.service.js') +const ViewLicenceBillsService = require('../services/licences/view-licence-bills.service') const ViewLicenceReturnsService = require('../services/licences/view-licence-returns.service') const ViewLicenceSummaryService = require('../services/licences/view-licence-summary.service') @@ -25,13 +26,22 @@ async function returnsRequired (request, h) { return h.redirect(`/system/return-requirements/${session.id}/start-date`) } +async function viewBills (request, h) { + const { params: { id }, auth, query: { page = 1 } } = request + + const data = await ViewLicenceBillsService.go(id, auth, page) + + return h.view('licences/view.njk', { + ...data + }) +} + async function viewSummary (request, h) { const { params: { id }, auth } = request const data = await ViewLicenceSummaryService.go(id, auth) return h.view('licences/view.njk', { - activeNavBar: 'search', ...data }) } @@ -42,7 +52,6 @@ async function viewReturns (request, h) { const data = await ViewLicenceReturnsService.go(id, auth, page) return h.view('licences/view.njk', { - activeNavBar: 'search', ...data }) } @@ -51,5 +60,6 @@ module.exports = { noReturnsRequired, returnsRequired, viewReturns, - viewSummary + viewSummary, + viewBills } diff --git a/app/presenters/licences/view-licence-bills.presenter.js b/app/presenters/licences/view-licence-bills.presenter.js new file mode 100644 index 0000000000..40bc171274 --- /dev/null +++ b/app/presenters/licences/view-licence-bills.presenter.js @@ -0,0 +1,21 @@ +'use strict' + +/** + * Formats data for the `/licences/{id}/bills` view licence bill page + * @module ViewLicenceBillsPresenter + */ + +/** + * Formats data for the `/licences/{id}/bills` view licence bill page + * + * @returns {Object} The data formatted for the view template + */ +function go (billsData) { + return { + activeTab: 'bills' + } +} + +module.exports = { + go +} diff --git a/app/presenters/licences/view-licence.presenter.js b/app/presenters/licences/view-licence.presenter.js index 0609378548..8c906a2266 100644 --- a/app/presenters/licences/view-licence.presenter.js +++ b/app/presenters/licences/view-licence.presenter.js @@ -33,7 +33,8 @@ function go (licence, auth) { pageTitle: `Licence ${licenceRef}`, registeredTo, roles: _authRoles(auth), - warning: _generateWarningMessage(ends) + warning: _generateWarningMessage(ends), + activeNavBar: 'search' } } diff --git a/app/routes/licence.routes.js b/app/routes/licence.routes.js index c71d066d8c..c2277ee1c4 100644 --- a/app/routes/licence.routes.js +++ b/app/routes/licence.routes.js @@ -3,6 +3,14 @@ const LicencesController = require('../controllers/licences.controller.js') const routes = [ + { + method: 'GET', + path: '/licences/{id}/bills', + handler: LicencesController.viewBills, + options: { + description: 'View a licence bills page' + } + }, { method: 'GET', path: '/licences/{id}/summary', diff --git a/app/services/licences/view-licence-bills.service.js b/app/services/licences/view-licence-bills.service.js new file mode 100644 index 0000000000..d524064ba2 --- /dev/null +++ b/app/services/licences/view-licence-bills.service.js @@ -0,0 +1,31 @@ +'use strict' + +/** + * Orchestrates fetching and presenting the data needed for the licence summary page + * @module ViewLicenceBillsService + */ + +const ViewLicenceService = require('./view-licence.service') +const ViewLicenceBillsPresenter = require('../../presenters/licences/view-licence-bills.presenter') + +/** + * Orchestrates fetching and presenting the data needed for the licence summary page + * + * @param {string} licenceId - The UUID of the licence + * + * @returns {Promise} an object representing the `pageData` needed by the licence summary template. + */ +async function go (licenceId, auth) { + const commonData = await ViewLicenceService.go(licenceId, auth) + + const pageData = ViewLicenceBillsPresenter.go() + + return { + ...commonData, + ...pageData + } +} + +module.exports = { + go +} diff --git a/app/views/licences/tabs/bills.njk b/app/views/licences/tabs/bills.njk new file mode 100644 index 0000000000..ee42a000b5 --- /dev/null +++ b/app/views/licences/tabs/bills.njk @@ -0,0 +1,9 @@ + +

Bills

+ {% if bills %} + + {% else %} +

No bills sent for this licence.

+ {% endif %} +
+ diff --git a/app/views/licences/view.njk b/app/views/licences/view.njk index bd3f8c3f5e..89646b2737 100644 --- a/app/views/licences/view.njk +++ b/app/views/licences/view.njk @@ -82,6 +82,9 @@ {% if activeTab === 'returns' %} {% include "licences/tabs/returns.njk" %} {% endif %} + {% if activeTab === 'bills' %} + {% include "licences/tabs/bills.njk" %} + {% endif %} {% endblock %} diff --git a/test/controllers/licences.controller.test.js b/test/controllers/licences.controller.test.js index 5025f42b60..9c3064d531 100644 --- a/test/controllers/licences.controller.test.js +++ b/test/controllers/licences.controller.test.js @@ -13,6 +13,7 @@ const Boom = require('@hapi/boom') // Things we need to stub const InitiateReturnRequirementSessionService = require('../../app/services/return-requirements/initiate-return-requirement-session.service.js') +const ViewLicenceBillsService = require('../../app/services/licences/view-licence-bills.service') const ViewLicenceSummaryService = require('../../app/services/licences/view-licence-summary.service') const ViewLicenceReturnsService = require('../../app/services/licences/view-licence-returns.service') @@ -151,6 +152,35 @@ describe('Licences controller', () => { }) }) + describe('GET /licences/{id}/bills', () => { + beforeEach(async () => { + options = { + method: 'GET', + url: '/licences/7861814c-ca19-43f2-be11-3c612f0d744b/bills', + auth: { + strategy: 'session', + credentials: { scope: [] } + } + } + }) + + describe('when a request is valid and has bills', () => { + beforeEach(async () => { + Sinon.stub(ViewLicenceBillsService, 'go').resolves(_viewLicenceBills()) + }) + + it('returns the page successfully', async () => { + const response = await server.inject(options) + + expect(response.statusCode).to.equal(200) + expect(response.payload).to.contain('Bills') + // Check the table titles + expect(response.payload).to.contain('Bills') + expect(response.payload).to.contain('No bills sent for this licence.') + }) + }) + }) + describe('GET /licences/{id}/summary', () => { beforeEach(async () => { options = { @@ -222,6 +252,13 @@ describe('Licences controller', () => { }) }) +function _viewLicenceBills () { + return { + activeTab: 'bills' + // bills: [] + } +} + function _viewLicenceReturns () { return { activeTab: 'returns', diff --git a/test/presenters/licences/view-licence-bills-presenter.test.js b/test/presenters/licences/view-licence-bills-presenter.test.js new file mode 100644 index 0000000000..b77ab38263 --- /dev/null +++ b/test/presenters/licences/view-licence-bills-presenter.test.js @@ -0,0 +1,33 @@ +'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 ViewLicenceBillsPresenter = require('../../../app/presenters/licences/view-licence-bills.presenter') + +describe('View Licence Bills presenter', () => { + let auth + + beforeEach(() => { + auth = undefined + }) + + describe('when provided with a bills data', () => { + it('correctly presents the data', () => { + const result = ViewLicenceBillsPresenter.go(_bills()) + + expect(result).to.equal({ + activeTab: 'bills' + }) + }) + }) +}) + +function _bills () { + return {} +} diff --git a/test/presenters/licences/view-licence-presenter.test.js b/test/presenters/licences/view-licence-presenter.test.js index e90de00846..07e7511356 100644 --- a/test/presenters/licences/view-licence-presenter.test.js +++ b/test/presenters/licences/view-licence-presenter.test.js @@ -24,6 +24,7 @@ describe('View Licence presenter', () => { const result = ViewLicencePresenter.go(licence) expect(result).to.equal({ + activeNavBar: 'search', licenceId: 'f1288f6c-8503-4dc1-b114-75c408a14bd0', licenceName: 'Unregistered licence', licenceRef: '01/123', diff --git a/test/services/licences/view-licence-bills.service.test.js b/test/services/licences/view-licence-bills.service.test.js new file mode 100644 index 0000000000..93d8c12002 --- /dev/null +++ b/test/services/licences/view-licence-bills.service.test.js @@ -0,0 +1,55 @@ +'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 + +// Things we need to stub +const ViewLicenceService = require('../../../app/services/licences/view-licence.service') +const ViewLicenceBillsPresenter = require('../../../app/presenters/licences/view-licence-bills.presenter') + +// Thing under test +const ViewLicenceBillsService = require('../../../app/services/licences/view-licence-bills.service') + +describe('View Licence service returns', () => { + const testId = '2c80bd22-a005-4cf4-a2a2-73812a9861de' + const auth = {} + + beforeEach(() => { + Sinon.stub(ViewLicenceBillsPresenter, 'go').returns(_billsPresenter()) + Sinon.stub(ViewLicenceService, 'go').resolves(_licence()) + }) + + afterEach(() => { + Sinon.restore() + }) + + describe.only('when a bill', () => { + describe('and it has no optional fields', () => { + it('will return all the mandatory data and default values for use in the licence returns page', async () => { + const result = await ViewLicenceBillsService.go(testId, auth) + + expect(result).to.equal({ + licenceName: 'fake licence', + bills: [], + activeTab: 'bills' + }) + }) + }) + }) +}) + +function _licence () { + return { licenceName: 'fake licence' } +} + +function _billsPresenter () { + return { + bills: [], + activeTab: 'bills' + } +} diff --git a/test/services/licences/view-licence.service.test.js b/test/services/licences/view-licence.service.test.js index f296f033d2..2ccb4396af 100644 --- a/test/services/licences/view-licence.service.test.js +++ b/test/services/licences/view-licence.service.test.js @@ -46,6 +46,7 @@ describe('View Licence service', () => { const result = await ViewLicenceService.go(testId) expect(result).to.equal({ + activeNavBar: 'search', licenceId: '2c80bd22-a005-4cf4-a2a2-73812a9861de', licenceName: 'Unregistered licence', licenceRef: '01/130/R01', From d14a4bca2b494c8c401845dfd99572965e52b7f8 Mon Sep 17 00:00:00 2001 From: jonathangoulding Date: Tue, 7 May 2024 10:16:59 +0100 Subject: [PATCH 02/17] fix: remove only tag --- test/services/licences/view-licence-bills.service.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/services/licences/view-licence-bills.service.test.js b/test/services/licences/view-licence-bills.service.test.js index 93d8c12002..1e5bd8646f 100644 --- a/test/services/licences/view-licence-bills.service.test.js +++ b/test/services/licences/view-licence-bills.service.test.js @@ -28,7 +28,7 @@ describe('View Licence service returns', () => { Sinon.restore() }) - describe.only('when a bill', () => { + describe('when a bill', () => { describe('and it has no optional fields', () => { it('will return all the mandatory data and default values for use in the licence returns page', async () => { const result = await ViewLicenceBillsService.go(testId, auth) From 2ed0e52b0daf43a3464e6f93cee1e9c21d7fe1d6 Mon Sep 17 00:00:00 2001 From: jonathangoulding Date: Tue, 7 May 2024 10:20:37 +0100 Subject: [PATCH 03/17] fix: linting --- .../licences/view-licence-bills-presenter.test.js | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/test/presenters/licences/view-licence-bills-presenter.test.js b/test/presenters/licences/view-licence-bills-presenter.test.js index b77ab38263..dcd72cc40d 100644 --- a/test/presenters/licences/view-licence-bills-presenter.test.js +++ b/test/presenters/licences/view-licence-bills-presenter.test.js @@ -4,19 +4,13 @@ const Lab = require('@hapi/lab') const Code = require('@hapi/code') -const { describe, it, beforeEach } = exports.lab = Lab.script() +const { describe, it } = exports.lab = Lab.script() const { expect } = Code // Thing under test const ViewLicenceBillsPresenter = require('../../../app/presenters/licences/view-licence-bills.presenter') describe('View Licence Bills presenter', () => { - let auth - - beforeEach(() => { - auth = undefined - }) - describe('when provided with a bills data', () => { it('correctly presents the data', () => { const result = ViewLicenceBillsPresenter.go(_bills()) From 19b691bfe4653008b712082d280f4a95f9bd92eb Mon Sep 17 00:00:00 2001 From: jonathangoulding Date: Tue, 7 May 2024 12:22:59 +0100 Subject: [PATCH 04/17] feat: add fetch bills data update bills mapper --- .../licences/view-licence-bills.presenter.js | 28 +++- .../licences/fetch-licence-bills.service.js | 54 ++++++++ .../licences/view-licence-bills.service.js | 7 +- app/views/licences/tabs/bills.njk | 41 +++++- test/controllers/licences.controller.test.js | 126 ++++++++++-------- .../view-licence-bills-presenter.test.js | 27 +++- .../view-licence-bills.service.test.js | 20 ++- 7 files changed, 238 insertions(+), 65 deletions(-) create mode 100644 app/services/licences/fetch-licence-bills.service.js diff --git a/app/presenters/licences/view-licence-bills.presenter.js b/app/presenters/licences/view-licence-bills.presenter.js index 40bc171274..f687d0515f 100644 --- a/app/presenters/licences/view-licence-bills.presenter.js +++ b/app/presenters/licences/view-licence-bills.presenter.js @@ -5,17 +5,41 @@ * @module ViewLicenceBillsPresenter */ +const { formatLongDate } = require('../base.presenter') + /** * Formats data for the `/licences/{id}/bills` view licence bill page * * @returns {Object} The data formatted for the view template */ -function go (billsData) { +function go (bills) { return { - activeTab: 'bills' + activeTab: 'bills', + bills: _formatBillsToTableRow(bills) } } +function _formatCurrencyToGBP (amount) { + return amount.toLocaleString('en-gb', { + style: 'currency', + currency: 'GBP' + }) +} +function _formatBillsToTableRow (bills) { + return bills.map((bill) => { + return { + billNumber: bill.invoiceNumber, + dateCreated: formatLongDate(new Date(bill.createdAt)), + account: bill.accountNumber, + runType: bill.batchType, + financialYear: bill.financialYearEnding, + total: _formatCurrencyToGBP(bill.netAmount), + accountId: bill.billingAccountId, + id: bill.id + } + }) +} + module.exports = { go } diff --git a/app/services/licences/fetch-licence-bills.service.js b/app/services/licences/fetch-licence-bills.service.js new file mode 100644 index 0000000000..c0814171d3 --- /dev/null +++ b/app/services/licences/fetch-licence-bills.service.js @@ -0,0 +1,54 @@ +'use strict' + +/** + * Fetches all return logs for a licence which is needed for the view '/licences/{id}/returns` page + * @module FetchLicenceBillsService + */ + +const LicenceModel = require('../../models/licence.model') +const BillLicenceModel = require('../../models/bill-licence.model') +const BillModel = require('../../models/bill.model') + +const DatabaseConfig = require('../../../config/database.config') + +/** + * Fetches all bills for a licence which is needed for the view '/licences/{id}/bills` page + * + * @param {string} licenceId - The UUID for the licence to fetch + * + * @returns {Promise} the data needed to populate the view licence page's returns tab + */ +async function go (licenceId, page) { + const { results, total } = await _fetch(licenceId, page) + + return { bills: JSON.parse(JSON.stringify(results)), pagination: { total } } +} + +async function _fetch (licenceId, page) { + const licence = await LicenceModel.query() + .findById(licenceId) + .select([ + 'id', + 'licenceRef' + ]) + + const billLicences = await BillLicenceModel.query() + .select('*') + .where('billLicences.licence_ref', licence.licenceRef) + .page(page - 1, DatabaseConfig.defaultPageSize) + + const billIds = JSON.parse(JSON.stringify(billLicences)).results.map(b => b.billId) + + return BillModel.query() + .findByIds(billIds) + .select('*') + // .where('bills.billing_account_id', billLicence.billId) + .orderBy([ + { column: 'created_at', order: 'desc' } + ]) + .page(page - 1, DatabaseConfig.defaultPageSize) +} + +module.exports = { + go +} diff --git a/app/services/licences/view-licence-bills.service.js b/app/services/licences/view-licence-bills.service.js index d524064ba2..4bd67b64b5 100644 --- a/app/services/licences/view-licence-bills.service.js +++ b/app/services/licences/view-licence-bills.service.js @@ -5,6 +5,7 @@ * @module ViewLicenceBillsService */ +const FetchLicenceBillsService = require('./fetch-licence-bills.service') const ViewLicenceService = require('./view-licence.service') const ViewLicenceBillsPresenter = require('../../presenters/licences/view-licence-bills.presenter') @@ -18,11 +19,13 @@ const ViewLicenceBillsPresenter = require('../../presenters/licences/view-licenc async function go (licenceId, auth) { const commonData = await ViewLicenceService.go(licenceId, auth) - const pageData = ViewLicenceBillsPresenter.go() + const billsData = await FetchLicenceBillsService.go(licenceId, 1) + const pageData = ViewLicenceBillsPresenter.go(billsData.bills) return { ...commonData, - ...pageData + ...pageData, + pagination: { total: 1 }, } } diff --git a/app/views/licences/tabs/bills.njk b/app/views/licences/tabs/bills.njk index ee42a000b5..db52e5bcdd 100644 --- a/app/views/licences/tabs/bills.njk +++ b/app/views/licences/tabs/bills.njk @@ -1,7 +1,46 @@

Bills

{% if bills %} - +
+ + + + + + + + + + + + {% for bill in bills %} + + + + + + + + + {% endfor %} + +
Bill numberDate createdBilling accountBill run typeFinancial yearBill total
+ + {{ bill.billNumber }} + + + {{ bill.dateCreated }} + + + {{ bill.account }} + + + {{ bill.runType }} + + {{ bill.financialYear }} + + {{ bill.total }} +
{% else %}

No bills sent for this licence.

{% endif %} diff --git a/test/controllers/licences.controller.test.js b/test/controllers/licences.controller.test.js index 9c3064d531..e71d01cb3a 100644 --- a/test/controllers/licences.controller.test.js +++ b/test/controllers/licences.controller.test.js @@ -175,87 +175,94 @@ describe('Licences controller', () => { expect(response.statusCode).to.equal(200) expect(response.payload).to.contain('Bills') // Check the table titles - expect(response.payload).to.contain('Bills') - expect(response.payload).to.contain('No bills sent for this licence.') + expect(response.payload).to.contain('Bill number') + expect(response.payload).to.contain('Date created') + expect(response.payload).to.contain('Billing account') + expect(response.payload).to.contain('Bill run type') + expect(response.payload).to.contain('Bill total') }) }) - }) - - describe('GET /licences/{id}/summary', () => { - beforeEach(async () => { - options = { - method: 'GET', - url: '/licences/7861814c-ca19-43f2-be11-3c612f0d744b/summary', - auth: { - strategy: 'session', - credentials: { scope: [] } - } - } - }) - - describe('when a request is valid', () => { + describe('when a request is valid and has no bills', () => { beforeEach(async () => { - Sinon.stub(ViewLicenceSummaryService, 'go').resolves(_viewLicenceSummary()) + Sinon.stub(ViewLicenceBillsService, 'go').resolves({ activeTab: 'bills'}) }) it('returns the page successfully', async () => { const response = await server.inject(options) expect(response.statusCode).to.equal(200) - expect(response.payload).to.contain('Summary') - expect(response.payload).to.contain('Effective from') - expect(response.payload).to.contain('End date') + expect(response.payload).to.contain('Bills') + // Check the table titles + expect(response.payload).to.contain('Bills') + expect(response.payload).to.contain('No bills sent for this licence.') }) }) + }) +}) - function _viewLicenceSummary () { - return { - id: '7861814c-ca19-43f2-be11-3c612f0d744b', - licenceRef: '01/130/R01', - region: 'Southern', - startDate: '1 November 2022', - endDate: '1 November 2032', - activeTab: 'summary' +describe('GET /licences/{id}/summary', () => { + beforeEach(async () => { + options = { + method: 'GET', + url: '/licences/7861814c-ca19-43f2-be11-3c612f0d744b/summary', + auth: { + strategy: 'session', + credentials: { scope: [] } } } }) - describe('GET /licences/{id}/returns', () => { + describe('when a request is valid', () => { beforeEach(async () => { - options = { - method: 'GET', - url: '/licences/7861814c-ca19-43f2-be11-3c612f0d744b/returns', - auth: { - strategy: 'session', - credentials: { scope: ['billing'] } - } - } + Sinon.stub(ViewLicenceSummaryService, 'go').resolves(_viewLicenceSummary()) }) - describe('when a request is valid and has returns', () => { - beforeEach(async () => { - Sinon.stub(ViewLicenceReturnsService, 'go').resolves(_viewLicenceReturns()) - }) + it('returns the page successfully', async () => { + const response = await server.inject(options) - it('returns the page successfully', async () => { - const response = await server.inject(options) + expect(response.statusCode).to.equal(200) + expect(response.payload).to.contain('Summary') + expect(response.payload).to.contain('Effective from') + expect(response.payload).to.contain('End date') + }) + }) +}) - expect(response.statusCode).to.equal(200) - expect(response.payload).to.contain('Returns') - // Check the table titles - expect(response.payload).to.contain('Return reference and dates') - expect(response.payload).to.contain('Purpose and description') - expect(response.payload).to.contain('Due date') - expect(response.payload).to.contain('Status') - }) +describe('GET /licences/{id}/returns', () => { + beforeEach(async () => { + options = { + method: 'GET', + url: '/licences/7861814c-ca19-43f2-be11-3c612f0d744b/returns', + auth: { + strategy: 'session', + credentials: { scope: ['billing'] } + } + } + }) + + describe('when a request is valid and has returns', () => { + beforeEach(async () => { + Sinon.stub(ViewLicenceReturnsService, 'go').resolves(_viewLicenceReturns()) + }) + + it('returns the page successfully', async () => { + const response = await server.inject(options) + + expect(response.statusCode).to.equal(200) + expect(response.payload).to.contain('Returns') + // Check the table titles + expect(response.payload).to.contain('Return reference and dates') + expect(response.payload).to.contain('Purpose and description') + expect(response.payload).to.contain('Due date') + expect(response.payload).to.contain('Status') }) }) }) function _viewLicenceBills () { return { - activeTab: 'bills' - // bills: [] + activeTab: 'bills', + bills: [] } } @@ -265,3 +272,14 @@ function _viewLicenceReturns () { returns: [{}] } } + +function _viewLicenceSummary () { + return { + id: '7861814c-ca19-43f2-be11-3c612f0d744b', + licenceRef: '01/130/R01', + region: 'Southern', + startDate: '1 November 2022', + endDate: '1 November 2032', + activeTab: 'summary' + } +} diff --git a/test/presenters/licences/view-licence-bills-presenter.test.js b/test/presenters/licences/view-licence-bills-presenter.test.js index dcd72cc40d..73e0f109d1 100644 --- a/test/presenters/licences/view-licence-bills-presenter.test.js +++ b/test/presenters/licences/view-licence-bills-presenter.test.js @@ -16,12 +16,35 @@ describe('View Licence Bills presenter', () => { const result = ViewLicenceBillsPresenter.go(_bills()) expect(result).to.equal({ - activeTab: 'bills' + activeTab: 'bills', + bills: [{ + account: 'acc123', + accountId: 'bicc1233', + billNumber: 'inv123', + dateCreated: '1 January 2020', + financialYear: '2021', + id: 'id123', + runType: 'Annual', + total: '£123,456,789.00' + }] }) }) + it('correctly formats the currency to UK standard', () => { + const result = ViewLicenceBillsPresenter.go(_bills()) + expect(result.bills[0].total).to.equal('£123,456,789.00') + }) }) }) function _bills () { - return {} + return [{ + invoiceNumber: 'inv123', + createdAt: new Date('2020-01-01'), + accountNumber: 'acc123', + batchType: 'Annual', + financialYearEnding: '2021', + netAmount: 123456789, + billingAccountId: 'bicc1233', + id: 'id123' + }] } diff --git a/test/services/licences/view-licence-bills.service.test.js b/test/services/licences/view-licence-bills.service.test.js index 1e5bd8646f..f453825e9e 100644 --- a/test/services/licences/view-licence-bills.service.test.js +++ b/test/services/licences/view-licence-bills.service.test.js @@ -9,17 +9,19 @@ const { describe, it, beforeEach, afterEach } = exports.lab = Lab.script() const { expect } = Code // Things we need to stub +const FetchLicenceBillsService = require('../../../app/services/licences/fetch-licence-bills.service') const ViewLicenceService = require('../../../app/services/licences/view-licence.service') const ViewLicenceBillsPresenter = require('../../../app/presenters/licences/view-licence-bills.presenter') // Thing under test const ViewLicenceBillsService = require('../../../app/services/licences/view-licence-bills.service') -describe('View Licence service returns', () => { +describe('View Licence service bills', () => { const testId = '2c80bd22-a005-4cf4-a2a2-73812a9861de' const auth = {} beforeEach(() => { + Sinon.stub(FetchLicenceBillsService, 'go').returns(_billsFetchService()) Sinon.stub(ViewLicenceBillsPresenter, 'go').returns(_billsPresenter()) Sinon.stub(ViewLicenceService, 'go').resolves(_licence()) }) @@ -30,13 +32,16 @@ describe('View Licence service returns', () => { describe('when a bill', () => { describe('and it has no optional fields', () => { - it('will return all the mandatory data and default values for use in the licence returns page', async () => { + it('will return all the mandatory data and default values for use in the licence bills page', async () => { const result = await ViewLicenceBillsService.go(testId, auth) expect(result).to.equal({ - licenceName: 'fake licence', + activeTab: 'bills', bills: [], - activeTab: 'bills' + licenceName: 'fake licence', + pagination: { + total: 1 + } }) }) }) @@ -53,3 +58,10 @@ function _billsPresenter () { activeTab: 'bills' } } + +function _billsFetchService () { + return { + bills: [], + pagination: { total: 1 }, + } +} From 2ac48d098b9f10738251bdb3fde80be9d6b3ab2a Mon Sep 17 00:00:00 2001 From: jonathangoulding Date: Tue, 7 May 2024 12:33:10 +0100 Subject: [PATCH 05/17] feat: batch type from bill run to Bill Run type --- .../licences/view-licence-bills.presenter.js | 2 +- .../licences/fetch-licence-bills.service.js | 5 +- .../licences/view-licence-bills.service.js | 2 +- app/views/licences/tabs/bills.njk | 2 +- test/controllers/licences.controller.test.js | 92 +++++++++---------- .../view-licence-bills.service.test.js | 2 +- 6 files changed, 54 insertions(+), 51 deletions(-) diff --git a/app/presenters/licences/view-licence-bills.presenter.js b/app/presenters/licences/view-licence-bills.presenter.js index f687d0515f..38436b9785 100644 --- a/app/presenters/licences/view-licence-bills.presenter.js +++ b/app/presenters/licences/view-licence-bills.presenter.js @@ -31,7 +31,7 @@ function _formatBillsToTableRow (bills) { billNumber: bill.invoiceNumber, dateCreated: formatLongDate(new Date(bill.createdAt)), account: bill.accountNumber, - runType: bill.batchType, + runType: bill.billRun.batchType, financialYear: bill.financialYearEnding, total: _formatCurrencyToGBP(bill.netAmount), accountId: bill.billingAccountId, diff --git a/app/services/licences/fetch-licence-bills.service.js b/app/services/licences/fetch-licence-bills.service.js index c0814171d3..b9423ee2a3 100644 --- a/app/services/licences/fetch-licence-bills.service.js +++ b/app/services/licences/fetch-licence-bills.service.js @@ -42,7 +42,10 @@ async function _fetch (licenceId, page) { return BillModel.query() .findByIds(billIds) .select('*') - // .where('bills.billing_account_id', billLicence.billId) + .withGraphFetched('billRun') + .modifyGraph('billRun', (builder) => { + builder.select(['batchType']) + }) .orderBy([ { column: 'created_at', order: 'desc' } ]) diff --git a/app/services/licences/view-licence-bills.service.js b/app/services/licences/view-licence-bills.service.js index 4bd67b64b5..fae170fc36 100644 --- a/app/services/licences/view-licence-bills.service.js +++ b/app/services/licences/view-licence-bills.service.js @@ -25,7 +25,7 @@ async function go (licenceId, auth) { return { ...commonData, ...pageData, - pagination: { total: 1 }, + pagination: { total: 1 } } } diff --git a/app/views/licences/tabs/bills.njk b/app/views/licences/tabs/bills.njk index db52e5bcdd..e776fba7ce 100644 --- a/app/views/licences/tabs/bills.njk +++ b/app/views/licences/tabs/bills.njk @@ -29,7 +29,7 @@ - {{ bill.runType }} + {{ bill.runType | title }} {{ bill.financialYear }} diff --git a/test/controllers/licences.controller.test.js b/test/controllers/licences.controller.test.js index e71d01cb3a..fe65bd6915 100644 --- a/test/controllers/licences.controller.test.js +++ b/test/controllers/licences.controller.test.js @@ -184,7 +184,7 @@ describe('Licences controller', () => { }) describe('when a request is valid and has no bills', () => { beforeEach(async () => { - Sinon.stub(ViewLicenceBillsService, 'go').resolves({ activeTab: 'bills'}) + Sinon.stub(ViewLicenceBillsService, 'go').resolves({ activeTab: 'bills' }) }) it('returns the page successfully', async () => { @@ -198,63 +198,63 @@ describe('Licences controller', () => { }) }) }) -}) - -describe('GET /licences/{id}/summary', () => { - beforeEach(async () => { - options = { - method: 'GET', - url: '/licences/7861814c-ca19-43f2-be11-3c612f0d744b/summary', - auth: { - strategy: 'session', - credentials: { scope: [] } - } - } - }) - describe('when a request is valid', () => { + describe('GET /licences/{id}/summary', () => { beforeEach(async () => { - Sinon.stub(ViewLicenceSummaryService, 'go').resolves(_viewLicenceSummary()) + options = { + method: 'GET', + url: '/licences/7861814c-ca19-43f2-be11-3c612f0d744b/summary', + auth: { + strategy: 'session', + credentials: { scope: [] } + } + } }) - it('returns the page successfully', async () => { - const response = await server.inject(options) + describe('when a request is valid', () => { + beforeEach(async () => { + Sinon.stub(ViewLicenceSummaryService, 'go').resolves(_viewLicenceSummary()) + }) - expect(response.statusCode).to.equal(200) - expect(response.payload).to.contain('Summary') - expect(response.payload).to.contain('Effective from') - expect(response.payload).to.contain('End date') - }) - }) -}) + it('returns the page successfully', async () => { + const response = await server.inject(options) -describe('GET /licences/{id}/returns', () => { - beforeEach(async () => { - options = { - method: 'GET', - url: '/licences/7861814c-ca19-43f2-be11-3c612f0d744b/returns', - auth: { - strategy: 'session', - credentials: { scope: ['billing'] } - } - } + expect(response.statusCode).to.equal(200) + expect(response.payload).to.contain('Summary') + expect(response.payload).to.contain('Effective from') + expect(response.payload).to.contain('End date') + }) + }) }) - describe('when a request is valid and has returns', () => { + describe('GET /licences/{id}/returns', () => { beforeEach(async () => { - Sinon.stub(ViewLicenceReturnsService, 'go').resolves(_viewLicenceReturns()) + options = { + method: 'GET', + url: '/licences/7861814c-ca19-43f2-be11-3c612f0d744b/returns', + auth: { + strategy: 'session', + credentials: { scope: ['billing'] } + } + } }) - it('returns the page successfully', async () => { - const response = await server.inject(options) + describe('when a request is valid and has returns', () => { + beforeEach(async () => { + Sinon.stub(ViewLicenceReturnsService, 'go').resolves(_viewLicenceReturns()) + }) + + it('returns the page successfully', async () => { + const response = await server.inject(options) - expect(response.statusCode).to.equal(200) - expect(response.payload).to.contain('Returns') - // Check the table titles - expect(response.payload).to.contain('Return reference and dates') - expect(response.payload).to.contain('Purpose and description') - expect(response.payload).to.contain('Due date') - expect(response.payload).to.contain('Status') + expect(response.statusCode).to.equal(200) + expect(response.payload).to.contain('Returns') + // Check the table titles + expect(response.payload).to.contain('Return reference and dates') + expect(response.payload).to.contain('Purpose and description') + expect(response.payload).to.contain('Due date') + expect(response.payload).to.contain('Status') + }) }) }) }) diff --git a/test/services/licences/view-licence-bills.service.test.js b/test/services/licences/view-licence-bills.service.test.js index f453825e9e..edde25db1d 100644 --- a/test/services/licences/view-licence-bills.service.test.js +++ b/test/services/licences/view-licence-bills.service.test.js @@ -62,6 +62,6 @@ function _billsPresenter () { function _billsFetchService () { return { bills: [], - pagination: { total: 1 }, + pagination: { total: 1 } } } From fea35d83a6f259073ce2a4a46974afcbfe4b6bcb Mon Sep 17 00:00:00 2001 From: jonathangoulding Date: Tue, 7 May 2024 13:08:35 +0100 Subject: [PATCH 06/17] feat: add pagination --- app/services/licences/fetch-licence-bills.service.js | 10 +--------- app/services/licences/view-licence-bills.service.js | 9 ++++++--- app/views/licences/tabs/bills.njk | 7 +++++++ .../licences/view-licence-bills.service.test.js | 10 ++++++---- 4 files changed, 20 insertions(+), 16 deletions(-) diff --git a/app/services/licences/fetch-licence-bills.service.js b/app/services/licences/fetch-licence-bills.service.js index b9423ee2a3..50b5c379f7 100644 --- a/app/services/licences/fetch-licence-bills.service.js +++ b/app/services/licences/fetch-licence-bills.service.js @@ -5,7 +5,6 @@ * @module FetchLicenceBillsService */ -const LicenceModel = require('../../models/licence.model') const BillLicenceModel = require('../../models/bill-licence.model') const BillModel = require('../../models/bill.model') @@ -25,16 +24,9 @@ async function go (licenceId, page) { } async function _fetch (licenceId, page) { - const licence = await LicenceModel.query() - .findById(licenceId) - .select([ - 'id', - 'licenceRef' - ]) - const billLicences = await BillLicenceModel.query() .select('*') - .where('billLicences.licence_ref', licence.licenceRef) + .where('billLicences.licence_id', licenceId) .page(page - 1, DatabaseConfig.defaultPageSize) const billIds = JSON.parse(JSON.stringify(billLicences)).results.map(b => b.billId) diff --git a/app/services/licences/view-licence-bills.service.js b/app/services/licences/view-licence-bills.service.js index fae170fc36..9954379a6b 100644 --- a/app/services/licences/view-licence-bills.service.js +++ b/app/services/licences/view-licence-bills.service.js @@ -8,6 +8,7 @@ const FetchLicenceBillsService = require('./fetch-licence-bills.service') const ViewLicenceService = require('./view-licence.service') const ViewLicenceBillsPresenter = require('../../presenters/licences/view-licence-bills.presenter') +const PaginatorPresenter = require('../../presenters/paginator.presenter') /** * Orchestrates fetching and presenting the data needed for the licence summary page @@ -16,16 +17,18 @@ const ViewLicenceBillsPresenter = require('../../presenters/licences/view-licenc * * @returns {Promise} an object representing the `pageData` needed by the licence summary template. */ -async function go (licenceId, auth) { +async function go (licenceId, auth, page) { const commonData = await ViewLicenceService.go(licenceId, auth) - const billsData = await FetchLicenceBillsService.go(licenceId, 1) + const billsData = await FetchLicenceBillsService.go(licenceId, page) const pageData = ViewLicenceBillsPresenter.go(billsData.bills) + const pagination = PaginatorPresenter.go(100, Number(page), `/system/licences/${licenceId}/bills`) + return { ...commonData, ...pageData, - pagination: { total: 1 } + pagination } } diff --git a/app/views/licences/tabs/bills.njk b/app/views/licences/tabs/bills.njk index e776fba7ce..e9cb5ac136 100644 --- a/app/views/licences/tabs/bills.njk +++ b/app/views/licences/tabs/bills.njk @@ -1,3 +1,5 @@ +{% from "govuk/components/pagination/macro.njk" import govukPagination %} +

Bills

{% if bills %} @@ -46,3 +48,8 @@ {% endif %}
+{% if bills and pagination.numberOfPages > 1 %} + {{ govukPagination(pagination.component) }} +{% endif %} + + diff --git a/test/services/licences/view-licence-bills.service.test.js b/test/services/licences/view-licence-bills.service.test.js index edde25db1d..c0ecb9f93b 100644 --- a/test/services/licences/view-licence-bills.service.test.js +++ b/test/services/licences/view-licence-bills.service.test.js @@ -10,6 +10,7 @@ const { expect } = Code // Things we need to stub const FetchLicenceBillsService = require('../../../app/services/licences/fetch-licence-bills.service') +const PaginatorPresenter = require('../../../app/presenters/paginator.presenter') const ViewLicenceService = require('../../../app/services/licences/view-licence.service') const ViewLicenceBillsPresenter = require('../../../app/presenters/licences/view-licence-bills.presenter') @@ -17,11 +18,14 @@ const ViewLicenceBillsPresenter = require('../../../app/presenters/licences/view const ViewLicenceBillsService = require('../../../app/services/licences/view-licence-bills.service') describe('View Licence service bills', () => { - const testId = '2c80bd22-a005-4cf4-a2a2-73812a9861de' const auth = {} + const page = 1 + const pagination = { page } + const testId = '2c80bd22-a005-4cf4-a2a2-73812a9861de' beforeEach(() => { Sinon.stub(FetchLicenceBillsService, 'go').returns(_billsFetchService()) + Sinon.stub(PaginatorPresenter, 'go').returns(pagination) Sinon.stub(ViewLicenceBillsPresenter, 'go').returns(_billsPresenter()) Sinon.stub(ViewLicenceService, 'go').resolves(_licence()) }) @@ -39,9 +43,7 @@ describe('View Licence service bills', () => { activeTab: 'bills', bills: [], licenceName: 'fake licence', - pagination: { - total: 1 - } + pagination: { page: 1 } }) }) }) From 03710a34c2ed5f15aebfbbb5d8e7952c682cf703 Mon Sep 17 00:00:00 2001 From: jonathangoulding Date: Tue, 7 May 2024 13:20:44 +0100 Subject: [PATCH 07/17] feat: update fetch service for licence bills --- .../licences/fetch-licence-bills.service.js | 24 ++++++++++--------- .../licences/view-licence-bills.service.js | 2 +- 2 files changed, 14 insertions(+), 12 deletions(-) diff --git a/app/services/licences/fetch-licence-bills.service.js b/app/services/licences/fetch-licence-bills.service.js index 50b5c379f7..a43455bb85 100644 --- a/app/services/licences/fetch-licence-bills.service.js +++ b/app/services/licences/fetch-licence-bills.service.js @@ -20,26 +20,28 @@ const DatabaseConfig = require('../../../config/database.config') async function go (licenceId, page) { const { results, total } = await _fetch(licenceId, page) - return { bills: JSON.parse(JSON.stringify(results)), pagination: { total } } + return { bills: results, pagination: { total } } } async function _fetch (licenceId, page) { - const billLicences = await BillLicenceModel.query() - .select('*') - .where('billLicences.licence_id', licenceId) - .page(page - 1, DatabaseConfig.defaultPageSize) - - const billIds = JSON.parse(JSON.stringify(billLicences)).results.map(b => b.billId) - return BillModel.query() - .findByIds(billIds) - .select('*') + .select([ + 'bills.id', + 'bills.invoiceNumber', + 'bills.accountNumber', + 'bills.financialYearEnding', + 'bills.netAmount', + 'bills.billingAccountId', + 'bills.createdAt', + ]) + .innerJoinRelated('billLicences') + .where('billLicences.licence_id', licenceId) .withGraphFetched('billRun') .modifyGraph('billRun', (builder) => { builder.select(['batchType']) }) .orderBy([ - { column: 'created_at', order: 'desc' } + { column: 'createdAt', order: 'desc' } ]) .page(page - 1, DatabaseConfig.defaultPageSize) } diff --git a/app/services/licences/view-licence-bills.service.js b/app/services/licences/view-licence-bills.service.js index 9954379a6b..eb827d9e08 100644 --- a/app/services/licences/view-licence-bills.service.js +++ b/app/services/licences/view-licence-bills.service.js @@ -23,7 +23,7 @@ async function go (licenceId, auth, page) { const billsData = await FetchLicenceBillsService.go(licenceId, page) const pageData = ViewLicenceBillsPresenter.go(billsData.bills) - const pagination = PaginatorPresenter.go(100, Number(page), `/system/licences/${licenceId}/bills`) + const pagination = PaginatorPresenter.go(billsData.pagination.total, Number(page), `/system/licences/${licenceId}/bills`) return { ...commonData, From 6a65c20913b07c487493e71b07f98635fc23d16a Mon Sep 17 00:00:00 2001 From: jonathangoulding Date: Tue, 7 May 2024 13:21:20 +0100 Subject: [PATCH 08/17] fix: linting --- app/services/licences/fetch-licence-bills.service.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/app/services/licences/fetch-licence-bills.service.js b/app/services/licences/fetch-licence-bills.service.js index a43455bb85..8313d0a5a2 100644 --- a/app/services/licences/fetch-licence-bills.service.js +++ b/app/services/licences/fetch-licence-bills.service.js @@ -5,7 +5,6 @@ * @module FetchLicenceBillsService */ -const BillLicenceModel = require('../../models/bill-licence.model') const BillModel = require('../../models/bill.model') const DatabaseConfig = require('../../../config/database.config') @@ -32,7 +31,7 @@ async function _fetch (licenceId, page) { 'bills.financialYearEnding', 'bills.netAmount', 'bills.billingAccountId', - 'bills.createdAt', + 'bills.createdAt' ]) .innerJoinRelated('billLicences') .where('billLicences.licence_id', licenceId) From bdc748d01db490bc0455af79b54cd44d5c081638 Mon Sep 17 00:00:00 2001 From: jonathangoulding Date: Tue, 7 May 2024 13:26:44 +0100 Subject: [PATCH 09/17] feat: format currency correctly --- .../licences/view-licence-bills.presenter.js | 12 +++--------- .../licences/view-licence-bills-presenter.test.js | 10 +++++----- 2 files changed, 8 insertions(+), 14 deletions(-) diff --git a/app/presenters/licences/view-licence-bills.presenter.js b/app/presenters/licences/view-licence-bills.presenter.js index 38436b9785..224b9696f8 100644 --- a/app/presenters/licences/view-licence-bills.presenter.js +++ b/app/presenters/licences/view-licence-bills.presenter.js @@ -5,7 +5,7 @@ * @module ViewLicenceBillsPresenter */ -const { formatLongDate } = require('../base.presenter') +const { formatLongDate, formatMoney } = require('../base.presenter') /** * Formats data for the `/licences/{id}/bills` view licence bill page @@ -19,21 +19,15 @@ function go (bills) { } } -function _formatCurrencyToGBP (amount) { - return amount.toLocaleString('en-gb', { - style: 'currency', - currency: 'GBP' - }) -} function _formatBillsToTableRow (bills) { return bills.map((bill) => { return { billNumber: bill.invoiceNumber, dateCreated: formatLongDate(new Date(bill.createdAt)), account: bill.accountNumber, - runType: bill.billRun.batchType, + runType: bill.billRun?.batchType, financialYear: bill.financialYearEnding, - total: _formatCurrencyToGBP(bill.netAmount), + total: formatMoney(bill.netAmount), accountId: bill.billingAccountId, id: bill.id } diff --git a/test/presenters/licences/view-licence-bills-presenter.test.js b/test/presenters/licences/view-licence-bills-presenter.test.js index 73e0f109d1..575991e222 100644 --- a/test/presenters/licences/view-licence-bills-presenter.test.js +++ b/test/presenters/licences/view-licence-bills-presenter.test.js @@ -24,14 +24,14 @@ describe('View Licence Bills presenter', () => { dateCreated: '1 January 2020', financialYear: '2021', id: 'id123', - runType: 'Annual', - total: '£123,456,789.00' + runType: 'annual', + total: '£1,234,567.89' }] }) }) it('correctly formats the currency to UK standard', () => { const result = ViewLicenceBillsPresenter.go(_bills()) - expect(result.bills[0].total).to.equal('£123,456,789.00') + expect(result.bills[0].total).to.equal('£1,234,567.89') }) }) }) @@ -41,10 +41,10 @@ function _bills () { invoiceNumber: 'inv123', createdAt: new Date('2020-01-01'), accountNumber: 'acc123', - batchType: 'Annual', financialYearEnding: '2021', netAmount: 123456789, billingAccountId: 'bicc1233', - id: 'id123' + id: 'id123', + billRun: { batchType: 'annual' } }] } From 64ba4215e2bf2279c4e57947cda896c61f632487 Mon Sep 17 00:00:00 2001 From: jonathangoulding Date: Tue, 7 May 2024 13:47:20 +0100 Subject: [PATCH 10/17] feat: add scope billing to page --- app/controllers/licences.controller.js | 8 +++++--- app/routes/licence.routes.js | 5 +++++ 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/app/controllers/licences.controller.js b/app/controllers/licences.controller.js index 3278f85c32..f6e284e207 100644 --- a/app/controllers/licences.controller.js +++ b/app/controllers/licences.controller.js @@ -10,6 +10,8 @@ const ViewLicenceBillsService = require('../services/licences/view-licence-bills const ViewLicenceReturnsService = require('../services/licences/view-licence-returns.service') const ViewLicenceSummaryService = require('../services/licences/view-licence-summary.service') + +const ViewLicencePage = 'licences/view.njk' async function noReturnsRequired (request, h) { const { id } = request.params @@ -31,7 +33,7 @@ async function viewBills (request, h) { const data = await ViewLicenceBillsService.go(id, auth, page) - return h.view('licences/view.njk', { + return h.view(ViewLicencePage, { ...data }) } @@ -41,7 +43,7 @@ async function viewSummary (request, h) { const data = await ViewLicenceSummaryService.go(id, auth) - return h.view('licences/view.njk', { + return h.view(ViewLicencePage, { ...data }) } @@ -51,7 +53,7 @@ async function viewReturns (request, h) { const data = await ViewLicenceReturnsService.go(id, auth, page) - return h.view('licences/view.njk', { + return h.view(ViewLicencePage, { ...data }) } diff --git a/app/routes/licence.routes.js b/app/routes/licence.routes.js index c2277ee1c4..65b61e2c6b 100644 --- a/app/routes/licence.routes.js +++ b/app/routes/licence.routes.js @@ -8,6 +8,11 @@ const routes = [ path: '/licences/{id}/bills', handler: LicencesController.viewBills, options: { + auth: { + access: { + scope: ['billing'] + } + }, description: 'View a licence bills page' } }, From c5801a180a579a2e445e9ac8e32d22267c80f561 Mon Sep 17 00:00:00 2001 From: jonathangoulding Date: Tue, 7 May 2024 13:55:27 +0100 Subject: [PATCH 11/17] fix: alpha ordering --- app/controllers/licences.controller.js | 5 +++-- .../licences/view-licence-bills.service.js | 2 +- .../licences/view-licence-bills-presenter.test.js | 15 ++++++++++----- 3 files changed, 14 insertions(+), 8 deletions(-) diff --git a/app/controllers/licences.controller.js b/app/controllers/licences.controller.js index f6e284e207..3e45160382 100644 --- a/app/controllers/licences.controller.js +++ b/app/controllers/licences.controller.js @@ -12,6 +12,7 @@ const ViewLicenceSummaryService = require('../services/licences/view-licence-sum const ViewLicencePage = 'licences/view.njk' + async function noReturnsRequired (request, h) { const { id } = request.params @@ -61,7 +62,7 @@ async function viewReturns (request, h) { module.exports = { noReturnsRequired, returnsRequired, + viewBills, viewReturns, - viewSummary, - viewBills + viewSummary } diff --git a/app/services/licences/view-licence-bills.service.js b/app/services/licences/view-licence-bills.service.js index eb827d9e08..67afe2ff90 100644 --- a/app/services/licences/view-licence-bills.service.js +++ b/app/services/licences/view-licence-bills.service.js @@ -6,8 +6,8 @@ */ const FetchLicenceBillsService = require('./fetch-licence-bills.service') -const ViewLicenceService = require('./view-licence.service') const ViewLicenceBillsPresenter = require('../../presenters/licences/view-licence-bills.presenter') +const ViewLicenceService = require('./view-licence.service') const PaginatorPresenter = require('../../presenters/paginator.presenter') /** diff --git a/test/presenters/licences/view-licence-bills-presenter.test.js b/test/presenters/licences/view-licence-bills-presenter.test.js index 575991e222..8ab40905b6 100644 --- a/test/presenters/licences/view-licence-bills-presenter.test.js +++ b/test/presenters/licences/view-licence-bills-presenter.test.js @@ -29,6 +29,11 @@ describe('View Licence Bills presenter', () => { }] }) }) + it('correctly formats the created date to convention', () => { + const result = ViewLicenceBillsPresenter.go(_bills()) + expect(result.bills[0].dateCreated).to.equal('1 January 2020') + }) + it('correctly formats the currency to UK standard', () => { const result = ViewLicenceBillsPresenter.go(_bills()) expect(result.bills[0].total).to.equal('£1,234,567.89') @@ -38,13 +43,13 @@ describe('View Licence Bills presenter', () => { function _bills () { return [{ - invoiceNumber: 'inv123', - createdAt: new Date('2020-01-01'), accountNumber: 'acc123', - financialYearEnding: '2021', - netAmount: 123456789, + billRun: { batchType: 'annual' }, billingAccountId: 'bicc1233', + createdAt: new Date('2020-01-01'), + financialYearEnding: '2021', id: 'id123', - billRun: { batchType: 'annual' } + invoiceNumber: 'inv123', + netAmount: 123456789 }] } From 34bf55ac42ce3baaf39339f68a084d73da8fec55 Mon Sep 17 00:00:00 2001 From: jonathangoulding Date: Tue, 7 May 2024 13:56:02 +0100 Subject: [PATCH 12/17] fix: lint spacing --- app/controllers/licences.controller.js | 1 - 1 file changed, 1 deletion(-) diff --git a/app/controllers/licences.controller.js b/app/controllers/licences.controller.js index 3e45160382..895841382e 100644 --- a/app/controllers/licences.controller.js +++ b/app/controllers/licences.controller.js @@ -10,7 +10,6 @@ const ViewLicenceBillsService = require('../services/licences/view-licence-bills const ViewLicenceReturnsService = require('../services/licences/view-licence-returns.service') const ViewLicenceSummaryService = require('../services/licences/view-licence-summary.service') - const ViewLicencePage = 'licences/view.njk' async function noReturnsRequired (request, h) { From fbc67bb4c9c80fe666dc10e9be5b8e55208fea65 Mon Sep 17 00:00:00 2001 From: jonathangoulding Date: Tue, 7 May 2024 14:04:34 +0100 Subject: [PATCH 13/17] fix: bills license test auth scope billing --- test/controllers/licences.controller.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/controllers/licences.controller.test.js b/test/controllers/licences.controller.test.js index fe65bd6915..234daba124 100644 --- a/test/controllers/licences.controller.test.js +++ b/test/controllers/licences.controller.test.js @@ -159,7 +159,7 @@ describe('Licences controller', () => { url: '/licences/7861814c-ca19-43f2-be11-3c612f0d744b/bills', auth: { strategy: 'session', - credentials: { scope: [] } + credentials: { scope: ['billing'] } } } }) From 4d40e9ebf78050994a1200536c59fbe0a5116447 Mon Sep 17 00:00:00 2001 From: jonathangoulding Date: Tue, 7 May 2024 14:29:34 +0100 Subject: [PATCH 14/17] test: add fetch bill licence test --- test/support/helpers/bill-licence.helper.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/support/helpers/bill-licence.helper.js b/test/support/helpers/bill-licence.helper.js index fe7ae89afc..f3e546d736 100644 --- a/test/support/helpers/bill-licence.helper.js +++ b/test/support/helpers/bill-licence.helper.js @@ -41,7 +41,7 @@ function defaults (data = {}) { const defaults = { billId: generateUUID(), licenceRef: LicenceHelper.generateLicenceRef(), - licenceId: generateUUID() + licenceId: data.licenceId || generateUUID() } return { From 360f5d7d0f529033337942da21c9e5744490982b Mon Sep 17 00:00:00 2001 From: jonathangoulding Date: Tue, 7 May 2024 14:31:44 +0100 Subject: [PATCH 15/17] test: add fetch bill licence test --- .../fetch-licence-bills.service.test.js | 73 +++++++++++++++++++ 1 file changed, 73 insertions(+) create mode 100644 test/services/licences/fetch-licence-bills.service.test.js diff --git a/test/services/licences/fetch-licence-bills.service.test.js b/test/services/licences/fetch-licence-bills.service.test.js new file mode 100644 index 0000000000..7e173c013f --- /dev/null +++ b/test/services/licences/fetch-licence-bills.service.test.js @@ -0,0 +1,73 @@ +'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 DatabaseSupport = require('../../support/database.js') +const BillLicenceHelper = require('../../support/helpers/bill-licence.helper') +const BillHelper = require('../../support/helpers/bill.helper') +const BillRunHelper = require('../../support/helpers/bill-run.helper') + +// Thing under test +const FetchLicenceBillService = require('../../../app/services/licences/fetch-licence-bills.service') + +describe('Fetch licence bills service', () => { + beforeEach(async () => { + await DatabaseSupport.clean() + }) + + describe('when the licence has bills', () => { + const createdDate = new Date('2022-01-01') + + const licenceId = '96d97293-1a62-4ad0-bcb6-24f68a203e6b' + const billId = '72988ec1-9fb2-4b87-b0a0-3c0be628a72c' + const billingAccountId = '0ba3b707-72ee-4296-b177-a19afff10688' + + beforeEach(async () => { + await BillLicenceHelper.add({ + licenceId, + billId + }) + + await BillRunHelper.add({ batchType: 'annual' }) + + await BillHelper.add( + { + id: billId, + invoiceNumber: '123', + accountNumber: 'T21404193A', + netAmount: 12345, + billingAccountId, + createdAt: createdDate + }) + // Add an extra record to ensure the bill is retrieved by the license id + await BillHelper.add() + }) + + it('returns results', async () => { + const result = await FetchLicenceBillService.go(licenceId, 1) + + expect(result.pagination).to.equal({ + total: 1 + }) + + expect(result.bills).to.equal( + [{ + accountNumber: 'T21404193A', + billRun: null, + billingAccountId, + createdAt: createdDate, + financialYearEnding: 2023, + id: billId, + invoiceNumber: '123', + netAmount: 12345 + }] + ) + }) + }) +}) From 86a820b82b10c1d8b97b04630b4a3d7bf089140a Mon Sep 17 00:00:00 2001 From: jonathangoulding Date: Tue, 7 May 2024 15:04:30 +0100 Subject: [PATCH 16/17] fix: use of bills for reference --- app/services/licences/fetch-licence-bills.service.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/services/licences/fetch-licence-bills.service.js b/app/services/licences/fetch-licence-bills.service.js index 8313d0a5a2..4fb99d027a 100644 --- a/app/services/licences/fetch-licence-bills.service.js +++ b/app/services/licences/fetch-licence-bills.service.js @@ -1,7 +1,7 @@ 'use strict' /** - * Fetches all return logs for a licence which is needed for the view '/licences/{id}/returns` page + * Fetches all return logs for a licence which is needed for the view '/licences/{id}/bills` page * @module FetchLicenceBillsService */ @@ -14,7 +14,7 @@ const DatabaseConfig = require('../../../config/database.config') * * @param {string} licenceId - The UUID for the licence to fetch * - * @returns {Promise} the data needed to populate the view licence page's returns tab + * @returns {Promise} the data needed to populate the view licence page's bills tab */ async function go (licenceId, page) { const { results, total } = await _fetch(licenceId, page) From 5dd8a74d2707f8670cd229d49c0d3d2d53c71866 Mon Sep 17 00:00:00 2001 From: jonathangoulding Date: Tue, 7 May 2024 16:08:32 +0100 Subject: [PATCH 17/17] fix: remove optional caning --- app/presenters/licences/view-licence-bills.presenter.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/presenters/licences/view-licence-bills.presenter.js b/app/presenters/licences/view-licence-bills.presenter.js index 224b9696f8..a770fa9dda 100644 --- a/app/presenters/licences/view-licence-bills.presenter.js +++ b/app/presenters/licences/view-licence-bills.presenter.js @@ -25,7 +25,7 @@ function _formatBillsToTableRow (bills) { billNumber: bill.invoiceNumber, dateCreated: formatLongDate(new Date(bill.createdAt)), account: bill.accountNumber, - runType: bill.billRun?.batchType, + runType: bill.billRun.batchType, financialYear: bill.financialYearEnding, total: formatMoney(bill.netAmount), accountId: bill.billingAccountId,