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

Match return to charge element service #638

Merged
merged 18 commits into from
Jan 15, 2024

Conversation

Beckyrose200
Copy link
Contributor

@Beckyrose200 Beckyrose200 commented Jan 5, 2024

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

This pull request refactors the 'MatchReturnsToChargeElementService' 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 AllocateReturnsToLicenceService could be split into two smaller services (MatchReturnsToChargeElementService and the AllocateReturnsToLicenceService), 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. As a result of this, we have some refactoring to do to tidy up the work we have done in our mob. We are keeping track of the refactoring to do in this issue DEFRA/water-abstraction-team#105. This PR will extract the MatchReturnsToChargeElementService out of our hacky branch and into its own PR along with the unit tests.
@Beckyrose200 Beckyrose200 self-assigned this Jan 5, 2024
@Beckyrose200 Beckyrose200 added the enhancement New feature or request label Jan 5, 2024
@Beckyrose200 Beckyrose200 marked this pull request as ready for review January 5, 2024 14:34
@Beckyrose200 Beckyrose200 marked this pull request as draft January 5, 2024 15:22
@Beckyrose200 Beckyrose200 marked this pull request as ready for review January 5, 2024 15:27
@Beckyrose200 Beckyrose200 requested a review from Jozzey January 8, 2024 10:34
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 suggested changes. If you are ok with them I'll approve and we can get this merged.

@Beckyrose200
Copy link
Contributor Author

All happy with the changes

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.

Just a similar comment about expanding the squash commit/PR description
to explain we realised we could simplify things by matching and allocating in separate services rather than doing it in one.

@Beckyrose200 Beckyrose200 merged commit fbe69e1 into main Jan 15, 2024
6 checks passed
@Beckyrose200 Beckyrose200 deleted the move-return-log-matching-into-own-service branch January 15, 2024 10:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants