Add LicenceRoleModel from crm_v2.roles #629
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-4292 > This is part of a series of changes to add the ability to identify the licence holder for a licence This change adds the migration, model, test helper and unit tests for a new `LicenceRoleModel`. We've already added [LicenceDocumentModel](#618). Though pointless, it's the only thing that points us to who the licence holder is. In the `crm_v2` schema it does this in `crm_v2.document_roles` which is the join table between `crm_v2.documents` and `crm_v2.roles`. `roles` is a lookup table of the 'role' a `crm_v2.contact` or `crm_c2.company` has, for example, licence holder! So, our next step is to create a model for 'roles' so we can then get on and join them in a future model. Our first blocker though is we can't use the name `roles` and that is already taken (`RoleModel` is used for the lookup table of user roles). Hence, we're creating a new `LicenceRoleModel` in this change! 😁
robertparkinson
approved these changes
Jan 3, 2024
Cruikshanks
added a commit
that referenced
this pull request
Jan 3, 2024
https://eaflood.atlassian.net/browse/WATER-4292 > This is part of a series of changes to add the ability to identify the licence holder for a licence This change adds the migration, model, test helper and unit tests for a new `LicenceDocumentRoleModel`. We added [LicenceDocumentModel](#618). Though pointless, it's the only thing that points us to who the licence holder is. In the `crm_v2` schema it does this in `crm_v2.document_roles` which is the join table between `crm_v2.documents` and `crm_v2.roles`. `roles` is a lookup table of the 'role' a `crm_v2.contact` or `crm_c2.company` has, for example, licence holder! We added that as [LicenceRoleModel](#629). We are now on the final step which is the adding the model that allows us to identify and extract the `crm_v2.document_roles` record that identifies _who_ is the current licence holder for a licence.
Cruikshanks
added a commit
that referenced
this pull request
Jan 3, 2024
https://eaflood.atlassian.net/browse/WATER-4292 > This is part of a series of changes to add the ability to identify the licence holder for a licence This change adds the migration, model, test helper and unit tests for a new `LicenceDocumentRoleModel`. We added [LicenceDocumentModel](#618). Though pointless, it's the only thing that points us to who the licence holder is. In the `crm_v2` schema it does this in `crm_v2.document_roles` which is the join table between `crm_v2.documents` and `crm_v2.roles`. `roles` is a lookup table of the 'role' a `crm_v2.contact` or `crm_c2.company` has, for example, licence holder! We added that as [LicenceRoleModel](#629). We are now on the final step which is adding the model that allows us to identify and extract the `crm_v2.document_roles` record that identifies _who_ is the current licence holder for a licence.
Cruikshanks
added a commit
that referenced
this pull request
Jan 3, 2024
https://eaflood.atlassian.net/browse/WATER-4292 > This is part of a series of changes to add the ability to identify the licence holder for a licence This change adds the migration, model, test helper and unit tests for a new `LicenceDocumentRoleModel`. We added [LicenceDocumentModel](#618). Though pointless, it's the only thing that points us to who the licence holder is. We then added [LicenceRoleModel](#629) and [LicenceDocumentRoleModel](#630) which in combination with `LicenceDocumentModel` will give us the licence holder. But we intend to work with `LicenceModel` instances. `LicenceDocumentModel` is just a really, _really_ cut down copy of the licence record which the previous team added for 'reasons'. So, this last change before we can get on and identify the licence holder is to setup the relationship between `LicenceModel` and `LicenceDocumentModel`.
Cruikshanks
added a commit
that referenced
this pull request
Jan 4, 2024
https://eaflood.atlassian.net/browse/WATER-4292 > This is part of a series of changes to add the ability to identify the licence holder for a licence This change adds the migration, model, test helper and unit tests for a new `LicenceDocumentRoleModel`. We added [LicenceDocumentModel](#618). Though pointless, it's the only thing that points us to who the licence holder is. We then added [LicenceRoleModel](#629) and [LicenceDocumentRoleModel](#630) which in combination with `LicenceDocumentModel` will give us the licence holder. But we intend to work with `LicenceModel` instances. `LicenceDocumentModel` is just a really, _really_ cut down copy of the licence record which the previous team added for 'reasons'. So, this last change before we can get on and identify the licence holder is to set up the relationship between `LicenceModel` and `LicenceDocumentModel`.
Cruikshanks
added a commit
that referenced
this pull request
Jan 7, 2024
https://eaflood.atlassian.net/browse/WATER-4292 > This is part of a series of changes to add the ability to identify the licence holder for a licence Arghhhhh! 😱🤬😩 We've not long ago added 3 new models to this project - [LicenceDocumentModel](#618) - [LicenceRoleModel](#629) - [LicenceDocumentRoleModel](#630) We then [linked the LicenceModel to LicenceDocumentModel](#632). All this was based on a first pass of what we thought the [water-abstraction-ui](https://github.com/DEFRA/water-abstraction-ui), [water-abstraction-service](https://github.com/DEFRA/water-abstraction-service) and [water-abstraction-tactical-crm](https://github.com/DEFRA/water-abstraction-tactical-crm) apps were doing to determine the licence holder. It reflected what we found in the tables behind the 3 models we added as well. We'd just [coded up a service](#639) to set the return requirement setup session with the name of the licence holder and begun testing when we found an issue in testing. We came across a licence where neither the company or contact reflected what we saw in the UI for the licence holder! In the UI beneath the licence holder name is a 'View licence contact details' link. We were able to figure out that the ID it uses is for a `crm.document_headers` record. And in there we found (surprise-surprise!) another JSONB field holding a bunch of data, including the licence holder name we were seeing on screen. Another dive into the legacy code revealed we'd taken a wrong turning. When the page is requested there is logic that grabs records from the tables we created models for. But they are not a used. In fact, not even the `crm.document_headers` record is used, other than to provide an ID for the link. What seems to happen (we could be wrong again!) is that the `permit.licence` table is queried and its `licence_data_value` field is extracted. If you haven't guessed already, this is another JSON field. Only this one seems to be a complete 'dump' of everything imported from NALD for the licence. This data gets passed to `src/lib/licence-transformer/nald-transformer.js` which transforms it from the NALD structure into a 'licence'. This includes a `licenceHolderFullName` property which is eventually what appears in the UI. We have absolutely _zero_ intent to start importing and transforming raw NALD data just to get the licence holder name. Thankfully, in the follow up testing we've been doing the information held in `crm.document_headers` has always matched what we see in the UI. So, this is the start of our second attempt to crack licence holder name starting with adding the `DocumentHeader` model.
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-4292
This change adds the migration, model, test helper and unit tests for a new
LicenceRoleModel.We've already added LicenceDocumentModel. Though pointless, it's the only thing that points us to who the licence holder is. In the
crm_v2schema it does this incrm_v2.document_roleswhich is the join table betweencrm_v2.documentsandcrm_v2.roles.rolesis a lookup table of the 'role' acrm_v2.contactorcrm_c2.companyhas, for example, licence holder!So, our next step is to create a model for 'roles' so we can then get on and join them in a future model. Our first blocker though is we can't use the name
rolesand that is already taken (RoleModelis used for the lookup table of user roles). Hence, we're creating a newLicenceRoleModelin this change! 😁