Refactor supplementary billing engine #834
Draft
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
Cruikshanks
added
the
housekeeping
Cruikshanks
force-pushed
the
supplementary-billing-refactor
branch
3 times, most recently
from
c0a8bbb to
88c7979
Compare
https://eaflood.atlassian.net/browse/WATER-4403 > Part of changes to exclude the current financial year from supplementary billing if the annual bill run has not been completed We have recently migrated the SROC annual billing engine into our project ( WATER-4345 ). The primary drive to do this was to enable them run annual bill runs in April at the same time as licensees are submitting their return data. This is when the service is under its greatest load so we wanted to ensure annual billing could take advantage of the improvements we made as part of the supplementary billing we implemented. Re-visiting supplementary billing as part of that migration meant we spotted further improvements we could make. For example, thanks to our models now being [view](https://www.postgresql.org/docs/current/sql-createview.html) based means tables being in different schemas is no longer a blocker to us creating relationships in the models. In our annual billing engine we are able to fetch everything at the billing account level removing the need for additional queries and the need to pre-generate billing objects. We were also able to implement a batch process, whereby multiple bills can be processed at the same time. We want to bring these improvements to our supplementary billing engine before we start making changes to support excluding the current financial year.
Cruikshanks
force-pushed
the
supplementary-billing-refactor
branch
from
88c7979 to
f1eab2a
Compare
The primary enabler in the annual billing improvements was moving to fetching the data at the billing account level. Currently, in supplementary we are fetching the charge versions and then pre-grouping them into bills (the pre-generate process). Part of that process was fetching the billing account details for each charge version group. If we start at the billing account level, we can remove that complexity _and_ more easily batch the processing of bills. So, the first step is to refactor `FetchBillingAccountsService` to get _everything_ needed just as we do in annual billing.
With FetchBillingAccountsService in place we can wave goodbye to FetchChargeVersions and PreGenerateBillingDataService. We're going to need to make changes to the services that depend on them, but those changes will remove the use of these services.
Now we've replaced FetchChargeVersionsService with FetchBillingAccountsService we need to update ProcessBillRunService to use it. As well as that we make some other improvements. - put functions into alphabetical order - removed unnecessary _logLevel() function - update comments - remove `is` prefix from boolean We also look ahead to the refactoring of `ProcessBillingPeriodService`. Currently, it is returning a flag to say whether any bills were generated as well as what licence ID's were processed. As it is iterating the charge versions it made sense at the time. Now, we're bringing ProcessBillingPeriodService inline with what we are doing in the annual engine. Plus, it's easier to follow if we figure this out within the ProcessBillRunService at the same time as we get fetch the billing accounts for the period.
Obviously, the key switch is we are iterating billing accounts. This also means we can bring in the same batching process for bills as we use in the annual billing engine. The PreGenerateBillingDataService is removed because we don't need to group charge versions by billing account and fetch the billing account details separately. Other than that we do general clean up such as ensuring the functions are alphabetical etc.
Cruikshanks
force-pushed
the
supplementary-billing-refactor
branch
from
f1eab2a to
736298b
Compare
When running the refactored engine through our acceptance tests we had one that repeatedly failed. Diving in we found that our refactoring had introduced a flaw. Because we used the updated process we'd adopted for the annual bill run engine we copied across where determining the charge period was happening. This was before we generate and process transactions for the charge version. It could be that a billing account was replaced by another prior to the billing period being processed. This would mean it's end date would be earlier than the billing period's start date resulting in an invalid charge period. For annual that means we can disregard it completely. For supplementary that just means we shouldn't bother to generate a transaction. It doesn't mean we shouldn't process the charge version and look for previous transactions which might result in a credit being raised. It means adding an involved and complex unit test. But it's a scenario we can't afford to get wrong.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
test/services/bill-runs/supplementary/process-billing-period.service.test.js
Dismissed
Show dismissed
Hide dismissed
There is no reason why the annual process can't determine the charge period in the same point in the process and thereby keeping the two engines consistent.
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-4403
We have recently migrated the SROC annual billing engine into our project ( WATER-4345 ). The primary drive to do this was to enable them to run annual bill runs in April at the same time as licensees are submitting their return data. This is when the service is under its greatest load so we wanted to ensure annual billing could take advantage of the improvements we made as part of the supplementary billing we implemented.
Re-visiting supplementary billing as part of that migration meant we spotted further improvements we could make. For example, thanks to our models now being view based means tables being in different schemas is no longer a blocker to us creating relationships in the models. In our annual billing engine, we can fetch everything at the billing account level removing the need for additional queries and the need to pre-generate billing objects.
We were also able to implement a batch process, whereby multiple bills can be processed at the same time. We want to bring these improvements to our supplementary billing engine before we start making changes to support excluding the current financial year.