From 6d84354a7660a8296cf469e3dbf076d9fb2d31fc Mon Sep 17 00:00:00 2001 From: Alan Cruikshanks Date: Tue, 9 Jul 2024 12:56:17 +0100 Subject: [PATCH 01/10] Fix dates in view licence set up tab https://eaflood.atlassian.net/browse/WATER-4556 > Part of the work to replace the legacy view licence page 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. This is because the record is not a charge version; it is a workflow record which when approved will become a charge version record. The current presenter logic is not distinguishing between the different types of workflow record hence the wrong date being displayed. But hold on. A quick review of the legacy version of the tab shows that when a workflow record does not have a status of `REVIEW` we don't show any dates at all! Then we spotted that there is inconsistency about whether we use a dash in the end date column. The charge information table does, but the return requirements and agreements tables don't. So, this change is a general fix up of dates in the tab. - for workflow records in `REVIEW` show the start date from the stored charge version details - for all tables, where end date is not set display a `-` - do not display any dates for workflow records that have a status other than `REVIEW` From 9eb615ac08140da06462f677b10b64b7a3187400 Mon Sep 17 00:00:00 2001 From: Alan Cruikshanks Date: Tue, 9 Jul 2024 13:12:09 +0100 Subject: [PATCH 02/10] Update presenter tests We do a little housekeeping to bring the presenter's tests a little closer to our conventions and deal with some linting issues. We also make the starting data more realistic by aligning the dates and not populating te end dates. Beyond that we change the assertions in the tests to match how we expect the data to be displayed in the page. --- app/presenters/licences/set-up.presenter.js | 13 ++++- .../licences/set-up.presenter.test.js | 51 +++++++++++-------- 2 files changed, 42 insertions(+), 22 deletions(-) diff --git a/app/presenters/licences/set-up.presenter.js b/app/presenters/licences/set-up.presenter.js index ee02cdca39..35ed71a866 100644 --- a/app/presenters/licences/set-up.presenter.js +++ b/app/presenters/licences/set-up.presenter.js @@ -221,7 +221,7 @@ function _workflows (workflows, auth) { endDate: '-', id: workflow.id, reason: workflow.data.chargeVersion?.changeReason?.description, - startDate: workflow.createdAt ? formatLongDate(workflow.createdAt) : '-', + startDate: _workflowStartDate(workflow), status: _status(workflow.status) } }) @@ -261,6 +261,17 @@ 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) + } + + return '-' +} + module.exports = { go } diff --git a/test/presenters/licences/set-up.presenter.test.js b/test/presenters/licences/set-up.presenter.test.js index fa2b3f2c69..cf275ba53c 100644 --- a/test/presenters/licences/set-up.presenter.test.js +++ b/test/presenters/licences/set-up.presenter.test.js @@ -26,7 +26,7 @@ describe('Licences - Set Up presenter', () => { const chargeVersion = { id: '0d514aa4-1550-46b1-8195-878957f2a5f8', startDate: new Date('2020-01-01'), - endDate: new Date('2020-09-01'), + endDate: null, status: 'current', changeReason: { description: 'Major change' }, licenceId: 'f91bf145-ce8e-481c-a842-4da90348062b' @@ -34,8 +34,8 @@ describe('Licences - Set Up presenter', () => { const returnVersion = { id: '0312e5eb-67ae-44fb-922c-b1a0b81bc08d', - startDate: new Date('2025-01-01'), - endDate: new Date('2025-02-01'), + startDate: new Date('2020-01-01'), + endDate: null, status: 'current', reason: 'change-to-special-agreement' } @@ -44,7 +44,12 @@ describe('Licences - Set Up presenter', () => { id: 'f547f465-0a62-45ff-9909-38825f05e0c4', createdAt: new Date('2020-01-01'), status: 'review', - data: { chargeVersion: { changeReason: { description: 'changed something' } } }, + data: { + chargeVersion: { + changeReason: { description: 'changed something' }, + dateRange: { startDate: '2022-04-01' } + } + }, licenceId: 'f91bf145-ce8e-481c-a842-4da90348062b' } @@ -118,9 +123,9 @@ describe('Licences - Set Up presenter', () => { text: 'Recalculate bills' } ], - signedOn: '', + signedOn: '-', description: 'Two-part tariff', - endDate: '', + endDate: '-', startDate: '1 January 2020' } ]) @@ -294,6 +299,7 @@ describe('Licences - Set Up presenter', () => { describe('when the licence is more than 6 years old and all the actions are available for an agreement', () => { beforeEach(() => { const sixYearsAndOneDayAgo = new Date() + sixYearsAndOneDayAgo.setDate(sixYearsAndOneDayAgo.getDate() - 1) sixYearsAndOneDayAgo.setFullYear(sixYearsAndOneDayAgo.getFullYear() - 6) @@ -317,14 +323,14 @@ describe('Licences - Set Up presenter', () => { workflows = [{ ...workflow }] }) - it('groups both types of data into the \'chargeInformation\' property', () => { + it('groups both types of data into the "chargeInformation" property', () => { const result = SetUpPresenter.go(chargeVersions, workflows, agreements, returnVersions, auth, commonData) expect(result.chargeInformation).to.equal([ { action: [], id: 'f547f465-0a62-45ff-9909-38825f05e0c4', - startDate: '1 January 2020', + startDate: '1 April 2022', endDate: '-', status: 'review', reason: 'changed something' @@ -336,7 +342,7 @@ describe('Licences - Set Up presenter', () => { }], id: '0d514aa4-1550-46b1-8195-878957f2a5f8', startDate: '1 January 2020', - endDate: '1 September 2020', + endDate: '-', status: 'approved', reason: 'Major change' } @@ -376,6 +382,7 @@ describe('Licences - Set Up presenter', () => { describe('where the end date is populated', () => { beforeEach(() => { chargeVersions = [{ ...chargeVersion }] + chargeVersions[0].endDate = new Date('2024-03-31') }) it('correctly presents the data with the end date', () => { @@ -388,7 +395,7 @@ describe('Licences - Set Up presenter', () => { }], id: '0d514aa4-1550-46b1-8195-878957f2a5f8', startDate: '1 January 2020', - endDate: '1 September 2020', + endDate: '31 March 2024', status: 'approved', reason: 'Major change' }]) @@ -401,7 +408,7 @@ describe('Licences - Set Up presenter', () => { chargeVersions = [] }) - describe('that have a status of \'review\'', () => { + describe('that have a status of "review"', () => { beforeEach(() => { workflows = [{ ...workflow }] }) @@ -420,7 +427,7 @@ describe('Licences - Set Up presenter', () => { text: 'Review' }], id: 'f547f465-0a62-45ff-9909-38825f05e0c4', - startDate: '1 January 2020', + startDate: '1 April 2024', endDate: '-', status: 'review', reason: 'changed something' @@ -435,7 +442,7 @@ describe('Licences - Set Up presenter', () => { expect(result.chargeInformation).to.equal([{ action: [], id: 'f547f465-0a62-45ff-9909-38825f05e0c4', - startDate: '1 January 2020', + startDate: '1 April 2024', endDate: '-', status: 'review', reason: 'changed something' @@ -444,7 +451,7 @@ describe('Licences - Set Up presenter', () => { }) }) - describe('that have a status of \'to_setup\'', () => { + describe('that have a status of "to_setup"', () => { beforeEach(() => { workflows = [{ ...workflow }] workflows[0].status = 'to_setup' @@ -464,7 +471,7 @@ describe('Licences - Set Up presenter', () => { text: 'Review' }], id: 'f547f465-0a62-45ff-9909-38825f05e0c4', - startDate: '1 January 2020', + startDate: '-', endDate: '-', status: 'to set up', reason: 'changed something' @@ -483,7 +490,7 @@ describe('Licences - Set Up presenter', () => { expect(result.chargeInformation).to.equal([{ action: [], id: 'f547f465-0a62-45ff-9909-38825f05e0c4', - startDate: '1 January 2020', + startDate: '-', endDate: '-', status: 'to set up', reason: 'changed something' @@ -509,9 +516,9 @@ describe('Licences - Set Up presenter', () => { text: 'View' } ], - endDate: '1 February 2025', + endDate: '-', reason: 'Change to special agreement', - startDate: '1 January 2025', + startDate: '1 January 2020', status: 'approved' } ]) @@ -533,9 +540,9 @@ describe('Licences - Set Up presenter', () => { text: 'View' } ], - endDate: '', + endDate: '-', reason: '', - startDate: '1 January 2025', + startDate: '1 January 2020', status: 'approved' } ]) @@ -586,6 +593,7 @@ describe('Licences - Set Up presenter', () => { describe('and the licence ends more than 6 years ago', () => { beforeEach(() => { const sixYearsAndOneDayAgo = new Date() + sixYearsAndOneDayAgo.setDate(sixYearsAndOneDayAgo.getDate() - 1) sixYearsAndOneDayAgo.setFullYear(sixYearsAndOneDayAgo.getFullYear() - 6) @@ -617,9 +625,10 @@ describe('Licences - Set Up presenter', () => { .equal('/licences/f91bf145-ce8e-481c-a842-4da90348062b/charge-information/create') }) }) - describe('and the licence \'ends\' more than 6 years ago', () => { + describe('and the licence "ends" more than 6 years ago', () => { beforeEach(() => { const sixYearsAndOneDayAgo = new Date() + sixYearsAndOneDayAgo.setDate(sixYearsAndOneDayAgo.getDate() - 1) sixYearsAndOneDayAgo.setFullYear(sixYearsAndOneDayAgo.getFullYear() - 6) From c8f25adb7257ec347634571d5c7ec3c1a15741a5 Mon Sep 17 00:00:00 2001 From: Alan Cruikshanks Date: Tue, 9 Jul 2024 13:21:33 +0100 Subject: [PATCH 03/10] Implement fix for workflow start date --- app/services/licences/view-licence-set-up.service.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/app/services/licences/view-licence-set-up.service.js b/app/services/licences/view-licence-set-up.service.js index 3eb68f0219..3bb3d5ee6d 100644 --- a/app/services/licences/view-licence-set-up.service.js +++ b/app/services/licences/view-licence-set-up.service.js @@ -25,7 +25,9 @@ async function go (licenceId, auth) { const agreements = await FetchAgreementsService.go(commonData.licenceRef) const chargeVersions = await FetchChargeVersionsService.go(licenceId) + console.log('🚀 ~ go ~ chargeVersions:', chargeVersions) const workflows = await FetchWorkflowsService.go(licenceId) + console.log('🚀 ~ go ~ workflows:', workflows) const returnVersions = await FetchReturnVersionsService.go(licenceId) const licenceSetUpData = SetUpPresenter From e95c8cb09b54eee2ed3ca028a972ca6250b2d7b0 Mon Sep 17 00:00:00 2001 From: Alan Cruikshanks Date: Tue, 9 Jul 2024 13:22:40 +0100 Subject: [PATCH 04/10] Make return requirements end date consistent --- app/presenters/licences/set-up.presenter.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/presenters/licences/set-up.presenter.js b/app/presenters/licences/set-up.presenter.js index 35ed71a866..db868bdd33 100644 --- a/app/presenters/licences/set-up.presenter.js +++ b/app/presenters/licences/set-up.presenter.js @@ -179,7 +179,7 @@ function _returnVersions (returnVersions = [{}]) { text: 'View', link: `/system/return-requirements/${returnVersion.id}/view` }], - endDate: returnVersion.endDate ? formatLongDate(returnVersion.endDate) : '', + endDate: returnVersion.endDate ? formatLongDate(returnVersion.endDate) : '-', reason: returnVersion.reason ? returnRequirementReasons[returnVersion.reason] : '', startDate: formatLongDate(returnVersion.startDate), status: _status(returnVersion.status) From 0c49015a249cf3cac54a6a07742133525441fac7 Mon Sep 17 00:00:00 2001 From: Alan Cruikshanks Date: Tue, 9 Jul 2024 13:25:09 +0100 Subject: [PATCH 05/10] Fix tests --- test/presenters/licences/set-up.presenter.test.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/presenters/licences/set-up.presenter.test.js b/test/presenters/licences/set-up.presenter.test.js index cf275ba53c..313d351cd5 100644 --- a/test/presenters/licences/set-up.presenter.test.js +++ b/test/presenters/licences/set-up.presenter.test.js @@ -427,7 +427,7 @@ describe('Licences - Set Up presenter', () => { text: 'Review' }], id: 'f547f465-0a62-45ff-9909-38825f05e0c4', - startDate: '1 April 2024', + startDate: '1 April 2022', endDate: '-', status: 'review', reason: 'changed something' @@ -442,7 +442,7 @@ describe('Licences - Set Up presenter', () => { expect(result.chargeInformation).to.equal([{ action: [], id: 'f547f465-0a62-45ff-9909-38825f05e0c4', - startDate: '1 April 2024', + startDate: '1 April 2022', endDate: '-', status: 'review', reason: 'changed something' From 3c416bb94476baaf2ce85e0ee579912685aa2313 Mon Sep 17 00:00:00 2001 From: Alan Cruikshanks Date: Tue, 9 Jul 2024 13:25:33 +0100 Subject: [PATCH 06/10] Make agreements dates consistent --- app/presenters/licences/set-up.presenter.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/presenters/licences/set-up.presenter.js b/app/presenters/licences/set-up.presenter.js index db868bdd33..576cc5dddd 100644 --- a/app/presenters/licences/set-up.presenter.js +++ b/app/presenters/licences/set-up.presenter.js @@ -55,9 +55,9 @@ function _agreements (commonData, agreements, auth) { return agreements.map((agreement) => { return { startDate: formatLongDate(agreement.startDate), - endDate: agreement.endDate ? formatLongDate(agreement.endDate) : '', + endDate: agreement.endDate ? formatLongDate(agreement.endDate) : '-', description: agreementDescriptions[_financialAgreementCode(agreement)], - signedOn: agreement.signedOn ? formatLongDate(agreement.signedOn) : '', + signedOn: agreement.signedOn ? formatLongDate(agreement.signedOn) : '-', action: _agreementActionLinks(commonData, agreement, auth) } }) From 26ea6260ae8bc305ee6effd944897275eebb0c68 Mon Sep 17 00:00:00 2001 From: Alan Cruikshanks Date: Tue, 9 Jul 2024 13:34:57 +0100 Subject: [PATCH 07/10] Remove debug console logs --- app/services/licences/view-licence-set-up.service.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/app/services/licences/view-licence-set-up.service.js b/app/services/licences/view-licence-set-up.service.js index 3bb3d5ee6d..3eb68f0219 100644 --- a/app/services/licences/view-licence-set-up.service.js +++ b/app/services/licences/view-licence-set-up.service.js @@ -25,9 +25,7 @@ async function go (licenceId, auth) { const agreements = await FetchAgreementsService.go(commonData.licenceRef) const chargeVersions = await FetchChargeVersionsService.go(licenceId) - console.log('🚀 ~ go ~ chargeVersions:', chargeVersions) const workflows = await FetchWorkflowsService.go(licenceId) - console.log('🚀 ~ go ~ workflows:', workflows) const returnVersions = await FetchReturnVersionsService.go(licenceId) const licenceSetUpData = SetUpPresenter From a20a3c1f2d0dd1202f7fc56460612bab04ea01b8 Mon Sep 17 00:00:00 2001 From: Alan Cruikshanks Date: Tue, 9 Jul 2024 13:38:08 +0100 Subject: [PATCH 08/10] Switch to no dashes --- app/presenters/licences/set-up.presenter.js | 12 ++++---- .../licences/set-up.presenter.test.js | 28 +++++++++---------- 2 files changed, 20 insertions(+), 20 deletions(-) diff --git a/app/presenters/licences/set-up.presenter.js b/app/presenters/licences/set-up.presenter.js index 576cc5dddd..64244abe36 100644 --- a/app/presenters/licences/set-up.presenter.js +++ b/app/presenters/licences/set-up.presenter.js @@ -55,9 +55,9 @@ function _agreements (commonData, agreements, auth) { return agreements.map((agreement) => { return { startDate: formatLongDate(agreement.startDate), - endDate: agreement.endDate ? formatLongDate(agreement.endDate) : '-', + endDate: agreement.endDate ? formatLongDate(agreement.endDate) : '', description: agreementDescriptions[_financialAgreementCode(agreement)], - signedOn: agreement.signedOn ? formatLongDate(agreement.signedOn) : '-', + signedOn: agreement.signedOn ? formatLongDate(agreement.signedOn) : '', action: _agreementActionLinks(commonData, agreement, auth) } }) @@ -135,7 +135,7 @@ function _chargeVersions (chargeVersions) { return { id: chargeVersion.id, startDate: formatLongDate(chargeVersion.startDate), - endDate: chargeVersion.endDate ? formatLongDate(chargeVersion.endDate) : '-', + endDate: chargeVersion.endDate ? formatLongDate(chargeVersion.endDate) : '', status: _status(chargeVersion.status), reason: chargeVersion.changeReason?.description, action: [ @@ -179,7 +179,7 @@ function _returnVersions (returnVersions = [{}]) { text: 'View', link: `/system/return-requirements/${returnVersion.id}/view` }], - endDate: returnVersion.endDate ? formatLongDate(returnVersion.endDate) : '-', + endDate: returnVersion.endDate ? formatLongDate(returnVersion.endDate) : '', reason: returnVersion.reason ? returnRequirementReasons[returnVersion.reason] : '', startDate: formatLongDate(returnVersion.startDate), status: _status(returnVersion.status) @@ -218,7 +218,7 @@ function _workflows (workflows, auth) { return workflows.map((workflow) => { return { action: _workflowAction(workflow, auth), - endDate: '-', + endDate: '', id: workflow.id, reason: workflow.data.chargeVersion?.changeReason?.description, startDate: _workflowStartDate(workflow), @@ -269,7 +269,7 @@ function _workflowStartDate (workflow) { return formatLongDate(startDate) } - return '-' + return '' } module.exports = { diff --git a/test/presenters/licences/set-up.presenter.test.js b/test/presenters/licences/set-up.presenter.test.js index 313d351cd5..b5d9bde7e3 100644 --- a/test/presenters/licences/set-up.presenter.test.js +++ b/test/presenters/licences/set-up.presenter.test.js @@ -14,7 +14,7 @@ const FeatureFlagsConfig = require('../../../config/feature-flags.config.js') // Thing under test const SetUpPresenter = require('../../../app/presenters/licences/set-up.presenter.js') -describe('Licences - Set Up presenter', () => { +describe.only('Licences - Set Up presenter', () => { const agreement = { id: '123', startDate: new Date('2020-01-01'), @@ -123,9 +123,9 @@ describe('Licences - Set Up presenter', () => { text: 'Recalculate bills' } ], - signedOn: '-', + signedOn: '', description: 'Two-part tariff', - endDate: '-', + endDate: '', startDate: '1 January 2020' } ]) @@ -331,7 +331,7 @@ describe('Licences - Set Up presenter', () => { action: [], id: 'f547f465-0a62-45ff-9909-38825f05e0c4', startDate: '1 April 2022', - endDate: '-', + endDate: '', status: 'review', reason: 'changed something' }, @@ -342,7 +342,7 @@ describe('Licences - Set Up presenter', () => { }], id: '0d514aa4-1550-46b1-8195-878957f2a5f8', startDate: '1 January 2020', - endDate: '-', + endDate: '', status: 'approved', reason: 'Major change' } @@ -372,7 +372,7 @@ describe('Licences - Set Up presenter', () => { }], id: '0d514aa4-1550-46b1-8195-878957f2a5f8', startDate: '1 January 2020', - endDate: '-', + endDate: '', status: 'approved', reason: 'Major change' }]) @@ -428,7 +428,7 @@ describe('Licences - Set Up presenter', () => { }], id: 'f547f465-0a62-45ff-9909-38825f05e0c4', startDate: '1 April 2022', - endDate: '-', + endDate: '', status: 'review', reason: 'changed something' }]) @@ -443,7 +443,7 @@ describe('Licences - Set Up presenter', () => { action: [], id: 'f547f465-0a62-45ff-9909-38825f05e0c4', startDate: '1 April 2022', - endDate: '-', + endDate: '', status: 'review', reason: 'changed something' }]) @@ -471,8 +471,8 @@ describe('Licences - Set Up presenter', () => { text: 'Review' }], id: 'f547f465-0a62-45ff-9909-38825f05e0c4', - startDate: '-', - endDate: '-', + startDate: '', + endDate: '', status: 'to set up', reason: 'changed something' }]) @@ -490,8 +490,8 @@ describe('Licences - Set Up presenter', () => { expect(result.chargeInformation).to.equal([{ action: [], id: 'f547f465-0a62-45ff-9909-38825f05e0c4', - startDate: '-', - endDate: '-', + startDate: '', + endDate: '', status: 'to set up', reason: 'changed something' }]) @@ -516,7 +516,7 @@ describe('Licences - Set Up presenter', () => { text: 'View' } ], - endDate: '-', + endDate: '', reason: 'Change to special agreement', startDate: '1 January 2020', status: 'approved' @@ -540,7 +540,7 @@ describe('Licences - Set Up presenter', () => { text: 'View' } ], - endDate: '-', + endDate: '', reason: '', startDate: '1 January 2020', status: 'approved' From d8e4aaee7250b3a554faff672e50dca10ea3a76f Mon Sep 17 00:00:00 2001 From: Alan Cruikshanks Date: Tue, 9 Jul 2024 13:41:16 +0100 Subject: [PATCH 09/10] Fix view licence set up service test --- .../licences/view-licence-set-up.service.test.js | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) 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 f8157baf09..f41ffabce9 100644 --- a/test/services/licences/view-licence-set-up.service.test.js +++ b/test/services/licences/view-licence-set-up.service.test.js @@ -63,7 +63,12 @@ describe('View Licence Set Up service', () => { id: 'f3fe1275-50ff-4f69-98cb-5a35c17654f3', createdAt: new Date('2020-01-01'), status: 'review', - data: { chargeVersion: { changeReason: { description: 'changed something' } } }, + data: { + chargeVersion: { + changeReason: { description: 'changed something' }, + dateRange: { startDate: '2022-04-01' } + } + }, licenceId: '456' } ]) @@ -107,10 +112,10 @@ describe('View Licence Set Up service', () => { chargeInformation: [ { action: [], - endDate: '-', + endDate: '', id: 'f3fe1275-50ff-4f69-98cb-5a35c17654f3', reason: 'changed something', - startDate: '1 January 2020', + startDate: '1 April 2022', status: 'review' }, { From b180842d1af2c1d2be88483b264b452a4f2d74eb Mon Sep 17 00:00:00 2001 From: Alan Cruikshanks Date: Tue, 9 Jul 2024 13:45:10 +0100 Subject: [PATCH 10/10] Doh! only() --- test/presenters/licences/set-up.presenter.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/presenters/licences/set-up.presenter.test.js b/test/presenters/licences/set-up.presenter.test.js index b5d9bde7e3..e42fd56539 100644 --- a/test/presenters/licences/set-up.presenter.test.js +++ b/test/presenters/licences/set-up.presenter.test.js @@ -14,7 +14,7 @@ const FeatureFlagsConfig = require('../../../config/feature-flags.config.js') // Thing under test const SetUpPresenter = require('../../../app/presenters/licences/set-up.presenter.js') -describe.only('Licences - Set Up presenter', () => { +describe('Licences - Set Up presenter', () => { const agreement = { id: '123', startDate: new Date('2020-01-01'),