Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Multi-Year SROC Supplementary Billing #228

Merged
merged 27 commits into from
Jun 6, 2023
Merged

Conversation

Jozzey
Copy link
Contributor

@Jozzey Jozzey commented May 17, 2023

https://eaflood.atlassian.net/browse/WATER-3984

Supplementary billing for SRoC currently only handles Financial Year 2022 - 2023, going forward it needs to pick up any changes to charge versions in any year from 2022. Once we have passed Annual Billing 2027 then the engine should only pick up changes to charge versions in the current year and the previous 5.

All transactions in the available billing period should be picked up in 1 bill run, and be triggered by a single action.

This PR adds the remainder of the functionality required for Multi-Year SROC Supplementary Billing not covered in the previous PR #226

https://eaflood.atlassian.net/browse/WATER-3984

Supplementary billing for SRoC currently only handles Financial Year 2022 - 2023, going forward it needs to pick up any changes to charge versions in any year from 2022. Once we have passed Annual Billing 2027 then the engine should only pick up changes to charge versions in the current year and previous 5.

All transactions in the available billing period should be picked up in 1 bill run, and be triggered by a single action.

This PR adds the remainder of the functionality required for Multi Year Billing not covered in the previous PR #226
@Jozzey Jozzey added the enhancement New feature or request label May 17, 2023
@Jozzey Jozzey self-assigned this May 17, 2023
@Jozzey Jozzey marked this pull request as ready for review May 25, 2023 14:19
@Jozzey Jozzey requested a review from StuAA78 May 25, 2023 15:09
Copy link
Contributor

@StuAA78 StuAA78 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One tiny jsdoc change I just spotted but otherwise all looking good! 😁

@Jozzey Jozzey requested a review from StuAA78 May 26, 2023 08:19
@Jozzey Jozzey added the do not merge Used for spikes and experiments label May 26, 2023
@Jozzey
Copy link
Contributor Author

Jozzey commented May 26, 2023

As this PR is going to significantly impact the supplementary billing testing that is currently being carried out. It shouldn't be merged until we get the OK from the business/QA.

Copy link
Member

@Cruikshanks Cruikshanks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The specific change is just about the order of const and let.

But I would also push for updating the first test ('when there are charge versions that should be considered for the next supplementary billing') in test/services/supplementary-billing/fetch-charge-versions.service.test.js. We should update the billing period to 2023-04-01 - 2024-03-31. The existing test SROC charge version should still be picked up. But we can also include one that starts in the billing period just for completeness.

The tests will then need to be updated to confirm both SROC charge versions are returned.

@Jozzey Jozzey requested a review from Cruikshanks May 26, 2023 13:41
@Cruikshanks Cruikshanks removed the do not merge Used for spikes and experiments label Jun 6, 2023
@Cruikshanks Cruikshanks merged commit 9400ec4 into main Jun 6, 2023
@Cruikshanks Cruikshanks deleted the multi-year-supp-billing branch June 6, 2023 10:11
Cruikshanks added a commit that referenced this pull request Jun 23, 2023
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.
Cruikshanks added a commit that referenced this pull request Jun 26, 2023
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 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()` 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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants