From e22357af73b9775d3df4f162be3cb4b0d6d68578 Mon Sep 17 00:00:00 2001 From: Alan Cruikshanks Date: Fri, 8 Nov 2024 10:09:23 +0000 Subject: [PATCH] Fix broken return version start date page (#1469) * Fix broken return version start date page https://eaflood.atlassian.net/browse/WATER-4687 In [Decouple start date logic](https://github.com/DEFRA/water-abstraction-system/pull/1461) we made a change to support adding quarterly returns. It was determined the logic for working out the start date from the values entered by the user was being duplicated in other places. The change ensured the logic stays in just the `SubmitStartDateService`, and added a new property `returnVersionStartDate` that other services could use instead. Unfortunately, an error was introduced. ```javascript // app/services/return-versions/setup/submit-start-date.service.js // Reminder! Because of the unique qualities of Javascript, Year and Day are literal values, month is an index! So, // January is actually 0, February is 1 etc. This is why we deduct 1 from the month. session.returnVersionStartDate = new Date(`${payload['start-date-year']}-${payload['start-date-month'] - 1}-${payload['start-date-day']}`).toISOString().split('T')[0] ``` This way of creating the date was added. It uses a [template literal](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Template_literals) to combine the values provided by the user and pass it as an argument o JavaScript's `new Date()`. But it also applies a common fix for the fact JavaScript expects month to be provided as an index, not literally! So, January = 0, February = 1 etc. The problem is `new Date('2024-04-01')` is not the same as `new Date(2024, 4, 1)`. The first _will_ return you 1 April 2024, the second will return 1 March 2024. JavaScript understands that 'month' is literal when you provide a string argument to `new Date()`. It is only when passing month as a separate value that you have to treat it as an index. The result for the return versions setup journey is that a user is entering, for example, `13 May 2019` on the start page, but we're storing (and would eventually persist!) `13 April 2019`. This change fixes the issue. * Simplify licence start date handling We don't need to call `toISOString()` and `split()`. It works perfectly, as it has done in other places in the code without these additions. * Oops! * Fix test that masked the issue The user has entered '26 11 2023' in the UI. Therefore, they should expect to see `26 November 2023` when `returnVersionStartDate` is used or displayed anywhere in the UI. The test wasn't highlighting something was wrong because it was asserting this fact. * Fix the issue * I like spacing, but not that much! --- .../return-versions/setup/submit-start-date.service.js | 6 ++---- .../setup/check/generate-return-version.service.test.js | 8 ++++---- .../setup/submit-start-date.service.test.js | 3 ++- 3 files changed, 8 insertions(+), 9 deletions(-) diff --git a/app/services/return-versions/setup/submit-start-date.service.js b/app/services/return-versions/setup/submit-start-date.service.js index f40bae32c7..7b6d8bd10e 100644 --- a/app/services/return-versions/setup/submit-start-date.service.js +++ b/app/services/return-versions/setup/submit-start-date.service.js @@ -65,11 +65,9 @@ async function _save (session, payload) { session.startDateDay = payload['start-date-day'] session.startDateMonth = payload['start-date-month'] session.startDateYear = payload['start-date-year'] - // Reminder! Because of the unique qualities of Javascript, Year and Day are literal values, month is an index! So, - // January is actually 0, February is 1 etc. This is why we deduct 1 from the month. - session.returnVersionStartDate = new Date(`${payload['start-date-year']}-${payload['start-date-month'] - 1}-${payload['start-date-day']}`).toISOString().split('T')[0] + session.returnVersionStartDate = new Date(`${payload['start-date-year']}-${payload['start-date-month']}-${payload['start-date-day']}`) } else { - session.returnVersionStartDate = new Date(session.licence.currentVersionStartDate).toISOString().split('T')[0] + session.returnVersionStartDate = new Date(session.licence.currentVersionStartDate) } return session.$update() diff --git a/test/services/return-versions/setup/check/generate-return-version.service.test.js b/test/services/return-versions/setup/check/generate-return-version.service.test.js index b5c0571312..7270bb0a07 100644 --- a/test/services/return-versions/setup/check/generate-return-version.service.test.js +++ b/test/services/return-versions/setup/check/generate-return-version.service.test.js @@ -61,7 +61,7 @@ describe('Return Versions Setup - Generate Return Version service', () => { ], currentVersionStartDate: '2023-02-13T00:00:00.000Z' }, - returnVersionStartDate: '2023-02-13', + returnVersionStartDate: '2023-02-13T00:00:00.000Z', requirements: ['return requirements data'], checkPageVisited: true, startDateOptions: 'licenceStartDate' @@ -81,7 +81,7 @@ describe('Return Versions Setup - Generate Return Version service', () => { expect(result.returnVersion.multipleUpload).to.be.false() expect(result.returnVersion.notes).to.be.undefined() expect(result.returnVersion.reason).to.equal(sessionData.reason) - expect(result.returnVersion.startDate).to.equal(new Date('2023-02-13 ')) + expect(result.returnVersion.startDate).to.equal(new Date('2023-02-13')) expect(result.returnVersion.status).to.equal('current') // Version number is 103 because this is the next version number after the previous version expect(result.returnVersion.version).to.equal(103) @@ -134,7 +134,7 @@ describe('Return Versions Setup - Generate Return Version service', () => { expect(result.returnVersion.multipleUpload).to.be.true() expect(result.returnVersion.notes).to.equal(sessionData.note.content) expect(result.returnVersion.reason).to.equal(sessionData.reason) - expect(result.returnVersion.startDate).to.equal(new Date('2023-02-13 ')) + expect(result.returnVersion.startDate).to.equal(new Date('2023-02-13')) expect(result.returnVersion.status).to.equal('current') // Version number is 1 because no previous return versions exist expect(result.returnVersion.version).to.equal(1) @@ -179,7 +179,7 @@ describe('Return Versions Setup - Generate Return Version service', () => { expect(result.returnVersion.multipleUpload).to.be.false() expect(result.returnVersion.notes).to.be.undefined() expect(result.returnVersion.reason).to.equal(sessionData.reason) - expect(result.returnVersion.startDate).to.equal(new Date('2023-02-13 ')) + expect(result.returnVersion.startDate).to.equal(new Date('2023-02-13')) expect(result.returnVersion.status).to.equal('current') expect(result.returnVersion.version).to.equal(1) }) diff --git a/test/services/return-versions/setup/submit-start-date.service.test.js b/test/services/return-versions/setup/submit-start-date.service.test.js index f36d1d66d9..58ad233bd8 100644 --- a/test/services/return-versions/setup/submit-start-date.service.test.js +++ b/test/services/return-versions/setup/submit-start-date.service.test.js @@ -64,6 +64,7 @@ describe('Return Versions Setup - Submit Start Date service', () => { const refreshedSession = await session.$query() expect(refreshedSession.startDateOptions).to.equal('licenceStartDate') + expect(new Date(refreshedSession.returnVersionStartDate)).to.equal(new Date('2023-01-01')) }) describe('and the page has been not been visited', () => { @@ -118,7 +119,7 @@ describe('Return Versions Setup - Submit Start Date service', () => { expect(refreshedSession.startDateDay).to.equal('26') expect(refreshedSession.startDateMonth).to.equal('11') expect(refreshedSession.startDateYear).to.equal('2023') - expect(refreshedSession.returnVersionStartDate).to.equal('2023-10-26') + expect(new Date(refreshedSession.returnVersionStartDate)).to.equal(new Date('2023-11-26')) }) it('returns the correct details the controller needs to redirect the journey', async () => {