Add a remove duplicate licence feature #883
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-4429 Usually once or twice a month we have to deal with a duplicate licence in the service. It is not actually a duplicate but an issue with NALD (the upstream service) that our [water-abstraction-import](https://github.com/DEFRA/water-abstraction-import) does not handle. We believe when a change is made to a licence in NALD it requires users to enter the licence reference again. This creates a new record in NALD but our import service recognises it is for the same licence because the references match. However, NALD is known for poor validation and doesn't trim data entered. So, often a user will accidentally add an extra space or newline character when entering the licence reference again. On import, computers being the way they are, will see this as a new licence so will create a new licence record with the same 'bad' reference. This causes an error for anything that attempts to retrieve the licence by reference in the service. The most visible casualty is the search page. We _should_ be fixing the import. But we're only just diving into that black-box of code and are reticent to change things we don't understand. Plus the reason we are now looking at the import code is because of the strong likelihood we will have to build a new import service this year. So, to avoid constantly having to create migration scripts and to allow us to respond sooner when an issue is spotted, this change adds a tool that will allow us to delete any 'bad' licence records.
Part of moving towards making the endpoints more visible in our controller tests.
We'll need a page that allows someone to enter the reference to be deleted. We'll not be adding a link to it anywhere as it's only us on the dev team that we intend to be using it. So you'll need to type it in when you need it! Because of that we've added it to our `/data` root. It is related to licences but we think as it is intended for our use `/data` is a better home. We've scoped it to `billing` which is a role a number of us have. Billing & Data is also the team that notices these problem licences and deal with them in NALD. So, with there being no 'admin' or 'dev' scope we think this is the best available.
We'll reference these tables when deleting related records to the duplicate licence. As this is the only reason for referencing them at the moment we have opted not to create models and views for them.
This will handle finding and removing the duplicate 'bad' licence record.
This will handle the licence reference entered into our page.
Cruikshanks
requested review from
Demwunz,
robertparkinson,
Jozzey,
Beckyrose200 and
rvsiyad
robertparkinson
previously approved these changes
Apr 4, 2024
| async function go (payload) { | ||
| const licenceRef = payload['licence-ref'] | ||
|
|
||
| if (!licenceRef || licenceRef.trim() === '') { |
There was a problem hiding this comment.
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.
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.
Pfft! Don't need your approval anymore! 😝
The test wasn't actually testing for a non-match before because we were passing in a reference that would find matches.
Beckyrose200
approved these changes
Apr 4, 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-4429
Usually once or twice a month we have to deal with a duplicate licence in the service. It is not a duplicate but an issue with NALD (the upstream service) that our water-abstraction-import does not handle.
We believe when a change is made to a licence in NALD it requires users to enter the licence reference again. This creates a new record in NALD but our import service recognises it is for the same licence because the references match.
However, NALD is known for poor validation and doesn't trim data entered. So, often a user will accidentally add an extra space or newline character when entering the licence reference again. On import, computers being the way they are, will see this as a new licence so will create a new licence record with the same 'bad' reference.
This causes an error for anything that attempts to retrieve the licence by reference in the service. The most visible casualty is the search page.
We should be fixing the import. But we're only just diving into that black box of code and are reticent to change things we don't understand. Plus the reason we are now looking at the import code is because of the strong likelihood we will have to build a new import service this year.
So, to avoid constantly having to create migration scripts and allow us to respond sooner when an issue is spotted, this change adds a tool that will allow us to delete any 'bad' licence records.