From 94bafb0001dc5f208650ef6eb741653c81f55168 Mon Sep 17 00:00:00 2001 From: Rebecca Ransome Date: Thu, 4 Apr 2024 12:00:19 +0100 Subject: [PATCH 1/6] Fix return link logic on two-part tariff licence review page During some refactoring of the match details page, it was noticed that the logic for generating the returnLog link in the licence review page was in the view and not the presenter. It was also noted that the link for unmatched returns was incorrectly taking the user to the full list of returns and not the actual return they clicked on. This PR fixes the unmatched returns link and moves the logic into the presenter. From cfc73e22a9f3e2b895dd54d57801dfae759e13d3 Mon Sep 17 00:00:00 2001 From: Rebecca Ransome Date: Thu, 4 Apr 2024 12:12:51 +0100 Subject: [PATCH 2/6] Fix return link and move logic into presenter --- .../review-licence.presenter.js | 54 +++++++++++-------- app/views/bill-runs/review-licence.njk | 7 +-- .../review-licence.presenter.test.js | 3 +- 3 files changed, 36 insertions(+), 28 deletions(-) diff --git a/app/presenters/bill-runs/two-part-tariff/review-licence.presenter.js b/app/presenters/bill-runs/two-part-tariff/review-licence.presenter.js index 3c74c29dae..dfe7a02c98 100644 --- a/app/presenters/bill-runs/two-part-tariff/review-licence.presenter.js +++ b/app/presenters/bill-runs/two-part-tariff/review-licence.presenter.js @@ -123,26 +123,6 @@ function _chargeElementCount (reviewChargeVersion) { return chargeElementCount } -function _returnStatus (returnLog) { - if (returnLog.returnStatus === 'due') { - return 'overdue' - } else if (returnLog.underQuery) { - return 'query' - } else { - return returnLog.returnStatus - } -} - -function _returnTotal (returnLog) { - const { returnStatus, allocated, quantity } = returnLog - - if (returnStatus === 'void' || returnStatus === 'received' || returnStatus === 'due') { - return '/' - } else { - return `${allocated} ML / ${quantity} ML` - } -} - function _contactName (billingAccount) { const contact = billingAccount.billingAccountAddresses[0].contact @@ -174,7 +154,8 @@ function _matchedReturns (returnLogs) { description: returnLog.description, purpose: returnLog.purposes[0].tertiary.description, returnTotal: _returnTotal(returnLog), - issues: returnLog.issues.length > 0 ? returnLog.issues.split(', ') : [''] + issues: returnLog.issues.length > 0 ? returnLog.issues.split(', ') : [''], + returnLink: _returnLink(returnLog) } ) } @@ -254,6 +235,34 @@ function _prepareReturnVolume (reviewChargeElement) { return returnVolumes } +function _returnLink (returnLog) { + if (['due', 'received', 'void'].includes(returnLog.returnStatus)) { + return `/return/internal?returnId=${returnLog.returnId}` + } + + return `/returns/return?id=${returnLog.returnId}` +} + +function _returnStatus (returnLog) { + if (returnLog.returnStatus === 'due') { + return 'overdue' + } else if (returnLog.underQuery) { + return 'query' + } else { + return returnLog.returnStatus + } +} + +function _returnTotal (returnLog) { + const { returnStatus, allocated, quantity } = returnLog + + if (returnStatus === 'void' || returnStatus === 'received' || returnStatus === 'due') { + return '/' + } else { + return `${allocated} ML / ${quantity} ML` + } +} + function _totalBillableReturns (reviewChargeReference) { let totalBillableReturns = 0 let totalQuantity = 0 @@ -282,7 +291,8 @@ function _unmatchedReturns (returnLogs) { description: returnLog.description, purpose: returnLog.purposes[0].tertiary.description, returnTotal: `${returnLog.allocated} / ${returnLog.quantity} ML`, - issues: returnLog.issues.length > 0 ? returnLog.issues.split(', ') : [''] + issues: returnLog.issues.length > 0 ? returnLog.issues.split(', ') : [''], + returnLink: _returnLink(returnLog) } ) } diff --git a/app/views/bill-runs/review-licence.njk b/app/views/bill-runs/review-licence.njk index ed413f12e8..e41536d0b1 100644 --- a/app/views/bill-runs/review-licence.njk +++ b/app/views/bill-runs/review-licence.njk @@ -85,17 +85,14 @@ {% if return.returnStatus == 'overdue' or return.returnStatus == 'query' %} {% set colour = "govuk-tag--red"%} - {% set returnLink = "/return/internal?returnId=" + return.returnId %} {% elif return.status == 'void' %} {% set colour = "govuk-tag--grey" %} - {% set returnLink = "/return/internal?returnId=" + return.returnId %} {% else %} {% set colour = "govuk-tag--green" %} - {% set returnLink = "/returns/return?id=" + return.returnId%} {% endif %} {% set action %} - {{ return.reference }} + {{ return.reference }}
{{return.dates}}
{% endset %} @@ -185,7 +182,7 @@ {% endif %} {% set action %} - {{ return.reference }} + {{ return.reference }}
{{return.dates}}
{% endset %} diff --git a/test/presenters/bill-runs/two-part-tariff/review-licence.presenter.test.js b/test/presenters/bill-runs/two-part-tariff/review-licence.presenter.test.js index 6a7a016b3f..5c2109343a 100644 --- a/test/presenters/bill-runs/two-part-tariff/review-licence.presenter.test.js +++ b/test/presenters/bill-runs/two-part-tariff/review-licence.presenter.test.js @@ -36,7 +36,8 @@ describe('Review Licence presenter', () => { description: 'Lands at Mosshayne Farm, Exeter & Broadclyst', purpose: 'Site description', returnTotal: '0 ML / 0 ML', - issues: [''] + issues: [''], + returnLink: '/returns/return?id=v1:1:01/60/28/3437:17061181:2022-04-01:2023-03-31' } ], unmatchedReturns: [], From 3691d523f8d439246f97620c69097ad5f6590354 Mon Sep 17 00:00:00 2001 From: Rebecca Ransome Date: Thu, 4 Apr 2024 16:37:51 +0100 Subject: [PATCH 3/6] Adding unit tests for review-licence presenter --- .../review-licence.presenter.test.js | 58 +++++++++++++++++-- 1 file changed, 52 insertions(+), 6 deletions(-) diff --git a/test/presenters/bill-runs/two-part-tariff/review-licence.presenter.test.js b/test/presenters/bill-runs/two-part-tariff/review-licence.presenter.test.js index 5c2109343a..cfb11f2871 100644 --- a/test/presenters/bill-runs/two-part-tariff/review-licence.presenter.test.js +++ b/test/presenters/bill-runs/two-part-tariff/review-licence.presenter.test.js @@ -4,7 +4,7 @@ const Lab = require('@hapi/lab') const Code = require('@hapi/code') -const { describe, it } = exports.lab = Lab.script() +const { describe, it, beforeEach } = exports.lab = Lab.script() const { expect } = Code // Thing under test @@ -83,6 +83,52 @@ describe('Review Licence presenter', () => { ] }) }) + + describe("the 'issues' property", () => { + describe('when the charge element has multiple issues', () => { + beforeEach(() => { + licence[0].reviewChargeVersions[0].reviewChargeReferences[0].reviewChargeElements[0].issues = 'Over abstraction, no returns' + }) + + it('correctly splits the issues into an array', () => { + const result = ReviewLicencePresenter.go(billRun, licence) + + expect(result.chargeData[0].chargeReferences[0].chargeElements[0].issues).to.equal(['Over abstraction', 'no returns']) + }) + }) + + describe('when the matched returns has multiple issues', () => { + beforeEach(() => { + licence[0].reviewReturns[0].issues = 'Over abstraction, no returns' + }) + + it('correctly splits the issues into an array', () => { + const result = ReviewLicencePresenter.go(billRun, licence) + + expect(result.matchedReturns[0].issues).to.equal(['Over abstraction', 'no returns']) + }) + }) + }) + + describe("the 'returnStatus' property", () => { + describe("when a return has a status of 'due'", () => { + beforeEach(() => { + licence[0].reviewReturns[0].returnStatus = 'due' + }) + + it("changes the status text to 'overdue'", () => { + const result = ReviewLicencePresenter.go(billRun, licence) + + expect(result.matchedReturns[0].returnStatus).to.equal('overdue') + }) + + it('formats the returns total correctly', () => { + const result = ReviewLicencePresenter.go(billRun, licence) + + expect(result.matchedReturns[0].returnTotal).to.equal('/') + }) + }) + }) }) }) @@ -104,7 +150,7 @@ function _licenceData () { licenceId: '786f0d83-eaf7-43c3-9de5-ec59e3de05ee', licenceRef: '01/49/80/4608', licenceHolder: 'Licence Holder Ltd', - issues: null, + issues: '', status: 'ready', reviewReturns: [{ id: '2264f443-5c16-4ca9-8522-f63e2d4e38be', @@ -127,14 +173,14 @@ function _licenceData () { description: 'Lands at Mosshayne Farm, Exeter & Broadclyst', startDate: new Date(' 2022-04-01'), endDate: new Date('2022-05-06'), - issues: [], + issues: '', reviewChargeElements: [{ id: 'e840f418-ca6b-4d96-9f36-bf684c78590f', reviewChargeReferenceId: '7759e0f9-5763-4b94-8d45-0621aea3edc1', chargeElementId: 'b1cd4f98-ad96-4901-9e21-4432f032492a', allocated: 0, chargeDatesOverlap: false, - issues: [], + issues: '', status: 'ready' }] }], @@ -165,7 +211,7 @@ function _licenceData () { chargeElementId: 'b1001716-cfb4-43c6-91f0-1863f4529223', allocated: 0, chargeDatesOverlap: false, - issues: [], + issues: '', status: 'ready', chargeElement: { description: 'Trickle Irrigation - Direct', @@ -192,7 +238,7 @@ function _licenceData () { description: 'Lands at Mosshayne Farm, Exeter & Broadclyst', startDate: new Date(' 2022-04-01'), endDate: new Date('2022-05-06'), - issues: [] + issues: '' }] }] }], From 0afbf9df78bb3f278fb10fa4b1d066129c4ce128 Mon Sep 17 00:00:00 2001 From: Rebecca Ransome Date: Thu, 4 Apr 2024 16:39:56 +0100 Subject: [PATCH 4/6] New unit test --- .../two-part-tariff/review-licence.presenter.test.js | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/test/presenters/bill-runs/two-part-tariff/review-licence.presenter.test.js b/test/presenters/bill-runs/two-part-tariff/review-licence.presenter.test.js index cfb11f2871..ad175b543e 100644 --- a/test/presenters/bill-runs/two-part-tariff/review-licence.presenter.test.js +++ b/test/presenters/bill-runs/two-part-tariff/review-licence.presenter.test.js @@ -127,6 +127,12 @@ describe('Review Licence presenter', () => { expect(result.matchedReturns[0].returnTotal).to.equal('/') }) + + it('formats the returns link correctly', () => { + const result = ReviewLicencePresenter.go(billRun, licence) + + expect(result.matchedReturns[0].returnLink).to.equal('/return/internal?returnId=v1:1:01/60/28/3437:17061181:2022-04-01:2023-03-31') + }) }) }) }) From b621915c875c61c7f2574065d8cc32b9caf3a595 Mon Sep 17 00:00:00 2001 From: Rebecca Ransome Date: Thu, 4 Apr 2024 16:52:00 +0100 Subject: [PATCH 5/6] Adding an unmatched return --- .../review-licence.presenter.test.js | 73 ++++++++++++++++++- 1 file changed, 70 insertions(+), 3 deletions(-) diff --git a/test/presenters/bill-runs/two-part-tariff/review-licence.presenter.test.js b/test/presenters/bill-runs/two-part-tariff/review-licence.presenter.test.js index ad175b543e..632d570731 100644 --- a/test/presenters/bill-runs/two-part-tariff/review-licence.presenter.test.js +++ b/test/presenters/bill-runs/two-part-tariff/review-licence.presenter.test.js @@ -11,9 +11,14 @@ const { expect } = Code const ReviewLicencePresenter = require('../../../../app/presenters/bill-runs/two-part-tariff/review-licence.presenter.js') describe('Review Licence presenter', () => { + let billRun + let licence + describe('when there is data to be presented for the review licence page', () => { - const billRun = _billRun() - const licence = _licenceData() + beforeEach(() => { + billRun = _billRun() + licence = _licenceData() + }) it('correctly presents the data', async () => { const result = ReviewLicencePresenter.go(billRun, licence) @@ -40,7 +45,19 @@ describe('Review Licence presenter', () => { returnLink: '/returns/return?id=v1:1:01/60/28/3437:17061181:2022-04-01:2023-03-31' } ], - unmatchedReturns: [], + unmatchedReturns: [ + { + dates: '1 April 2022 to 6 May 2022', + description: 'Lands at Mosshayne Farm, Exeter & Broadclyst', + issues: [''], + purpose: 'Site description', + reference: '10031343', + returnId: 'v2:1:01/60/28/3437:17061181:2022-04-01:2023-03-31', + returnLink: '/returns/return?id=v2:1:01/60/28/3437:17061181:2022-04-01:2023-03-31', + returnStatus: 'completed', + returnTotal: '0 / 0 ML' + } + ], chargeData: [ { financialYear: '2022 to 2023', @@ -108,6 +125,18 @@ describe('Review Licence presenter', () => { expect(result.matchedReturns[0].issues).to.equal(['Over abstraction', 'no returns']) }) }) + + describe('when the unmatched returns has multiple issues', () => { + beforeEach(() => { + licence[0].reviewReturns[1].issues = 'Over abstraction, no returns' + }) + + it('correctly splits the issues into an array', () => { + const result = ReviewLicencePresenter.go(billRun, licence) + + expect(result.unmatchedReturns[0].issues).to.equal(['Over abstraction', 'no returns']) + }) + }) }) describe("the 'returnStatus' property", () => { @@ -135,6 +164,20 @@ describe('Review Licence presenter', () => { }) }) }) + + describe("the 'underQuery' property", () => { + describe('when a return is under query', () => { + beforeEach(() => { + licence[0].reviewReturns[0].underQuery = true + }) + + it("changes the returns status to be 'under query'", () => { + const result = ReviewLicencePresenter.go(billRun, licence) + + expect(result.matchedReturns[0].returnStatus).to.equal('query') + }) + }) + }) }) }) @@ -189,6 +232,30 @@ function _licenceData () { issues: '', status: 'ready' }] + }, + { + id: '4864f643-5c16-5ca9-8512-f63e1d4e58be', + reviewLicenceId: '78a99c1c-26d3-4163-ab58-084cd78594ab', + returnId: 'v2:1:01/60/28/3437:17061181:2022-04-01:2023-03-31', + returnReference: '10031343', + quantity: 0, + allocated: 0, + underQuery: false, + returnStatus: 'completed', + nilReturn: false, + abstractionOutsidePeriod: false, + receivedDate: new Date('2022-06-03'), + dueDate: new Date('2022-06-03'), + purposes: [{ + tertiary: { + description: 'Site description' + } + }], + description: 'Lands at Mosshayne Farm, Exeter & Broadclyst', + startDate: new Date(' 2022-04-01'), + endDate: new Date('2022-05-06'), + issues: '', + reviewChargeElements: [] }], reviewChargeVersions: [{ id: '3de5634a-da26-4241-87e9-7248a4b83a69', From c299c0e8f9d7a6cd9610009d1a6e9e6fcb3ea3af Mon Sep 17 00:00:00 2001 From: Rebecca Ransome Date: Thu, 4 Apr 2024 17:00:30 +0100 Subject: [PATCH 6/6] Refactor presenter --- .../two-part-tariff/review-licence.presenter.js | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/app/presenters/bill-runs/two-part-tariff/review-licence.presenter.js b/app/presenters/bill-runs/two-part-tariff/review-licence.presenter.js index dfe7a02c98..73285129c2 100644 --- a/app/presenters/bill-runs/two-part-tariff/review-licence.presenter.js +++ b/app/presenters/bill-runs/two-part-tariff/review-licence.presenter.js @@ -246,21 +246,21 @@ function _returnLink (returnLog) { function _returnStatus (returnLog) { if (returnLog.returnStatus === 'due') { return 'overdue' - } else if (returnLog.underQuery) { + } + + if (returnLog.underQuery) { return 'query' - } else { - return returnLog.returnStatus } + return returnLog.returnStatus } function _returnTotal (returnLog) { const { returnStatus, allocated, quantity } = returnLog - if (returnStatus === 'void' || returnStatus === 'received' || returnStatus === 'due') { + if (['due', 'received', 'void'].includes(returnStatus)) { return '/' - } else { - return `${allocated} ML / ${quantity} ML` } + return `${allocated} ML / ${quantity} ML` } function _totalBillableReturns (reviewChargeReference) {