Decouple start date logic #1461
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
https://eaflood.atlassian.net/browse/WATER-4687 As part of work to introduce quarterly returns we have noticed some logic leaking out of the start date functionality. Outside of this service we do not care how the start date is calculated. This change introduces a variable 'returnVersionStartDate' to the session for services outside of start to use. Start date has been left as intact as possible and this change has been added when the start date saves the session so the internal workings have been left alone.
jonathangoulding
added
the
housekeeping
jonathangoulding
requested review from
Jozzey,
robertparkinson and
rvsiyad
and removed request for
robertparkinson,
Jozzey and
rvsiyad
robertparkinson
previously approved these changes
Nov 1, 2024
They are not used / broken
Cruikshanks
approved these changes
Nov 4, 2024
Cruikshanks
added a commit
that referenced
this pull request
Nov 8, 2024
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.
jonathangoulding
pushed a commit
that referenced
this pull request
Nov 8, 2024
* 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!
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
https://eaflood.atlassian.net/browse/WATER-4687
As part of work to introduce quarterly returns we have noticed some logic leaking out of the start date functionality.
Outside of this service we do not care how the start date is calculated.
This change introduces a variable 'returnVersionStartDate' to the session for services outside of start to use.
Start date has been left as intact as possible and this change has been added when the start date saves the session so the internal workings have been left alone.