Add reissuing to ProcessBillingBatchService
#290
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-4036 We update `ProcessBillingBatchService` to call our new `ReissueInvoicesService`.
We don't yet have `ReissueInvoicesService` approved and merged. We therefore create a dummy service in the meantime to stand in for it.
We noticed that we don't have any docs in `ProcessBillingBatchService`. We add some brief placeholder docs to be fleshed out later.
We originally had a `isBatchPopulated` flag which we set `false` at the start of processing, then updated to `true` if the call to `ProcessBillingPeriodService` came back as `true` at any point. To avoid duplicating logic when we introduce reissuing, we refactor this to add the result to an array and then set `isBatchPopulated` based on whether at least one result in the array is truthy.
We add in a call to `ReissueInvoicesService` and add its return value to our results array
We refactor our logging function to accept different messages, and add logging for our reissue invoices process
We update our unit tests to stub `ReissueInvoicesService` and test that its return value is used to determine whether or not the billing batch is empty
We refactor `ProcessBillingBatch` to shorten the number of lines in `go`
In the interests of getting this PR merged asap we're going to leave the docs to be fleshed out at a later date. However we will at least get the right `@param` details in there
We noticed that `UnflagUnbilledLicencesService` is documented as returning a value, but it actually returns an Objection query object. We therefore refactor it so that it executes the query and then returns its result.
As we don't want to enable reissuing in all environments, we hide it behind a feature flag, set in `.env` as `ENABLE_REISSUING_BILLING_BATCHES`.
Some early feedback was that we don't care so much about the time taken to reissue invoices -- we're more concerned with how long the overall process takes. So we revert the time logging to how things were prior to this PR -- that is, we log the overall process time, and we do this within the `go` function to ensure it covers the entire process (ie. reissuing as well as processing the billing periods)
StuAA78
requested review from
Cruikshanks,
Jozzey and
Beckyrose200
and removed request for
Cruikshanks and
Jozzey
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-4036
We update
ProcessBillingBatchServiceto call our newReissueInvoicesService. This has involved a small refactor toProcessBillingBatchServiceso that each stage is called in its own function, leaving the maingofunction clean.We also hide the reissuing behind a feature flag, which is set by
ENABLE_REISSUING_BILLING_BATCHESin.env. This enables us to selectively enable reissuing in environments so that it isn't prematurely deployed.