Add LicenceDocumentModel from crm_v2.documents #618
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 `LicenceDocumentModel`. TBH, it is a pointless model! This seems to be a bridge between the `water.licences` table we now have and the `crm.document` that was created back when the idea was to have a generic 'repository' for all types of documents, not just water abstraction licences. We know the schema `crm_v2` is the result of an incomplete attempt to move away from that idea. But for whatever reason, rather than link the `water.licences` records directly to contact and company information instead the previous team stuck with the concept of a 'document' So, we need to add this model to then be able to identify the the relevant `crm_v2.document_roles` records we need, to find the current licence holder! 🥳😩
diff --git a/db/migrations/legacy/20231221145235_create-crm-v2-documents.js b/db/migrations/legacy/20231221145235_create-crm-v2-documents.js
new file mode 100644
index 0000000..e1ac921
--- /dev/null
+++ b/db/migrations/legacy/20231221145235_create-crm-v2-documents.js
@@ -0,0 +1,36 @@
+'use strict'
+
+const tableName = 'documents'
+
+exports.up = function (knex) {
+ return knex
+ .schema
+ .withSchema('crm_v2')
+ .createTable(tableName, (table) => {
+ // Primary Key
+ table.uuid('document_id').primary().defaultTo(knex.raw('gen_random_uuid()'))
+
+ // Data
+ table.string('regime').notNullable()
+ table.string('document_type').notNullable()
+ table.string('document_ref').notNullable()
+ table.date('start_date').notNullable()
+ table.date('end_date').notNullable()
+ table.string('external_id')
+ table.boolean('is_test').notNullable().defaultTo(false)
+ table.timestamp('date_deleted')
+
+ // Legacy timestamps
+ table.timestamp('date_created').notNullable().defaultTo(knex.fn.now())
+ table.timestamp('date_updated').notNullable().defaultTo(knex.fn.now())
+
+ table.unique(['regime', 'document_type', 'document_ref'], { useConstraint: true })
+ })
+}
+
+exports.down = function (knex) {
+ return knex
+ .schema
+ .withSchema('crm_v2')
+ .dropTableIfExists(tableName)
+}
diff --git a/db/migrations/public/20231221144424_create-licence-documents-view.js b/db/migrations/public/20231221144424_create-licence-documents-view.js
new file mode 100644
index 0000000..1966f78
--- /dev/null
+++ b/db/migrations/public/20231221144424_create-licence-documents-view.js
@@ -0,0 +1,29 @@
+'use strict'
+
+const viewName = 'licence_documents'
+
+exports.up = function (knex) {
+ return knex
+ .schema
+ .createView(viewName, (view) => {
+ // NOTE: We have commented out unused columns from the source table
+ view.as(knex('documents').withSchema('crm_v2').select([
+ 'document_id AS id',
+ // 'regime',
+ // 'document_type',
+ 'document_ref AS licence_ref',
+ 'start_date',
+ 'end_date',
+ // 'is_test',
+ 'date_deleted AS deleted_at',
+ 'date_created AS created_at',
+ 'date_updated AS updated_at'
+ ]))
+ })
+}
+
+exports.down = function (knex) {
+ return knex
+ .schema
+ .dropViewIfExists(viewName)
+}
Cruikshanks
requested review from
Demwunz,
robertparkinson,
Jozzey and
Beckyrose200
robertparkinson
approved these changes
Dec 21, 2023
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 `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! 😁
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 `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! 😁
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
LicenceDocumentModel.TBH, it is a pointless model! This seems to be a bridge between the
water.licencestable we now have and thecrm.documentthat was created back when the idea was to have a generic 'repository' for all types of documents, not just water abstraction licences. We know the schemacrm_v2is the result of an incomplete attempt to move away from that idea. But for whatever reason, rather than link thewater.licencesrecords directly to contact and company information instead the previous team stuck with the concept of a 'document'So, we need to add this model to then be able to identify the relevant
crm_v2.document_rolesrecords we need, to find the current licence holder! 🥳😩