-
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
Move licence-changes to endpoint and split work #1600
Merged
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-4546 > Part of the work to migrate management of return versions to WRLS from NALD During the import from NALD, if a licence end date is changed, for example, if it has been revoked in NALD, our return version functionality needs to know about it. This is so we can reissue the return logs for the licence to match the changed end date. We are actively trying to move away from the legacy code base, so this work was always going to be done in here. Initially, it would be triggered by our [own import process](https://eaflood.atlassian.net/browse/WATER-4535), which was due to replace the legacy one. But that was when we thought ReSP would be taking over from NALD. Now, the plan is for WRLS to encompass the final abstraction leg and take over from NALD. There is little point in replacing a complex import we intend to switch off in the next year. But, knowing we'd need something in the interim, we created the [/jobs/licence-changes job](#1593). The intent was to schedule this after the first [NALD import job](https://github.com/DEFRA/water-abstraction-team/blob/main/jobs/import.md#nald-import) had completed (the one that downloads and extracts the NALD data) but before the main [licence import job](https://github.com/DEFRA/water-abstraction-team/blob/main/jobs/import.md#licence-import). That way, our job could compare the NALD and WRLS licence records to determine if an end date has changed. But then we had our 'doh!' moment. Our reissue return logs engine expects to be given a licence ID and the date the change applies. However, the WRLS licence record needs to have been updated for it to determine the start and end dates of the new return logs. If we schedule `/jobs/licence-changes` before the licence import job, it will see the change but won't reissue anything because the WRLS record won't have been updated. If we schedule it after, it won't see any difference and won't trigger the reissue. Doh! So, we've needed to change tact on this completely. Now, we intend to - _trigger_ `/licences/end-dates/check` from the [NALD import job](https://github.com/DEFRA/water-abstraction-team/blob/main/jobs/import.md#nald-import) rather than schedule it - refactor `/jobs/licence-changes` as `/licences/end-dates`, but instead of processing changed licences it [stores the details of the change in a table](DEFRA/water-abstraction-service#2666) - add a new `/licences/end-dates/process` endpoint that processes these 'licence change' records, and which will be triggered from the [licence import job](https://github.com/DEFRA/water-abstraction-team/blob/main/jobs/import.md#licence-import) This change handles the refactoring and creating of the endpoints, plus addition of a new model for for the new table.
Cruikshanks
added
the
housekeeping
Refactoring, tidying up or other work which supports the project
label
Jan 6, 2025
We also rename the config for the process as it is no longer a job.
Cruikshanks
force-pushed
the
update-licence-changes-to-store-changes
branch
from
January 7, 2025 08:45
7fd71a2
to
a4a0d82
Compare
Cruikshanks
force-pushed
the
update-licence-changes-to-store-changes
branch
from
January 7, 2025 10:39
a4a0d82
to
a474f3d
Compare
Not sure why we were selecting from `NALD_ABS_LIC_VERSIONS` and then joining to `NALD_ABS_LICENCES` but as the service is called `FetchLicencesService`, and all the data is coming from `NALD_ABS_LICENCES` it makes sense to refactor the query to treat it as the main table to select from. Now it reads as expected, select all licences with a 'CURR' licence version.
Our constraint causes the original version of the test to fail.
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-4546
During the import from NALD, if a licence end date is changed, for example, if it has been revoked in NALD, our return version functionality needs to know about it. This is so we can reissue the return logs for the licence to match the changed end date.
We are actively trying to move away from the legacy code base, so this work was always going to be done in here. Initially, it would be triggered by our own import process, which was due to replace the legacy one. But that was when we thought ReSP would be taking over from NALD.
Now, the plan is for WRLS to encompass the final abstraction leg and take over from NALD. There is little point in replacing a complex import we intend to switch off in the next year.
But, knowing we'd need something in the interim, we created the /jobs/licence-changes job. The intent was to schedule this after the first NALD import job had completed (the one that downloads and extracts the NALD data) but before the main licence import
job. That way, our job could compare the NALD and WRLS licence records to determine if an end date has changed.
But then we had our 'doh!' moment.
Our reissue return logs engine expects to be given a licence ID and the date the change applies. However, the WRLS licence record needs to have been updated for it to determine the start and end dates of the new return logs.
If we schedule
/jobs/licence-changes
before the licence import job, it will see the change but won't reissue anything because the WRLS record won't have been updated. If we schedule it after, it won't see any difference and won't trigger the reissue.Doh!
So, we've needed to change tact on this completely. Now, we intend to
/licences/end-dates/check
from the NALD import job rather than schedule it/jobs/licence-changes
as/licences/end-dates
, but instead of processing changed licences, it stores the details of the change in a table/licences/end-dates/process
endpoint that processes these 'licence change' records, which will be triggered from the licence import jobThis change handles the refactoring and creation of the endpoints, plus the addition of a new model for the new table.