From 8d5af349aaaf160d69439333b966ea847cfb1c08 Mon Sep 17 00:00:00 2001 From: Alan Cruikshanks Date: Tue, 9 Jul 2024 15:54:56 +0100 Subject: [PATCH 1/4] Fix workflow start date in view licence set up tab https://eaflood.atlassian.net/browse/WATER-4556 In testing of the new view licence page, specifically the licence set up tab, it was spotted that when a charge version has a status of `REVIEW` that the date the record was created rather than the start date is displayed in the charge information table. We fixed that in [Fix dates in view licence set up tab](https://github.com/DEFRA/water-abstraction-system/pull/1175). But hot on its heels someone has pointed out the same applies for a workflow record with a status of `changes_requested`. So, this change fixes it so we display the charge version start date for a workflow record with a status of `changes_requested` in the charge information table of the Licence setup tab in our new view licence page. From 708a2f144f2262b46f3b7fe45f46430b957ccc7b Mon Sep 17 00:00:00 2001 From: Alan Cruikshanks Date: Tue, 9 Jul 2024 16:06:19 +0100 Subject: [PATCH 2/4] Add tests 4 workflow with changes_requested status --- .../licences/set-up.presenter.test.js | 44 +++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/test/presenters/licences/set-up.presenter.test.js b/test/presenters/licences/set-up.presenter.test.js index e42fd56539..40235fb8ce 100644 --- a/test/presenters/licences/set-up.presenter.test.js +++ b/test/presenters/licences/set-up.presenter.test.js @@ -451,6 +451,50 @@ describe('Licences - Set Up presenter', () => { }) }) + describe('that have a status of "changes_requested"', () => { + beforeEach(() => { + workflows = [{ ...workflow }] + workflows[0].status = 'changes_requested' + }) + + describe('and the user is permitted to review workflow records', () => { + beforeEach(() => { + auth.credentials.scope = ['billing', 'charge_version_workflow_reviewer'] + }) + + it('correctly presents the data and workflow actions', () => { + const result = SetUpPresenter.go(chargeVersions, workflows, agreements, returnVersions, auth, commonData) + + expect(result.chargeInformation).to.equal([{ + action: [{ + link: '/licences/f91bf145-ce8e-481c-a842-4da90348062b/charge-information/f547f465-0a62-45ff-9909-38825f05e0c4/review', + text: 'Review' + }], + id: 'f547f465-0a62-45ff-9909-38825f05e0c4', + startDate: '1 April 2022', + endDate: '', + status: 'change request', + reason: 'changed something' + }]) + }) + }) + + describe('and the user is not permitted to review workflow records', () => { + it('correctly presents the data and workflow actions', () => { + const result = SetUpPresenter.go(chargeVersions, workflows, agreements, returnVersions, auth, commonData) + + expect(result.chargeInformation).to.equal([{ + action: [], + id: 'f547f465-0a62-45ff-9909-38825f05e0c4', + startDate: '1 April 2022', + endDate: '', + status: 'change request', + reason: 'changed something' + }]) + }) + }) + }) + describe('that have a status of "to_setup"', () => { beforeEach(() => { workflows = [{ ...workflow }] From 18fe2a473b39fb83b4a76981bbbb4acd432a258e Mon Sep 17 00:00:00 2001 From: Alan Cruikshanks Date: Tue, 9 Jul 2024 16:09:17 +0100 Subject: [PATCH 3/4] Implement fix --- app/presenters/licences/set-up.presenter.js | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/app/presenters/licences/set-up.presenter.js b/app/presenters/licences/set-up.presenter.js index 64244abe36..f854d2f4f7 100644 --- a/app/presenters/licences/set-up.presenter.js +++ b/app/presenters/licences/set-up.presenter.js @@ -262,14 +262,14 @@ function _workflowActionReviewer (workflow) { } function _workflowStartDate (workflow) { - if (workflow.status === 'review') { - // Stored as JSON the date is returned as a string. So, we need to convert it to a date type first - const startDate = new Date(workflow.data.chargeVersion.dateRange.startDate) - - return formatLongDate(startDate) + if (workflow.status === 'to_setup') { + return '' } - return '' + // Stored as JSON the date is returned as a string. So, we need to convert it to a date type first + const startDate = new Date(workflow.data.chargeVersion.dateRange.startDate) + + return formatLongDate(startDate) } module.exports = { From 323fbaa493234558e39bff50e9f9d8cc25ed48a2 Mon Sep 17 00:00:00 2001 From: Alan Cruikshanks Date: Tue, 9 Jul 2024 16:35:13 +0100 Subject: [PATCH 4/4] Housekeeping - match tags to data and convention This changes updates the status tags for charge versions (including workflow) and return versions so that - they only expect what we actually store in the DB (these are custom types so can only be these values) - that the tag macros follow the convention we've used in others We also split out the tag handling for return and charge versions to make it easier to see what we would expect to handle and to give us more flexibility should that change. --- app/presenters/licences/set-up.presenter.js | 22 ++------- .../return-requirements/view.presenter.js | 11 +---- app/views/licences/tabs/set-up.njk | 8 ++-- .../macros/charge-version-status-tag.njk | 45 +++++++++++-------- .../macros/return-version-status-tag.njk | 27 +++++++++++ app/views/return-requirements/view.njk | 3 +- .../licences/set-up.presenter.test.js | 18 ++++---- .../view.presenter.test.js | 2 +- .../view-licence-set-up.service.test.js | 4 +- .../return-requirements/view.service.test.js | 2 +- 10 files changed, 78 insertions(+), 64 deletions(-) create mode 100644 app/views/macros/return-version-status-tag.njk diff --git a/app/presenters/licences/set-up.presenter.js b/app/presenters/licences/set-up.presenter.js index f854d2f4f7..b5ae9d3e97 100644 --- a/app/presenters/licences/set-up.presenter.js +++ b/app/presenters/licences/set-up.presenter.js @@ -136,7 +136,7 @@ function _chargeVersions (chargeVersions) { id: chargeVersion.id, startDate: formatLongDate(chargeVersion.startDate), endDate: chargeVersion.endDate ? formatLongDate(chargeVersion.endDate) : '', - status: _status(chargeVersion.status), + status: chargeVersion.status, reason: chargeVersion.changeReason?.description, action: [ { @@ -182,7 +182,7 @@ function _returnVersions (returnVersions = [{}]) { endDate: returnVersion.endDate ? formatLongDate(returnVersion.endDate) : '', reason: returnVersion.reason ? returnRequirementReasons[returnVersion.reason] : '', startDate: formatLongDate(returnVersion.startDate), - status: _status(returnVersion.status) + status: returnVersion.status } }) } @@ -198,22 +198,6 @@ function _returnVersionsLinks (commonData, enableRequirementsForReturns) { return {} } -function _status (status) { - const statuses = { - current: 'approved', - draft: 'draft', - approved: 'approved', - replaced: 'replaced', - superseded: 'replaced', - invalid: 'invalid', - review: 'review', - changes_requested: 'change request', - to_setup: 'to set up' - } - - return statuses[status] -} - function _workflows (workflows, auth) { return workflows.map((workflow) => { return { @@ -222,7 +206,7 @@ function _workflows (workflows, auth) { id: workflow.id, reason: workflow.data.chargeVersion?.changeReason?.description, startDate: _workflowStartDate(workflow), - status: _status(workflow.status) + status: workflow.status } }) } diff --git a/app/presenters/return-requirements/view.presenter.js b/app/presenters/return-requirements/view.presenter.js index 4aaf92de1e..3d458c4cba 100644 --- a/app/presenters/return-requirements/view.presenter.js +++ b/app/presenters/return-requirements/view.presenter.js @@ -34,7 +34,7 @@ function go (requirementsForReturns) { reason: returnRequirementReasons[reason] || '', requirements: _requirements(returnRequirements), startDate: formatLongDate(startDate), - status: _status(status), + status, createdDate: formatLongDate(createdAt), createdBy: user ? user.username : 'Migrated from NALD' } @@ -129,15 +129,6 @@ function _requirements (requirements) { }) } -function _status (status) { - const statuses = { - current: 'approved', - superseded: 'replaced' - } - - return statuses[status] -} - module.exports = { go } diff --git a/app/views/licences/tabs/set-up.njk b/app/views/licences/tabs/set-up.njk index 2d509c42a0..c9dcc8d392 100644 --- a/app/views/licences/tabs/set-up.njk +++ b/app/views/licences/tabs/set-up.njk @@ -1,6 +1,8 @@ {% from "govuk/components/button/macro.njk" import govukButton %} {% from "govuk/components/table/macro.njk" import govukTable %} -{% from "macros/charge-version-status-tag.njk" import statusTag %} + +{% from "macros/charge-version-status-tag.njk" import statusTag as chargeVersionStatusTag %} +{% from "macros/return-version-status-tag.njk" import statusTag as returnVersionStatusTag %} {% macro createLink(data) %} {% for linkItem in data.action %} @@ -30,7 +32,7 @@ text: returnsRequirement.reason }, { - html: statusTag(returnsRequirement.status) + html: returnVersionStatusTag(returnsRequirement.status, true) }, { html: createLink(returnsRequirement) @@ -99,7 +101,7 @@ text: chargeVersion.reason }, { - html: statusTag(chargeVersion.status) + html: chargeVersionStatusTag(chargeVersion.status, true) }, { html: createLink(chargeVersion) diff --git a/app/views/macros/charge-version-status-tag.njk b/app/views/macros/charge-version-status-tag.njk index 2501f16e0a..16a8e43d48 100644 --- a/app/views/macros/charge-version-status-tag.njk +++ b/app/views/macros/charge-version-status-tag.njk @@ -1,26 +1,35 @@ {% from "govuk/components/tag/macro.njk" import govukTag %} -{% macro statusTag(status) %} - {% if status === 'approved' %} - {% set classes = "govuk-tag--green" %} - {% elif status === 'replaced' %} - {% set classes = "govuk-tag--grey" %} - {% elif status === 'superseded' %} - {% set classes = "govuk-tag--grey" %} - {% elif status === 'invalid' %} - {% set classes = "govuk-tag--red" %} - {% elif status === 'review' %} - {% set classes = "govuk-tag--orange" %} - {% elif status === 'change request' %} - {% set classes = "govuk-tag--blue" %} - {% elif status === 'to set up' %} - {% set classes = "govuk-tag--blue" %} +{% macro statusTag(status, inline=false) %} + {# Default the text to the status as that covers most tags #} + {% set text = status %} + + {% if status === 'current' %} + {% set color = "govuk-tag--green" %} + {% set text = 'approved' %} + {% elif status === 'superseded' %} + {% set color = "govuk-tag--grey" %} + {% set text = 'replaced' %} + {% elif status === 'review' %} + {% set color = "govuk-tag--orange" %} + {% elif status === 'changes_requested' %} + {% set color = "govuk-tag--blue" %} + {% set text = 'change request' %} + {% elif status === 'to_setup' %} + {% set color = "govuk-tag--blue" %} + {% set text = 'to set up' %} + {% else %} + {% set color = "govuk-tag--blue" %} + {% endif %} + + {% if inline %} + {% set fontSize = "" %} {% else %} - {% set classes = "" %} + {% set fontSize = "govuk-!-font-size-27" %} {% endif %} {{ govukTag({ - text: status, - classes: classes + text: text, + classes: color + ' ' + fontSize }) }} {% endmacro %} diff --git a/app/views/macros/return-version-status-tag.njk b/app/views/macros/return-version-status-tag.njk new file mode 100644 index 0000000000..61f29041ac --- /dev/null +++ b/app/views/macros/return-version-status-tag.njk @@ -0,0 +1,27 @@ +{% from "govuk/components/tag/macro.njk" import govukTag %} + +{% macro statusTag(status, inline=false) %} + {# Default the text to the status as that covers most tags #} + {% set text = status %} + + {% if status === 'current' %} + {% set color = "govuk-tag--green" %} + {% set text = 'approved' %} + {% elif status === 'superseded' %} + {% set color = "govuk-tag--grey" %} + {% set text = 'replaced' %} + {% else %} + {% set color = "govuk-tag--blue" %} + {% endif %} + + {% if inline %} + {% set fontSize = "" %} + {% else %} + {% set fontSize = "govuk-!-font-size-27" %} + {% endif %} + + {{ govukTag({ + text: text, + classes: color + ' ' + fontSize + }) }} +{% endmacro %} diff --git a/app/views/return-requirements/view.njk b/app/views/return-requirements/view.njk index 458ce50972..20a36ad720 100644 --- a/app/views/return-requirements/view.njk +++ b/app/views/return-requirements/view.njk @@ -1,7 +1,8 @@ {% extends 'layout.njk' %} {% from "govuk/components/back-link/macro.njk" import govukBackLink %} {% from "govuk/components/summary-list/macro.njk" import govukSummaryList %} -{% from "macros/charge-version-status-tag.njk" import statusTag %} + +{% from "macros/return-version-status-tag.njk" import statusTag %} {% macro newLine(array) %} {% for item in array %} diff --git a/test/presenters/licences/set-up.presenter.test.js b/test/presenters/licences/set-up.presenter.test.js index 40235fb8ce..1883dcee10 100644 --- a/test/presenters/licences/set-up.presenter.test.js +++ b/test/presenters/licences/set-up.presenter.test.js @@ -343,7 +343,7 @@ describe('Licences - Set Up presenter', () => { id: '0d514aa4-1550-46b1-8195-878957f2a5f8', startDate: '1 January 2020', endDate: '', - status: 'approved', + status: 'current', reason: 'Major change' } ]) @@ -373,7 +373,7 @@ describe('Licences - Set Up presenter', () => { id: '0d514aa4-1550-46b1-8195-878957f2a5f8', startDate: '1 January 2020', endDate: '', - status: 'approved', + status: 'current', reason: 'Major change' }]) }) @@ -396,7 +396,7 @@ describe('Licences - Set Up presenter', () => { id: '0d514aa4-1550-46b1-8195-878957f2a5f8', startDate: '1 January 2020', endDate: '31 March 2024', - status: 'approved', + status: 'current', reason: 'Major change' }]) }) @@ -473,7 +473,7 @@ describe('Licences - Set Up presenter', () => { id: 'f547f465-0a62-45ff-9909-38825f05e0c4', startDate: '1 April 2022', endDate: '', - status: 'change request', + status: 'changes_requested', reason: 'changed something' }]) }) @@ -488,7 +488,7 @@ describe('Licences - Set Up presenter', () => { id: 'f547f465-0a62-45ff-9909-38825f05e0c4', startDate: '1 April 2022', endDate: '', - status: 'change request', + status: 'changes_requested', reason: 'changed something' }]) }) @@ -517,7 +517,7 @@ describe('Licences - Set Up presenter', () => { id: 'f547f465-0a62-45ff-9909-38825f05e0c4', startDate: '', endDate: '', - status: 'to set up', + status: 'to_setup', reason: 'changed something' }]) }) @@ -536,7 +536,7 @@ describe('Licences - Set Up presenter', () => { id: 'f547f465-0a62-45ff-9909-38825f05e0c4', startDate: '', endDate: '', - status: 'to set up', + status: 'to_setup', reason: 'changed something' }]) }) @@ -563,7 +563,7 @@ describe('Licences - Set Up presenter', () => { endDate: '', reason: 'Change to special agreement', startDate: '1 January 2020', - status: 'approved' + status: 'current' } ]) }) @@ -587,7 +587,7 @@ describe('Licences - Set Up presenter', () => { endDate: '', reason: '', startDate: '1 January 2020', - status: 'approved' + status: 'current' } ]) }) diff --git a/test/presenters/return-requirements/view.presenter.test.js b/test/presenters/return-requirements/view.presenter.test.js index d032b29779..4568bc8c93 100644 --- a/test/presenters/return-requirements/view.presenter.test.js +++ b/test/presenters/return-requirements/view.presenter.test.js @@ -110,7 +110,7 @@ describe('Return Requirements - View presenter', () => { } ], startDate: '21 April 2023', - status: 'approved' + status: 'current' }) }) diff --git a/test/services/licences/view-licence-set-up.service.test.js b/test/services/licences/view-licence-set-up.service.test.js index f41ffabce9..05233afdae 100644 --- a/test/services/licences/view-licence-set-up.service.test.js +++ b/test/services/licences/view-licence-set-up.service.test.js @@ -129,7 +129,7 @@ describe('View Licence Set Up service', () => { id: 'c0601335-b6ad-4651-b54b-c586f8d22ac3', reason: 'Missing thing', startDate: '1 January 2020', - status: 'approved' + status: 'current' } ], licenceId: testId, @@ -153,7 +153,7 @@ describe('View Licence Set Up service', () => { endDate: '1 February 2025', reason: 'Change to special agreement', startDate: '1 January 2025', - status: 'approved' + status: 'current' } ] }) diff --git a/test/services/return-requirements/view.service.test.js b/test/services/return-requirements/view.service.test.js index 3557ccdee0..c29136acfe 100644 --- a/test/services/return-requirements/view.service.test.js +++ b/test/services/return-requirements/view.service.test.js @@ -79,7 +79,7 @@ describe('Return Requirements - View service', () => { } ], startDate: '1 April 2022', - status: 'approved' + status: 'current' }) }) })