Add Licence holder name to view licence page #679
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-4324 Migrating the licence summary page from the water-abstraction-ui to the water-abstraction-system. This ticket is for adding the registered user name or contact details to the page or unregistered if none are found.
https://eaflood.atlassian.net/browse/WATER-4325 https://eaflood.atlassian.net/browse/WATER-4263 We have had two tickets come through that both need to determine if and when a licence 'ends'. A licence can 'end' for 3 reasons: - because it is _revoked_ - because it is _lapsed_ - because it is _expired_ The previous delivery team chose to encode these as 3 separate date fields on the licence record. So, if a field is populated it means the licence 'ends' for that reason on that day. But it's not that simple! 😁 More than one of these fields may be populated. For example, a licence was due to expire on 2023-08-10 but was then revoked on 2022-04-27. So, to determine the reason you need to select the _earliest_ date. But there's more! 😲 We have examples where 2 of the fields might be populated with the same date (and 1 licence where all 3 have the same date!) So, how do you determine the 'end' reason then? If more than one date field is populated and they hold the earliest date value then we select based on priority; revoked -> lapsed -> expired. Both tickets are connected to PRs that have to handle all or some of this logic. - [Returns required journey - Select start date page iteration 2 (1 of 7)](#646) - [Add warning text to view licence page](#670) Both PRs will also have to write numerous tests to check the right date is being used, and we have a strong suspicion we are already doing something similar in supplementary billing and that we'll need the 'end' in future tickets. So, enough is enough! This refactoring change moves the logic to the model, specifically, as a [custom Objection.js instance method](https://vincit.github.io/objection.js/api/model/instance-methods.html). This is something we've done in a [previous project](https://github.com/DEFRA/sroc-charging-module-api) and in this project to determine a `ContactModel`s name. We can then remove the duplication both in logic and unit tests. This change updates the model, adds the new tests, and then refactors existing usages to use the model's `$ends()` method.
https://eaflood.atlassian.net/browse/WATER-4188 https://eaflood.atlassian.net/browse/WATER-4328 Much like in [Move licence end date logic to model](#681) we have another two tickets that need to identify the licence holder for a licence. We've already figured out how to do that in [Add licence ref. and holder to rtn. req. session](#639). But that implementation was specific to return requirements and unlike `myContact.$name()` and `myLicence.$ends()` to determine the licence holder you need to have pulled through a series of related models. So, for the same reasons we want to move this piece of logic to the model to make it easier to reuse. However, there will be differences in implementation. This change adds the logic to the model and then refactors `InitiateReturnRequirementSessionService` to demonstrate how to use it.
Cruikshanks
requested changes
Jan 26, 2024
There was a problem hiding this comment.
Absolutely nothing wrong with the code! 😂
But would you mind updating the PR title and description to reflect these are the changes for adding the licence holder to the summary tab; WATER-4328.
Then I can relax that we don't squash this in with the wrong details 😰😁
Cruikshanks
added a commit
that referenced
this pull request
Jan 27, 2024
These are some of the changes made in #679 to support displaying the licence holder in the view page. We will not retain them at the end of this PR. But `FetchLicenceService` is where we intend to first make use of our new registered user modifier. To make sure it works and doesn't break anything we're copying what will soon be merged before we then start playing around!
robertparkinson
changed the title
Cruikshanks
approved these changes
Jan 29, 2024
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-4328
Migrating the licence summary page from the water-abstraction-ui to the water-abstraction-system.
This ticket is for adding the Lice holder name to the summary section of the page.