Skip to content

Commit

Permalink
Fix broken return version start date page (#1469)
Browse files Browse the repository at this point in the history
* Fix broken return version start date page

https://eaflood.atlassian.net/browse/WATER-4687

In [Decouple start date logic](#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!
  • Loading branch information
Cruikshanks authored Nov 8, 2024
1 parent 8582ebf commit e22357a
Show file tree
Hide file tree
Showing 3 changed files with 8 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand All @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down Expand Up @@ -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 () => {
Expand Down

0 comments on commit e22357a

Please sign in to comment.