-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add a remove duplicate licence feature #883
Merged
+790
−31
Merged
Changes from 13 commits
Commits
Show all changes
17 commits
Select commit
Hold shift + click to select a range
6af505c
Add a remove duplicate licence feature
Cruikshanks 63b98ea
Housekeeping - re-group controller tests
Cruikshanks 0ed157a
Add our new routes to /data
Cruikshanks 7f07aa3
Add view
Cruikshanks 06bb7c9
Add legacy migration tables for unit testing
Cruikshanks 954640a
Add DeDuplicateLicenceService
Cruikshanks 0475272
Add SubmitDeduplicateService
Cruikshanks 9df40fa
Add new endpoints to /data controller
Cruikshanks efb875e
Add new routes
Cruikshanks f01ac40
Merge branch 'main' into remove-duplicate-licence-tool
Cruikshanks 2d6b30d
Resolve SonarCloud issues
Cruikshanks 59ef73c
Doh! Remove console.log
Cruikshanks 1983698
Comment corrections
Cruikshanks 81f37a0
Fix error link in view
Cruikshanks e9c36d6
Doh! Again! Add missed controller tests
Cruikshanks 3dfee18
Correct test and demonstrate possible scenario
Cruikshanks d0c380e
Merge branch 'main' into remove-duplicate-licence-tool
Cruikshanks File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
175 changes: 175 additions & 0 deletions
175
app/services/data/deduplicate/de-duplicate-licence.service.js
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,175 @@ | ||
'use strict' | ||
|
||
/** | ||
* De-duplicates a licence by removing the record with whitespace and its related records | ||
* @module DeDuplicateService | ||
*/ | ||
|
||
const { db } = require('../../../../db/db.js') | ||
const LicenceModel = require('../../../models/licence.model.js') | ||
|
||
/** | ||
* De-duplicates a licence by removing the record with whitespace and its related records | ||
* | ||
* If first matches all licences against the licence reference provided. For example, if '01/120' is the reference we | ||
* will match | ||
* | ||
* - '01/120' | ||
* - 'WA/01/120' | ||
* - '02/01/120/R01' | ||
* - ' 01/120' | ||
* - ' 01/120 ' | ||
* - '01/120 ' | ||
* - '\n01/120' | ||
* - '\n01/120\n' | ||
* - '01/120\n' | ||
* | ||
* We then use a regex to filter out any result that does not include whitespace. These are the invalid licence records | ||
* that need to be removed. We iterate through them (though we expect there to only ever be 1 result) and first delete | ||
* all related records before then deleting the main licence record. | ||
* | ||
* We've opted to use {@link https://knexjs.org/guide/raw.html | Knex.raw()} rather than | ||
* {@link https://vincit.github.io/objection.js/ | Objection.js} because not all the tables referenced have models. So, | ||
* rather than switch between different ways of querying we stick to one for the removal. | ||
* | ||
* @param {string} licenceRef - Reference for the licence with a duplicate record | ||
* | ||
* @returns {Promise} the service does not resolve to a value | ||
*/ | ||
async function go (licenceRef) { | ||
const invalidLicences = await _determineInvalidLicences(licenceRef) | ||
|
||
// NOTE: In theory there could be more than one 'bad' licence for the matching licence reference! But mainly we do | ||
// this as a loop because it is an easy way to deal with the fact we have an array. | ||
for (const invalidLicence of invalidLicences) { | ||
const { id: licenceId, licenceRef: invalidLicenceRef } = invalidLicence | ||
|
||
// NOTE: Order is important. Some tables have to be cleared before others. So resist trying to make the calls | ||
// alphabetical! | ||
await _documentRoles(invalidLicenceRef) | ||
await _documents(invalidLicenceRef) | ||
await _documentHeaders(invalidLicenceRef) | ||
await _permitLicences(invalidLicenceRef) | ||
await _returns(invalidLicenceRef) | ||
await _licenceVersionPurposeConditions(licenceId) | ||
await _licenceVersionPurposes(licenceId) | ||
await _licenceVersions(licenceId) | ||
await _returnRequirementPurposes(licenceId) | ||
await _returnRequirements(licenceId) | ||
await _returnVersions(licenceId) | ||
await _licences(licenceId) | ||
} | ||
} | ||
|
||
async function _determineInvalidLicences (licenceRef) { | ||
const licences = await LicenceModel.query() | ||
.select([ | ||
'id', | ||
'licenceRef' | ||
]) | ||
.whereILike('licenceRef', `%${licenceRef}%`) | ||
|
||
// NOTE: Match any string which has whitespace at the start or the end of the string | ||
// ^ asserts position at start of the string | ||
// \s matches any whitespace character | ||
const whitespaceAtStartRegex = /^\s/ | ||
// \s matches any whitespace character | ||
// $ asserts position at end of the string | ||
const whitespaceAtEndRegex = /\s$/ | ||
|
||
const invalidLicences = licences.filter((licence) => { | ||
return whitespaceAtStartRegex.test(licence.licenceRef) || whitespaceAtEndRegex.test(licence.licenceRef) | ||
}) | ||
|
||
return invalidLicences | ||
} | ||
|
||
async function _documentHeaders (licenceRef) { | ||
return db.raw(` | ||
DELETE FROM crm.document_header WHERE system_external_id = ?; | ||
`, licenceRef) | ||
} | ||
|
||
async function _documentRoles (licenceRef) { | ||
return db.raw(` | ||
DELETE FROM "crm_v2"."document_roles" WHERE document_id IN ( | ||
SELECT document_id FROM "crm_v2"."documents" WHERE document_ref = ? | ||
); | ||
`, licenceRef) | ||
} | ||
|
||
async function _documents (licenceRef) { | ||
return db.raw(` | ||
DELETE FROM "crm_v2"."documents" WHERE document_ref = ?; | ||
`, licenceRef) | ||
} | ||
|
||
async function _licences (licenceId) { | ||
return db.raw(` | ||
DELETE FROM water.licences WHERE licence_id = ?; | ||
`, licenceId) | ||
} | ||
|
||
async function _licenceVersionPurposeConditions (licenceId) { | ||
return db.raw(` | ||
DELETE FROM water.licence_version_purpose_conditions WHERE licence_version_purpose_id IN ( | ||
SELECT licence_version_purpose_id FROM water.licence_version_purposes WHERE licence_version_id IN ( | ||
SELECT licence_version_id FROM water.licence_versions WHERE licence_id = ? | ||
) | ||
); | ||
`, licenceId) | ||
} | ||
|
||
async function _licenceVersionPurposes (licenceId) { | ||
return db.raw(` | ||
DELETE FROM water.licence_version_purposes WHERE licence_version_id IN ( | ||
SELECT licence_version_id FROM water.licence_versions WHERE licence_id = ? | ||
); | ||
`, licenceId) | ||
} | ||
|
||
async function _licenceVersions (licenceId) { | ||
return db.raw(` | ||
DELETE FROM water.licence_versions WHERE licence_id = ?; | ||
`, licenceId) | ||
} | ||
|
||
async function _permitLicences (licenceRef) { | ||
return db.raw(` | ||
DELETE FROM permit.licence WHERE licence_ref = ?; | ||
`, licenceRef) | ||
} | ||
|
||
async function _returnRequirementPurposes (licenceId) { | ||
return db.raw(` | ||
DELETE FROM water.return_requirement_purposes WHERE return_requirement_id IN ( | ||
SELECT return_requirement_id FROM water.return_requirements WHERE return_version_id IN ( | ||
SELECT return_version_id FROM water.return_versions WHERE licence_id = ? | ||
) | ||
); | ||
`, licenceId) | ||
} | ||
|
||
async function _returnRequirements (licenceId) { | ||
return db.raw(` | ||
DELETE FROM water.return_requirements WHERE return_version_id IN ( | ||
SELECT return_version_id FROM water.return_versions WHERE licence_id = ? | ||
); | ||
`, licenceId) | ||
} | ||
|
||
async function _returns (licenceRef) { | ||
return db.raw(` | ||
DELETE FROM "returns"."returns" WHERE licence_ref = ?; | ||
`, licenceRef) | ||
} | ||
|
||
async function _returnVersions (licenceId) { | ||
return db.raw(` | ||
DELETE FROM water.return_versions WHERE licence_id = ?; | ||
`, licenceId) | ||
} | ||
|
||
module.exports = { | ||
go | ||
} |
47 changes: 47 additions & 0 deletions
47
app/services/data/deduplicate/submit-deduplicate.service.js
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,47 @@ | ||
'use strict' | ||
|
||
/** | ||
* Handles the user submission for the `/data/deduplicate` page | ||
* @module SubmitDeduplicateService | ||
*/ | ||
|
||
const DeDuplicateService = require('./de-duplicate-licence.service.js') | ||
|
||
/** | ||
* Handles the user submission for the `/data/deduplicate` page | ||
* | ||
* It will first validate that the user has entered a reference. If they haven't we return an error that can be used by | ||
* the Nunjucks view to let the user know. | ||
* | ||
* If they have we first parse it, removing any whitespace and converting any lowercase characters to uppercase. This | ||
* parsed reference is then passed to the `DeDuplicateService` to do the actual removal of the duplicate licence. | ||
* | ||
* Once complete we return the parsed reference back to the controller. As a handy way of confirming if it worked the | ||
* controller will redirect the user to the search page and have it search for the licence reference. If the tool has | ||
* done its job the page should no longer error. | ||
* | ||
* @returns {Promise<Object>} an object containing a parsed version of the licence reference submitted else an error | ||
* message if nothing was entered | ||
*/ | ||
async function go (payload) { | ||
const licenceRef = payload['licence-ref'] | ||
|
||
if (!licenceRef || licenceRef.trim() === '') { | ||
return { | ||
error: { | ||
text: 'Enter a licence reference to de-dupe' | ||
} | ||
} | ||
} | ||
|
||
const parsedLicenceRef = licenceRef.trim().toUpperCase() | ||
await DeDuplicateService.go(parsedLicenceRef) | ||
|
||
return { | ||
licenceRef: parsedLicenceRef | ||
} | ||
} | ||
|
||
module.exports = { | ||
go | ||
} |
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,38 @@ | ||
{% extends 'layout.njk' %} | ||
{% from "govuk/components/button/macro.njk" import govukButton %} | ||
{% from "govuk/components/error-summary/macro.njk" import govukErrorSummary %} | ||
{% from "govuk/components/input/macro.njk" import govukInput %} | ||
|
||
{% block content %} | ||
{% if error %} | ||
{{ govukErrorSummary({ | ||
titleText: "There is a problem", | ||
errorList: [ | ||
{ | ||
text: error.text, | ||
href: "#licence-ref-error" | ||
} | ||
] | ||
}) }} | ||
{% endif %} | ||
|
||
<div class="govuk-body"> | ||
<form method="post"> | ||
{{ govukInput({ | ||
classes: "govuk-!-width-one-third", | ||
errorMessage: error, | ||
label: { | ||
text: pageTitle, | ||
classes: "govuk-label--l", | ||
isPageHeading: true | ||
}, | ||
hint: { | ||
text: "Enter the licence reference for de-duping" | ||
}, | ||
name: "licence-ref" | ||
}) }} | ||
|
||
{{ govukButton({ text: "Remove duplicate" }) }} | ||
</form> | ||
</div> | ||
{% endblock %} |
35 changes: 35 additions & 0 deletions
35
db/migrations/legacy/20221108007023-water-return-versions.js
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,35 @@ | ||
'use strict' | ||
|
||
const tableName = 'return_versions' | ||
|
||
exports.up = function (knex) { | ||
return knex | ||
.schema | ||
.withSchema('water') | ||
.createTable(tableName, (table) => { | ||
// Primary Key | ||
table.uuid('return_version_id').primary().defaultTo(knex.raw('gen_random_uuid()')) | ||
|
||
// Data | ||
table.uuid('licence_id').notNullable() | ||
table.integer('version_number').notNullable() | ||
table.date('start_date').notNullable() | ||
table.date('end_date') | ||
table.string('status water').notNullable() | ||
table.string('external_id') | ||
|
||
// Legacy timestamps | ||
table.timestamp('date_created', { useTz: false }).notNullable() | ||
table.timestamp('date_updated', { useTz: false }) | ||
|
||
// Constraints | ||
table.unique(['external_id'], { useConstraint: true }) | ||
}) | ||
} | ||
|
||
exports.down = function (knex) { | ||
return knex | ||
.schema | ||
.withSchema('water') | ||
.dropTableIfExists(tableName) | ||
} |
Oops, something went wrong.
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what if someone puts a space in the middle of the reference? will the
whereILike
handle that ok?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, but in so far as it won't match anything. The import process may be flaky but we've not experienced a licence ref with whitespace in the reference.
But this comment was helpful! It's made me see my test for a non-matching scenario isn't doing what I thought it was. So, I can correct the test and demonstrate how this is handled at the same time (does mean I'm gonna need this re-approved once I've made the change 😬 )
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pfft! Don't need your approval anymore! 😝