-
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
Remove the replacement legacy import code #1585
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-4535 This time last year (December 2023) we'd completed a workshop with our ReSP colleagues, and had begun working to a vision that would conclude with NALD being replaced by ReSP. A key part of this would be a complete change in the way we import our licence abstraction (ABS) data. The legacy process that looks at _all_ licences, processing each one to determine new and changed licences, would be replaced by single licence 'events'. ReSP would provide an event bus from to which our service would be a subscriber. As MVP (because details were scant) we would treat any event as a trigger to process a licence and determine if it is new or changed. In order to be as 'ready' as possible, we saw an opportunity to replace the existing legacy import and create the basis for that MVP. We would rebuild what the legacy import is doing but refactor the process to be licence specific. For example, the legacy import has a CRM step for all licences, then an abstraction step, then a returns step. We'd refactor that to do the CRM step, then the abstraction step, then the returns step _for a single licence_. This work was done under [ReSP Import abstraction data prep - water.licence](https://eaflood.atlassian.net/browse/WATER-4535). We got it to a point where the contacts and abstraction steps could be tested, once we'd figured out just how we'd go about verifying it was importing things correctly! We'd then need to tackle returns information. Piggy-backing on this work were changes need for two-part tariff billing and return version management. These two features needed to know when a licence 'end dates' changed in order to correctly flag it for billing, and in case return logs needed to be reissued. This was no small job, and got as far as it did thanks to the perseverance of @jonathangoulding . Unfortunately, events overtook this work and a new change in direction has been agreed. Now WRLS itself will replace NALD. This means licences will be created and managed within the service directly. There will be no need for an import process _at all_. It having changed direction once, there was always the chance it would change back! So, we decided to leave the work done within the app. If nothing else, it's a handy resource to refer to when trying to figure out what the legacy import is actually doing. But two-part tariff billing and return version management still need to know if a licence's end dates have changed. So, we've added a new job (see [WATER-4728 New 'import' job to trigger return log and supplementary flag checks](https://eaflood.atlassian.net/browse/WATER-4728)) at `/jobs/import-licences` specifically designed to trigger these two processes. The problem is when looking at the work around, sometimes folks are doing it in this new job. Other times they are updating and amending the legacy licence import replacement we built. There is a concern that something is going to be missed or done incorrectly because the team is getting confused between the two. So, leaning into the knowledge that source control means deleted code is never truly lost, this change aims to remove the confusion by deleting the replacement legacy import functionality we added.
Cruikshanks
added
the
housekeeping
Refactoring, tidying up or other work which supports the project
label
Dec 24, 2024
Cruikshanks
added a commit
that referenced
this pull request
Dec 28, 2024
https://eaflood.atlassian.net/browse/WATER-4535 In [Remove the replacement legacy import code](#1585) we removed the code we added for replacing the legacy import service (see the PR for the reasons why). Or we thought we did! Turns out we missed a bunch of stuff. Doh! This change removes the stuff we missed.
Cruikshanks
added a commit
that referenced
this pull request
Dec 28, 2024
https://eaflood.atlassian.net/browse/WATER-4535 In [Remove the replacement legacy import code](#1585), we removed the code we had added to replace the legacy import service (see the PR for the reasons why). Or we thought we did! Turns out we missed a bunch of stuff. Doh! This change removes the stuff we missed.
Cruikshanks
added a commit
that referenced
this pull request
Jan 3, 2025
https://eaflood.atlassian.net/browse/WATER-4728 After completing [Remove the replacement legacy import code](#1585) 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 - determining that a licence has a changed 'end date' - determining what that changed date is 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 `FetchLicencesServices` uses! 😱 😁
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-4535
By this time last year (December 2023), we completed a workshop with our ReSP colleagues and began working toward a vision that would conclude with NALD being replaced by ReSP. A key part of this vision would be a complete change in how we import our licence abstraction (ABS) data.
The legacy process that examines all licences, processing each one to determine whether it is new or changed, would be replaced by single licence 'events'. ReSP would provide an event bus to which our service would subscribe. As MVP (because details were scant), we would treat any event as a trigger to process a licence and determine whether it is new or changed.
To be as ready as possible, we saw an opportunity to replace the existing legacy import and create the basis for that MVP. We would rebuild what the legacy import does but refactor the process to be licence-specific. For example, the legacy import has a CRM step for all licences, an abstraction step for all, and a returns step for all. We'd refactor that to do the CRM step, the abstraction step, and the returns step for a single licence.
This work was done under ReSP Import abstraction data prep—water.licence. Once we figured out how to verify that it was importing things correctly, we could test the contacts and abstraction steps. We'd then need to tackle returns information.
Piggybacking on this work was the need to change two parts: tariff billing and return version management. These two features needed to know when a licence 'end dates' were changed to flag it for billing correctly and in case return logs were required to be reissued.
This was no small job, and it got as far as it did thanks to the perseverance of @jonathangoulding . Unfortunately, events overtook this work and a new change in direction has been agreed. Now, WRLS itself will replace NALD. This means licences will be created and directly managed within the service. There will be no need for an import process at all.
If it had changed direction once, there was always the chance it would change back! So, we decided to leave the work done within the app. If nothing else, it's a handy resource for figuring out what the legacy import is doing.
However, management of the two-part tariff billing and return version still needs to know if a licence's end dates have changed. So, we've added a new job (see WATER-4728 New 'import' job to trigger return log and supplementary flag checks) at
/jobs/import-licences
specifically designed to trigger these two processes.The problem is that folks sometimes do it in this new job when it comes to managing two-part tariffs and returns. Other times, they update and amend the legacy licence import replacement we built. There is a concern that something will be missed or done incorrectly because the team is confused between the two.
So, leaning into the knowledge that source control means deleted code is never truly lost, this change aims to remove the confusion by deleting the replacement legacy import functionality we added.