Skip to content

Commit

Permalink
Fix including unnecessary supp. billing trans.
Browse files Browse the repository at this point in the history
https://eaflood.atlassian.net/browse/WATER-3989

Recent testing found an issue following the change we made in [Inc. changes to agreements & charges in supp bill](#207). Although the final result was correct, we were always create invoices with transaction lines for scenarios we should have been returning an `EMPTY` bill run.

The reason was our changed logic to compare calculated transaction lines to previous ones. It was always returning 'false' i.e. the lines were always different when they should be matching and cancelling each other out.

The problem was an unexpected way the previous team was handling Section 130 agreements. Whether a charge version has one or not is held in the linked `charge_elements` table in the adjustments field, for example

```json
{"s126": null, "s127": true, "s130": false, "charge": null, "winter": false, "aggregate": null}
```

We generate our calculated transaction lines using this source data so hold `section130Agreement` as a boolean. What we found when we looked at the `billing_transactions` table is that this is a varchar not a boolean. It looks like the initial build of PRESROC billing stored values other than 'true' or 'false'. But that then changed to 'true' and 'false' and these are the only values used for SROC.

This means we weren't aware `FetchPreviousBillingTransactionsService` was return previous transactions with `section130Agreement` held as a string. So, when we came to compare it was causing it to always determine the transactions were different.

So, this change fixes the issue by using [Knex raw](https://knexjs.org/guide/raw.html) to cast the value in our query to a boolean to match what we get and use in `FetchChargeVersionsService`. Doing at there will be the most performant and means it matches our expectations of what type `section130Agreement` will be.
  • Loading branch information
Cruikshanks committed May 4, 2023
1 parent 8a92792 commit c9b165a
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,12 @@ async function _fetch (licenceId, invoiceAccountId, financialYearEnding) {
'bt.volume',
'bt.section126Factor',
'bt.section127Agreement',
'bt.section130Agreement',
// NOTE: This field is a varchar in the DB for historic reasons. It seems some early PRESROC transactions
// recorded values other than 'true' or 'false'. For SROC though, it will only ever be true/false. We generate
// our calculated billing transaction lines based on the Section130 flag against charge_elements which is always
// a boolean. So, to avoid issues when we need to compare the values we cast this to a boolean when fetching the
// data.
db.raw('bt.section_130_agreement::boolean'),
'bt.isTwoPartSecondPartCharge',
'bt.scheme',
'bt.aggregateFactor',
Expand Down
1 change: 1 addition & 0 deletions test/support/helpers/water/billing-transaction.helper.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ function defaults (data = {}) {
isCredit: false,
loss: 'medium',
season: 'all year',
section130Agreement: 'false',
scheme: 'sroc',
source: 'non-tidal',
volume: 11
Expand Down

0 comments on commit c9b165a

Please sign in to comment.