Rename Import job as Licence Changes and refactor #1593
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
We shouldn't be taking dates from superseded licence versions, on from whatever is flagged as the current licence version for the licence from NALD.
By including both the NALD and WRLS dates in our original fetch service we can save an additional 74K queries against the database. What our job needs to know is _if_ a licence has a changed end date. Billing flags and return logs then need to know _when_. Currently, we are passing the raw data down and they are doing there own variation of determining what the `changeDate` should be. However, its the same for both, so why not determine that in the service that is already processing the end dates!? The result is a new DetermineEarliestLicenceChangedDate service that replaces DetermineLicenceEndDateChanged.
First, this service wasn't actually generating the return logs. It was actually determining the change date for the licence and then passing this result to ProcessLicenceReturnLogsService. Now we have DetermineEarliestLicenceChangedDateService that work is done for us, so this service has become redundant.
With the name change and use of DetermineEarliestLicenceChangedDateService it was easier to create a new service to handle the processing of each licence by the job. Another feature that was missed was the inclusion of the feature flag. Until WRLS takes over management of requirements for returns, we shouldn't be reissuing return logs during import. This new service gives us an opportunity to add the functionality in.
We needed to change the service name to follow our convention of the service matching with the job endpoint and name. We also didn't see the need to solve the problem of importing an ESM only dependency any different than we already do with Got in our `BaseRequest` module. That, plus some expansion of comments and tidying up resulted in this new service to replace the existing one.
We no longer support sending a licence ref to our return logs jobs endpoint so these tests are redundant. We also swallow the log message that appears, caused by the 404 test.
The payload we are sending now has a different structure.
The key change is all the work the service was doing to determine the earliest change date can be removed because this has already been determined.
With the changes we have made the test endpoints needed to be updated to reflect what the `/jobs/licence-changes` would actually sending through to the billing flags and return logs service when a licence due to be imported from NALD is found to have a changed end date.
Cruikshanks
added
the
housekeeping
Cruikshanks
added a commit
that referenced
this pull request
Jan 6, 2025
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.
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-4728
After completing Remove the replacement legacy import code we could review the replacement 'import job' we created in a clearer light.
The immediate thing that rang out was that we are not importing a license! The job is connected to the import licence process, but specifically, it cares about changes to the licence. Hence the name change.
After that, we could see the job and the two downstream processes it kicks off from a bird's eye view.
What jumped out is they were both doing there own variation of the same thing: working out what the 'change date' is. Plus, in the case of return logs it was re-quering the DB for information that could be retrieved in the job's
FetchLicenceService.So, in this change we have refactored the job to handle both
As the job has this information, it can then pass it to the downstream services, which means we can delete the redundant code from them.
Along the way, we do some housekeeping: updating the job to match the existing jobs, enhancing the comments, and fixing a whoopsie in the query
FetchLicencesServicesuses! 😱 😁