Skip to content
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

Refactor Prepare Licence For Allocation Service #656

Merged
merged 9 commits into from
Jan 15, 2024

Conversation

Beckyrose200
Copy link
Contributor

@Beckyrose200 Beckyrose200 commented Jan 11, 2024

https://eaflood.atlassian.net/browse/WATER-4319

This pull request refactors the 'PrepareLicenceForAllocationService' as part of our ongoing effort to enhance the two-part-tariff service. Initially we began by creating a 'hack' branch to build and iterate on the two-part-tariff functionality. In the process of working on these branches, we identified a cleaner and more performant approach to implement the overall service. We realised that the PrepareLicenceForAllocationService could be split into two smaller services, making it easier in the code to follow the two-part-tariff process and making it easier for testing.

https://eaflood.atlassian.net/browse/WATER-4319

As part of the work we have been doing, we've been hacking away at a branch in a mob to build the two-part-tariff service. We have started refactoring this hacky branch and pulling out the various services into their own PR's.  DEFRA/water-abstraction-team#105. Since then we have found a much cleaner, more performate way to right the overall service that we have now added into another separate hacky branch. This PR is updating the PrepareLicenceForAllocationService that has already been pulled out and refactoring it to match our new hacky branch.
@Beckyrose200 Beckyrose200 added the housekeeping Refactoring, tidying up or other work which supports the project label Jan 11, 2024
@Beckyrose200 Beckyrose200 self-assigned this Jan 11, 2024
@Beckyrose200 Beckyrose200 marked this pull request as ready for review January 12, 2024 09:36
Copy link
Member

@Cruikshanks Cruikshanks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I pushed some corrections. If you are ok with them I'll approve and we can get this merged.

@Beckyrose200
Copy link
Contributor Author

Looks all good to me :)

Copy link
Member

@Cruikshanks Cruikshanks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only thing I would ask before merging is that the squashed commit & PR description are updated.

The 'hack' branches once 2PT is complete will be lost. So, we can reference things we're doing on them because there will be nothing to look up.

So, we need to explain in this commit/description that we've realised we can simplify the two-part tariff process and services involved by splitting the old PrepareLicence into 2 separate services.

@Beckyrose200 Beckyrose200 merged commit b5b13cb into main Jan 15, 2024
5 of 6 checks passed
@Beckyrose200 Beckyrose200 deleted the refactor-prepare-licence-for-allocation-service branch January 15, 2024 10:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
housekeeping Refactoring, tidying up or other work which supports the project
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants