From 0e9117be4288dbc6e670541cac5bc5c67729616b Mon Sep 17 00:00:00 2001 From: jonathangoulding Date: Tue, 11 Jun 2024 14:33:36 +0100 Subject: [PATCH 01/23] View Licence Returns no return messages When a licence has returns show the returns table. When the licence does not have returns and or requirements then the messages show need to show different messages. --- app/services/licences/view-licence-returns.service.js | 10 ++++++---- app/services/licences/view-licence.service.js | 2 +- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/app/services/licences/view-licence-returns.service.js b/app/services/licences/view-licence-returns.service.js index fca6cc8e0f..93324d3ed0 100644 --- a/app/services/licences/view-licence-returns.service.js +++ b/app/services/licences/view-licence-returns.service.js @@ -5,15 +5,17 @@ * @module ViewLicenceSummaryService */ -const FetchLicenceReturnsService = require('./fetch-licence-returns.service') -const ViewLicenceService = require('./view-licence.service') -const ViewLicenceReturnsPresenter = require('../../presenters/licences/view-licence-returns.presenter') -const PaginatorPresenter = require('../../presenters/paginator.presenter') +const FetchLicenceReturnsService = require('./fetch-licence-returns.service.js') +const PaginatorPresenter = require('../../presenters/paginator.presenter.js') +const ViewLicenceReturnsPresenter = require('../../presenters/licences/view-licence-returns.presenter.js') +const ViewLicenceService = require('./view-licence.service.js') /** * Orchestrates fetching and presenting the data needed for the licence summary page * * @param {string} licenceId - The UUID of the licence + * @param {Object} auth - The auth object taken from `request.auth` containing user details + * @param {Object} page - The current page for the pagination service * * @returns {Promise} an object representing the `pageData` needed by the licence summary template. */ diff --git a/app/services/licences/view-licence.service.js b/app/services/licences/view-licence.service.js index 78056b1135..1b766efa36 100644 --- a/app/services/licences/view-licence.service.js +++ b/app/services/licences/view-licence.service.js @@ -12,7 +12,7 @@ const ViewLicencePresenter = require('../../presenters/licences/view-licence.pre * Orchestrates fetching and presenting the data needed for the licence summary page * * @param {string} licenceId - The UUID of the licence - * @param {string} auth - Auth object + * @param {Object} auth - The auth object taken from `request.auth` containing user details * @returns {Promise} an object representing the `pageData` needed by the licence summary template. */ async function go (licenceId, auth) { From b2535dc6feb980023f958aa1293245301a208c60 Mon Sep 17 00:00:00 2001 From: jonathangoulding Date: Tue, 11 Jun 2024 15:56:18 +0100 Subject: [PATCH 02/23] feat: add fetch for returns requirements check --- .../view-licence-returns.presenter.js | 5 ++- .../fetch-licence-has-requirements.service.js | 34 +++++++++++++++ .../licences/view-licence-returns.service.js | 5 ++- app/views/licences/tabs/returns.njk | 11 +++-- test/controllers/licences.controller.test.js | 31 +++++++++++--- .../view-licence-returns-presenter.test.js | 3 +- ...h-licence-has-requirements.service.test.js | 42 +++++++++++++++++++ .../view-licence-returns.service.test.js | 13 +++--- 8 files changed, 126 insertions(+), 18 deletions(-) create mode 100644 app/services/licences/fetch-licence-has-requirements.service.js create mode 100644 test/services/licences/fetch-licence-has-requirements.service.test.js diff --git a/app/presenters/licences/view-licence-returns.presenter.js b/app/presenters/licences/view-licence-returns.presenter.js index 00468e7305..5fb76e9371 100644 --- a/app/presenters/licences/view-licence-returns.presenter.js +++ b/app/presenters/licences/view-licence-returns.presenter.js @@ -5,7 +5,7 @@ * @module ViewLicenceReturnsPresenter */ -const { formatLongDate } = require('../base.presenter') +const { formatLongDate } = require('../base.presenter.js') /** * Formats common data for the `/licences/{id}/*` view licence pages @@ -17,7 +17,8 @@ function go (returnsData) { return { activeTab: 'returns', - returns + returns, + hasReturns: returns.length > 0 } } diff --git a/app/services/licences/fetch-licence-has-requirements.service.js b/app/services/licences/fetch-licence-has-requirements.service.js new file mode 100644 index 0000000000..c58dbed0cc --- /dev/null +++ b/app/services/licences/fetch-licence-has-requirements.service.js @@ -0,0 +1,34 @@ +'use strict' + +/** + * Fetches requirements for a licence and returns true or false if present + * @module FetchLicenceHasRequirementsService + */ + +const ReturnVersionModel = require('../../models/return-version.model.js') + +/** + * Fetches requirements for a licence and returns true or false if present + * + * @param {string} licenceId - The UUID for the licence id to fetch + * + * @returns {Promise} the result of check if a licence has requirements for returns + */ +async function go (licenceId) { + const requirement = await _fetch(licenceId) + + return !!requirement +} + +async function _fetch (licenceId, page) { + return ReturnVersionModel.query() + .select([ + 'id' + ]) + .where('licenceId', licenceId) + .first() +} + +module.exports = { + go +} diff --git a/app/services/licences/view-licence-returns.service.js b/app/services/licences/view-licence-returns.service.js index 93324d3ed0..4323ae6eb1 100644 --- a/app/services/licences/view-licence-returns.service.js +++ b/app/services/licences/view-licence-returns.service.js @@ -6,6 +6,7 @@ */ const FetchLicenceReturnsService = require('./fetch-licence-returns.service.js') +const FetchLicenceHasRequirementsService = require('./fetch-licence-has-requirements.service.js') const PaginatorPresenter = require('../../presenters/paginator.presenter.js') const ViewLicenceReturnsPresenter = require('../../presenters/licences/view-licence-returns.presenter.js') const ViewLicenceService = require('./view-licence.service.js') @@ -23,6 +24,7 @@ async function go (licenceId, auth, page) { const commonData = await ViewLicenceService.go(licenceId, auth) const returnsData = await FetchLicenceReturnsService.go(licenceId, page) + const hasRequirements = await FetchLicenceHasRequirementsService.go(licenceId) const pageData = ViewLicenceReturnsPresenter.go(returnsData) const pagination = PaginatorPresenter.go(returnsData.pagination.total, Number(page), `/system/licences/${licenceId}/returns`) @@ -30,7 +32,8 @@ async function go (licenceId, auth, page) { return { ...pageData, ...commonData, - pagination + pagination, + hasRequirements } } diff --git a/app/views/licences/tabs/returns.njk b/app/views/licences/tabs/returns.njk index 9f0d4401b6..467f97dd0e 100644 --- a/app/views/licences/tabs/returns.njk +++ b/app/views/licences/tabs/returns.njk @@ -21,7 +21,7 @@

Returns

- {% if returns.length > 0 %} + {% if hasReturns %} @@ -43,12 +43,15 @@ {% endfor %} - {% else %} -

No returns found

+ + {% elseif hasReturns == false and hasRequirements == false %} +

No requirements for returns have been set up for this licence.

+ {% elseif hasRequirements and hasReturns == false %} +

No returns for this licence.

{% endif %}
Return reference and dates
-{% if returns and pagination.numberOfPages > 1 %} +{% if hasReturns and pagination.numberOfPages > 1 %} {{ govukPagination(pagination.component) }} {% endif %} diff --git a/test/controllers/licences.controller.test.js b/test/controllers/licences.controller.test.js index 7fd11ba2fb..5fc06b6985 100644 --- a/test/controllers/licences.controller.test.js +++ b/test/controllers/licences.controller.test.js @@ -421,21 +421,41 @@ describe('Licences controller', () => { }) }) - describe('when a request is valid and has NO returns', () => { + describe('when a request is valid and has requirements but NO returns', () => { beforeEach(async () => { Sinon.stub(ViewLicenceReturnsService, 'go').resolves({ activeTab: 'returns', - returns: [] + returns: [], + hasReturns: false, + hasRequirements: true }) }) - it('returns the page successfully', async () => { + it('returns the page successfully with the message \'No returns for this licence.\'', 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('No returns found') + expect(response.payload).to.contain('No returns for this licence.') + }) + }) + + describe('when a request is valid and has NO requirements OR returns', () => { + beforeEach(async () => { + Sinon.stub(ViewLicenceReturnsService, 'go').resolves({ + activeTab: 'returns', + returns: [], + hasReturns: false, + hasRequirements: false + }) + }) + + it('returns the page successfully with the message \'No returns for this licence.\'', async () => { + const response = await server.inject(options) + + expect(response.statusCode).to.equal(200) + expect(response.payload).to.contain('Returns') + expect(response.payload).to.contain('No requirements for returns have been set up for this licence.') }) }) }) @@ -477,6 +497,7 @@ function _viewLicenceContactDetails () { function _viewLicenceReturns () { return { activeTab: 'returns', + hasReturns: true, returns: [{ id: 'returns-id' }] } } diff --git a/test/presenters/licences/view-licence-returns-presenter.test.js b/test/presenters/licences/view-licence-returns-presenter.test.js index c65c2135f7..cec850bfba 100644 --- a/test/presenters/licences/view-licence-returns-presenter.test.js +++ b/test/presenters/licences/view-licence-returns-presenter.test.js @@ -8,7 +8,7 @@ const { describe, it, beforeEach } = exports.lab = Lab.script() const { expect } = Code // Thing under test -const ViewLicenceReturnsPresenter = require('../../../app/presenters/licences/view-licence-returns.presenter') +const ViewLicenceReturnsPresenter = require('../../../app/presenters/licences/view-licence-returns.presenter.js') describe('View Licence returns presenter', () => { let returnData @@ -23,6 +23,7 @@ describe('View Licence returns presenter', () => { expect(result).to.equal({ activeTab: 'returns', + hasReturns: true, returns: [ { id: 'mock-id-1', diff --git a/test/services/licences/fetch-licence-has-requirements.service.test.js b/test/services/licences/fetch-licence-has-requirements.service.test.js new file mode 100644 index 0000000000..cf4f63a7aa --- /dev/null +++ b/test/services/licences/fetch-licence-has-requirements.service.test.js @@ -0,0 +1,42 @@ +'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 ReturnVersionHelper = require('../../support/helpers/return-version.helper.js') + +// Thing under test +const FetchLicenceHasRequirementsService = + require('../../../app/services/licences/fetch-licence-has-requirements.service.js') + +describe.only('Fetch Licence Has Requirements service', () => { + let returnVersion + + beforeEach(async () => { + await DatabaseSupport.clean() + }) + + describe('when the licence has return versions data', () => { + beforeEach(async () => { + returnVersion = await ReturnVersionHelper.add() + }) + + it('returns true a licence has any return versions', async () => { + const result = await FetchLicenceHasRequirementsService.go(returnVersion.licenceId) + + expect(result).to.be.true() + }) + + it('returns false no return versions for licence', async () => { + const result = await FetchLicenceHasRequirementsService.go('ed3b9b1a-94e0-480c-8ad6-60e05f5fa9f4') + + expect(result).to.be.false() + }) + }) +}) diff --git a/test/services/licences/view-licence-returns.service.test.js b/test/services/licences/view-licence-returns.service.test.js index 8365c50c79..8ea59c1420 100644 --- a/test/services/licences/view-licence-returns.service.test.js +++ b/test/services/licences/view-licence-returns.service.test.js @@ -9,13 +9,14 @@ 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 PaginatorPresenter = require('../../../app/presenters/paginator.presenter') -const ViewLicenceReturnsPresenter = require('../../../app/presenters/licences/view-licence-returns.presenter') -const FetchLicenceReturnsService = require('../../../app/services/licences/fetch-licence-returns.service') +const FetchLicenceReturnsService = require('../../../app/services/licences/fetch-licence-returns.service.js') +const FetchLicenceHasRequirementsService = require('../../../app/services/licences/fetch-licence-has-requirements.service.js') +const PaginatorPresenter = require('../../../app/presenters/paginator.presenter.js') +const ViewLicenceReturnsPresenter = require('../../../app/presenters/licences/view-licence-returns.presenter.js') +const ViewLicenceService = require('../../../app/services/licences/view-licence.service.js') // Thing under test -const ViewLicenceReturnsService = require('../../../app/services/licences/view-licence-returns.service') +const ViewLicenceReturnsService = require('../../../app/services/licences/view-licence-returns.service.js') describe('View Licence service returns', () => { const testId = '2c80bd22-a005-4cf4-a2a2-73812a9861de' @@ -24,6 +25,7 @@ describe('View Licence service returns', () => { const pagination = { page } beforeEach(() => { + Sinon.stub(FetchLicenceHasRequirementsService, 'go').resolves(true) Sinon.stub(FetchLicenceReturnsService, 'go').resolves(_returnsFetch()) Sinon.stub(PaginatorPresenter, 'go').returns(pagination) Sinon.stub(ViewLicenceReturnsPresenter, 'go').returns(_returnsPresenter()) @@ -41,6 +43,7 @@ describe('View Licence service returns', () => { expect(result).to.equal({ licenceName: 'fake licence', + hasRequirements: true, returns: [], activeTab: 'returns', pagination: { page: 1 } From bb96e667ea275f586be786be23df7159fc012639 Mon Sep 17 00:00:00 2001 From: jonathangoulding Date: Tue, 11 Jun 2024 15:57:57 +0100 Subject: [PATCH 03/23] chore: pre pr check --- .../licences/fetch-licence-has-requirements.service.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/services/licences/fetch-licence-has-requirements.service.js b/app/services/licences/fetch-licence-has-requirements.service.js index c58dbed0cc..e920448349 100644 --- a/app/services/licences/fetch-licence-has-requirements.service.js +++ b/app/services/licences/fetch-licence-has-requirements.service.js @@ -1,14 +1,14 @@ 'use strict' /** - * Fetches requirements for a licence and returns true or false if present + * Fetches requirements for a licence and returns and returns a boolean if the licence has requirements for returns * @module FetchLicenceHasRequirementsService */ const ReturnVersionModel = require('../../models/return-version.model.js') /** - * Fetches requirements for a licence and returns true or false if present + * Fetches requirements for a licence and returns a boolean if the licence has requirements for returns * * @param {string} licenceId - The UUID for the licence id to fetch * From f523b2bba6b7ae329818ae7796018d5633ab6fd3 Mon Sep 17 00:00:00 2001 From: jonathangoulding Date: Tue, 11 Jun 2024 16:00:01 +0100 Subject: [PATCH 04/23] chore: pre pr check --- app/services/licences/view-licence-returns.service.js | 3 ++- .../licences/fetch-licence-has-requirements.service.test.js | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/app/services/licences/view-licence-returns.service.js b/app/services/licences/view-licence-returns.service.js index 4323ae6eb1..201f4166a4 100644 --- a/app/services/licences/view-licence-returns.service.js +++ b/app/services/licences/view-licence-returns.service.js @@ -24,9 +24,10 @@ async function go (licenceId, auth, page) { const commonData = await ViewLicenceService.go(licenceId, auth) const returnsData = await FetchLicenceReturnsService.go(licenceId, page) - const hasRequirements = await FetchLicenceHasRequirementsService.go(licenceId) const pageData = ViewLicenceReturnsPresenter.go(returnsData) + const hasRequirements = await FetchLicenceHasRequirementsService.go(licenceId) + const pagination = PaginatorPresenter.go(returnsData.pagination.total, Number(page), `/system/licences/${licenceId}/returns`) return { diff --git a/test/services/licences/fetch-licence-has-requirements.service.test.js b/test/services/licences/fetch-licence-has-requirements.service.test.js index cf4f63a7aa..c7663e91ed 100644 --- a/test/services/licences/fetch-licence-has-requirements.service.test.js +++ b/test/services/licences/fetch-licence-has-requirements.service.test.js @@ -15,7 +15,7 @@ const ReturnVersionHelper = require('../../support/helpers/return-version.helper const FetchLicenceHasRequirementsService = require('../../../app/services/licences/fetch-licence-has-requirements.service.js') -describe.only('Fetch Licence Has Requirements service', () => { +describe('Fetch Licence Has Requirements service', () => { let returnVersion beforeEach(async () => { From f64f88e6eb05bba6be4ddf6828dc4b2ee12ac5a9 Mon Sep 17 00:00:00 2001 From: jonathangoulding Date: Tue, 11 Jun 2024 16:00:16 +0100 Subject: [PATCH 05/23] chore: pre pr check --- app/services/licences/view-licence-returns.service.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/services/licences/view-licence-returns.service.js b/app/services/licences/view-licence-returns.service.js index 201f4166a4..fff040b969 100644 --- a/app/services/licences/view-licence-returns.service.js +++ b/app/services/licences/view-licence-returns.service.js @@ -33,8 +33,8 @@ async function go (licenceId, auth, page) { return { ...pageData, ...commonData, - pagination, - hasRequirements + hasRequirements, + pagination } } From 219424dcdcf127fd39cb613ad3e9d8833c30189d Mon Sep 17 00:00:00 2001 From: jonathangoulding Date: Tue, 11 Jun 2024 16:03:04 +0100 Subject: [PATCH 06/23] chore: pre pr check --- 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 5fc06b6985..93db5e72f1 100644 --- a/test/controllers/licences.controller.test.js +++ b/test/controllers/licences.controller.test.js @@ -440,7 +440,7 @@ describe('Licences controller', () => { }) }) - describe('when a request is valid and has NO requirements OR returns', () => { + describe('when a request is valid and has NO requirements and NO returns', () => { beforeEach(async () => { Sinon.stub(ViewLicenceReturnsService, 'go').resolves({ activeTab: 'returns', From 6672c2928b0dbc5eb8fd0e1fcf1469f0dba7168d Mon Sep 17 00:00:00 2001 From: jonathangoulding Date: Tue, 11 Jun 2024 16:10:47 +0100 Subject: [PATCH 07/23] fix: old style test --- .../view-licence-returns-presenter.test.js | 124 +++++++++--------- 1 file changed, 59 insertions(+), 65 deletions(-) diff --git a/test/presenters/licences/view-licence-returns-presenter.test.js b/test/presenters/licences/view-licence-returns-presenter.test.js index cec850bfba..4fbd132792 100644 --- a/test/presenters/licences/view-licence-returns-presenter.test.js +++ b/test/presenters/licences/view-licence-returns-presenter.test.js @@ -13,11 +13,50 @@ const ViewLicenceReturnsPresenter = require('../../../app/presenters/licences/vi describe('View Licence returns presenter', () => { let returnData + const returnItem = { + id: 'mock-id-1', + dueDate: '2012-11-28T00:00:00.000Z', + status: 'completed', + startDate: '2020/01/02', + endDate: '2020/02/01', + metadata: { + purposes: [ + { + alias: 'SPRAY IRRIGATION', + primary: { + code: 'A', + description: 'Agriculture' + }, + tertiary: { + code: '400', + description: 'Spray Irrigation - Direct' + }, + secondary: { + code: 'AGR', + description: 'General Agriculture' + } + } + ], + description: 'empty description' + }, + returnReference: '1068' + } + beforeEach(() => { - returnData = _returnData() + returnData = { + returns: [ + { ...returnItem }, + { + ...returnItem, + id: 'mock-id-2', + status: 'due', + returnReference: '1069' + } + ] + } }) - describe('when provided with a populated licence', () => { + describe('when provided returns', () => { it('correctly presents the data', () => { const result = ViewLicenceReturnsPresenter.go(returnData) @@ -38,7 +77,7 @@ describe('View Licence returns presenter', () => { id: 'mock-id-2', reference: '1069', purpose: 'SPRAY IRRIGATION', - dueDate: '28 November 2019', + dueDate: '28 November 2012', status: 'OVERDUE', dates: '2 January 2020 to 1 February 2020', description: 'empty description' @@ -47,67 +86,22 @@ describe('View Licence returns presenter', () => { }) }) }) -}) -function _returnData () { - return { - returns: [ - { - id: 'mock-id-1', - dueDate: '2012-11-28T00:00:00.000Z', - status: 'completed', - startDate: '2020/01/02', - endDate: '2020/02/01', - metadata: { - purposes: [ - { - alias: 'SPRAY IRRIGATION', - primary: { - code: 'A', - description: 'Agriculture' - }, - tertiary: { - code: '400', - description: 'Spray Irrigation - Direct' - }, - secondary: { - code: 'AGR', - description: 'General Agriculture' - } - } - ], - description: 'empty description' - }, - returnReference: '1068' - }, - { - id: 'mock-id-2', - dueDate: '2019-11-28T00:00:00.000Z', - status: 'due', - startDate: '2020/01/02', - endDate: '2020/02/01', - metadata: { - description: 'empty description', - purposes: [ - { - alias: 'SPRAY IRRIGATION', - primary: { - code: 'A', - description: 'Agriculture' - }, - tertiary: { - code: '400', - description: 'Spray Irrigation - Direct' - }, - secondary: { - code: 'AGR', - description: 'General Agriculture' - } - } - ] - }, - returnReference: '1069' + describe('when no returns', () => { + beforeEach(() => { + returnData = { + returns: [] } - ] - } -} + }) + + it('correctly returns the no returns data', () => { + const result = ViewLicenceReturnsPresenter.go(returnData) + + expect(result).to.equal({ + activeTab: 'returns', + hasReturns: false, + returns: [] + }) + }) + }) +}) From 01596a93872ab88d646a8589d9e49d704de0ab15 Mon Sep 17 00:00:00 2001 From: jonathangoulding Date: Tue, 11 Jun 2024 16:15:12 +0100 Subject: [PATCH 08/23] chore: pre pr check --- .../licences/view-licence-returns-presenter.test.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/presenters/licences/view-licence-returns-presenter.test.js b/test/presenters/licences/view-licence-returns-presenter.test.js index 4fbd132792..a562bb4ded 100644 --- a/test/presenters/licences/view-licence-returns-presenter.test.js +++ b/test/presenters/licences/view-licence-returns-presenter.test.js @@ -56,7 +56,7 @@ describe('View Licence returns presenter', () => { } }) - describe('when provided returns', () => { + describe('when provided with returns data', () => { it('correctly presents the data', () => { const result = ViewLicenceReturnsPresenter.go(returnData) @@ -87,14 +87,14 @@ describe('View Licence returns presenter', () => { }) }) - describe('when no returns', () => { + describe('when provided with NO returns data', () => { beforeEach(() => { returnData = { returns: [] } }) - it('correctly returns the no returns data', () => { + it('correctly returns no returns data ', () => { const result = ViewLicenceReturnsPresenter.go(returnData) expect(result).to.equal({ From 552c5ed4931dcc47b40ab27c12de9901a7e11585 Mon Sep 17 00:00:00 2001 From: jonathangoulding Date: Tue, 11 Jun 2024 16:16:38 +0100 Subject: [PATCH 09/23] chore: pre pr check --- app/services/licences/fetch-licence-has-requirements.service.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/services/licences/fetch-licence-has-requirements.service.js b/app/services/licences/fetch-licence-has-requirements.service.js index e920448349..ab495feb84 100644 --- a/app/services/licences/fetch-licence-has-requirements.service.js +++ b/app/services/licences/fetch-licence-has-requirements.service.js @@ -20,7 +20,7 @@ async function go (licenceId) { return !!requirement } -async function _fetch (licenceId, page) { +async function _fetch (licenceId) { return ReturnVersionModel.query() .select([ 'id' From ddf7ea9a10f8e7a816cf891fc7fd9c478041e823 Mon Sep 17 00:00:00 2001 From: jonathangoulding Date: Wed, 12 Jun 2024 11:15:25 +0100 Subject: [PATCH 10/23] refactor: has requirements into the returns service --- .../fetch-licence-has-requirements.service.js | 34 --------------- .../licences/view-licence-returns.service.js | 16 ++++++- ...h-licence-has-requirements.service.test.js | 42 ------------------- .../view-licence-returns.service.test.js | 37 ++++++++++++---- 4 files changed, 44 insertions(+), 85 deletions(-) delete mode 100644 app/services/licences/fetch-licence-has-requirements.service.js delete mode 100644 test/services/licences/fetch-licence-has-requirements.service.test.js diff --git a/app/services/licences/fetch-licence-has-requirements.service.js b/app/services/licences/fetch-licence-has-requirements.service.js deleted file mode 100644 index ab495feb84..0000000000 --- a/app/services/licences/fetch-licence-has-requirements.service.js +++ /dev/null @@ -1,34 +0,0 @@ -'use strict' - -/** - * Fetches requirements for a licence and returns and returns a boolean if the licence has requirements for returns - * @module FetchLicenceHasRequirementsService - */ - -const ReturnVersionModel = require('../../models/return-version.model.js') - -/** - * Fetches requirements for a licence and returns a boolean if the licence has requirements for returns - * - * @param {string} licenceId - The UUID for the licence id to fetch - * - * @returns {Promise} the result of check if a licence has requirements for returns - */ -async function go (licenceId) { - const requirement = await _fetch(licenceId) - - return !!requirement -} - -async function _fetch (licenceId) { - return ReturnVersionModel.query() - .select([ - 'id' - ]) - .where('licenceId', licenceId) - .first() -} - -module.exports = { - go -} diff --git a/app/services/licences/view-licence-returns.service.js b/app/services/licences/view-licence-returns.service.js index fff040b969..7e8544f3bb 100644 --- a/app/services/licences/view-licence-returns.service.js +++ b/app/services/licences/view-licence-returns.service.js @@ -6,8 +6,8 @@ */ const FetchLicenceReturnsService = require('./fetch-licence-returns.service.js') -const FetchLicenceHasRequirementsService = require('./fetch-licence-has-requirements.service.js') const PaginatorPresenter = require('../../presenters/paginator.presenter.js') +const ReturnVersionModel = require('../../models/return-version.model.js') const ViewLicenceReturnsPresenter = require('../../presenters/licences/view-licence-returns.presenter.js') const ViewLicenceService = require('./view-licence.service.js') @@ -26,7 +26,7 @@ async function go (licenceId, auth, page) { const returnsData = await FetchLicenceReturnsService.go(licenceId, page) const pageData = ViewLicenceReturnsPresenter.go(returnsData) - const hasRequirements = await FetchLicenceHasRequirementsService.go(licenceId) + const hasRequirements = await LicenceHasRequirements(licenceId) const pagination = PaginatorPresenter.go(returnsData.pagination.total, Number(page), `/system/licences/${licenceId}/returns`) @@ -38,6 +38,18 @@ async function go (licenceId, auth, page) { } } +async function LicenceHasRequirements (licenceId) { + const requirement = await ReturnVersionModel.query() + .select([ + 'id' + ]) + .where('licenceId', licenceId) + .first() + .limit(1) + + return !!requirement +} + module.exports = { go } diff --git a/test/services/licences/fetch-licence-has-requirements.service.test.js b/test/services/licences/fetch-licence-has-requirements.service.test.js deleted file mode 100644 index c7663e91ed..0000000000 --- a/test/services/licences/fetch-licence-has-requirements.service.test.js +++ /dev/null @@ -1,42 +0,0 @@ -'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 ReturnVersionHelper = require('../../support/helpers/return-version.helper.js') - -// Thing under test -const FetchLicenceHasRequirementsService = - require('../../../app/services/licences/fetch-licence-has-requirements.service.js') - -describe('Fetch Licence Has Requirements service', () => { - let returnVersion - - beforeEach(async () => { - await DatabaseSupport.clean() - }) - - describe('when the licence has return versions data', () => { - beforeEach(async () => { - returnVersion = await ReturnVersionHelper.add() - }) - - it('returns true a licence has any return versions', async () => { - const result = await FetchLicenceHasRequirementsService.go(returnVersion.licenceId) - - expect(result).to.be.true() - }) - - it('returns false no return versions for licence', async () => { - const result = await FetchLicenceHasRequirementsService.go('ed3b9b1a-94e0-480c-8ad6-60e05f5fa9f4') - - expect(result).to.be.false() - }) - }) -}) diff --git a/test/services/licences/view-licence-returns.service.test.js b/test/services/licences/view-licence-returns.service.test.js index 8ea59c1420..b4b2b8668d 100644 --- a/test/services/licences/view-licence-returns.service.test.js +++ b/test/services/licences/view-licence-returns.service.test.js @@ -9,8 +9,9 @@ const { describe, it, beforeEach, afterEach } = exports.lab = Lab.script() const { expect } = Code // Things we need to stub +const DatabaseSupport = require('../../support/database.js') +const ReturnVersionHelper = require('../../support/helpers/return-version.helper.js') const FetchLicenceReturnsService = require('../../../app/services/licences/fetch-licence-returns.service.js') -const FetchLicenceHasRequirementsService = require('../../../app/services/licences/fetch-licence-has-requirements.service.js') const PaginatorPresenter = require('../../../app/presenters/paginator.presenter.js') const ViewLicenceReturnsPresenter = require('../../../app/presenters/licences/view-licence-returns.presenter.js') const ViewLicenceService = require('../../../app/services/licences/view-licence.service.js') @@ -18,14 +19,18 @@ const ViewLicenceService = require('../../../app/services/licences/view-licence. // Thing under test const ViewLicenceReturnsService = require('../../../app/services/licences/view-licence-returns.service.js') -describe('View Licence service returns', () => { - const testId = '2c80bd22-a005-4cf4-a2a2-73812a9861de' +describe.only('View Licence service returns', () => { + let returnVersion + const page = 1 const auth = {} const pagination = { page } - beforeEach(() => { - Sinon.stub(FetchLicenceHasRequirementsService, 'go').resolves(true) + beforeEach(async () => { + await DatabaseSupport.clean() + + returnVersion = await ReturnVersionHelper.add() + Sinon.stub(FetchLicenceReturnsService, 'go').resolves(_returnsFetch()) Sinon.stub(PaginatorPresenter, 'go').returns(pagination) Sinon.stub(ViewLicenceReturnsPresenter, 'go').returns(_returnsPresenter()) @@ -36,10 +41,10 @@ describe('View Licence service returns', () => { Sinon.restore() }) - describe('when a return', () => { + describe.only('when a return', () => { 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 ViewLicenceReturnsService.go(testId, auth, page) + const result = await ViewLicenceReturnsService.go(returnVersion.licenceId, auth, page) expect(result).to.equal({ licenceName: 'fake licence', @@ -50,6 +55,24 @@ describe('View Licence service returns', () => { }) }) }) + + describe('the "hasRequirements" property', () => { + describe('when the licence has requirements', () => { + it('will return true', async () => { + const result = await ViewLicenceReturnsService.go(returnVersion.licenceId, auth, page) + + expect(result.hasRequirements).to.be.true() + }) + }) + + describe('when the licence does not have requirements', () => { + it('will return false', async () => { + const result = await ViewLicenceReturnsService.go('a36e7556-fd86-4fbd-a20a-92fcb9f92836', auth, page) + + expect(result.hasRequirements).to.be.false() + }) + }) + }) }) }) From 6c51c5f55e87eeec3633c955349dee431e8a8b96 Mon Sep 17 00:00:00 2001 From: jonathangoulding Date: Wed, 12 Jun 2024 11:16:39 +0100 Subject: [PATCH 11/23] chore: rename to private method --- app/services/licences/view-licence-returns.service.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/services/licences/view-licence-returns.service.js b/app/services/licences/view-licence-returns.service.js index 7e8544f3bb..6203c35060 100644 --- a/app/services/licences/view-licence-returns.service.js +++ b/app/services/licences/view-licence-returns.service.js @@ -26,7 +26,7 @@ async function go (licenceId, auth, page) { const returnsData = await FetchLicenceReturnsService.go(licenceId, page) const pageData = ViewLicenceReturnsPresenter.go(returnsData) - const hasRequirements = await LicenceHasRequirements(licenceId) + const hasRequirements = await _licenceHasRequirements(licenceId) const pagination = PaginatorPresenter.go(returnsData.pagination.total, Number(page), `/system/licences/${licenceId}/returns`) @@ -38,7 +38,7 @@ async function go (licenceId, auth, page) { } } -async function LicenceHasRequirements (licenceId) { +async function _licenceHasRequirements (licenceId) { const requirement = await ReturnVersionModel.query() .select([ 'id' From 3fe3d754cd8e7037b971f785d70172ff703e71b0 Mon Sep 17 00:00:00 2001 From: jonathangoulding Date: Wed, 12 Jun 2024 11:17:01 +0100 Subject: [PATCH 12/23] chore: remove .only --- test/services/licences/view-licence-returns.service.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/services/licences/view-licence-returns.service.test.js b/test/services/licences/view-licence-returns.service.test.js index b4b2b8668d..92a8bc61f2 100644 --- a/test/services/licences/view-licence-returns.service.test.js +++ b/test/services/licences/view-licence-returns.service.test.js @@ -19,7 +19,7 @@ const ViewLicenceService = require('../../../app/services/licences/view-licence. // Thing under test const ViewLicenceReturnsService = require('../../../app/services/licences/view-licence-returns.service.js') -describe.only('View Licence service returns', () => { +describe('View Licence service returns', () => { let returnVersion const page = 1 From a5ba5ad2e6ecd9904218c7de6879fa36c5d0a581 Mon Sep 17 00:00:00 2001 From: jonathangoulding Date: Wed, 12 Jun 2024 11:21:36 +0100 Subject: [PATCH 13/23] chore: remove .only --- test/services/licences/view-licence-returns.service.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/services/licences/view-licence-returns.service.test.js b/test/services/licences/view-licence-returns.service.test.js index 92a8bc61f2..e6181aeb11 100644 --- a/test/services/licences/view-licence-returns.service.test.js +++ b/test/services/licences/view-licence-returns.service.test.js @@ -41,7 +41,7 @@ describe('View Licence service returns', () => { Sinon.restore() }) - describe.only('when a return', () => { + describe('when a return', () => { 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 ViewLicenceReturnsService.go(returnVersion.licenceId, auth, page) From ace05de256935040a41efa798b5d0489fc1d6c52 Mon Sep 17 00:00:00 2001 From: jonathangoulding Date: Wed, 12 Jun 2024 12:04:20 +0100 Subject: [PATCH 14/23] refactor: error message implementation use njk component for table --- .../view-licence-returns.presenter.js | 23 +++++- .../fetch-licence-has-requirements.service.js | 34 ++++++++ .../licences/view-licence-returns.service.js | 21 +---- app/views/licences/tabs/returns.njk | 79 +++++++++++-------- test/controllers/licences.controller.test.js | 40 +++------- .../view-licence-returns-presenter.test.js | 67 ++++++++++------ ...h-licence-has-requirements.service.test.js | 42 ++++++++++ .../view-licence-returns.service.test.js | 64 ++++----------- 8 files changed, 216 insertions(+), 154 deletions(-) create mode 100644 app/services/licences/fetch-licence-has-requirements.service.js create mode 100644 test/services/licences/fetch-licence-has-requirements.service.test.js diff --git a/app/presenters/licences/view-licence-returns.presenter.js b/app/presenters/licences/view-licence-returns.presenter.js index 5fb76e9371..93b0284157 100644 --- a/app/presenters/licences/view-licence-returns.presenter.js +++ b/app/presenters/licences/view-licence-returns.presenter.js @@ -10,15 +10,20 @@ const { formatLongDate } = require('../base.presenter.js') /** * Formats common data for the `/licences/{id}/*` view licence pages * + * @param {module:ReturnLogModel[]} returnsData - The session for the return requirement journey + * @param {boolean} hasRequirements - If the licence has return versions then it has requirements + * * @returns {Object} The data formatted for the view template */ -function go (returnsData) { - const returns = _formatReturnToTableRow(returnsData.returns) +function go (returnsData, hasRequirements) { + const returns = _formatReturnToTableRow(returnsData) + + const hasReturns = returns.length > 0 return { activeTab: 'returns', returns, - hasReturns: returns.length > 0 + noReturnsMessage: _noReturnsMessage(hasReturns, hasRequirements) } } @@ -54,6 +59,18 @@ function _formatStatus (status) { return 'NO STATUS' } +function _noReturnsMessage (hasReturns, hasRequirements) { + if (!hasReturns && !hasRequirements) { + return 'No requirements for returns have been set up for this licence.' + } + + if (hasRequirements && !hasReturns) { + return 'No returns for this licence.' + } + + return null +} + module.exports = { go } diff --git a/app/services/licences/fetch-licence-has-requirements.service.js b/app/services/licences/fetch-licence-has-requirements.service.js new file mode 100644 index 0000000000..ab495feb84 --- /dev/null +++ b/app/services/licences/fetch-licence-has-requirements.service.js @@ -0,0 +1,34 @@ +'use strict' + +/** + * Fetches requirements for a licence and returns and returns a boolean if the licence has requirements for returns + * @module FetchLicenceHasRequirementsService + */ + +const ReturnVersionModel = require('../../models/return-version.model.js') + +/** + * Fetches requirements for a licence and returns a boolean if the licence has requirements for returns + * + * @param {string} licenceId - The UUID for the licence id to fetch + * + * @returns {Promise} the result of check if a licence has requirements for returns + */ +async function go (licenceId) { + const requirement = await _fetch(licenceId) + + return !!requirement +} + +async function _fetch (licenceId) { + return ReturnVersionModel.query() + .select([ + 'id' + ]) + .where('licenceId', licenceId) + .first() +} + +module.exports = { + go +} diff --git a/app/services/licences/view-licence-returns.service.js b/app/services/licences/view-licence-returns.service.js index 6203c35060..4f52fe61e6 100644 --- a/app/services/licences/view-licence-returns.service.js +++ b/app/services/licences/view-licence-returns.service.js @@ -6,8 +6,8 @@ */ const FetchLicenceReturnsService = require('./fetch-licence-returns.service.js') +const FetchLicenceHasRequirementsService = require('./fetch-licence-has-requirements.service.js') const PaginatorPresenter = require('../../presenters/paginator.presenter.js') -const ReturnVersionModel = require('../../models/return-version.model.js') const ViewLicenceReturnsPresenter = require('../../presenters/licences/view-licence-returns.presenter.js') const ViewLicenceService = require('./view-licence.service.js') @@ -23,33 +23,20 @@ const ViewLicenceService = require('./view-licence.service.js') async function go (licenceId, auth, page) { const commonData = await ViewLicenceService.go(licenceId, auth) - const returnsData = await FetchLicenceReturnsService.go(licenceId, page) - const pageData = ViewLicenceReturnsPresenter.go(returnsData) + const hasRequirements = await FetchLicenceHasRequirementsService.go(licenceId) - const hasRequirements = await _licenceHasRequirements(licenceId) + const returnsData = await FetchLicenceReturnsService.go(licenceId, page) + const pageData = ViewLicenceReturnsPresenter.go(returnsData.returns, hasRequirements) const pagination = PaginatorPresenter.go(returnsData.pagination.total, Number(page), `/system/licences/${licenceId}/returns`) return { ...pageData, ...commonData, - hasRequirements, pagination } } -async function _licenceHasRequirements (licenceId) { - const requirement = await ReturnVersionModel.query() - .select([ - 'id' - ]) - .where('licenceId', licenceId) - .first() - .limit(1) - - return !!requirement -} - module.exports = { go } diff --git a/app/views/licences/tabs/returns.njk b/app/views/licences/tabs/returns.njk index 467f97dd0e..dd6e797240 100644 --- a/app/views/licences/tabs/returns.njk +++ b/app/views/licences/tabs/returns.njk @@ -19,37 +19,54 @@ {% endif %} {% endmacro %} - -

Returns

- {% if hasReturns %} - - - - - - - - - - {% for return in returns %} - - - - - - - {% endfor %} - - - {% elseif hasReturns == false and hasRequirements == false %} -

No requirements for returns have been set up for this licence.

- {% elseif hasRequirements and hasReturns == false %} -

No returns for this licence.

- {% endif %} -
Return reference and datesPurpose and descriptionDue dateStatus
- {{ return.reference }} -

{{ return.dates }}

- 		{{ return.purpose }}

{{ return.description }}

{{ return.dueDate }}{{ formatStatus(return.status) }}
+

Returns

+ +{% if noReturnsMessage %} +

{{ noReturnsMessage }}

+{% else %} + + {% macro referneceColum(return) %} + {{ return.reference }} +

{{ return.dates }}

+ {% endmacro %} + + {% set returnRows = [] %} + + {% for returnItem in returns %} + {% set returnRows = (returnRows.push([ + { + html: referneceColum(returnItem) + }, + { + html: returnItem.purpose + "

" + returnItem.description + "

" + }, + { + text: returnItem.dueDate + }, + { + html: formatStatus(returnItem.status) + } + ]), returnRows) %} + {% endfor %} + + {{ govukTable({ + head: [ + { + text: 'Return reference and dates' + }, + { + text: 'Purpose and description' + }, + { + text: 'Due date' + }, + { + text: 'Status' + } + ], + rows: returnRows + }) }} +{% endif %} {% if hasReturns and pagination.numberOfPages > 1 %} {{ govukPagination(pagination.component) }} diff --git a/test/controllers/licences.controller.test.js b/test/controllers/licences.controller.test.js index 93db5e72f1..dc7f051078 100644 --- a/test/controllers/licences.controller.test.js +++ b/test/controllers/licences.controller.test.js @@ -403,9 +403,15 @@ describe('Licences controller', () => { } }) - describe('when a request is valid and has returns', () => { + describe('when a request is valid and has returns and the licence has requirements', () => { beforeEach(async () => { - Sinon.stub(ViewLicenceReturnsService, 'go').resolves(_viewLicenceReturns()) + Sinon.stub(ViewLicenceReturnsService, 'go').resolves({ + activeTab: 'returns', + returns: [ + { id: 'returns-id' } + ], + noReturnsMessage: null + }) }) it('returns the page successfully', async () => { @@ -426,8 +432,7 @@ describe('Licences controller', () => { Sinon.stub(ViewLicenceReturnsService, 'go').resolves({ activeTab: 'returns', returns: [], - hasReturns: false, - hasRequirements: true + noReturnsMessage: 'No returns for this licence.' }) }) @@ -439,25 +444,6 @@ describe('Licences controller', () => { expect(response.payload).to.contain('No returns for this licence.') }) }) - - describe('when a request is valid and has NO requirements and NO returns', () => { - beforeEach(async () => { - Sinon.stub(ViewLicenceReturnsService, 'go').resolves({ - activeTab: 'returns', - returns: [], - hasReturns: false, - hasRequirements: false - }) - }) - - it('returns the page successfully with the message \'No returns for this licence.\'', async () => { - const response = await server.inject(options) - - expect(response.statusCode).to.equal(200) - expect(response.payload).to.contain('Returns') - expect(response.payload).to.contain('No requirements for returns have been set up for this licence.') - }) - }) }) }) @@ -494,14 +480,6 @@ function _viewLicenceContactDetails () { } } -function _viewLicenceReturns () { - return { - activeTab: 'returns', - hasReturns: true, - returns: [{ id: 'returns-id' }] - } -} - function _viewLicenceSummary () { return { id: '7861814c-ca19-43f2-be11-3c612f0d744b', diff --git a/test/presenters/licences/view-licence-returns-presenter.test.js b/test/presenters/licences/view-licence-returns-presenter.test.js index a562bb4ded..ed13c56ecd 100644 --- a/test/presenters/licences/view-licence-returns-presenter.test.js +++ b/test/presenters/licences/view-licence-returns-presenter.test.js @@ -11,7 +11,8 @@ const { expect } = Code const ViewLicenceReturnsPresenter = require('../../../app/presenters/licences/view-licence-returns.presenter.js') describe('View Licence returns presenter', () => { - let returnData + let returnsData + let hasRequirements const returnItem = { id: 'mock-id-1', @@ -43,26 +44,25 @@ describe('View Licence returns presenter', () => { } beforeEach(() => { - returnData = { - returns: [ - { ...returnItem }, - { - ...returnItem, - id: 'mock-id-2', - status: 'due', - returnReference: '1069' - } - ] - } + hasRequirements = true + returnsData = [ + { ...returnItem }, + { + ...returnItem, + id: 'mock-id-2', + status: 'due', + returnReference: '1069' + } + ] }) describe('when provided with returns data', () => { it('correctly presents the data', () => { - const result = ViewLicenceReturnsPresenter.go(returnData) + const result = ViewLicenceReturnsPresenter.go(returnsData, hasRequirements) expect(result).to.equal({ activeTab: 'returns', - hasReturns: true, + noReturnsMessage: null, returns: [ { id: 'mock-id-1', @@ -87,20 +87,37 @@ describe('View Licence returns presenter', () => { }) }) - describe('when provided with NO returns data', () => { - beforeEach(() => { - returnData = { - returns: [] - } + describe('the "noReturnsMessage" property', () => { + describe('when a licence has returns and requirements', () => { + it('returns null', () => { + const result = ViewLicenceReturnsPresenter.go(returnsData, hasRequirements) + + expect(result.noReturnsMessage).to.be.null() + }) }) - it('correctly returns no returns data ', () => { - const result = ViewLicenceReturnsPresenter.go(returnData) + describe('when a licence has NO returns and NO requirements', () => { + beforeEach(() => { + returnsData = [] + hasRequirements = false + }) - expect(result).to.equal({ - activeTab: 'returns', - hasReturns: false, - returns: [] + it('presents the No returns and No requirements message "No requirements for returns have been set up for this licence."', () => { + const result = ViewLicenceReturnsPresenter.go(returnsData, hasRequirements) + + expect(result.noReturnsMessage).to.equal('No requirements for returns have been set up for this licence.') + }) + }) + + describe('when a licence has returns but no requirements', () => { + beforeEach(() => { + returnsData = [] + }) + + it('presents the returns but no requirements message "No returns for this licence."', () => { + const result = ViewLicenceReturnsPresenter.go(returnsData, hasRequirements) + + expect(result.noReturnsMessage).to.equal('No returns for this licence.') }) }) }) diff --git a/test/services/licences/fetch-licence-has-requirements.service.test.js b/test/services/licences/fetch-licence-has-requirements.service.test.js new file mode 100644 index 0000000000..c7663e91ed --- /dev/null +++ b/test/services/licences/fetch-licence-has-requirements.service.test.js @@ -0,0 +1,42 @@ +'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 ReturnVersionHelper = require('../../support/helpers/return-version.helper.js') + +// Thing under test +const FetchLicenceHasRequirementsService = + require('../../../app/services/licences/fetch-licence-has-requirements.service.js') + +describe('Fetch Licence Has Requirements service', () => { + let returnVersion + + beforeEach(async () => { + await DatabaseSupport.clean() + }) + + describe('when the licence has return versions data', () => { + beforeEach(async () => { + returnVersion = await ReturnVersionHelper.add() + }) + + it('returns true a licence has any return versions', async () => { + const result = await FetchLicenceHasRequirementsService.go(returnVersion.licenceId) + + expect(result).to.be.true() + }) + + it('returns false no return versions for licence', async () => { + const result = await FetchLicenceHasRequirementsService.go('ed3b9b1a-94e0-480c-8ad6-60e05f5fa9f4') + + expect(result).to.be.false() + }) + }) +}) diff --git a/test/services/licences/view-licence-returns.service.test.js b/test/services/licences/view-licence-returns.service.test.js index e6181aeb11..991dd8b47d 100644 --- a/test/services/licences/view-licence-returns.service.test.js +++ b/test/services/licences/view-licence-returns.service.test.js @@ -9,9 +9,8 @@ const { describe, it, beforeEach, afterEach } = exports.lab = Lab.script() const { expect } = Code // Things we need to stub -const DatabaseSupport = require('../../support/database.js') -const ReturnVersionHelper = require('../../support/helpers/return-version.helper.js') const FetchLicenceReturnsService = require('../../../app/services/licences/fetch-licence-returns.service.js') +const FetchLicenceHasRequirementsService = require('../../../app/services/licences/fetch-licence-has-requirements.service.js') const PaginatorPresenter = require('../../../app/presenters/paginator.presenter.js') const ViewLicenceReturnsPresenter = require('../../../app/presenters/licences/view-licence-returns.presenter.js') const ViewLicenceService = require('../../../app/services/licences/view-licence.service.js') @@ -20,21 +19,29 @@ const ViewLicenceService = require('../../../app/services/licences/view-licence. const ViewLicenceReturnsService = require('../../../app/services/licences/view-licence-returns.service.js') describe('View Licence service returns', () => { - let returnVersion - + const testId = '2c80bd22-a005-4cf4-a2a2-73812a9861de' const page = 1 const auth = {} const pagination = { page } beforeEach(async () => { - await DatabaseSupport.clean() + Sinon.stub(FetchLicenceHasRequirementsService, 'go').returns(true) - returnVersion = await ReturnVersionHelper.add() + Sinon.stub(FetchLicenceReturnsService, 'go').resolves({ + pagination: { total: 1 }, + returns: [] + }) - Sinon.stub(FetchLicenceReturnsService, 'go').resolves(_returnsFetch()) Sinon.stub(PaginatorPresenter, 'go').returns(pagination) - Sinon.stub(ViewLicenceReturnsPresenter, 'go').returns(_returnsPresenter()) - Sinon.stub(ViewLicenceService, 'go').resolves(_licence()) + + Sinon.stub(ViewLicenceReturnsPresenter, 'go').returns({ + returns: [], + activeTab: 'returns' + }) + + Sinon.stub(ViewLicenceService, 'go').resolves({ + licenceName: 'fake licence' + }) }) afterEach(() => { @@ -44,52 +51,15 @@ describe('View Licence service returns', () => { describe('when a return', () => { 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 ViewLicenceReturnsService.go(returnVersion.licenceId, auth, page) + const result = await ViewLicenceReturnsService.go(testId, auth, page) expect(result).to.equal({ licenceName: 'fake licence', - hasRequirements: true, returns: [], activeTab: 'returns', pagination: { page: 1 } }) }) }) - - describe('the "hasRequirements" property', () => { - describe('when the licence has requirements', () => { - it('will return true', async () => { - const result = await ViewLicenceReturnsService.go(returnVersion.licenceId, auth, page) - - expect(result.hasRequirements).to.be.true() - }) - }) - - describe('when the licence does not have requirements', () => { - it('will return false', async () => { - const result = await ViewLicenceReturnsService.go('a36e7556-fd86-4fbd-a20a-92fcb9f92836', auth, page) - - expect(result.hasRequirements).to.be.false() - }) - }) - }) }) }) - -function _licence () { - return { licenceName: 'fake licence' } -} - -function _returnsFetch () { - return { - pagination: { total: 1 }, - returns: [] - } -} - -function _returnsPresenter () { - return { - returns: [], - activeTab: 'returns' - } -} From ed310eda9a670806e88b32b16fe8f86169b8bb7a Mon Sep 17 00:00:00 2001 From: jonathangoulding Date: Wed, 12 Jun 2024 12:09:31 +0100 Subject: [PATCH 15/23] refactor: rename to DetermineLicenceHasReturnVersionsService --- ....js => determine-licence-has-return-versions.service.js} | 6 +++--- app/services/licences/view-licence-returns.service.js | 4 ++-- ...> determine-licence-has-return-versions.service.test.js} | 2 +- test/services/licences/view-licence-returns.service.test.js | 4 ++-- 4 files changed, 8 insertions(+), 8 deletions(-) rename app/services/licences/{fetch-licence-has-requirements.service.js => determine-licence-has-return-versions.service.js} (67%) rename test/services/licences/{fetch-licence-has-requirements.service.test.js => determine-licence-has-return-versions.service.test.js} (92%) diff --git a/app/services/licences/fetch-licence-has-requirements.service.js b/app/services/licences/determine-licence-has-return-versions.service.js similarity index 67% rename from app/services/licences/fetch-licence-has-requirements.service.js rename to app/services/licences/determine-licence-has-return-versions.service.js index ab495feb84..8f7f88b2e9 100644 --- a/app/services/licences/fetch-licence-has-requirements.service.js +++ b/app/services/licences/determine-licence-has-return-versions.service.js @@ -1,14 +1,14 @@ 'use strict' /** - * Fetches requirements for a licence and returns and returns a boolean if the licence has requirements for returns - * @module FetchLicenceHasRequirementsService + * Determines if a licence has requirements + * @module DetermineLicenceHasReturnVersionsService */ const ReturnVersionModel = require('../../models/return-version.model.js') /** - * Fetches requirements for a licence and returns a boolean if the licence has requirements for returns + * Determines if a licence has requirements * * @param {string} licenceId - The UUID for the licence id to fetch * diff --git a/app/services/licences/view-licence-returns.service.js b/app/services/licences/view-licence-returns.service.js index 4f52fe61e6..dbbe8472ea 100644 --- a/app/services/licences/view-licence-returns.service.js +++ b/app/services/licences/view-licence-returns.service.js @@ -5,8 +5,8 @@ * @module ViewLicenceSummaryService */ +const DetermineLicenceHasReturnVersionsService = require('./determine-licence-has-return-versions.service.js') const FetchLicenceReturnsService = require('./fetch-licence-returns.service.js') -const FetchLicenceHasRequirementsService = require('./fetch-licence-has-requirements.service.js') const PaginatorPresenter = require('../../presenters/paginator.presenter.js') const ViewLicenceReturnsPresenter = require('../../presenters/licences/view-licence-returns.presenter.js') const ViewLicenceService = require('./view-licence.service.js') @@ -23,7 +23,7 @@ const ViewLicenceService = require('./view-licence.service.js') async function go (licenceId, auth, page) { const commonData = await ViewLicenceService.go(licenceId, auth) - const hasRequirements = await FetchLicenceHasRequirementsService.go(licenceId) + const hasRequirements = await DetermineLicenceHasReturnVersionsService.go(licenceId) const returnsData = await FetchLicenceReturnsService.go(licenceId, page) const pageData = ViewLicenceReturnsPresenter.go(returnsData.returns, hasRequirements) diff --git a/test/services/licences/fetch-licence-has-requirements.service.test.js b/test/services/licences/determine-licence-has-return-versions.service.test.js similarity index 92% rename from test/services/licences/fetch-licence-has-requirements.service.test.js rename to test/services/licences/determine-licence-has-return-versions.service.test.js index c7663e91ed..041c712ae2 100644 --- a/test/services/licences/fetch-licence-has-requirements.service.test.js +++ b/test/services/licences/determine-licence-has-return-versions.service.test.js @@ -13,7 +13,7 @@ const ReturnVersionHelper = require('../../support/helpers/return-version.helper // Thing under test const FetchLicenceHasRequirementsService = - require('../../../app/services/licences/fetch-licence-has-requirements.service.js') + require('../../../app/services/licences/determine-licence-has-return-versions.service.js') describe('Fetch Licence Has Requirements service', () => { let returnVersion diff --git a/test/services/licences/view-licence-returns.service.test.js b/test/services/licences/view-licence-returns.service.test.js index 991dd8b47d..3ad8f1970c 100644 --- a/test/services/licences/view-licence-returns.service.test.js +++ b/test/services/licences/view-licence-returns.service.test.js @@ -9,8 +9,8 @@ const { describe, it, beforeEach, afterEach } = exports.lab = Lab.script() const { expect } = Code // Things we need to stub +const DetermineLicenceHasReturnVersionsService = require('../../../app/services/licences/determine-licence-has-return-versions.service.js') const FetchLicenceReturnsService = require('../../../app/services/licences/fetch-licence-returns.service.js') -const FetchLicenceHasRequirementsService = require('../../../app/services/licences/fetch-licence-has-requirements.service.js') const PaginatorPresenter = require('../../../app/presenters/paginator.presenter.js') const ViewLicenceReturnsPresenter = require('../../../app/presenters/licences/view-licence-returns.presenter.js') const ViewLicenceService = require('../../../app/services/licences/view-licence.service.js') @@ -25,7 +25,7 @@ describe('View Licence service returns', () => { const pagination = { page } beforeEach(async () => { - Sinon.stub(FetchLicenceHasRequirementsService, 'go').returns(true) + Sinon.stub(DetermineLicenceHasReturnVersionsService, 'go').returns(true) Sinon.stub(FetchLicenceReturnsService, 'go').resolves({ pagination: { total: 1 }, From d774fb28ad6d28a599a0bbc5c66aa637abdf81dc Mon Sep 17 00:00:00 2001 From: Jonathan Goulding <58443816+jonathangoulding@users.noreply.github.com> Date: Wed, 12 Jun 2024 17:14:40 +0100 Subject: [PATCH 16/23] Update app/views/licences/tabs/returns.njk Co-authored-by: Alan Cruikshanks --- app/views/licences/tabs/returns.njk | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/licences/tabs/returns.njk b/app/views/licences/tabs/returns.njk index dd6e797240..430570e79b 100644 --- a/app/views/licences/tabs/returns.njk +++ b/app/views/licences/tabs/returns.njk @@ -25,7 +25,7 @@

{{ noReturnsMessage }}

{% else %} - {% macro referneceColum(return) %} + {% macro referenceColumn(return) %} {{ return.reference }}

{{ return.dates }}

{% endmacro %} From 705fd1ca1eafbd615e44d8b8c17a68119b915e28 Mon Sep 17 00:00:00 2001 From: Jonathan Goulding <58443816+jonathangoulding@users.noreply.github.com> Date: Wed, 12 Jun 2024 17:14:46 +0100 Subject: [PATCH 17/23] Update app/views/licences/tabs/returns.njk Co-authored-by: Alan Cruikshanks --- app/views/licences/tabs/returns.njk | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/licences/tabs/returns.njk b/app/views/licences/tabs/returns.njk index 430570e79b..60e0eb8f02 100644 --- a/app/views/licences/tabs/returns.njk +++ b/app/views/licences/tabs/returns.njk @@ -35,7 +35,7 @@ {% for returnItem in returns %} {% set returnRows = (returnRows.push([ { - html: referneceColum(returnItem) + html: referenceColumn(returnItem) }, { html: returnItem.purpose + "

" + returnItem.description + "

" From e46c807b16132e3d965ee1853e8a71065e96fe93 Mon Sep 17 00:00:00 2001 From: Jonathan Goulding <58443816+jonathangoulding@users.noreply.github.com> Date: Wed, 12 Jun 2024 17:15:58 +0100 Subject: [PATCH 18/23] Update app/services/licences/determine-licence-has-return-versions.service.js Co-authored-by: Alan Cruikshanks --- .../licences/determine-licence-has-return-versions.service.js | 1 + 1 file changed, 1 insertion(+) diff --git a/app/services/licences/determine-licence-has-return-versions.service.js b/app/services/licences/determine-licence-has-return-versions.service.js index 8f7f88b2e9..652a511e4f 100644 --- a/app/services/licences/determine-licence-has-return-versions.service.js +++ b/app/services/licences/determine-licence-has-return-versions.service.js @@ -26,6 +26,7 @@ async function _fetch (licenceId) { 'id' ]) .where('licenceId', licenceId) + .limit(1) .first() } From f5b2ec3eed64e863479e7c72aba30eb59cbf06d7 Mon Sep 17 00:00:00 2001 From: Jonathan Goulding <58443816+jonathangoulding@users.noreply.github.com> Date: Wed, 12 Jun 2024 17:16:16 +0100 Subject: [PATCH 19/23] Update app/services/licences/determine-licence-has-return-versions.service.js Co-authored-by: Alan Cruikshanks --- .../licences/determine-licence-has-return-versions.service.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/services/licences/determine-licence-has-return-versions.service.js b/app/services/licences/determine-licence-has-return-versions.service.js index 652a511e4f..56b0521ca5 100644 --- a/app/services/licences/determine-licence-has-return-versions.service.js +++ b/app/services/licences/determine-licence-has-return-versions.service.js @@ -10,7 +10,7 @@ const ReturnVersionModel = require('../../models/return-version.model.js') /** * Determines if a licence has requirements * - * @param {string} licenceId - The UUID for the licence id to fetch + * @param {string} licenceId - The UUID of the licence to determine if return versions exist * * @returns {Promise} the result of check if a licence has requirements for returns */ From d6e80c23c82cf018d56cf630852ae4cccbc747a8 Mon Sep 17 00:00:00 2001 From: Jonathan Goulding <58443816+jonathangoulding@users.noreply.github.com> Date: Wed, 12 Jun 2024 17:16:30 +0100 Subject: [PATCH 20/23] Update app/services/licences/determine-licence-has-return-versions.service.js Co-authored-by: Alan Cruikshanks --- .../licences/determine-licence-has-return-versions.service.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/services/licences/determine-licence-has-return-versions.service.js b/app/services/licences/determine-licence-has-return-versions.service.js index 56b0521ca5..c9a6ea4009 100644 --- a/app/services/licences/determine-licence-has-return-versions.service.js +++ b/app/services/licences/determine-licence-has-return-versions.service.js @@ -12,7 +12,7 @@ const ReturnVersionModel = require('../../models/return-version.model.js') * * @param {string} licenceId - The UUID of the licence to determine if return versions exist * - * @returns {Promise} the result of check if a licence has requirements for returns + * @returns {Promise} true if the licence has return versions else false */ async function go (licenceId) { const requirement = await _fetch(licenceId) From c639d27121dd010cf325a0625d2dd626dda6827f Mon Sep 17 00:00:00 2001 From: Jonathan Goulding <58443816+jonathangoulding@users.noreply.github.com> Date: Wed, 12 Jun 2024 17:18:47 +0100 Subject: [PATCH 21/23] Update app/views/licences/tabs/returns.njk Co-authored-by: Alan Cruikshanks --- app/views/licences/tabs/returns.njk | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/licences/tabs/returns.njk b/app/views/licences/tabs/returns.njk index 60e0eb8f02..db2861f6a5 100644 --- a/app/views/licences/tabs/returns.njk +++ b/app/views/licences/tabs/returns.njk @@ -68,7 +68,7 @@ }) }} {% endif %} -{% if hasReturns and pagination.numberOfPages > 1 %} +{% if pagination.numberOfPages > 1 %} {{ govukPagination(pagination.component) }} {% endif %} From 6eef66e00bc3640786e0adb6d15089cea49d037d Mon Sep 17 00:00:00 2001 From: jonathangoulding Date: Wed, 12 Jun 2024 17:22:59 +0100 Subject: [PATCH 22/23] fix: pr isssues --- app/views/licences/tabs/returns.njk | 2 +- test/controllers/licences.controller.test.js | 20 +------------------- 2 files changed, 2 insertions(+), 20 deletions(-) diff --git a/app/views/licences/tabs/returns.njk b/app/views/licences/tabs/returns.njk index 60e0eb8f02..db2861f6a5 100644 --- a/app/views/licences/tabs/returns.njk +++ b/app/views/licences/tabs/returns.njk @@ -68,7 +68,7 @@ }) }} {% endif %} -{% if hasReturns and pagination.numberOfPages > 1 %} +{% if pagination.numberOfPages > 1 %} {{ govukPagination(pagination.component) }} {% endif %} diff --git a/test/controllers/licences.controller.test.js b/test/controllers/licences.controller.test.js index dc7f051078..57ba36f6ae 100644 --- a/test/controllers/licences.controller.test.js +++ b/test/controllers/licences.controller.test.js @@ -403,7 +403,7 @@ describe('Licences controller', () => { } }) - describe('when a request is valid and has returns and the licence has requirements', () => { + describe('when a request is valid', () => { beforeEach(async () => { Sinon.stub(ViewLicenceReturnsService, 'go').resolves({ activeTab: 'returns', @@ -426,24 +426,6 @@ describe('Licences controller', () => { expect(response.payload).to.contain('Status') }) }) - - describe('when a request is valid and has requirements but NO returns', () => { - beforeEach(async () => { - Sinon.stub(ViewLicenceReturnsService, 'go').resolves({ - activeTab: 'returns', - returns: [], - noReturnsMessage: 'No returns for this licence.' - }) - }) - - it('returns the page successfully with the message \'No returns for this licence.\'', async () => { - const response = await server.inject(options) - - expect(response.statusCode).to.equal(200) - expect(response.payload).to.contain('Returns') - expect(response.payload).to.contain('No returns for this licence.') - }) - }) }) }) From 30bab8d472d19cc094d422e70a7144fb3750cedc Mon Sep 17 00:00:00 2001 From: Jonathan Goulding <58443816+jonathangoulding@users.noreply.github.com> Date: Wed, 12 Jun 2024 17:24:12 +0100 Subject: [PATCH 23/23] Update test/services/licences/determine-licence-has-return-versions.service.test.js Co-authored-by: Alan Cruikshanks --- ...ine-licence-has-return-versions.service.test.js | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/test/services/licences/determine-licence-has-return-versions.service.test.js b/test/services/licences/determine-licence-has-return-versions.service.test.js index 041c712ae2..f103d363e7 100644 --- a/test/services/licences/determine-licence-has-return-versions.service.test.js +++ b/test/services/licences/determine-licence-has-return-versions.service.test.js @@ -16,24 +16,26 @@ const FetchLicenceHasRequirementsService = require('../../../app/services/licences/determine-licence-has-return-versions.service.js') describe('Fetch Licence Has Requirements service', () => { - let returnVersion + const licenceId = 'e004c0c9-0316-42fc-a6e3-5ae9a271b3c6' beforeEach(async () => { await DatabaseSupport.clean() }) - describe('when the licence has return versions data', () => { + describe('when the licence has return versions', () => { beforeEach(async () => { - returnVersion = await ReturnVersionHelper.add() + await ReturnVersionHelper.add({ licenceId }) }) - it('returns true a licence has any return versions', async () => { - const result = await FetchLicenceHasRequirementsService.go(returnVersion.licenceId) + it('returns true', async () => { + const result = await FetchLicenceHasRequirementsService.go(licenceId) expect(result).to.be.true() }) + }) - it('returns false no return versions for licence', async () => { + describe('when the licence does not have return versions', () => { + it('returns false', async () => { const result = await FetchLicenceHasRequirementsService.go('ed3b9b1a-94e0-480c-8ad6-60e05f5fa9f4') expect(result).to.be.false()