Fix supp. billing not crediting old accounts #280
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-4044 Assume the following scenario. A charge version is added to a licence starting on 1 April 2022 against invoice account **INVAC001**. The licence is flagged for supplementary billing which is run during the 2023-2024 billing period. This will will generate 2 bills for **INVAC001**; one in 2024 and one in 2023 ([multi-year billing!](#228)) Then another charge version is added starting on 1 June 2022 against invoice account **INVAC002**. Again the licence is flagged for supplementary billing which is run during the 2023-2024 billing period. What should happen is - 2024 - **INVAC001** receives a whole year credit - **INVAC002** receives a whole year debit - 2023 - **INVAC001** receives a part year credit and debit - **INVAC002** receives a part year debit So, 4 bills in total which result in a £0 bill run. What is happening is no credit is generated for **INVAC001** in 2024. The reason is `FetchChargeVersionsService` was written to exclude charge versions which start and finish outside the billing period being processed. But that exclusion means we never check them for previous transactions, which in this scenario would result in **INVAC001's** debit being found so a credit generated. This change resolves the issue by removing the clause in our query which excludes charge versions that end before the billing period start date. To support this we also need to amend the `_periodIsInvalid()` in `DetermineChargePeriodService`. > Note - removal of this clause means we might pick up charge versions that don't need to be processed at all. We did manage to implement an Objection.js query that used a [UNION](https://www.w3schools.com/sql/sql_union.asp) to prevent this. But it made `FetchChargeVersionsService` vastly more complex purely for a negligible performance bump.
We thought we were helping things by making the references in the query explicit. But we then started getting 'missing FROM clause' errors. It is due to how Objection.js is generating the SQL which we could fix by adding yet more code. Better to just put things back how they were and leave alone!
We remove the test that checks charge versions with end dates before the period start date are not returned. We enhance the existing tests that check the results to cover our new scenario.
Cruikshanks
added a commit
that referenced
this pull request
Jun 27, 2023
https://eaflood.atlassian.net/browse/WATER-4044 In [Fix supp. billing not crediting old accounts](#280) as sought to resolve a problem in multi-year billing that saw debits raised in a period not being credited. Imagine a charge version that starts 2022-04-01 with no end date. The first bill run would generate a debit for both 2023-24 and 2022-23. Should we add a new charge version that starts 2022-06-01 the service will automatically add an end date to the first one (2022-05-31). Our engine on the second run, when fetching the charge versions for 2023-24 would now skip the first charge version because it's outside the billing period. But by skipping it we overlook checking for any previous transactions to credit, which we need to do in this case. We fixed it by simply removing the end date clause in `FetchChargeVersionsService`. Now to be included a charge version just needs to start before the billing period end date (2024-03-31). We got lucky when we ran the fix against the scenario that identified this problem. A lot of complexity comes from working out just when a charge version is 'chargeable' and whether their abstraction period overlaps this. In the original scenario, all things aligned to calculate zero billable days. Further testing by our QA found a scenario where the bill run always errored. Essentially, we were calculating a negative billable days and trying to send it to the charging module API, which was rejecting it. As part of the original fix, we'd removed a check in our `DetermineChargePeriodService` that said the charge version's end date couldn't be before the billing period's start date. We had to as we were now fetching charge versions where this was the case. In the new scenario there was a charge version for 2022-09-07 to 2022-11-30 which when processed in 2023-204 was determining a charge period of - **start date** 2023-04-01 - **end date** 2022-11-30 End date before the start date was causing a problem further down the process hence us trying to send a negative billable days value to the charging module. This change puts back the check we removed in `DetermineChargePeriodService`, and now use it to tell us if the charge version and billing period are incompatible. And if they are, instead of throwing an error we return a result that says, “Don't bother calculating this one”. We skip unnecessary processing and avoid trying to send an invalid transaction to the Charging Module API.
Cruikshanks
added a commit
that referenced
this pull request
Jun 28, 2023
https://eaflood.atlassian.net/browse/WATER-4044 In [Fix supp. billing not crediting old accounts](#280) as sought to resolve a problem in multi-year billing that saw debits raised in a period not being credited. Imagine a charge version that starts 2022-04-01 with no end date. The first bill run would generate a debit for both 2023-24 and 2022-23. Should we add a new charge version that starts 2022-06-01 the service will automatically add an end date to the first one (2022-05-31). Our engine on the second run, when fetching the charge versions for 2023-24 would now skip the first charge version because it's outside the billing period. But by skipping it we overlook checking for any previous transactions to credit, which we need to do in this case. We fixed it by simply removing the end date clause in `FetchChargeVersionsService`. Now to be included a charge version just needs to start before the billing period end date (2024-03-31). We got lucky when we ran the fix against the scenario that identified this problem. A lot of complexity comes from working out just when a charge version is 'chargeable' and whether their abstraction period overlaps this. In the original scenario, all things aligned to calculate zero billable days. Further testing by our QA found a scenario where the bill run always errored. Essentially, we were calculating negative billable days and trying to send it to the charging module API, which was rejecting it. As part of the original fix, we'd removed a check in our `DetermineChargePeriodService` that said the charge version's end date couldn't be before the billing period's start date. We had to as we were now fetching charge versions where this was the case. In the new scenario, there was a charge version for 2022-09-07 to 2022-11-30 which when processed in 2023-204 was determining a charge period of - **start date** 2023-04-01 - **end date** 2022-11-30 End date before the start date was causing a problem further down the process hence us trying to send a negative billable days value to the charging module. This change puts back the check we removed in `DetermineChargePeriodService`, and now use it to tell us if the charge version and billing period are incompatible. And if they are, instead of throwing an error we return a result that says, “Don't bother calculating this one”. We skip unnecessary processing and avoid trying to send an invalid transaction to the Charging Module API.
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-4044
Assume the following scenario. A charge version is added to a licence starting on 1 April 2022 against invoice account INVAC001. The licence is flagged for supplementary billing which is run during the 2023-2024 billing period.
This will will generate 2 bills for INVAC001; one in 2024 and one in 2023 (multi-year billing!)
Then another charge version is added starting on 1 June 2022 against invoice account INVAC002. Again the licence is flagged for supplementary billing which is run during the 2023-2024 billing period.
What should happen is
So, 4 bills in total which result in a £0 bill run.
What is happening is no credit is generated for INVAC001 in 2024. The reason is
FetchChargeVersionsServicewas written to exclude charge versions which start and finish outside the billing period being processed. But that exclusion means we never check them for previous transactions, which in this scenario would result in INVAC001's debit being found so a credit is generated.This change resolves the issue by removing the clause in our query which excludes charge versions that end before the billing period start date. To support this we also need to amend the
_periodIsInvalid()inDetermineChargePeriodService.