Fix licence ends before billing period #293
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 We recently implemented a [fix to stop calculating negative billable days for out of period charge versions](#286). Each billable period (financial year) we process we have to look at all charge versions that start before the period ends. This is to catch those which were previously debited in the period but recent changes now cause them to end before the billable period. We've since spotted another scenario that causes a charge period to be returned where the start date is after the end date. A charge version that starts before the billable period but is linked to a licence that ended before the billable period results in an invalid charge period - charge version start date 2022-04-01 - licence revoked date 2023-01-11 - billable period start date 2023-04-01 - billable period end date 2024-03-31 Our current `_periodIsIncompatible()` check would determine the charge version has a compatible period. So, the service would then grab the latest start date; billable period start date. Next it would grab the earliest end date; licence revoked date. The result is a charge period that results in negative billable days ```javascript return { startDate: new Date('2023-04-01'), endDate: new Date('2023-01-11') } ``` This change moves the `_periodIsIncompatible()` check to after we've determined the start and end dates. This should mean no matter the reason for determining a start date after the end date, the service will see it is incompatible and return the null result needed to stop `ProcessBillingPeriodService` trying to calculate billable days.
When we originally wrote this service we were only having to deal with the current year, which also happened to be the first year of SROC (2022-04-01 to 2023-03-31). We are now trying to add scenarios that cover charge versions that start before the 'current' billable period. We can use 2021 to 2022, but that seems wrong when we know it's not actually a valid SROC billing period. So, before we start adding our test cases we update the existing tests by bumping all the years forward by 1. We can then add our scenarios for stuff that starts and ends before the billing period of 2023-2024 and _still_ have everything be inside SROC.
It was a revoked date that was the root of our problem scenario but the same could happen for expired and lapsed licences. So, we've extended the existing tests to cover what happens if the licences ends before the billing period. They all currently fail. Next we just need to implement the fix.
Now matches comments how how we refer to the period in other services.
We realised the service was working out the billing period using the `financialYearEnding` value passed in. Where does it get this value? Why, from the `ProcessBillingPeriodService` passing in `billingPeriod.endDate.getFullYear()` 🤦 So, we take this chance to update the service to just expect the billing period to have been worked out and passed in.
After making the change to have `DetermineChargePeriodService` just accept a `billingPeriod` param we'd broken supplementary billing! A quick check found that `DetermineMinimumChargeService` was calling `DetermineChargePeriodService` to work out the charge period. It was still using the financial year end arg. So, initial though was we'll need to get the billing period to it as well. But then we realised that `DetermineMinimumChargeService` is only called by `ProcessBillingPeriodService` and this happens straight after it's called `DetermineChargePeriodService`. We already have the `chargePeriod`. There is no need to calculate it again.
Overlooked this when we moved from passing in the financial year end.
We found some duplication amongst the tests for DetermineChargePeriodService. So, we went back through them all and tried to reorganise so that what is covered is clearer and the duplication is removed.
We realised there is another possible scenario not currently catered for. What if the charge version being processed starts after the expired date of the licence, and both those dates are within the billing period? The service is going to use the charge version's start date as that is the latest but the licence expired date as that is the earliest. This results in another charge period where the start date is after the end date.
Now obvious we are looking at chargePeriod vs billingPeriod in `_periodIsIncompatible()`.
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
We recently implemented a fix to stop calculating negative billable days for out-of-period charge versions.
For each billable period (financial year) we have to look at all charge versions that start before the period ends. This is to catch those which were previously debited in the period but recent changes now cause them to end before it.
We've since spotted another scenario that causes a charge period to be returned where the start date is after the end date. A charge version that starts before the billable period but is linked to a licence that ended before the billable period results in an invalid charge period
Our current
_periodIsIncompatible()check would determine whether the charge version has a compatible period. So, the service would grab the latest start date; billable period start date. Next, it would grab the earliest end date; the licence revoked date.The result is a charge period that results in negative billable days
This change moves the
_periodIsIncompatible()check to after we've determined the start and end dates. This should mean no matter the reason for determining a start date after the end date, the service will see it is incompatible and return the null result needed to stopProcessBillingPeriodServicefrom trying to calculate billable days.As part of these changes, we spotted that
DetermineChargePeriodServicewas working out the billing period when it was readily available to pass inDetermineMinimumChargeServicewas usingDetermineChargePeriodServiceto work out the charge period when again, it was readily available to pass inDetermineChargePeriodServicehad become duplicatedThis cleanup was done as part of this change.