From 6a7e06a9cf26ee32a8df4a64f0d831ab4db8e029 Mon Sep 17 00:00:00 2001 From: jonathangoulding Date: Wed, 1 May 2024 15:47:13 +0100 Subject: [PATCH 01/33] Add View License Returns page The existing service handling view license 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 returns controller, service and presenter to handle the view license returns page. This will share he same view as the summary page and load the same 'common data' established in previous work. Both pieces of work are part of - https://eaflood.atlassian.net/browse/WATER-4444 Previous work as mentioned above - https://github.com/DEFRA/water-abstraction-system/pull/957 --- app/controllers/licences.controller.js | 16 +++++++- .../view-licence-returns.presenter.js | 22 ++++++++++ app/routes/licence.routes.js | 18 ++++++++- .../licences/view-license-returns.service.js | 29 ++++++++++++++ app/views/licences/tabs/returns.njk | 1 + app/views/licences/view.njk | 3 ++ test/controllers/licences.controller.test.js | 33 +++++++++++++++ .../view-licence-=returns-presenter.test.js | 33 +++++++++++++++ .../view-license-returns.service.test.js | 40 +++++++++++++++++++ 9 files changed, 191 insertions(+), 4 deletions(-) create mode 100644 app/presenters/licences/view-licence-returns.presenter.js create mode 100644 app/services/licences/view-license-returns.service.js create mode 100644 app/views/licences/tabs/returns.njk create mode 100644 test/presenters/licences/view-licence-=returns-presenter.test.js create mode 100644 test/services/licences/view-license-returns.service.test.js diff --git a/app/controllers/licences.controller.js b/app/controllers/licences.controller.js index 9b4cbaec4f..37d4bde6ee 100644 --- a/app/controllers/licences.controller.js +++ b/app/controllers/licences.controller.js @@ -7,7 +7,7 @@ const InitiateReturnRequirementSessionService = require('../services/return-requirements/initiate-return-requirement-session.service.js') const ViewLicenceSummaryService = require('../services/licences/view-license-summary.service') - +const ViewLicenceReturnsService = require('../services/licences/view-license-returns.service') async function noReturnsRequired (request, h) { const { id } = request.params @@ -35,8 +35,20 @@ async function viewSummary (request, h) { }) } +async function viewReturns (request, h) { + const { params: { id }, auth } = request + + const data = await ViewLicenceReturnsService.go(id, auth) + + return h.view('licences/view.njk', { + activeNavBar: 'search', + ...data + }) +} + module.exports = { noReturnsRequired, returnsRequired, - viewSummary + viewSummary, + viewReturns } diff --git a/app/presenters/licences/view-licence-returns.presenter.js b/app/presenters/licences/view-licence-returns.presenter.js new file mode 100644 index 0000000000..a6d92f94f2 --- /dev/null +++ b/app/presenters/licences/view-licence-returns.presenter.js @@ -0,0 +1,22 @@ +'use strict' + +/** + * Formats data for common licence data `/licences/{id}` page's + * @module ViewLicenceReturnsPresenter + */ + +/** + * Formats data for common licence data `/licences/{id}` page's + * + * @returns {Object} The data formatted for the view template + */ +function go () { + return { + activeTab: 'returns', + message: 'hello returns' + } +} + +module.exports = { + go +} diff --git a/app/routes/licence.routes.js b/app/routes/licence.routes.js index 562afaf3d9..e32286c744 100644 --- a/app/routes/licence.routes.js +++ b/app/routes/licence.routes.js @@ -13,9 +13,23 @@ const routes = [ scope: ['billing'] } }, - description: 'View a licence page' + description: 'View a summary licence page' } - }, { + }, + { + method: 'GET', + path: '/licences/{id}/returns', + handler: LicencesController.viewReturns, + options: { + auth: { + access: { + scope: ['billing'] + } + }, + description: 'View a returns licence page' + } + }, + { method: 'GET', path: '/licences/{id}/no-returns-required', handler: LicencesController.noReturnsRequired, diff --git a/app/services/licences/view-license-returns.service.js b/app/services/licences/view-license-returns.service.js new file mode 100644 index 0000000000..3dd1e36466 --- /dev/null +++ b/app/services/licences/view-license-returns.service.js @@ -0,0 +1,29 @@ +'use strict' + +/** + * Orchestrates fetching and presenting the data needed for the licence summary page + * @module ViewLicenceSummaryService + */ + +const ViewLicenceService = require('./view-licence.service') +const ViewLicenceReturnsPresenter = require('../../presenters/licences/view-licence-returns.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 = ViewLicenceReturnsPresenter.go() + + return { + ...pageData, + ...commonData + } +} + +module.exports = { + go +} diff --git a/app/views/licences/tabs/returns.njk b/app/views/licences/tabs/returns.njk new file mode 100644 index 0000000000..c45d548d7a --- /dev/null +++ b/app/views/licences/tabs/returns.njk @@ -0,0 +1 @@ +

{{ message }}

diff --git a/app/views/licences/view.njk b/app/views/licences/view.njk index 5a451d1fca..bd3f8c3f5e 100644 --- a/app/views/licences/view.njk +++ b/app/views/licences/view.njk @@ -79,6 +79,9 @@ {% if activeTab === 'summary' %} {% include "licences/tabs/summary.njk" %} {% endif %} + {% if activeTab === 'returns' %} + {% include "licences/tabs/returns.njk" %} + {% endif %} {% endblock %} diff --git a/test/controllers/licences.controller.test.js b/test/controllers/licences.controller.test.js index 4ce890e007..9e64d15cea 100644 --- a/test/controllers/licences.controller.test.js +++ b/test/controllers/licences.controller.test.js @@ -14,6 +14,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 ViewLicenceSummaryService = require('../../app/services/licences/view-license-summary.service') +const ViewLicenceReturnsService = require('../../app/services/licences/view-license-returns.service') // For running our service const { init } = require('../../app/server.js') @@ -188,4 +189,36 @@ describe('Licences controller', () => { } } }) + + describe.only('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', () => { + 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') + }) + }) + + function _viewLicenceReturns () { + return { + activeTab: 'returns' + } + } + }) }) diff --git a/test/presenters/licences/view-licence-=returns-presenter.test.js b/test/presenters/licences/view-licence-=returns-presenter.test.js new file mode 100644 index 0000000000..0dee8c8168 --- /dev/null +++ b/test/presenters/licences/view-licence-=returns-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 ViewLicenceReturnsPresenter = require('../../../app/presenters/licences/view-licence-returns.presenter') + +describe('View Licence returns presenter', () => { + let licence + beforeEach(() => { + licence = _licence() + }) + + describe('when provided with a populated licence', () => { + it('correctly presents the data', () => { + const result = ViewLicenceReturnsPresenter.go(licence) + + expect(result).to.equal({ + activeTab: 'returns', + message: 'hello returns' + }) + }) + }) +}) + +function _licence () { + return {} +} diff --git a/test/services/licences/view-license-returns.service.test.js b/test/services/licences/view-license-returns.service.test.js new file mode 100644 index 0000000000..9e8ba7a603 --- /dev/null +++ b/test/services/licences/view-license-returns.service.test.js @@ -0,0 +1,40 @@ +'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') +// Thing under test +const ViewLicenceReturnsService = require('../../../app/services/licences/view-license-returns.service') + +describe('View Licence service returns', () => { + const testId = '2c80bd22-a005-4cf4-a2a2-73812a9861de' + + beforeEach(() => { + Sinon.stub(ViewLicenceService, 'go').resolves({ licenceName: 'fake license' }) + }) + + afterEach(() => { + Sinon.restore() + }) + + 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(testId) + + expect(result).to.equal({ + activeTab: 'returns', + licenceName: 'fake license', + message: 'hello returns' + }) + }) + }) + }) +}) From da60d31912bf92614c1ade3578d68d499094079f Mon Sep 17 00:00:00 2001 From: jonathangoulding Date: Wed, 1 May 2024 15:49:49 +0100 Subject: [PATCH 02/33] test: remove .only --- 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 9e64d15cea..064ab811c2 100644 --- a/test/controllers/licences.controller.test.js +++ b/test/controllers/licences.controller.test.js @@ -190,7 +190,7 @@ describe('Licences controller', () => { } }) - describe.only('GET /licences/{id}/returns', () => { + describe('GET /licences/{id}/returns', () => { beforeEach(async () => { options = { method: 'GET', From a324ad363fbba71edaa5fcad91555158096bc216 Mon Sep 17 00:00:00 2001 From: jonathangoulding Date: Wed, 1 May 2024 16:35:38 +0100 Subject: [PATCH 03/33] feat: add initial structure to view and presenter --- .../view-licence-returns.presenter.js | 21 +++++++++++- app/views/licences/tabs/returns.njk | 34 ++++++++++++++++++- test/controllers/licences.controller.test.js | 5 +++ ...=> view-licence-returns-presenter.test.js} | 9 ++++- 4 files changed, 66 insertions(+), 3 deletions(-) rename test/presenters/licences/{view-licence-=returns-presenter.test.js => view-licence-returns-presenter.test.js} (75%) diff --git a/app/presenters/licences/view-licence-returns.presenter.js b/app/presenters/licences/view-licence-returns.presenter.js index a6d92f94f2..3ecac085d2 100644 --- a/app/presenters/licences/view-licence-returns.presenter.js +++ b/app/presenters/licences/view-licence-returns.presenter.js @@ -5,18 +5,37 @@ * @module ViewLicenceReturnsPresenter */ +const { formatLongDate } = require('../base.presenter') + /** * Formats data for common licence data `/licences/{id}` page's * * @returns {Object} The data formatted for the view template */ function go () { + const fakeReturn = [{ + reference: 'Mock reference', + purpose: 'mock purpose', + dueDate: '2023-04-28', + status: 'complete' + }] return { activeTab: 'returns', - message: 'hello returns' + returns: _formatToTableRow(fakeReturn) } } +function _formatToTableRow (returns) { + return returns.map(r => { + return { + reference: r.reference, + purpose: r.purpose, + dueDate: formatLongDate(new Date(r.dueDate)), + status: r.status.toLowerCase() + } + }) +} + module.exports = { go } diff --git a/app/views/licences/tabs/returns.njk b/app/views/licences/tabs/returns.njk index c45d548d7a..44116fc8f3 100644 --- a/app/views/licences/tabs/returns.njk +++ b/app/views/licences/tabs/returns.njk @@ -1 +1,33 @@ -

{{ message }}

+{% from "govuk/components/table/macro.njk" import govukTable %} +{% from "govuk/components/tag/macro.njk" import govukTag %} + +{% macro formatStatus(status) %} + {% if status === 'complete' %} + {{ govukTag({ + text: status, + classes: "govuk-tag--green" + }) }} + {% endif %} +{% endmacro %} + + + + + + + + + + + + + {% for return in returns %} + + + + + + + {% endfor %} + +
Returns
Return reference and datesPurpose and descriptionDue dateStatus
{{ return.reference }}{{ return.purpose }}{{ return.dueDate }}{{ formatStatus(return.status) }}
diff --git a/test/controllers/licences.controller.test.js b/test/controllers/licences.controller.test.js index 064ab811c2..ba5738b9cd 100644 --- a/test/controllers/licences.controller.test.js +++ b/test/controllers/licences.controller.test.js @@ -212,6 +212,11 @@ describe('Licences controller', () => { 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/presenters/licences/view-licence-=returns-presenter.test.js b/test/presenters/licences/view-licence-returns-presenter.test.js similarity index 75% rename from test/presenters/licences/view-licence-=returns-presenter.test.js rename to test/presenters/licences/view-licence-returns-presenter.test.js index 0dee8c8168..84e7a6c955 100644 --- a/test/presenters/licences/view-licence-=returns-presenter.test.js +++ b/test/presenters/licences/view-licence-returns-presenter.test.js @@ -22,7 +22,14 @@ describe('View Licence returns presenter', () => { expect(result).to.equal({ activeTab: 'returns', - message: 'hello returns' + returns: [ + { + dueDate: '28 April 2023', // date should be formatted + purpose: 'mock purpose', + reference: 'Mock reference', + status: 'complete' // lower case + } + ] }) }) }) From 4ed39e2d0b1b6593fc7915fb3a016dd170bf042f Mon Sep 17 00:00:00 2001 From: jonathangoulding Date: Thu, 2 May 2024 14:26:47 +0100 Subject: [PATCH 04/33] feat: add license return fetch service get data from returns table format data to presenter update tests --- .../view-licence-returns.presenter.js | 40 +++++++--- .../licences/fetch-licence-returns.service.js | 52 +++++++++++++ .../licences/view-license-returns.service.js | 5 +- app/views/licences/tabs/returns.njk | 13 +++- .../view-licence-returns-presenter.test.js | 78 ++++++++++++++++--- .../view-license-returns.service.test.js | 75 +++++++++++++++++- 6 files changed, 238 insertions(+), 25 deletions(-) create mode 100644 app/services/licences/fetch-licence-returns.service.js diff --git a/app/presenters/licences/view-licence-returns.presenter.js b/app/presenters/licences/view-licence-returns.presenter.js index 3ecac085d2..8d4083f6f9 100644 --- a/app/presenters/licences/view-licence-returns.presenter.js +++ b/app/presenters/licences/view-licence-returns.presenter.js @@ -12,26 +12,42 @@ const { formatLongDate } = require('../base.presenter') * * @returns {Object} The data formatted for the view template */ -function go () { - const fakeReturn = [{ - reference: 'Mock reference', - purpose: 'mock purpose', - dueDate: '2023-04-28', - status: 'complete' - }] +function go (returnsData) { + const returns = _formatReturnToTableRow(returnsData) + return { activeTab: 'returns', - returns: _formatToTableRow(fakeReturn) + returnsUrl: 'return/internal', + returns } } -function _formatToTableRow (returns) { +function _formatPurpose (purpose) { + const [firstPurpose] = purpose + return firstPurpose.alias ? firstPurpose.alias : firstPurpose.tertiary.description + /* Old Nunjucks converted to presenter logic + {% macro returnRequirementPurposes(return) %} + {% for purpose in return.returnRequirement.returnRequirementPurposes %} + {{ ', ' if not loop.first }} + {{ purpose.purposeAlias | titleCase if purpose.purposeAlias else purpose.purposeUse.name | titleCase }} + {% endfor %} + {% endmacro %} + */ +} + +function _formatStatus (status) { + if (status === 'completed') return 'COMPLETE' + if (status === 'due') return 'OVERDUE' + return 'NO STATUS' +} +function _formatReturnToTableRow (returns) { return returns.map(r => { return { - reference: r.reference, - purpose: r.purpose, + id: r.id, + reference: r.returnReference || 'No REF !!', + purpose: _formatPurpose(r.metadata.purposes), dueDate: formatLongDate(new Date(r.dueDate)), - status: r.status.toLowerCase() + status: _formatStatus(r.status) } }) } diff --git a/app/services/licences/fetch-licence-returns.service.js b/app/services/licences/fetch-licence-returns.service.js new file mode 100644 index 0000000000..892e5794cc --- /dev/null +++ b/app/services/licences/fetch-licence-returns.service.js @@ -0,0 +1,52 @@ +'use strict' + +/** + * Fetches data needed for the view '/licences/{id}` page + * @module FetchLicenceReturnsService + */ + +const LicenceModel = require('../../models/licence.model.js') +const ReturnLogModel = require('../../models/return-log.model') + +/** + * Fetch the matching licence and return data needed for the view licence page + * + * Was built to provide the data needed for the '/licences/{id}' page + * + * @param {string} id The UUID for the licence to fetch + * + * @returns {Promise} the data needed to populate the view licence page and some elements of the summary tab + */ +async function go (id) { + const licence = await _fetchLicence(id) + const data = await _data(licence) + + return data +} + +async function _data (returns) { + return returns +} + +async function _fetchLicence (id) { + const licenseData = await LicenceModel.query() + .findById(id) + .select([ + 'licenceRef' + ]) + + const result = await ReturnLogModel.query() + .select([ + 'id', + 'dueDate', + 'status', + 'metadata', + 'return_reference' + ]) + .where('licence_ref', licenseData.licenceRef) + return result +} + +module.exports = { + go +} diff --git a/app/services/licences/view-license-returns.service.js b/app/services/licences/view-license-returns.service.js index 3dd1e36466..eb5a8d7f30 100644 --- a/app/services/licences/view-license-returns.service.js +++ b/app/services/licences/view-license-returns.service.js @@ -5,6 +5,7 @@ * @module ViewLicenceSummaryService */ +const FetchLicenceReturnsService = require('./fetch-licence-returns.service') const ViewLicenceService = require('./view-licence.service') const ViewLicenceReturnsPresenter = require('../../presenters/licences/view-licence-returns.presenter') /** @@ -16,7 +17,9 @@ const ViewLicenceReturnsPresenter = require('../../presenters/licences/view-lice */ async function go (licenceId, auth) { const commonData = await ViewLicenceService.go(licenceId, auth) - const pageData = ViewLicenceReturnsPresenter.go() + + const returnsData = await FetchLicenceReturnsService.go(licenceId) + const pageData = ViewLicenceReturnsPresenter.go(returnsData) return { ...pageData, diff --git a/app/views/licences/tabs/returns.njk b/app/views/licences/tabs/returns.njk index 44116fc8f3..97b8f5c925 100644 --- a/app/views/licences/tabs/returns.njk +++ b/app/views/licences/tabs/returns.njk @@ -2,11 +2,19 @@ {% from "govuk/components/tag/macro.njk" import govukTag %} {% macro formatStatus(status) %} - {% if status === 'complete' %} + {% if status === 'COMPLETE' %} {{ govukTag({ text: status, classes: "govuk-tag--green" }) }} + + {% elseif status === 'OVERDUE' %} + {{ govukTag({ + text: status, + classes: "govuk-tag--red" + }) }} + {% else %} + {{ status }} {% endif %} {% endmacro %} @@ -23,7 +31,8 @@ {% for return in returns %} - {{ return.reference }} + {{ return.reference }} {{ return.purpose }} {{ return.dueDate }} {{ formatStatus(return.status) }} diff --git a/test/presenters/licences/view-licence-returns-presenter.test.js b/test/presenters/licences/view-licence-returns-presenter.test.js index 84e7a6c955..02229cc615 100644 --- a/test/presenters/licences/view-licence-returns-presenter.test.js +++ b/test/presenters/licences/view-licence-returns-presenter.test.js @@ -11,23 +11,32 @@ const { expect } = Code const ViewLicenceReturnsPresenter = require('../../../app/presenters/licences/view-licence-returns.presenter') describe('View Licence returns presenter', () => { - let licence + let returnData beforeEach(() => { - licence = _licence() + returnData = _returnData() }) describe('when provided with a populated licence', () => { it('correctly presents the data', () => { - const result = ViewLicenceReturnsPresenter.go(licence) + const result = ViewLicenceReturnsPresenter.go(returnData) expect(result).to.equal({ activeTab: 'returns', + returnsUrl: 'return/internal', returns: [ { - dueDate: '28 April 2023', // date should be formatted - purpose: 'mock purpose', - reference: 'Mock reference', - status: 'complete' // lower case + id: 'mock-id-1', + reference: '1068', + purpose: 'SPRAY IRRIGATION', + dueDate: '28 November 2012', + status: 'COMPLETE' + }, + { + id: 'mock-id-2', + reference: '1069', + purpose: 'SPRAY IRRIGATION', + dueDate: '28 November 2019', + status: 'OVERDUE' } ] }) @@ -35,6 +44,57 @@ describe('View Licence returns presenter', () => { }) }) -function _licence () { - return {} +function _returnData () { + return [ + { + id: 'mock-id-1', + dueDate: '2012-11-28T00:00:00.000Z', + status: 'completed', + metadata: { + purposes: [ + { + alias: 'SPRAY IRRIGATION', + primary: { + code: 'A', + description: 'Agriculture' + }, + tertiary: { + code: '400', + description: 'Spray Irrigation - Direct' + }, + secondary: { + code: 'AGR', + description: 'General Agriculture' + } + } + ] + }, + returnReference: '1068' + }, + { + id: 'mock-id-2', + dueDate: '2019-11-28T00:00:00.000Z', + status: 'due', + metadata: { + purposes: [ + { + alias: 'SPRAY IRRIGATION', + primary: { + code: 'A', + description: 'Agriculture' + }, + tertiary: { + code: '400', + description: 'Spray Irrigation - Direct' + }, + secondary: { + code: 'AGR', + description: 'General Agriculture' + } + } + ] + }, + returnReference: '1069' + } + ] } diff --git a/test/services/licences/view-license-returns.service.test.js b/test/services/licences/view-license-returns.service.test.js index 9e8ba7a603..f8ae6b4bda 100644 --- a/test/services/licences/view-license-returns.service.test.js +++ b/test/services/licences/view-license-returns.service.test.js @@ -10,6 +10,7 @@ const { expect } = Code // Things we need to stub const ViewLicenceService = require('../../../app/services/licences/view-licence.service') +const FetchLicenceReturnsService = require('../../../app/services/licences/fetch-licence-returns.service') // Thing under test const ViewLicenceReturnsService = require('../../../app/services/licences/view-license-returns.service') @@ -18,6 +19,7 @@ describe('View Licence service returns', () => { beforeEach(() => { Sinon.stub(ViewLicenceService, 'go').resolves({ licenceName: 'fake license' }) + Sinon.stub(FetchLicenceReturnsService, 'go').resolves(_returnData()) }) afterEach(() => { @@ -32,9 +34,80 @@ describe('View Licence service returns', () => { expect(result).to.equal({ activeTab: 'returns', licenceName: 'fake license', - message: 'hello returns' + returnsUrl: 'return/internal', + returns: [ + { + id: 'mock-id-1', + reference: '1068', + purpose: 'SPRAY IRRIGATION', + dueDate: '28 November 2012', + status: 'COMPLETE' + }, + { + id: 'mock-id-2', + reference: '1069', + purpose: 'SPRAY IRRIGATION', + dueDate: '28 November 2019', + status: 'OVERDUE' + } + ] }) }) }) }) }) + +function _returnData () { + return [ + { + id: 'mock-id-1', + dueDate: '2012-11-28T00:00:00.000Z', + status: 'completed', + metadata: { + purposes: [ + { + alias: 'SPRAY IRRIGATION', + primary: { + code: 'A', + description: 'Agriculture' + }, + tertiary: { + code: '400', + description: 'Spray Irrigation - Direct' + }, + secondary: { + code: 'AGR', + description: 'General Agriculture' + } + } + ] + }, + returnReference: '1068' + }, + { + id: 'mock-id-2', + dueDate: '2019-11-28T00:00:00.000Z', + status: 'due', + metadata: { + purposes: [ + { + alias: 'SPRAY IRRIGATION', + primary: { + code: 'A', + description: 'Agriculture' + }, + tertiary: { + code: '400', + description: 'Spray Irrigation - Direct' + }, + secondary: { + code: 'AGR', + description: 'General Agriculture' + } + } + ] + }, + returnReference: '1069' + } + ] +} From 1215af80c3f66d6a1e99ddffb427c17cf3b5c0e0 Mon Sep 17 00:00:00 2001 From: jonathangoulding Date: Thu, 2 May 2024 14:30:30 +0100 Subject: [PATCH 05/33] chore: fix lint issue --- app/views/licences/tabs/returns.njk | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/app/views/licences/tabs/returns.njk b/app/views/licences/tabs/returns.njk index 97b8f5c925..2e4221ca72 100644 --- a/app/views/licences/tabs/returns.njk +++ b/app/views/licences/tabs/returns.njk @@ -31,8 +31,7 @@ {% for return in returns %} - {{ return.reference }} + {{ return.reference }} {{ return.purpose }} {{ return.dueDate }} {{ formatStatus(return.status) }} From 973b186be2924f50395b46fbb0ec21030063d610 Mon Sep 17 00:00:00 2001 From: jonathangoulding Date: Thu, 2 May 2024 14:44:13 +0100 Subject: [PATCH 06/33] feat: add start to end date text add description to table row items --- .../licences/view-licence-returns.presenter.js | 4 +++- .../licences/fetch-licence-returns.service.js | 2 ++ app/views/licences/tabs/returns.njk | 7 +++++-- .../view-licence-returns-presenter.test.js | 16 +++++++++++++--- .../view-license-returns.service.test.js | 17 +++++++++++++---- 5 files changed, 36 insertions(+), 10 deletions(-) diff --git a/app/presenters/licences/view-licence-returns.presenter.js b/app/presenters/licences/view-licence-returns.presenter.js index 8d4083f6f9..d9208a4375 100644 --- a/app/presenters/licences/view-licence-returns.presenter.js +++ b/app/presenters/licences/view-licence-returns.presenter.js @@ -46,8 +46,10 @@ function _formatReturnToTableRow (returns) { id: r.id, reference: r.returnReference || 'No REF !!', purpose: _formatPurpose(r.metadata.purposes), + description: r.metadata.description, dueDate: formatLongDate(new Date(r.dueDate)), - status: _formatStatus(r.status) + status: _formatStatus(r.status), + dates: `${formatLongDate(new Date(r.startDate))} to ${formatLongDate(new Date(r.endDate))}` } }) } diff --git a/app/services/licences/fetch-licence-returns.service.js b/app/services/licences/fetch-licence-returns.service.js index 892e5794cc..8a34980969 100644 --- a/app/services/licences/fetch-licence-returns.service.js +++ b/app/services/licences/fetch-licence-returns.service.js @@ -38,6 +38,8 @@ async function _fetchLicence (id) { const result = await ReturnLogModel.query() .select([ 'id', + 'start_date', + 'end_date', 'dueDate', 'status', 'metadata', diff --git a/app/views/licences/tabs/returns.njk b/app/views/licences/tabs/returns.njk index 2e4221ca72..75faa5a9f4 100644 --- a/app/views/licences/tabs/returns.njk +++ b/app/views/licences/tabs/returns.njk @@ -31,8 +31,11 @@ {% for return in returns %} - {{ return.reference }} - {{ return.purpose }} + + {{ return.reference }} +

{{ return.dates }}

+ + {{ return.purpose }}

{{ return.description }}

{{ return.dueDate }} {{ formatStatus(return.status) }} diff --git a/test/presenters/licences/view-licence-returns-presenter.test.js b/test/presenters/licences/view-licence-returns-presenter.test.js index 02229cc615..513439aace 100644 --- a/test/presenters/licences/view-licence-returns-presenter.test.js +++ b/test/presenters/licences/view-licence-returns-presenter.test.js @@ -29,14 +29,18 @@ describe('View Licence returns presenter', () => { reference: '1068', purpose: 'SPRAY IRRIGATION', dueDate: '28 November 2012', - status: 'COMPLETE' + status: 'COMPLETE', + dates: '2 January 2020 to 1 February 2020', + description: 'empty description' }, { id: 'mock-id-2', reference: '1069', purpose: 'SPRAY IRRIGATION', dueDate: '28 November 2019', - status: 'OVERDUE' + status: 'OVERDUE', + dates: '2 January 2020 to 1 February 2020', + description: 'empty description' } ] }) @@ -50,6 +54,8 @@ function _returnData () { id: 'mock-id-1', dueDate: '2012-11-28T00:00:00.000Z', status: 'completed', + startDate: '2020/01/02', + endDate: '2020/02/01', metadata: { purposes: [ { @@ -67,7 +73,8 @@ function _returnData () { description: 'General Agriculture' } } - ] + ], + description: 'empty description' }, returnReference: '1068' }, @@ -75,7 +82,10 @@ function _returnData () { 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', diff --git a/test/services/licences/view-license-returns.service.test.js b/test/services/licences/view-license-returns.service.test.js index f8ae6b4bda..af484d3bf4 100644 --- a/test/services/licences/view-license-returns.service.test.js +++ b/test/services/licences/view-license-returns.service.test.js @@ -41,14 +41,18 @@ describe('View Licence service returns', () => { reference: '1068', purpose: 'SPRAY IRRIGATION', dueDate: '28 November 2012', - status: 'COMPLETE' + status: 'COMPLETE', + dates: '2 January 2020 to 1 February 2020', + description: 'empty description' }, { id: 'mock-id-2', reference: '1069', purpose: 'SPRAY IRRIGATION', dueDate: '28 November 2019', - status: 'OVERDUE' + status: 'OVERDUE', + dates: '2 January 2020 to 1 February 2020', + description: 'empty description' } ] }) @@ -56,13 +60,14 @@ describe('View Licence service returns', () => { }) }) }) - function _returnData () { return [ { id: 'mock-id-1', dueDate: '2012-11-28T00:00:00.000Z', status: 'completed', + startDate: '2020/01/02', + endDate: '2020/02/01', metadata: { purposes: [ { @@ -80,7 +85,8 @@ function _returnData () { description: 'General Agriculture' } } - ] + ], + description: 'empty description' }, returnReference: '1068' }, @@ -88,7 +94,10 @@ function _returnData () { 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', From aedd94a43ed4c6e98875fb7127d735f2df932f56 Mon Sep 17 00:00:00 2001 From: jonathangoulding Date: Thu, 2 May 2024 16:43:43 +0100 Subject: [PATCH 07/33] feat: add pagination --- app/controllers/licences.controller.js | 4 ++-- .../licences/view-licence-returns.presenter.js | 2 +- .../licences/fetch-licence-returns.service.js | 17 ++++++++++++----- .../licences/view-license-returns.service.js | 10 +++++++--- app/views/licences/tabs/returns.njk | 6 ++++++ 5 files changed, 28 insertions(+), 11 deletions(-) diff --git a/app/controllers/licences.controller.js b/app/controllers/licences.controller.js index 37d4bde6ee..edbaf262dd 100644 --- a/app/controllers/licences.controller.js +++ b/app/controllers/licences.controller.js @@ -36,9 +36,9 @@ async function viewSummary (request, h) { } async function viewReturns (request, h) { - const { params: { id }, auth } = request + const { params: { id }, auth, query: { page = 1 } } = request - const data = await ViewLicenceReturnsService.go(id, auth) + const data = await ViewLicenceReturnsService.go(id, auth, page) return h.view('licences/view.njk', { activeNavBar: 'search', diff --git a/app/presenters/licences/view-licence-returns.presenter.js b/app/presenters/licences/view-licence-returns.presenter.js index d9208a4375..a3e9556554 100644 --- a/app/presenters/licences/view-licence-returns.presenter.js +++ b/app/presenters/licences/view-licence-returns.presenter.js @@ -13,7 +13,7 @@ const { formatLongDate } = require('../base.presenter') * @returns {Object} The data formatted for the view template */ function go (returnsData) { - const returns = _formatReturnToTableRow(returnsData) + const returns = _formatReturnToTableRow(returnsData.returns) return { activeTab: 'returns', diff --git a/app/services/licences/fetch-licence-returns.service.js b/app/services/licences/fetch-licence-returns.service.js index 8a34980969..82eb342637 100644 --- a/app/services/licences/fetch-licence-returns.service.js +++ b/app/services/licences/fetch-licence-returns.service.js @@ -7,6 +7,7 @@ const LicenceModel = require('../../models/licence.model.js') const ReturnLogModel = require('../../models/return-log.model') +const DatabaseConfig = require('../../../config/database.config') /** * Fetch the matching licence and return data needed for the view licence page @@ -17,24 +18,25 @@ const ReturnLogModel = require('../../models/return-log.model') * * @returns {Promise} the data needed to populate the view licence page and some elements of the summary tab */ -async function go (id) { - const licence = await _fetchLicence(id) +async function go (id, page) { + const licence = await _fetchLicence(id, page) const data = await _data(licence) return data } -async function _data (returns) { - return returns +async function _data ({ results, total }) { + return { returns: results, pagination: { total } } } -async function _fetchLicence (id) { +async function _fetchLicence (id, page) { const licenseData = await LicenceModel.query() .findById(id) .select([ 'licenceRef' ]) + // order by due date const result = await ReturnLogModel.query() .select([ 'id', @@ -46,6 +48,11 @@ async function _fetchLicence (id) { 'return_reference' ]) .where('licence_ref', licenseData.licenceRef) + .orderBy([ + { column: 'dueDate', order: 'desc' } + ]) + .page(page - 1, DatabaseConfig.defaultPageSize) + return result } diff --git a/app/services/licences/view-license-returns.service.js b/app/services/licences/view-license-returns.service.js index eb5a8d7f30..0151e2dbe7 100644 --- a/app/services/licences/view-license-returns.service.js +++ b/app/services/licences/view-license-returns.service.js @@ -8,6 +8,7 @@ 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') /** * Orchestrates fetching and presenting the data needed for the licence summary page * @@ -15,15 +16,18 @@ const ViewLicenceReturnsPresenter = require('../../presenters/licences/view-lice * * @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 returnsData = await FetchLicenceReturnsService.go(licenceId) + const returnsData = await FetchLicenceReturnsService.go(licenceId, page) const pageData = ViewLicenceReturnsPresenter.go(returnsData) + const pagination = PaginatorPresenter.go(returnsData.pagination.total, Number(page), '') + return { ...pageData, - ...commonData + ...commonData, + pagination } } diff --git a/app/views/licences/tabs/returns.njk b/app/views/licences/tabs/returns.njk index 75faa5a9f4..d7bb38f2fd 100644 --- a/app/views/licences/tabs/returns.njk +++ b/app/views/licences/tabs/returns.njk @@ -1,3 +1,4 @@ +{% from "govuk/components/pagination/macro.njk" import govukPagination %} {% from "govuk/components/table/macro.njk" import govukTable %} {% from "govuk/components/tag/macro.njk" import govukTag %} @@ -30,6 +31,7 @@ {% for return in returns %} +{# Fix addtional bottom row#} {{ return.reference }} @@ -42,3 +44,7 @@ {% endfor %} + +{% if pagination.numberOfPages > 1 %} + {{ govukPagination(pagination.component) }} +{% endif %} From 76aa5756f09691928af4a0e6aedce2215397f1c1 Mon Sep 17 00:00:00 2001 From: jonathangoulding Date: Fri, 3 May 2024 09:45:56 +0100 Subject: [PATCH 08/33] feat: add pagination and update tests --- .../licences/view-license-returns.service.js | 2 +- app/views/licences/tabs/returns.njk | 6 +- .../view-licence-returns-presenter.test.js | 112 ++++++++--------- .../view-license-returns.service.test.js | 115 +++++------------- 4 files changed, 92 insertions(+), 143 deletions(-) diff --git a/app/services/licences/view-license-returns.service.js b/app/services/licences/view-license-returns.service.js index 0151e2dbe7..707cebbc45 100644 --- a/app/services/licences/view-license-returns.service.js +++ b/app/services/licences/view-license-returns.service.js @@ -22,7 +22,7 @@ async function go (licenceId, auth, page) { const returnsData = await FetchLicenceReturnsService.go(licenceId, page) const pageData = ViewLicenceReturnsPresenter.go(returnsData) - const pagination = PaginatorPresenter.go(returnsData.pagination.total, Number(page), '') + const pagination = PaginatorPresenter.go(returnsData.pagination.total, Number(page), `/system/licences/${licenceId}/returns`) return { ...pageData, diff --git a/app/views/licences/tabs/returns.njk b/app/views/licences/tabs/returns.njk index d7bb38f2fd..7a50ce5aef 100644 --- a/app/views/licences/tabs/returns.njk +++ b/app/views/licences/tabs/returns.njk @@ -32,12 +32,12 @@ {% for return in returns %} {# Fix addtional bottom row#} - + {{ return.reference }} -

{{ return.dates }}

+

{{ return.dates }}

- {{ return.purpose }}

{{ return.description }}

+ {{ return.purpose }}

{{ return.description }}

{{ return.dueDate }} {{ formatStatus(return.status) }} diff --git a/test/presenters/licences/view-licence-returns-presenter.test.js b/test/presenters/licences/view-licence-returns-presenter.test.js index 513439aace..0f4e4ef5dc 100644 --- a/test/presenters/licences/view-licence-returns-presenter.test.js +++ b/test/presenters/licences/view-licence-returns-presenter.test.js @@ -49,62 +49,64 @@ describe('View Licence returns presenter', () => { }) function _returnData () { - return [ - { - 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' + 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' + ], + description: 'empty description' + }, + returnReference: '1068' }, - 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' + { + 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' - } - ] + ] + }, + returnReference: '1069' + } + ] + } } diff --git a/test/services/licences/view-license-returns.service.test.js b/test/services/licences/view-license-returns.service.test.js index af484d3bf4..590d291408 100644 --- a/test/services/licences/view-license-returns.service.test.js +++ b/test/services/licences/view-license-returns.service.test.js @@ -10,16 +10,22 @@ 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') // Thing under test const ViewLicenceReturnsService = require('../../../app/services/licences/view-license-returns.service') describe('View Licence service returns', () => { const testId = '2c80bd22-a005-4cf4-a2a2-73812a9861de' - + const page = 1 + const auth = {} + const pagination = { page } beforeEach(() => { - Sinon.stub(ViewLicenceService, 'go').resolves({ licenceName: 'fake license' }) - Sinon.stub(FetchLicenceReturnsService, 'go').resolves(_returnData()) + Sinon.stub(FetchLicenceReturnsService, 'go').resolves(_returnsFetch()) + Sinon.stub(PaginatorPresenter, 'go').returns(pagination) + Sinon.stub(ViewLicenceReturnsPresenter, 'go').returns(_returnsPresenter()) + Sinon.stub(ViewLicenceService, 'go').resolves(_license()) }) afterEach(() => { @@ -29,94 +35,35 @@ 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(testId) + const result = await ViewLicenceReturnsService.go(testId, auth, page) expect(result).to.equal({ - activeTab: 'returns', licenceName: 'fake license', returnsUrl: 'return/internal', - returns: [ - { - id: 'mock-id-1', - reference: '1068', - purpose: 'SPRAY IRRIGATION', - dueDate: '28 November 2012', - status: 'COMPLETE', - dates: '2 January 2020 to 1 February 2020', - description: 'empty description' - }, - { - id: 'mock-id-2', - reference: '1069', - purpose: 'SPRAY IRRIGATION', - dueDate: '28 November 2019', - status: 'OVERDUE', - dates: '2 January 2020 to 1 February 2020', - description: 'empty description' - } - ] + returns: [], + activeTab: 'returns', + pagination: { page: 1 } }) }) }) }) }) -function _returnData () { - return [ - { - 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' - } - ] + +function _returnsPresenter () { + return { + returnsUrl: 'return/internal', + returns: [], + activeTab: 'returns' + } +} + +function _returnsFetch () { + return { + pagination: { total: 1 }, + returns: [] + } +} + +function _license () { + return { licenceName: 'fake license' } } From 60cbb6dd151e21662715c877d3678f6cbfbb29da Mon Sep 17 00:00:00 2001 From: jonathangoulding Date: Fri, 3 May 2024 13:46:28 +0100 Subject: [PATCH 09/33] fix: route guard remove comment block update fetch service returns to use returnlog relation --- .../view-licence-returns.presenter.js | 8 --- app/routes/licence.routes.js | 5 -- .../licences/fetch-licence-returns.service.js | 51 +++++++------------ 3 files changed, 18 insertions(+), 46 deletions(-) diff --git a/app/presenters/licences/view-licence-returns.presenter.js b/app/presenters/licences/view-licence-returns.presenter.js index a3e9556554..0f133b59e6 100644 --- a/app/presenters/licences/view-licence-returns.presenter.js +++ b/app/presenters/licences/view-licence-returns.presenter.js @@ -25,14 +25,6 @@ function go (returnsData) { function _formatPurpose (purpose) { const [firstPurpose] = purpose return firstPurpose.alias ? firstPurpose.alias : firstPurpose.tertiary.description - /* Old Nunjucks converted to presenter logic - {% macro returnRequirementPurposes(return) %} - {% for purpose in return.returnRequirement.returnRequirementPurposes %} - {{ ', ' if not loop.first }} - {{ purpose.purposeAlias | titleCase if purpose.purposeAlias else purpose.purposeUse.name | titleCase }} - {% endfor %} - {% endmacro %} - */ } function _formatStatus (status) { diff --git a/app/routes/licence.routes.js b/app/routes/licence.routes.js index e32286c744..bae8ae4c7b 100644 --- a/app/routes/licence.routes.js +++ b/app/routes/licence.routes.js @@ -21,11 +21,6 @@ const routes = [ path: '/licences/{id}/returns', handler: LicencesController.viewReturns, options: { - auth: { - access: { - scope: ['billing'] - } - }, description: 'View a returns licence page' } }, diff --git a/app/services/licences/fetch-licence-returns.service.js b/app/services/licences/fetch-licence-returns.service.js index 82eb342637..a899f81c35 100644 --- a/app/services/licences/fetch-licence-returns.service.js +++ b/app/services/licences/fetch-licence-returns.service.js @@ -1,59 +1,44 @@ 'use strict' /** - * Fetches data needed for the view '/licences/{id}` page + * Fetches all return logs for a licence which is needed for the view '/licences/{id}/returns` page * @module FetchLicenceReturnsService */ -const LicenceModel = require('../../models/licence.model.js') const ReturnLogModel = require('../../models/return-log.model') + const DatabaseConfig = require('../../../config/database.config') /** - * Fetch the matching licence and return data needed for the view licence page - * - * Was built to provide the data needed for the '/licences/{id}' page + * Fetches all return logs for a licence which is needed for the view '/licences/{id}/returns` page * - * @param {string} id The UUID for the licence to fetch + * @param {string} licenceId - The UUID for the licence to fetch * - * @returns {Promise} the data needed to populate the view licence page and some elements of the summary tab + * @returns {Promise} the data needed to populate the view licence page's returns tab */ -async function go (id, page) { - const licence = await _fetchLicence(id, page) - const data = await _data(licence) - - return data -} +async function go (licenceId, page) { + const { results, total } = await _fetch(licenceId, page) -async function _data ({ results, total }) { return { returns: results, pagination: { total } } } -async function _fetchLicence (id, page) { - const licenseData = await LicenceModel.query() - .findById(id) +async function _fetch (licenceId, page) { + return ReturnLogModel.query() .select([ - 'licenceRef' + 'returnLogs.id', + 'returnLogs.dueDate', + 'returnLogs.endDate', + 'returnLogs.metadata', + 'returnLogs.returnReference', + 'returnLogs.startDate', + 'returnLogs.status' ]) - - // order by due date - const result = await ReturnLogModel.query() - .select([ - 'id', - 'start_date', - 'end_date', - 'dueDate', - 'status', - 'metadata', - 'return_reference' - ]) - .where('licence_ref', licenseData.licenceRef) + .innerJoinRelated('licence') + .where('licence.id', licenceId) .orderBy([ { column: 'dueDate', order: 'desc' } ]) .page(page - 1, DatabaseConfig.defaultPageSize) - - return result } module.exports = { From 5eaca75319a241db42e31a99a1dbb8e8f9af97d1 Mon Sep 17 00:00:00 2001 From: jonathangoulding Date: Fri, 3 May 2024 14:04:40 +0100 Subject: [PATCH 10/33] feat: add no returns and returns not needed to view and controller --- app/views/licences/tabs/returns.njk | 58 ++++++++++++-------- test/controllers/licences.controller.test.js | 42 ++++++++++++-- 2 files changed, 72 insertions(+), 28 deletions(-) diff --git a/app/views/licences/tabs/returns.njk b/app/views/licences/tabs/returns.njk index 7a50ce5aef..40344f2880 100644 --- a/app/views/licences/tabs/returns.njk +++ b/app/views/licences/tabs/returns.njk @@ -20,31 +20,41 @@ {% endmacro %} - - - - - - - - - - - {% for return in returns %} -{# Fix addtional bottom row#} - - - - - - - {% endfor %} - +

Returns

+ {% if returns %} + + + + + + + + + + + {% for return in returns %} + {# Fix addtional bottom row#} + + + + + + + {% endfor %} + + {% elseif returnNeeded == true %} +

Returns do not need to be submitted for this license.

+

Set up new requirements for returns

+ {% else %} +

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

+

Set up new requirements for returns

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

{{ return.dates }}

-
{{ return.purpose }}

{{ return.description }}

{{ return.dueDate }}{{ formatStatus(return.status) }}
Returns
Return reference and datesPurpose and descriptionDue dateStatus
+ {{ return.reference }} +

{{ return.dates }}

+
{{ return.purpose }}

{{ return.description }}

{{ return.dueDate }}{{ formatStatus(return.status) }}
-{% if pagination.numberOfPages > 1 %} +{% if returns 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 ba5738b9cd..78acc4519a 100644 --- a/test/controllers/licences.controller.test.js +++ b/test/controllers/licences.controller.test.js @@ -151,7 +151,7 @@ describe('Licences controller', () => { }) }) - describe('GET /licences/{id}/summary', () => { + describe.only('GET /licences/{id}/summary', () => { beforeEach(async () => { options = { method: 'GET', @@ -190,7 +190,7 @@ describe('Licences controller', () => { } }) - describe('GET /licences/{id}/returns', () => { + describe.only('GET /licences/{id}/returns', () => { beforeEach(async () => { options = { method: 'GET', @@ -202,7 +202,7 @@ describe('Licences controller', () => { } }) - describe('when a request is valid', () => { + describe('when a request is valid and has returns', () => { beforeEach(async () => { Sinon.stub(ViewLicenceReturnsService, 'go').resolves(_viewLicenceReturns()) }) @@ -220,9 +220,43 @@ describe('Licences controller', () => { }) }) + describe('when a request is valid and a return is not needed', () => { + beforeEach(async () => { + const returnData = _viewLicenceReturns() + delete returnData.returns + returnData.returnNeeded = true + Sinon.stub(ViewLicenceReturnsService, 'go').resolves(returnData) + }) + + it('returns the page successfully', async () => { + const response = await server.inject(options) + + expect(response.statusCode).to.equal(200) + expect(response.payload).to.contain('Returns') + expect(response.payload).to.contain('Returns do not need to be submitted for this license.') + }) + }) + + describe('when a request is valid and no returns', () => { + beforeEach(async () => { + const returnData = _viewLicenceReturns() + delete returnData.returns + Sinon.stub(ViewLicenceReturnsService, 'go').resolves(returnData) + }) + + it('returns the page successfully', 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.') + }) + }) + function _viewLicenceReturns () { return { - activeTab: 'returns' + activeTab: 'returns', + returns: [{}] } } }) From f6061c67240a4b75da321ba78a9a47e8b2526342 Mon Sep 17 00:00:00 2001 From: jonathangoulding Date: Fri, 3 May 2024 14:16:35 +0100 Subject: [PATCH 11/33] feat: update href for no returns link --- app/presenters/licences/view-licence.presenter.js | 4 +++- app/services/licences/fetch-licence.service.js | 1 + app/views/licences/tabs/returns.njk | 4 ++-- test/services/licences/fetch-licence.service.test.js | 2 ++ 4 files changed, 8 insertions(+), 3 deletions(-) diff --git a/app/presenters/licences/view-licence.presenter.js b/app/presenters/licences/view-licence.presenter.js index 1343155bd5..b9d9b7de47 100644 --- a/app/presenters/licences/view-licence.presenter.js +++ b/app/presenters/licences/view-licence.presenter.js @@ -21,10 +21,12 @@ function go (licence, auth) { includeInSrocBilling, licenceName, licenceRef, - registeredTo + registeredTo, + id } = licence return { + licenseId: id, licenceName, licenceRef, notification: _determineNotificationBanner(includeInPresrocBilling, includeInSrocBilling), diff --git a/app/services/licences/fetch-licence.service.js b/app/services/licences/fetch-licence.service.js index acfe726fc4..bcc06be487 100644 --- a/app/services/licences/fetch-licence.service.js +++ b/app/services/licences/fetch-licence.service.js @@ -39,6 +39,7 @@ async function _fetchLicence (id) { const result = await LicenceModel.query() .findById(id) .select([ + 'id', 'include_in_presroc_billing', 'include_in_sroc_billing', 'licenceRef', diff --git a/app/views/licences/tabs/returns.njk b/app/views/licences/tabs/returns.njk index 40344f2880..d0f3c0319c 100644 --- a/app/views/licences/tabs/returns.njk +++ b/app/views/licences/tabs/returns.njk @@ -47,10 +47,10 @@ {% elseif returnNeeded == true %}

Returns do not need to be submitted for this license.

-

Set up new requirements for returns

+

Set up new requirements for returns

{% else %}

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

-

Set up new requirements for returns

+

Set up new requirements for returns

{% endif %} diff --git a/test/services/licences/fetch-licence.service.test.js b/test/services/licences/fetch-licence.service.test.js index 1c3b61a854..283613e33c 100644 --- a/test/services/licences/fetch-licence.service.test.js +++ b/test/services/licences/fetch-licence.service.test.js @@ -28,6 +28,7 @@ describe('Fetch licence service', () => { describe('when there is no optional data in the model', () => { beforeEach(async () => { licence = await LicenceHelper.add({ + id: 'a8256ea1-4509-4992-b30f-d011509e5f62', expiredDate: null, include_in_presroc_billing: 'yes', include_in_sroc_billing: true, @@ -42,6 +43,7 @@ describe('Fetch licence service', () => { it('returns results', async () => { const result = await FetchLicenceService.go(licence.id) + expect(result.id).to.equal('a8256ea1-4509-4992-b30f-d011509e5f62') expect(result.ends).to.equal(null) expect(result.expiredDate).to.equal(null) expect(result.lapsedDate).to.equal(null) From 0f55f59622e8e8bc771e5a9489e0bafce6e29534 Mon Sep 17 00:00:00 2001 From: jonathangoulding Date: Fri, 3 May 2024 14:25:14 +0100 Subject: [PATCH 12/33] fix: remove only text update the the if in license returns to share the link --- app/views/licences/tabs/returns.njk | 67 ++++++++++---------- test/controllers/licences.controller.test.js | 2 +- 2 files changed, 36 insertions(+), 33 deletions(-) diff --git a/app/views/licences/tabs/returns.njk b/app/views/licences/tabs/returns.njk index d0f3c0319c..a56921f959 100644 --- a/app/views/licences/tabs/returns.njk +++ b/app/views/licences/tabs/returns.njk @@ -21,40 +21,43 @@

Returns

- {% if returns %} - - - - - - - - - - - {% for return in returns %} - {# Fix addtional bottom row#} - - - - - - - {% endfor %} - - {% elseif returnNeeded == true %} -

Returns do not need to be submitted for this license.

-

Set up new requirements for returns

- {% else %} -

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

-

Set up new requirements for returns

- {% endif %} + {% if returns %} + + + + + + + + + + + {% for return in returns %} + {# Fix addtional bottom row #} + + + + + + + {% endfor %} + + + {% else %} + {% if returnNeeded == true %} +

Returns do not need to be submitted for this license.

+ {% else %} +

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

+ {% endif %} +

Set up new + requirements for returns

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

{{ return.dates }}

-
{{ return.purpose }}

{{ return.description }}

{{ return.dueDate }}{{ formatStatus(return.status) }}
Returns
Return reference and datesPurpose and descriptionDue dateStatus
+ {{ return.reference }} +

{{ return.dates }}

+
{{ return.purpose }}

{{ return.description }}

{{ return.dueDate }}{{ formatStatus(return.status) }}
-{% if returns and pagination.numberOfPages > 1 %} +{% if returns 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 78acc4519a..a8bc5ead28 100644 --- a/test/controllers/licences.controller.test.js +++ b/test/controllers/licences.controller.test.js @@ -151,7 +151,7 @@ describe('Licences controller', () => { }) }) - describe.only('GET /licences/{id}/summary', () => { + describe('GET /licences/{id}/summary', () => { beforeEach(async () => { options = { method: 'GET', From eb90d683c45a4bca0835a4091194ff755c84b928 Mon Sep 17 00:00:00 2001 From: jonathangoulding Date: Fri, 3 May 2024 14:26:36 +0100 Subject: [PATCH 13/33] fix: remove only text --- 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 a8bc5ead28..54a07a50e7 100644 --- a/test/controllers/licences.controller.test.js +++ b/test/controllers/licences.controller.test.js @@ -190,7 +190,7 @@ describe('Licences controller', () => { } }) - describe.only('GET /licences/{id}/returns', () => { + describe('GET /licences/{id}/returns', () => { beforeEach(async () => { options = { method: 'GET', From 9aa09f14d506f855d39bf6619f453f4f408acf0a Mon Sep 17 00:00:00 2001 From: jonathangoulding Date: Fri, 3 May 2024 14:40:07 +0100 Subject: [PATCH 14/33] fix: remove requirement no longer needed rename spelling --- app/controllers/licences.controller.js | 4 +- ...r.js => view-licence-summary.presenter.js} | 0 ...ice.js => view-licence-returns.service.js} | 0 ...ice.js => view-licence-summary.service.js} | 2 +- app/views/licences/tabs/returns.njk | 9 +---- test/controllers/licences.controller.test.js | 37 +------------------ .../view-bill-summaries.presenter.test.js | 2 +- .../view-licence-summary.presenter.test.js | 2 +- ...s => view-licence-returns.service.test.js} | 2 +- ...s => view-licence-summary.service.test.js} | 2 +- 10 files changed, 10 insertions(+), 50 deletions(-) rename app/presenters/licences/{view-license-summary.presenter.js => view-licence-summary.presenter.js} (100%) rename app/services/licences/{view-license-returns.service.js => view-licence-returns.service.js} (100%) rename app/services/licences/{view-license-summary.service.js => view-licence-summary.service.js} (96%) rename test/services/licences/{view-license-returns.service.test.js => view-licence-returns.service.test.js} (98%) rename test/services/licences/{view-license-summary.service.test.js => view-licence-summary.service.test.js} (99%) diff --git a/app/controllers/licences.controller.js b/app/controllers/licences.controller.js index edbaf262dd..23f2582a34 100644 --- a/app/controllers/licences.controller.js +++ b/app/controllers/licences.controller.js @@ -6,8 +6,8 @@ */ const InitiateReturnRequirementSessionService = require('../services/return-requirements/initiate-return-requirement-session.service.js') -const ViewLicenceSummaryService = require('../services/licences/view-license-summary.service') -const ViewLicenceReturnsService = require('../services/licences/view-license-returns.service') +const ViewLicenceSummaryService = require('../services/licences/view-licence-summary.service') +const ViewLicenceReturnsService = require('../services/licences/view-licence-returns.service') async function noReturnsRequired (request, h) { const { id } = request.params diff --git a/app/presenters/licences/view-license-summary.presenter.js b/app/presenters/licences/view-licence-summary.presenter.js similarity index 100% rename from app/presenters/licences/view-license-summary.presenter.js rename to app/presenters/licences/view-licence-summary.presenter.js diff --git a/app/services/licences/view-license-returns.service.js b/app/services/licences/view-licence-returns.service.js similarity index 100% rename from app/services/licences/view-license-returns.service.js rename to app/services/licences/view-licence-returns.service.js diff --git a/app/services/licences/view-license-summary.service.js b/app/services/licences/view-licence-summary.service.js similarity index 96% rename from app/services/licences/view-license-summary.service.js rename to app/services/licences/view-licence-summary.service.js index e714e28a1d..6f5d0dbdba 100644 --- a/app/services/licences/view-license-summary.service.js +++ b/app/services/licences/view-licence-summary.service.js @@ -7,7 +7,7 @@ const FetchLicenceAbstractionConditionsService = require('./fetch-licence-abstraction-conditions.service.js') const FetchLicenceSummaryService = require('./fetch-license-summary.service') -const ViewLicenceSummaryPresenter = require('../../presenters/licences/view-license-summary.presenter') +const ViewLicenceSummaryPresenter = require('../../presenters/licences/view-licence-summary.presenter') const ViewLicenceService = require('./view-licence.service') /** diff --git a/app/views/licences/tabs/returns.njk b/app/views/licences/tabs/returns.njk index a56921f959..7c1dbeb3ba 100644 --- a/app/views/licences/tabs/returns.njk +++ b/app/views/licences/tabs/returns.njk @@ -45,15 +45,8 @@ {% endfor %} - {% else %} - {% if returnNeeded == true %} -

Returns do not need to be submitted for this license.

- {% else %} -

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

- {% endif %} -

Set up new - requirements for returns

+

No returns found

{% endif %} diff --git a/test/controllers/licences.controller.test.js b/test/controllers/licences.controller.test.js index 54a07a50e7..8bfc08551c 100644 --- a/test/controllers/licences.controller.test.js +++ b/test/controllers/licences.controller.test.js @@ -13,8 +13,8 @@ const Boom = require('@hapi/boom') // Things we need to stub const InitiateReturnRequirementSessionService = require('../../app/services/return-requirements/initiate-return-requirement-session.service.js') -const ViewLicenceSummaryService = require('../../app/services/licences/view-license-summary.service') -const ViewLicenceReturnsService = require('../../app/services/licences/view-license-returns.service') +const ViewLicenceSummaryService = require('../../app/services/licences/view-licence-summary.service') +const ViewLicenceReturnsService = require('../../app/services/licences/view-licence-returns.service') // For running our service const { init } = require('../../app/server.js') @@ -220,39 +220,6 @@ describe('Licences controller', () => { }) }) - describe('when a request is valid and a return is not needed', () => { - beforeEach(async () => { - const returnData = _viewLicenceReturns() - delete returnData.returns - returnData.returnNeeded = true - Sinon.stub(ViewLicenceReturnsService, 'go').resolves(returnData) - }) - - it('returns the page successfully', async () => { - const response = await server.inject(options) - - expect(response.statusCode).to.equal(200) - expect(response.payload).to.contain('Returns') - expect(response.payload).to.contain('Returns do not need to be submitted for this license.') - }) - }) - - describe('when a request is valid and no returns', () => { - beforeEach(async () => { - const returnData = _viewLicenceReturns() - delete returnData.returns - Sinon.stub(ViewLicenceReturnsService, 'go').resolves(returnData) - }) - - it('returns the page successfully', 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.') - }) - }) - function _viewLicenceReturns () { return { activeTab: 'returns', diff --git a/test/presenters/bill-runs/view-bill-summaries.presenter.test.js b/test/presenters/bill-runs/view-bill-summaries.presenter.test.js index 9b4d167f8e..b904edc9a2 100644 --- a/test/presenters/bill-runs/view-bill-summaries.presenter.test.js +++ b/test/presenters/bill-runs/view-bill-summaries.presenter.test.js @@ -167,7 +167,7 @@ describe('View Bill Summaries presenter', () => { }) describe("the bill 'licences' property", () => { - it('splits the licenses provided by , and places the resulting references into an array', () => { + it('splits the licences provided by , and places the resulting references into an array', () => { const result = ViewBillSummariesPresenter.go(billSummaries) expect(result[0].bills[0].licences).to.equal(['17/53/001/A/101', '17/53/002/B/205', '17/53/002/C/308']) diff --git a/test/presenters/licences/view-licence-summary.presenter.test.js b/test/presenters/licences/view-licence-summary.presenter.test.js index 50d6a13793..9c11fba334 100644 --- a/test/presenters/licences/view-licence-summary.presenter.test.js +++ b/test/presenters/licences/view-licence-summary.presenter.test.js @@ -8,7 +8,7 @@ const { describe, it, beforeEach } = exports.lab = Lab.script() const { expect } = Code // Thing under test -const ViewLicenceSummaryPresenter = require('../../../app/presenters/licences/view-license-summary.presenter') +const ViewLicenceSummaryPresenter = require('../../../app/presenters/licences/view-licence-summary.presenter') describe('View Licence Summary presenter', () => { let licenceAbstractionConditions diff --git a/test/services/licences/view-license-returns.service.test.js b/test/services/licences/view-licence-returns.service.test.js similarity index 98% rename from test/services/licences/view-license-returns.service.test.js rename to test/services/licences/view-licence-returns.service.test.js index 590d291408..cc780bbbc0 100644 --- a/test/services/licences/view-license-returns.service.test.js +++ b/test/services/licences/view-licence-returns.service.test.js @@ -14,7 +14,7 @@ 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') // Thing under test -const ViewLicenceReturnsService = require('../../../app/services/licences/view-license-returns.service') +const ViewLicenceReturnsService = require('../../../app/services/licences/view-licence-returns.service') describe('View Licence service returns', () => { const testId = '2c80bd22-a005-4cf4-a2a2-73812a9861de' diff --git a/test/services/licences/view-license-summary.service.test.js b/test/services/licences/view-licence-summary.service.test.js similarity index 99% rename from test/services/licences/view-license-summary.service.test.js rename to test/services/licences/view-licence-summary.service.test.js index d00c7c9e60..29e1ec6ca5 100644 --- a/test/services/licences/view-license-summary.service.test.js +++ b/test/services/licences/view-licence-summary.service.test.js @@ -16,7 +16,7 @@ const FetchLicenceAbstractionConditionsService = require('../../../app/services/ const FetchLicenceSummaryService = require('../../../app/services/licences/fetch-license-summary.service') const ViewLicenceService = require('../../../app/services/licences/view-licence.service') // Thing under test -const ViewLicenceSummaryService = require('../../../app/services/licences/view-license-summary.service') +const ViewLicenceSummaryService = require('../../../app/services/licences/view-licence-summary.service') describe('View Licence service summary', () => { const testId = '2c80bd22-a005-4cf4-a2a2-73812a9861de' From 9d9763d8bb7a2d786d2893842e8b761688deb262 Mon Sep 17 00:00:00 2001 From: jonathangoulding Date: Fri, 3 May 2024 14:41:34 +0100 Subject: [PATCH 15/33] fix: remove additional title --- app/views/licences/tabs/returns.njk | 1 - 1 file changed, 1 deletion(-) diff --git a/app/views/licences/tabs/returns.njk b/app/views/licences/tabs/returns.njk index 7c1dbeb3ba..9e5ad7ec2e 100644 --- a/app/views/licences/tabs/returns.njk +++ b/app/views/licences/tabs/returns.njk @@ -22,7 +22,6 @@

Returns

{% if returns %} - From 19c1567045d652dc9e1f2ccc745b557b5b644fb8 Mon Sep 17 00:00:00 2001 From: jonathangoulding Date: Fri, 3 May 2024 14:46:26 +0100 Subject: [PATCH 16/33] fix: rename licence spelling --- ...ummary.service.js => fetch-licence-summary.service.js} | 0 app/services/licences/view-licence-summary.service.js | 2 +- test/presenters/licences/view-licence-presenter.test.js | 6 +++--- .../licences/fetch-licence-summary.service.test.js | 2 +- .../licences/view-licence-returns.service.test.js | 8 ++++---- .../licences/view-licence-summary.service.test.js | 6 +++--- 6 files changed, 12 insertions(+), 12 deletions(-) rename app/services/licences/{fetch-license-summary.service.js => fetch-licence-summary.service.js} (100%) diff --git a/app/services/licences/fetch-license-summary.service.js b/app/services/licences/fetch-licence-summary.service.js similarity index 100% rename from app/services/licences/fetch-license-summary.service.js rename to app/services/licences/fetch-licence-summary.service.js diff --git a/app/services/licences/view-licence-summary.service.js b/app/services/licences/view-licence-summary.service.js index 6f5d0dbdba..3913b1233a 100644 --- a/app/services/licences/view-licence-summary.service.js +++ b/app/services/licences/view-licence-summary.service.js @@ -6,7 +6,7 @@ */ const FetchLicenceAbstractionConditionsService = require('./fetch-licence-abstraction-conditions.service.js') -const FetchLicenceSummaryService = require('./fetch-license-summary.service') +const FetchLicenceSummaryService = require('./fetch-licence-summary.service') const ViewLicenceSummaryPresenter = require('../../presenters/licences/view-licence-summary.presenter') const ViewLicenceService = require('./view-licence.service') diff --git a/test/presenters/licences/view-licence-presenter.test.js b/test/presenters/licences/view-licence-presenter.test.js index dc57a01778..532619a446 100644 --- a/test/presenters/licences/view-licence-presenter.test.js +++ b/test/presenters/licences/view-licence-presenter.test.js @@ -151,7 +151,7 @@ describe('View Licence presenter', () => { it('returns the notification for PRESROC', () => { const result = ViewLicencePresenter.go(licence) - expect(result.notification).to.equal('This license has been marked for the next supplementary bill run for the old charge scheme.') + expect(result.notification).to.equal('This licence has been marked for the next supplementary bill run for the old charge scheme.') }) }) @@ -163,7 +163,7 @@ describe('View Licence presenter', () => { it('returns the notification for SROC', () => { const result = ViewLicencePresenter.go(licence) - expect(result.notification).to.equal('This license has been marked for the next supplementary bill run.') + expect(result.notification).to.equal('This licence has been marked for the next supplementary bill run.') }) }) @@ -176,7 +176,7 @@ describe('View Licence presenter', () => { it('returns the notification for SROC & PRESROC)', () => { const result = ViewLicencePresenter.go(licence) - expect(result.notification).to.equal('This license has been marked for the next supplementary bill runs for the current and old charge schemes.') + expect(result.notification).to.equal('This licence has been marked for the next supplementary bill runs for the current and old charge schemes.') }) }) }) diff --git a/test/services/licences/fetch-licence-summary.service.test.js b/test/services/licences/fetch-licence-summary.service.test.js index e82ad385c6..1ce9a90a9e 100644 --- a/test/services/licences/fetch-licence-summary.service.test.js +++ b/test/services/licences/fetch-licence-summary.service.test.js @@ -27,7 +27,7 @@ const PermitLicenceHelper = require('../../support/helpers/permit-licence.helper const RegionHelper = require('../../support/helpers/region.helper.js') // Thing under test -const FetchLicenceSummaryService = require('../../../app/services/licences/fetch-license-summary.service') +const FetchLicenceSummaryService = require('../../../app/services/licences/fetch-licence-summary.service') describe('Fetch licence summary service', () => { let licence diff --git a/test/services/licences/view-licence-returns.service.test.js b/test/services/licences/view-licence-returns.service.test.js index cc780bbbc0..2c24c241df 100644 --- a/test/services/licences/view-licence-returns.service.test.js +++ b/test/services/licences/view-licence-returns.service.test.js @@ -25,7 +25,7 @@ describe('View Licence service returns', () => { Sinon.stub(FetchLicenceReturnsService, 'go').resolves(_returnsFetch()) Sinon.stub(PaginatorPresenter, 'go').returns(pagination) Sinon.stub(ViewLicenceReturnsPresenter, 'go').returns(_returnsPresenter()) - Sinon.stub(ViewLicenceService, 'go').resolves(_license()) + Sinon.stub(ViewLicenceService, 'go').resolves(_licence()) }) afterEach(() => { @@ -38,7 +38,7 @@ describe('View Licence service returns', () => { const result = await ViewLicenceReturnsService.go(testId, auth, page) expect(result).to.equal({ - licenceName: 'fake license', + licenceName: 'fake licence', returnsUrl: 'return/internal', returns: [], activeTab: 'returns', @@ -64,6 +64,6 @@ function _returnsFetch () { } } -function _license () { - return { licenceName: 'fake license' } +function _licence () { + return { licenceName: 'fake licence' } } diff --git a/test/services/licences/view-licence-summary.service.test.js b/test/services/licences/view-licence-summary.service.test.js index 29e1ec6ca5..c466ea28ab 100644 --- a/test/services/licences/view-licence-summary.service.test.js +++ b/test/services/licences/view-licence-summary.service.test.js @@ -13,7 +13,7 @@ const LicenceModel = require('../../../app/models/licence.model.js') // Things we need to stub const FetchLicenceAbstractionConditionsService = require('../../../app/services/licences/fetch-licence-abstraction-conditions.service.js') -const FetchLicenceSummaryService = require('../../../app/services/licences/fetch-license-summary.service') +const FetchLicenceSummaryService = require('../../../app/services/licences/fetch-licence-summary.service') const ViewLicenceService = require('../../../app/services/licences/view-licence.service') // Thing under test const ViewLicenceSummaryService = require('../../../app/services/licences/view-licence-summary.service') @@ -29,7 +29,7 @@ describe('View Licence service summary', () => { purposeIds: [], numberOfConditions: 0 }) - Sinon.stub(ViewLicenceService, 'go').resolves({ licenceName: 'fake license' }) + Sinon.stub(ViewLicenceService, 'go').resolves({ licenceName: 'fake licence' }) }) afterEach(() => { @@ -64,7 +64,7 @@ describe('View Licence service summary', () => { endDate: null, id: '2c80bd22-a005-4cf4-a2a2-73812a9861de', licenceHolder: 'Unregistered licence', - licenceName: 'fake license', + licenceName: 'fake licence', monitoringStations: [], purposes: null, region: 'South West', From 39637ac9751bf6e434e1c67094f7adaef29bb6e1 Mon Sep 17 00:00:00 2001 From: jonathangoulding Date: Fri, 3 May 2024 14:52:10 +0100 Subject: [PATCH 17/33] fix: clean up dead code --- app/presenters/licences/view-licence-returns.presenter.js | 3 ++- app/views/licences/tabs/returns.njk | 1 - 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/presenters/licences/view-licence-returns.presenter.js b/app/presenters/licences/view-licence-returns.presenter.js index 0f133b59e6..3683f08e05 100644 --- a/app/presenters/licences/view-licence-returns.presenter.js +++ b/app/presenters/licences/view-licence-returns.presenter.js @@ -32,11 +32,12 @@ function _formatStatus (status) { if (status === 'due') return 'OVERDUE' return 'NO STATUS' } + function _formatReturnToTableRow (returns) { return returns.map(r => { return { id: r.id, - reference: r.returnReference || 'No REF !!', + reference: r.returnReference, purpose: _formatPurpose(r.metadata.purposes), description: r.metadata.description, dueDate: formatLongDate(new Date(r.dueDate)), diff --git a/app/views/licences/tabs/returns.njk b/app/views/licences/tabs/returns.njk index 9e5ad7ec2e..26f168122e 100644 --- a/app/views/licences/tabs/returns.njk +++ b/app/views/licences/tabs/returns.njk @@ -32,7 +32,6 @@ {% for return in returns %} - {# Fix addtional bottom row #} diff --git a/test/presenters/licences/view-licence-returns-presenter.test.js b/test/presenters/licences/view-licence-returns-presenter.test.js index 0f4e4ef5dc..90274a0f02 100644 --- a/test/presenters/licences/view-licence-returns-presenter.test.js +++ b/test/presenters/licences/view-licence-returns-presenter.test.js @@ -22,7 +22,6 @@ describe('View Licence returns presenter', () => { expect(result).to.equal({ activeTab: 'returns', - returnsUrl: 'return/internal', returns: [ { id: 'mock-id-1', diff --git a/test/services/licences/view-licence-returns.service.test.js b/test/services/licences/view-licence-returns.service.test.js index 6153035381..630e5a9c1d 100644 --- a/test/services/licences/view-licence-returns.service.test.js +++ b/test/services/licences/view-licence-returns.service.test.js @@ -41,7 +41,6 @@ describe('View Licence service returns', () => { expect(result).to.equal({ licenceName: 'fake licence', - returnsUrl: 'return/internal', returns: [], activeTab: 'returns', pagination: { page: 1 } @@ -53,7 +52,6 @@ describe('View Licence service returns', () => { function _returnsPresenter () { return { - returnsUrl: 'return/internal', returns: [], activeTab: 'returns' } From 14e837a4c136726ac72fc3481cec02f2802d6f56 Mon Sep 17 00:00:00 2001 From: Jonathan Goulding <58443816+jonathangoulding@users.noreply.github.com> Date: Fri, 3 May 2024 17:21:02 +0100 Subject: [PATCH 25/33] fix: pr Co-authored-by: Alan Cruikshanks --- app/controllers/licences.controller.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/controllers/licences.controller.js b/app/controllers/licences.controller.js index cfa334c03e..821e7b8f1d 100644 --- a/app/controllers/licences.controller.js +++ b/app/controllers/licences.controller.js @@ -6,8 +6,9 @@ */ const InitiateReturnRequirementSessionService = require('../services/return-requirements/initiate-return-requirement-session.service.js') -const ViewLicenceSummaryService = require('../services/licences/view-licence-summary.service') const ViewLicenceReturnsService = require('../services/licences/view-licence-returns.service') +const ViewLicenceSummaryService = require('../services/licences/view-licence-summary.service') + async function noReturnsRequired (request, h) { const { id } = request.params From e12cb229bc5181ba631a5c67f2f53ad2acd79d0b Mon Sep 17 00:00:00 2001 From: Jonathan Goulding <58443816+jonathangoulding@users.noreply.github.com> Date: Fri, 3 May 2024 17:21:21 +0100 Subject: [PATCH 26/33] fix: pr Co-authored-by: Alan Cruikshanks --- app/presenters/licences/view-licence-returns.presenter.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/presenters/licences/view-licence-returns.presenter.js b/app/presenters/licences/view-licence-returns.presenter.js index 4932a6d734..11add58940 100644 --- a/app/presenters/licences/view-licence-returns.presenter.js +++ b/app/presenters/licences/view-licence-returns.presenter.js @@ -8,7 +8,7 @@ const { formatLongDate } = require('../base.presenter') /** - * Formats data for common licence data `/licences/{id}` page's + * Formats common data for the `/licences/{id}/*` view licence pages * * @returns {Object} The data formatted for the view template */ From 467dbf55f3584d51de2271a2f2155e15e6e15397 Mon Sep 17 00:00:00 2001 From: jonathangoulding Date: Fri, 3 May 2024 17:23:45 +0100 Subject: [PATCH 27/33] fix: reorder license presenter functions alpha --- .../view-licence-returns.presenter.js | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/app/presenters/licences/view-licence-returns.presenter.js b/app/presenters/licences/view-licence-returns.presenter.js index 4932a6d734..a5acb66c92 100644 --- a/app/presenters/licences/view-licence-returns.presenter.js +++ b/app/presenters/licences/view-licence-returns.presenter.js @@ -27,26 +27,26 @@ function _formatPurpose (purpose) { return firstPurpose.alias ? firstPurpose.alias : firstPurpose.tertiary.description } -function _formatStatus (status) { - if (status === 'completed') return 'COMPLETE' - if (status === 'due') return 'OVERDUE' - return 'NO STATUS' -} - function _formatReturnToTableRow (returns) { return returns.map((r) => { return { - id: r.id, - reference: r.returnReference, - purpose: _formatPurpose(r.metadata.purposes), + dates: `${formatLongDate(new Date(r.startDate))} to ${formatLongDate(new Date(r.endDate))}`, description: r.metadata.description, dueDate: formatLongDate(new Date(r.dueDate)), - status: _formatStatus(r.status), - dates: `${formatLongDate(new Date(r.startDate))} to ${formatLongDate(new Date(r.endDate))}` + id: r.id, + purpose: _formatPurpose(r.metadata.purposes), + reference: r.returnReference, + status: _formatStatus(r.status) } }) } +function _formatStatus (status) { + if (status === 'completed') return 'COMPLETE' + if (status === 'due') return 'OVERDUE' + return 'NO STATUS' +} + module.exports = { go } From d36ca6a6f88a29bb8376a4d3afaba4734c7cfa8a Mon Sep 17 00:00:00 2001 From: jonathangoulding Date: Fri, 3 May 2024 17:25:20 +0100 Subject: [PATCH 28/33] fix: reorder license service test functions alpha --- .../licences/view-licence-returns.service.test.js | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/test/services/licences/view-licence-returns.service.test.js b/test/services/licences/view-licence-returns.service.test.js index 630e5a9c1d..8365c50c79 100644 --- a/test/services/licences/view-licence-returns.service.test.js +++ b/test/services/licences/view-licence-returns.service.test.js @@ -50,11 +50,8 @@ describe('View Licence service returns', () => { }) }) -function _returnsPresenter () { - return { - returns: [], - activeTab: 'returns' - } +function _licence () { + return { licenceName: 'fake licence' } } function _returnsFetch () { @@ -64,6 +61,9 @@ function _returnsFetch () { } } -function _licence () { - return { licenceName: 'fake licence' } +function _returnsPresenter () { + return { + returns: [], + activeTab: 'returns' + } } From 156c73aa5da4b0cfbe528e3b0918fce043a97f5e Mon Sep 17 00:00:00 2001 From: jonathangoulding Date: Fri, 3 May 2024 17:26:54 +0100 Subject: [PATCH 29/33] fix: test helper function outside of describe --- test/controllers/licences.controller.test.js | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/test/controllers/licences.controller.test.js b/test/controllers/licences.controller.test.js index 340b4d8163..5025f42b60 100644 --- a/test/controllers/licences.controller.test.js +++ b/test/controllers/licences.controller.test.js @@ -219,12 +219,12 @@ describe('Licences controller', () => { expect(response.payload).to.contain('Status') }) }) - - function _viewLicenceReturns () { - return { - activeTab: 'returns', - returns: [{}] - } - } }) }) + +function _viewLicenceReturns () { + return { + activeTab: 'returns', + returns: [{}] + } +} From 9bfd3d075ae911e2cddc45f9f61b667a652ac6ba Mon Sep 17 00:00:00 2001 From: Jonathan Goulding <58443816+jonathangoulding@users.noreply.github.com> Date: Fri, 3 May 2024 17:27:35 +0100 Subject: [PATCH 30/33] fix: pr Co-authored-by: Alan Cruikshanks --- test/presenters/licences/view-licence-returns-presenter.test.js | 1 + 1 file changed, 1 insertion(+) diff --git a/test/presenters/licences/view-licence-returns-presenter.test.js b/test/presenters/licences/view-licence-returns-presenter.test.js index 90274a0f02..c65c2135f7 100644 --- a/test/presenters/licences/view-licence-returns-presenter.test.js +++ b/test/presenters/licences/view-licence-returns-presenter.test.js @@ -12,6 +12,7 @@ const ViewLicenceReturnsPresenter = require('../../../app/presenters/licences/vi describe('View Licence returns presenter', () => { let returnData + beforeEach(() => { returnData = _returnData() }) From 47661e34b81cd88cbdaaf5865422e93cc40c947e Mon Sep 17 00:00:00 2001 From: jonathangoulding Date: Fri, 3 May 2024 17:29:23 +0100 Subject: [PATCH 31/33] fix: sonar return blocks --- .../licences/view-licence-returns.presenter.js | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/app/presenters/licences/view-licence-returns.presenter.js b/app/presenters/licences/view-licence-returns.presenter.js index e7b829fe56..00468e7305 100644 --- a/app/presenters/licences/view-licence-returns.presenter.js +++ b/app/presenters/licences/view-licence-returns.presenter.js @@ -42,8 +42,14 @@ function _formatReturnToTableRow (returns) { } function _formatStatus (status) { - if (status === 'completed') return 'COMPLETE' - if (status === 'due') return 'OVERDUE' + if (status === 'completed') { + return 'COMPLETE' + } + + if (status === 'due') { + return 'OVERDUE' + } + return 'NO STATUS' } From f55d8de7d39ef99b4a7edb39acc2003bf318f77b Mon Sep 17 00:00:00 2001 From: jonathangoulding Date: Fri, 3 May 2024 17:40:13 +0100 Subject: [PATCH 32/33] fix: fetch licence test service --- .../fetch-licence-returns.service.test.js | 26 +++++++++++-------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/test/services/licences/fetch-licence-returns.service.test.js b/test/services/licences/fetch-licence-returns.service.test.js index a34b8eecf8..f81cae3f4f 100644 --- a/test/services/licences/fetch-licence-returns.service.test.js +++ b/test/services/licences/fetch-licence-returns.service.test.js @@ -9,10 +9,10 @@ const { expect } = Code // Test helpers const DatabaseSupport = require('../../support/database.js') +const FetchLicenceReturnsService = require('../../../app/services/licences/fetch-licence-returns.service') const ReturnLogHelper = require('../../support/helpers/return-log.helper.js') // Thing under test -const FetchLicenceReturnsService = require('../../../app/services/licences/fetch-licence-returns.service') const LicenceHelper = require('../../support/helpers/licence.helper') describe('Fetch licence returns service', () => { @@ -23,13 +23,12 @@ describe('Fetch licence returns service', () => { }) describe('when the licence has return logs', () => { - let firstReturn - let latestReturn const dueDate = new Date('2020-04-01') const endDate = new Date('2020-06-01') const startDate = new Date('2020-02-01') const latestDueDate = new Date('2020-05-01') - const returnData = { + const firstReturn = { + id: '5fef371e-d0b5-4fd5-a7ff-a67d04b3f451', dueDate, endDate, metadata: '323', @@ -38,18 +37,23 @@ describe('Fetch licence returns service', () => { status: '32' } + const latestReturn = { + ...firstReturn, + id: 'b80f87a3-a274-4232-b536-750670d79928', + returnReference: '123', + dueDate: latestDueDate + } + beforeEach(async () => { const license = await LicenceHelper.add({ id: licenceId }) - returnData.licenceRef = license.licenceRef - firstReturn = await ReturnLogHelper.add(returnData) - latestReturn = await ReturnLogHelper.add({ - ...returnData, - returnReference: '123', - dueDate: latestDueDate - }) + firstReturn.licenceRef = license.licenceRef + latestReturn.licenceRef = license.licenceRef + + await ReturnLogHelper.add(firstReturn) + await ReturnLogHelper.add(latestReturn) }) it('returns results', async () => { From b0752088233628d125769b26439e6f0fb97fa9e6 Mon Sep 17 00:00:00 2001 From: Jonathan Goulding <58443816+jonathangoulding@users.noreply.github.com> Date: Fri, 3 May 2024 17:44:19 +0100 Subject: [PATCH 33/33] fix: reorder Co-authored-by: Alan Cruikshanks --- test/services/licences/fetch-licence-returns.service.test.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/services/licences/fetch-licence-returns.service.test.js b/test/services/licences/fetch-licence-returns.service.test.js index f81cae3f4f..38d0deb9a7 100644 --- a/test/services/licences/fetch-licence-returns.service.test.js +++ b/test/services/licences/fetch-licence-returns.service.test.js @@ -9,11 +9,11 @@ const { expect } = Code // Test helpers const DatabaseSupport = require('../../support/database.js') -const FetchLicenceReturnsService = require('../../../app/services/licences/fetch-licence-returns.service') +const LicenceHelper = require('../../support/helpers/licence.helper') const ReturnLogHelper = require('../../support/helpers/return-log.helper.js') // Thing under test -const LicenceHelper = require('../../support/helpers/licence.helper') +const FetchLicenceReturnsService = require('../../../app/services/licences/fetch-licence-returns.service') describe('Fetch licence returns service', () => { const licenceId = 'fef693fd-eb6f-478d-9f79-ab24749c5dc6'
Returns
Return reference and dates
{{ return.reference }} From cda3e80a363d5ca3bc24848765e33075366814a3 Mon Sep 17 00:00:00 2001 From: jonathangoulding Date: Fri, 3 May 2024 14:55:44 +0100 Subject: [PATCH 18/33] fix: spelling test issues --- app/presenters/licences/view-licence.presenter.js | 4 ++-- test/presenters/licences/view-licence-presenter.test.js | 1 + test/services/licences/view-licence.service.test.js | 1 + 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/app/presenters/licences/view-licence.presenter.js b/app/presenters/licences/view-licence.presenter.js index b9d9b7de47..0609378548 100644 --- a/app/presenters/licences/view-licence.presenter.js +++ b/app/presenters/licences/view-licence.presenter.js @@ -26,7 +26,7 @@ function go (licence, auth) { } = licence return { - licenseId: id, + licenceId: id, licenceName, licenceRef, notification: _determineNotificationBanner(includeInPresrocBilling, includeInSrocBilling), @@ -38,7 +38,7 @@ function go (licence, auth) { } function _determineNotificationBanner (includeInPresrocBilling, includeInSrocBilling) { - const baseMessage = 'This license has been marked for the next supplementary bill run' + const baseMessage = 'This licence has been marked for the next supplementary bill run' if (includeInPresrocBilling === 'yes' && includeInSrocBilling === true) { return baseMessage + 's for the current and old charge schemes.' diff --git a/test/presenters/licences/view-licence-presenter.test.js b/test/presenters/licences/view-licence-presenter.test.js index 532619a446..e90de00846 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({ + licenceId: 'f1288f6c-8503-4dc1-b114-75c408a14bd0', licenceName: 'Unregistered licence', licenceRef: '01/123', notification: null, diff --git a/test/services/licences/view-licence.service.test.js b/test/services/licences/view-licence.service.test.js index 70f58eb6ec..acecc747c2 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({ + licenseId: '2c80bd22-a005-4cf4-a2a2-73812a9861de', licenceName: 'Unregistered licence', licenceRef: '01/130/R01', notification: null, From 4835239bf82c89f9025428e2040fa31cf8b4949a Mon Sep 17 00:00:00 2001 From: jonathangoulding Date: Fri, 3 May 2024 15:43:33 +0100 Subject: [PATCH 19/33] fix: test --- test/services/licences/view-licence.service.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/services/licences/view-licence.service.test.js b/test/services/licences/view-licence.service.test.js index acecc747c2..f296f033d2 100644 --- a/test/services/licences/view-licence.service.test.js +++ b/test/services/licences/view-licence.service.test.js @@ -46,7 +46,7 @@ describe('View Licence service', () => { const result = await ViewLicenceService.go(testId) expect(result).to.equal({ - licenseId: '2c80bd22-a005-4cf4-a2a2-73812a9861de', + licenceId: '2c80bd22-a005-4cf4-a2a2-73812a9861de', licenceName: 'Unregistered licence', licenceRef: '01/130/R01', notification: null, From e9aa37b4950e31759d1bc4fa248e859dd7a2b3f6 Mon Sep 17 00:00:00 2001 From: jonathangoulding Date: Fri, 3 May 2024 16:12:52 +0100 Subject: [PATCH 20/33] test: fetch returns --- .../fetch-licence-returns.service.test.js | 65 +++++++++++++++++++ 1 file changed, 65 insertions(+) create mode 100644 test/services/licences/fetch-licence-returns.service.test.js diff --git a/test/services/licences/fetch-licence-returns.service.test.js b/test/services/licences/fetch-licence-returns.service.test.js new file mode 100644 index 0000000000..73ed495f45 --- /dev/null +++ b/test/services/licences/fetch-licence-returns.service.test.js @@ -0,0 +1,65 @@ +'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 ReturnLogHelper = require('../../support/helpers/return-log.helper.js') + +// Thing under test +const FetchLicenceReturnsService = require('../../../app/services/licences/fetch-licence-returns.service') +const LicenceHelper = require('../../support/helpers/licence.helper') + +describe('Fetch licence returns service', () => { + let returnsLogData + const licenceId = 'fef693fd-eb6f-478d-9f79-ab24749c5dc6' + + beforeEach(async () => { + await DatabaseSupport.clean() + }) + + describe('when there is no optional data in the model', () => { + const returnData = { + dueDate: new Date('2020-04-01'), + endDate: new Date('2020-06-01'), + metadata: '323', + returnReference: '32', + startDate: new Date('2020-02-01'), + status: '32' + } + + beforeEach(async () => { + const license = await LicenceHelper.add({ + id: licenceId + }) + + returnData.licenceRef = license.licenceRef + returnsLogData = await ReturnLogHelper.add(returnData) + }) + + it('returns results', async () => { + const result = await FetchLicenceReturnsService.go(licenceId, 1) + + expect(result.pagination).to.equal({ + total: 1 + }) + expect(result.returns).to.equal( + [{ + dueDate: new Date('2020-04-01'), + endDate: new Date('2020-06-01'), + id: returnsLogData.id, + metadata: 323, + returnReference: '32', + startDate: new Date('2020-02-01'), + status: '32' + } + ] + ) + }) + }) +}) From 4750d1d7f49ae7855419d391d3e78525013e3eae Mon Sep 17 00:00:00 2001 From: jonathangoulding Date: Fri, 3 May 2024 16:22:26 +0100 Subject: [PATCH 21/33] test: fetch returns with multiple returns --- .../fetch-licence-returns.service.test.js | 52 +++++++++++++------ 1 file changed, 37 insertions(+), 15 deletions(-) diff --git a/test/services/licences/fetch-licence-returns.service.test.js b/test/services/licences/fetch-licence-returns.service.test.js index 73ed495f45..2981ec5574 100644 --- a/test/services/licences/fetch-licence-returns.service.test.js +++ b/test/services/licences/fetch-licence-returns.service.test.js @@ -16,7 +16,6 @@ const FetchLicenceReturnsService = require('../../../app/services/licences/fetch const LicenceHelper = require('../../support/helpers/licence.helper') describe('Fetch licence returns service', () => { - let returnsLogData const licenceId = 'fef693fd-eb6f-478d-9f79-ab24749c5dc6' beforeEach(async () => { @@ -24,12 +23,18 @@ describe('Fetch licence returns service', () => { }) describe('when there is no optional data in the model', () => { + let firstReturn + let latestReturn + const dueDate = new Date('2020-04-01') + const endDate = new Date('2020-06-01') + const startDate = new Date('2020-02-01') + const latestDueDate = new Date('2020-05-01') const returnData = { - dueDate: new Date('2020-04-01'), - endDate: new Date('2020-06-01'), + dueDate, + endDate, metadata: '323', returnReference: '32', - startDate: new Date('2020-02-01'), + startDate, status: '32' } @@ -39,25 +44,42 @@ describe('Fetch licence returns service', () => { }) returnData.licenceRef = license.licenceRef - returnsLogData = await ReturnLogHelper.add(returnData) + firstReturn = await ReturnLogHelper.add(returnData) + latestReturn = await ReturnLogHelper.add({ + ...returnData, + returnReference: '123', + dueDate: latestDueDate + }) }) it('returns results', async () => { const result = await FetchLicenceReturnsService.go(licenceId, 1) expect(result.pagination).to.equal({ - total: 1 + total: 2 }) + // This should be ordered by due date + // id: returnsLogData.id, expect(result.returns).to.equal( - [{ - dueDate: new Date('2020-04-01'), - endDate: new Date('2020-06-01'), - id: returnsLogData.id, - metadata: 323, - returnReference: '32', - startDate: new Date('2020-02-01'), - status: '32' - } + [ + { + dueDate: latestDueDate, + endDate, + id: latestReturn.id, + metadata: 323, + returnReference: '123', + startDate, + status: '32' + }, + { + dueDate, + endDate, + id: firstReturn.id, + metadata: 323, + returnReference: '32', + startDate, + status: '32' + } ] ) }) From 2327a0b5308d115bea4ac27d8fd4789b585a64fa Mon Sep 17 00:00:00 2001 From: jonathangoulding Date: Fri, 3 May 2024 16:27:04 +0100 Subject: [PATCH 22/33] test: fetch returns with multiple returns --- test/services/licences/fetch-licence-returns.service.test.js | 1 - 1 file changed, 1 deletion(-) diff --git a/test/services/licences/fetch-licence-returns.service.test.js b/test/services/licences/fetch-licence-returns.service.test.js index 2981ec5574..cf7d44cb4e 100644 --- a/test/services/licences/fetch-licence-returns.service.test.js +++ b/test/services/licences/fetch-licence-returns.service.test.js @@ -59,7 +59,6 @@ describe('Fetch licence returns service', () => { total: 2 }) // This should be ordered by due date - // id: returnsLogData.id, expect(result.returns).to.equal( [ { From aa2092c0eb57a691502b278cdaa2d6428f8d6acf Mon Sep 17 00:00:00 2001 From: Jonathan Goulding <58443816+jonathangoulding@users.noreply.github.com> Date: Fri, 3 May 2024 17:12:07 +0100 Subject: [PATCH 23/33] fix: pr alpha sort white spaces content --- app/controllers/licences.controller.js | 4 ++-- app/presenters/licences/view-licence-returns.presenter.js | 5 +++-- app/routes/licence.routes.js | 4 ++-- app/services/licences/view-licence-returns.service.js | 1 + test/services/licences/fetch-licence-returns.service.test.js | 2 +- test/services/licences/view-licence-returns.service.test.js | 2 ++ 6 files changed, 11 insertions(+), 7 deletions(-) diff --git a/app/controllers/licences.controller.js b/app/controllers/licences.controller.js index 23f2582a34..cfa334c03e 100644 --- a/app/controllers/licences.controller.js +++ b/app/controllers/licences.controller.js @@ -49,6 +49,6 @@ async function viewReturns (request, h) { module.exports = { noReturnsRequired, returnsRequired, - viewSummary, - viewReturns + viewReturns, + viewSummary } diff --git a/app/presenters/licences/view-licence-returns.presenter.js b/app/presenters/licences/view-licence-returns.presenter.js index 3683f08e05..6f021260bc 100644 --- a/app/presenters/licences/view-licence-returns.presenter.js +++ b/app/presenters/licences/view-licence-returns.presenter.js @@ -1,7 +1,7 @@ 'use strict' /** - * Formats data for common licence data `/licences/{id}` page's + * Formats common data for the `/licences/{id}/*` view licence pages * @module ViewLicenceReturnsPresenter */ @@ -24,6 +24,7 @@ function go (returnsData) { function _formatPurpose (purpose) { const [firstPurpose] = purpose + return firstPurpose.alias ? firstPurpose.alias : firstPurpose.tertiary.description } @@ -34,7 +35,7 @@ function _formatStatus (status) { } function _formatReturnToTableRow (returns) { - return returns.map(r => { + return returns.map((r) => { return { id: r.id, reference: r.returnReference, diff --git a/app/routes/licence.routes.js b/app/routes/licence.routes.js index 1b9ae4f438..c71d066d8c 100644 --- a/app/routes/licence.routes.js +++ b/app/routes/licence.routes.js @@ -8,7 +8,7 @@ const routes = [ path: '/licences/{id}/summary', handler: LicencesController.viewSummary, options: { - description: 'View a summary licence page' + description: 'View a licence summary page' } }, { @@ -16,7 +16,7 @@ const routes = [ path: '/licences/{id}/returns', handler: LicencesController.viewReturns, options: { - description: 'View a returns licence page' + description: 'View a licence returns page' } }, { diff --git a/app/services/licences/view-licence-returns.service.js b/app/services/licences/view-licence-returns.service.js index 707cebbc45..fca6cc8e0f 100644 --- a/app/services/licences/view-licence-returns.service.js +++ b/app/services/licences/view-licence-returns.service.js @@ -9,6 +9,7 @@ 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') + /** * Orchestrates fetching and presenting the data needed for the licence summary page * diff --git a/test/services/licences/fetch-licence-returns.service.test.js b/test/services/licences/fetch-licence-returns.service.test.js index cf7d44cb4e..a34b8eecf8 100644 --- a/test/services/licences/fetch-licence-returns.service.test.js +++ b/test/services/licences/fetch-licence-returns.service.test.js @@ -22,7 +22,7 @@ describe('Fetch licence returns service', () => { await DatabaseSupport.clean() }) - describe('when there is no optional data in the model', () => { + describe('when the licence has return logs', () => { let firstReturn let latestReturn const dueDate = new Date('2020-04-01') diff --git a/test/services/licences/view-licence-returns.service.test.js b/test/services/licences/view-licence-returns.service.test.js index 2c24c241df..6153035381 100644 --- a/test/services/licences/view-licence-returns.service.test.js +++ b/test/services/licences/view-licence-returns.service.test.js @@ -13,6 +13,7 @@ const ViewLicenceService = require('../../../app/services/licences/view-licence. 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') + // Thing under test const ViewLicenceReturnsService = require('../../../app/services/licences/view-licence-returns.service') @@ -21,6 +22,7 @@ describe('View Licence service returns', () => { const page = 1 const auth = {} const pagination = { page } + beforeEach(() => { Sinon.stub(FetchLicenceReturnsService, 'go').resolves(_returnsFetch()) Sinon.stub(PaginatorPresenter, 'go').returns(pagination) From b895c74d55c5800be2fe4e3588ddbc44d0585ad1 Mon Sep 17 00:00:00 2001 From: jonathangoulding Date: Fri, 3 May 2024 17:18:46 +0100 Subject: [PATCH 24/33] fix: hard code returns url in the njk --- app/presenters/licences/view-licence-returns.presenter.js | 3 +-- app/views/licences/tabs/returns.njk | 2 +- .../presenters/licences/view-licence-returns-presenter.test.js | 1 - test/services/licences/view-licence-returns.service.test.js | 2 -- 4 files changed, 2 insertions(+), 6 deletions(-) diff --git a/app/presenters/licences/view-licence-returns.presenter.js b/app/presenters/licences/view-licence-returns.presenter.js index 6f021260bc..4932a6d734 100644 --- a/app/presenters/licences/view-licence-returns.presenter.js +++ b/app/presenters/licences/view-licence-returns.presenter.js @@ -17,14 +17,13 @@ function go (returnsData) { return { activeTab: 'returns', - returnsUrl: 'return/internal', returns } } function _formatPurpose (purpose) { const [firstPurpose] = purpose - + return firstPurpose.alias ? firstPurpose.alias : firstPurpose.tertiary.description } diff --git a/app/views/licences/tabs/returns.njk b/app/views/licences/tabs/returns.njk index 26f168122e..51ac8cc585 100644 --- a/app/views/licences/tabs/returns.njk +++ b/app/views/licences/tabs/returns.njk @@ -34,7 +34,7 @@ {% for return in returns %}
- {{ return.reference }} + {{ return.reference }}

{{ return.dates }}

{{ return.purpose }}

{{ return.description }}