Refactor, unit tests & add NewBillingBatchService #236
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
I just can't get this to work properly at the moment.
We really want to test what the ProcessBillingBatchService service _was_ doing in it's `_process()` method. But the way we have designed this is to kick off the processing whilst still returning a response to the client. This made it extremely painful to test. Then inspiration hit; if we move what `_process()` is doing to its own service we can then test that as normal! Key thing is that new service really needed to keep the name `ProcessBillingBatchService`! So, we introduce a new service that will be called from the controller, will do all the work of determining the billing periods and creating the initial billing batch record. It will also kick of our ProcessBillingBatchService whilst sending a response back to the controller.
Have it call our new service.
Cruikshanks
added
the
housekeeping
Also make sure we await the HandleErroredBillingBatchService.
StuAA78
requested changes
May 22, 2023
app/services/supplementary-billing/new-billing-batch.service.js
Outdated
Show resolved
Hide resolved
app/services/supplementary-billing/process-billing-batch.service.js
Outdated
Show resolved
Hide resolved
Co-authored-by: Stuart Adair <43574728+StuAA78@users.noreply.github.com>
Co-authored-by: Stuart Adair <43574728+StuAA78@users.noreply.github.com>
Co-authored-by: Stuart Adair <43574728+StuAA78@users.noreply.github.com>
In review it was agreed it is more expected that a service returns true if something is done, and false if it isn't.
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
This is a follow on from Refactor start of processing a supp. billing batch. In there we needed to refactor the
ProcessBillingBatchServiceto not mix processing a given billing period and the overall billing batch. So, we createdProcessBillingPeriodServiceand split the functionality between the 2.But we parked writing any unit tests for our new
ProcessBillingBatchServicetill this PR, else the change would have been massive. Only we hit an issue when we started writing them.ProcessBillingBatchServicein 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
ProcessBillingBatchServicein 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!