Add LicenceDocumentHeaderModel #640
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 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.
Also ensure it is included in our database helper clean up.
Cruikshanks
changed the title
Cruikshanks
added
do not merge
Cruikshanks
requested review from
Demwunz,
robertparkinson,
Jozzey and
Beckyrose200
robertparkinson
previously approved these changes
Jan 8, 2024
|
Thank you for reviewing this @robertparkinson . But further testing has shown that even though the legacy code doesn't usethe As the |
|
Just going to keep this around until we have confirmation from our QA colleagues that yes, licence holder looks good just in case we find we do need to revert to using this table instead. |
|
I think we are going to need this anyway. We are currently working on rebuilding the view licence page and replacing the legacy UI version (WATER-4322 ). A number of the links in the summary tab depend on having the document header ID. So, even if we do nothing else with this model we're going to need it to grab the correct ID for the links. |
robertparkinson
approved these changes
Jan 15, 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-4322
When viewing a licence using the legacy water-abstraction-ui the summary tab features several links to other pages, some of which give expanded details for the item shown.
Those links rely on having the
crm.document_headerID for the licence currently being viewed. Our work on WATER-4292 (identifying the licence holder) has shown that much of the data in the table has already been imported elsewhere.But without the ID those links and pages won't work and they are beyond the scope of what we are doing with the view licence page.
So, to allow us to quickly get the related
crm.document_header(that we have chosen to callLicenceDocumentHeader) for a licence this change adds the model to the project.