Add registered user modifier to LicenceModel #693
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 > This is part of a series of changes to allow us to replace the legacy view licence page When viewing a licence using the [legacy water-abstraction-ui](https://github.com/DEFRA/water-abstraction-ui) the page will display information on who the registered user is. The registered is an external user who has declared that they manage the licence via the external UI. They are not the licence holder, just the person declared to be responsible for managing it. Unfortunately, this functionality dates from the very start of the service and the data is in one of the oldest legacy schemas; `crm`. It seems this was one of the things which the previous team never managed to migrate to `crm_v2`, their intended better CRM structure. We have to go through a number of tables using a specific query to identify the registered user and whether they have set a custom name for the licence. This change adds a [modifier](https://vincit.github.io/objection.js/recipes/modifiers.html#modifiers) to the `LicenceModel` so we only have to do this once!
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!
There were fields which didn't appear to be used when we create the migration. No doubt we didn't check pre-prod properly because if we had we would have seen that a number of these fields are populated. Specifically, we need `companyEntityId` in order to link through to the new tables we've added to get the primary user. And we need `document_name` as this is where **Licence name** comes from (shown as a field in the external summary and above the licence ref in the internal summary).
Here is a working example of the query we need to add to get a licence's registered username and if set the licence name they assigned. We're having to use an standard join rather than a `withGraphFetched` because the link between `LicenceDocumentHeader` and `LicenceEntityRole` is via the `companyEntityId` which just seems _too_ tenuous. So, we haven't defined a relationship for it in the model hence the need for the stand INNER JOIN. We're also having to drop down into Knex for part of it in order to add an extra clause to the JOIN. We only want the record that is set as the `primary_user`. Because we are using standard joins we can't deal with this in the WHERE clause. We could have left it out but then it'd be prudent to add extra logic to ensure the `LicenceEntityRole` definitely has the role `primary_user`. This means you can use the result without extra checking.
This proves we can migrate the part of the query we've enhanced to a modifier and it still works (we console.Log() the result and confirmed it doesn't blow up for those with and without a registered user).
The new name better matches how the value is referred to in the UI. The instance methods make it easier to access the values and give us something to build our tests against.
First the type default didn't match the comments or reflect what we intended to set it to. Also, after checking the real data when the type is `individual` we expect the name to be there email address. So, this change corrects these mistakes.
As stated in the first commit, these will come through in a different PR. But hopefully we have demonstrated how the modifier and custom instance methods can be used to provide the registered user and licence name details.
|
Assigned this one just to you @robertparkinson for review as, hopefully, this will support you in your ticket to add 'registered to' details to the view licence page. Within the commits I did you will see how to change |
Cruikshanks
added a commit
that referenced
this pull request
Jan 27, 2024
https://eaflood.atlassian.net/browse/WATER-4322 Whilst working on [Add registered user modifier to LicenceModel](#693) we spotted the that the new view licence page is missing its title. So this PR change was originally intended to fix just that. As part of it we spotted that there was no direct unit test for the `ViewLicencePresenter` so we included adding that as well. But when doing so we spotted an inconsistency with the end date and the warning. If the licence's `expiredDate` is null or less than today's date then we return `null`. We only want to show the **End date** field when a licence is not yet 'ended' but has an expiry date. But if the licence has an expired date equal to today then we will return a formatted date string and show the field in the view. The problem is the logic in `_generateWarningMessage()` determines a warning message is needed if a licence's end date is equal to today or less. Basically, if a licence's `expiredDate` matches the current date (today) we'll show both the **End date** field and the warning message. So, this change also clears that issue up!
robertparkinson
approved these changes
Jan 29, 2024
Cruikshanks
added a commit
that referenced
this pull request
Jan 29, 2024
https://eaflood.atlassian.net/browse/WATER-4322 Whilst working on [Add registered user modifier to LicenceModel](#693) we spotted that the new view licence page is missing its title. As part of this, we identified that there was no direct unit test for the `ViewLicencePresenter`, so we also added that.
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-4324
When viewing a licence using the legacy water-abstraction-ui the page will display information on who the registered user is.
The registered is an external user who has declared that they manage the licence via the external UI. They are not the licence holder, just the person declared to be responsible for managing it.
Unfortunately, this functionality dates from the very start of the service and the data is in one of the oldest legacy schemas;
crm. It seems this was one of the things which the previous team never managed to migrate tocrm_v2, their intended better CRM structure.We have to go through a number of tables using a specific query to identify the registered user and whether they have set a custom name for the licence.
This change adds a modifier to the
LicenceModelso we only have to do this once!