Refactor start of processing a supp. billing batch #233
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-3977 https://eaflood.atlassian.net/browse/WATER-3984 Both to help resolve some issues found when cleaning up our existing process billing batch service and to support changes need for handling multi-year bill runs we need to make some changes. The key one is `ProcessBillingBatchService` is no longer appropriate because it is actually focused on process a single billing period. So, we renamed it to `ProcessBillingPeriodService` and have created a new `ProcessBillingBatchService` responsible for orchestrating the _whole_ process. As the primary service now, we've updated the controller to call it instead of `InitiateBillingBatchService`. This resulted in use needing to make further changes in it and the services it uses. Along the way we spotted opportunities to clean up some other things; passing in params only to find they are hard coded elsewhere, or having params named differently to how they are used within a service.
Cruikshanks
added
the
housekeeping
Jozzey
requested changes
May 18, 2023
app/services/supplementary-billing/create-billing-batch.service.js
Outdated
Show resolved
Hide resolved
StuAA78
approved these changes
May 18, 2023
Cruikshanks
added a commit
that referenced
this pull request
May 22, 2023
https://eaflood.atlassian.net/browse/WATER-3977 https://eaflood.atlassian.net/browse/WATER-3984 This is a follow on from [Refactor start of processing a supp. billing batch](#233). In there we needed to refactor the `ProcessBillingBatchService` to not mix processing a given billing period and the overall billing batch. So, we created `ProcessBillingPeriodService` and split the functionality between the 2. But we parked writing any unit tests for our new `ProcessBillingBatchService` till this PR, else the change would have been massive. Only we hit an issue when we started writing them. `ProcessBillingBatchService` in its new form was determining the billing periods, initiating the billing batch record, and generating a response back to the client all whilst kicking off the process for each billing period and handling any errors thrown. It was also responsible for finalising the bill run. A bit much! The final issue was that background processing was not `awaited`. This made it a nightmare to try and test all this behaviour. We realised we needed to get the 'processing' bit in its own service if only to make it testable. So, this change adds a new `NewBillingBatchService` (yes - we really need to sit down and firm up our naming conventions!) _This_ is now what is called when kicking off a new billing batch. It deals with determining the billing periods, initiating the billing batch, and generating the response for the client. Essentially, it manages everything to do with a new bill run. It also kicks off `ProcessBillingBatchService` in the background, which is now solely focused on processing a billing batch; generating the data, handling errors, updating the billing batch status, and finalising it. With this in place, it meant we could also write some unit tests!
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-3977
https://eaflood.atlassian.net/browse/WATER-3984
Both to help resolve some issues found when cleaning up our existing process billing batch service and to support changes need for handling multi-year bill runs we need to make some changes.
The key one is
ProcessBillingBatchServiceis no longer appropriate because it is actually focused on processing a single billing period. So, we renamed it toProcessBillingPeriodServiceand have created a newProcessBillingBatchServiceresponsible for orchestrating the whole process.As the primary service now, we've updated the controller to call it instead of
InitiateBillingBatchService. This resulted in us needing to make further changes in it and the services it uses.Along the way we spotted opportunities to clean up some other things; passing in params only to find they are hard coded elsewhere, or having params named differently to how they are used within a service.