diff --git a/app/services/supplementary-billing/process-billing-transactions.service.js b/app/services/supplementary-billing/process-billing-transactions.service.js index ef0db3ad20..8ed6256304 100644 --- a/app/services/supplementary-billing/process-billing-transactions.service.js +++ b/app/services/supplementary-billing/process-billing-transactions.service.js @@ -39,21 +39,55 @@ async function go (calculatedTransactions, billingInvoice, billingInvoiceLicence } /** + * Compares a calculated transaction to the transactions from previous bill runs + * * Takes a single calculated debit transaction and checks to see if the provided array of reversed (credit) transactions - * contains a transaction that will cancel it out, returning `true` or `false` to indicate if it does or doesn't. Since - * the calculated debit transactions have not yet been sent to the Charging Module, we look at `chargeType`, - * `chargeCategoryCode` and `billableDays` as any given combination of these will always result in the same value coming - * back from the Charging Module. + * contains a transaction that will cancel it out, returning `true` or `false` to indicate if it does or doesn't. + * + * We compare those properties which determine the charge value calculated by the charging module. If the calculated + * transaction's properties matches one in reversedTransactions we return true. This will tell the calling method + * to not include the calculated transaction in the bill run. We also remove the matched transaction from + * reversedTransactions. + * + * The key properties are charge type, category code, and billable days. But we also need to compare agreements and + * additional charges because if those have changed, we'll need to credit the previous transaction and calculate the + * new debit value. Because what we are checking does not match up to what you see in the UI we have this reference + * + * - Abatement agreement - section126Factor + * - Two-part tariff agreement - section127Agreement + * - Canal and River Trust agreement - section130Agreement + * - Aggregate - aggregateFactor + * - Charge Adjustment - adjustmentFactor + * - Winter discount - isWinterOnly + * + * - Additional charges - isSupportedSource + * - Additional charges - supportedSourceName + * - Additional charges - isWaterCompanyCharge * * NOTE: This function will mutate the provided array of reversed transactions if one of the transactions in it will * cancel the calculated transaction; in this case, we remove the reversed transaction from the array as it can only * cancel one calculated transaction. */ function _cancelCalculatedTransaction (calculatedTransaction, reversedTransactions) { + // When we put together this matching logic our instincts were to try and do something 'better' than this long, + // chained && statement. But whatever we came up with was + // - more complex + // - less performant + // We found this easy to see what properties are being compared. Plus the moment something doesn't match we bail. So, + // much as it feels 'wrong', we are sticking with it! const result = reversedTransactions.findIndex((reversedTransaction) => { return reversedTransaction.chargeType === calculatedTransaction.chargeType && reversedTransaction.chargeCategoryCode === calculatedTransaction.chargeCategoryCode && - reversedTransaction.billableDays === calculatedTransaction.billableDays + reversedTransaction.billableDays === calculatedTransaction.billableDays && + reversedTransaction.section126Factor === calculatedTransaction.section126Factor && + reversedTransaction.section127Agreement === calculatedTransaction.section127Agreement && + reversedTransaction.section130Agreement === calculatedTransaction.section130Agreement && + reversedTransaction.aggregateFactor === calculatedTransaction.aggregateFactor && + reversedTransaction.adjustmentFactor === calculatedTransaction.adjustmentFactor && + reversedTransaction.isWinterOnly === calculatedTransaction.isWinterOnly && + reversedTransaction.isSupportedSource === calculatedTransaction.isSupportedSource && + reversedTransaction.supportedSourceName === calculatedTransaction.supportedSourceName && + reversedTransaction.isWaterCompanyCharge === calculatedTransaction.isWaterCompanyCharge }) if (result === -1) { diff --git a/test/services/supplementary-billing/process-billing-transactions.service.test.js b/test/services/supplementary-billing/process-billing-transactions.service.test.js index 8dce07ba24..b929c67802 100644 --- a/test/services/supplementary-billing/process-billing-transactions.service.test.js +++ b/test/services/supplementary-billing/process-billing-transactions.service.test.js @@ -23,99 +23,36 @@ describe('Process billing batch service', () => { endDate: new Date('2023-03-31') } - const calculatedTransactions = [ - { - billingTransactionId: '61abdc15-7859-4783-9622-6cb8de7f2461', - billingInvoiceLicenceId: billingInvoiceLicence.billingInvoiceLicenceId, - isCredit: false, - status: 'candidate', - chargeType: 'standard', - chargeCategoryCode: '4.10.1', - billableDays: 365, - purposes: 'CALCULATED_TRANSACTION_1' - }, - { - billingTransactionId: 'a903cdd3-1804-4237-aeb9-70ef9008469d', - billingInvoiceLicenceId: billingInvoiceLicence.billingInvoiceLicenceId, - isCredit: false, - status: 'candidate', - chargeType: 'standard', - chargeCategoryCode: '5.11.2', - billableDays: 265, - purposes: 'CALCULATED_TRANSACTION_2' - }, - { - billingTransactionId: '34453414-0ecb-49ce-8442-619d22c882f0', - billingInvoiceLicenceId: billingInvoiceLicence.billingInvoiceLicenceId, - isCredit: false, - status: 'candidate', - chargeType: 'standard', - chargeCategoryCode: '6.12.3', - billableDays: 100, - purposes: 'CALCULATED_TRANSACTION_3' - } - ] - - const allPreviousTransactions = [ - { - billingTransactionId: '8d68eb26-d054-47a7-aee8-cd93a24fa860', - billingInvoiceLicenceId: '110ab2e2-6076-4d5a-a56f-b17a048eb269', - isCredit: false, - status: 'candidate', - chargeType: 'standard', - chargeCategoryCode: '4.10.1', - billableDays: 365, - purposes: ['I_WILL_BE_REMOVED_1'] - }, - { - billingTransactionId: '63742bee-cb1b-44f1-86f6-c7d546f59c88', - billingInvoiceLicenceId: '110ab2e2-6076-4d5a-a56f-b17a048eb269', - isCredit: false, - status: 'candidate', - chargeType: 'standard', - chargeCategoryCode: '5.11.2', - billableDays: 265, - purposes: ['I_WILL_BE_REMOVED_2'] - }, - { - billingTransactionId: '4f254a70-d36e-46eb-9e4b-59503c3d5994', - billingInvoiceLicenceId: '110ab2e2-6076-4d5a-a56f-b17a048eb269', - isCredit: false, - status: 'candidate', - chargeType: 'standard', - chargeCategoryCode: '6.12.3', - billableDays: 100, - purposes: ['I_WILL_BE_REMOVED_3'] - }, - { - billingTransactionId: '733bdb55-5386-4fdb-a581-1e98f9a35bed', - billingInvoiceLicenceId: '110ab2e2-6076-4d5a-a56f-b17a048eb269', - isCredit: false, - status: 'candidate', - chargeType: 'standard', - chargeCategoryCode: '9.9.9', - billableDays: 180, - purposes: ['I_WILL_NOT_BE_REMOVED'] - } - ] - afterEach(() => { Sinon.restore() }) describe('when the billing invoice, licence and period', () => { + let calculatedTransactions + + beforeEach(() => { + calculatedTransactions = [ + _generateCalculatedTransaction('4.10.1', 365, 'CALCULATED_TRANSACTION_1'), + _generateCalculatedTransaction('5.11.2', 265, 'CALCULATED_TRANSACTION_2'), + _generateCalculatedTransaction('6.12.3', 100, 'CALCULATED_TRANSACTION_3') + ] + }) + describe('match to transactions on a previous billing batch', () => { describe('and the calculated transactions provided', () => { let previousTransactions - describe('completely cancel out the previous transactions from the last billing batch', () => { + describe('match all the previous transactions from the last billing batch', () => { beforeEach(() => { - previousTransactions = [allPreviousTransactions[0], allPreviousTransactions[1]] + previousTransactions = [ + _generatePreviousTransaction('4.10.1', 365, 'I_WILL_BE_REMOVED_1'), + _generatePreviousTransaction('5.11.2', 265, 'I_WILL_BE_REMOVED_2') + ] Sinon.stub(FetchPreviousBillingTransactionsService, 'go').resolves(previousTransactions) }) - it('returns the uncanceled calculated transactions', async () => { + it('returns the matched calculated transactions', async () => { const result = await ProcessBillingTransactionsService.go( calculatedTransactions, billingInvoice, @@ -128,14 +65,18 @@ describe('Process billing batch service', () => { }) }) - describe('partially cancel out the previous transactions from the last billing batch', () => { + describe('are cancelled out by the previous transactions from the last billing batch', () => { beforeEach(() => { - previousTransactions = [allPreviousTransactions[0], allPreviousTransactions[1], allPreviousTransactions[3]] + previousTransactions = [ + _generatePreviousTransaction('4.10.1', 365, 'I_WILL_BE_REMOVED_1'), + _generatePreviousTransaction('5.11.2', 265, 'I_WILL_BE_REMOVED_2'), + _generatePreviousTransaction('6.12.3', 100, 'I_WILL_BE_REMOVED_3') + ] Sinon.stub(FetchPreviousBillingTransactionsService, 'go').resolves(previousTransactions) }) - it('returns the uncanceled calculated and reversed transactions', async () => { + it('returns no transactions', async () => { const result = await ProcessBillingTransactionsService.go( calculatedTransactions, billingInvoice, @@ -143,51 +84,57 @@ describe('Process billing batch service', () => { billingPeriod ) - expect(result).to.have.length(2) - expect(result[0].purposes).to.equal('CALCULATED_TRANSACTION_3') - expect(result[1].purposes).to.equal('I_WILL_NOT_BE_REMOVED') + expect(result).to.be.empty() }) }) - describe('are cancelled out by the previous transactions from the last billing batch', () => { + describe('are empty', () => { beforeEach(() => { - previousTransactions = [allPreviousTransactions[0], allPreviousTransactions[1], allPreviousTransactions[2]] + previousTransactions = [ + _generatePreviousTransaction('4.10.1', 365, 'I_WILL_NOT_BE_REMOVED_1'), + _generatePreviousTransaction('5.11.2', 265, 'I_WILL_NOT_BE_REMOVED_2') + ] Sinon.stub(FetchPreviousBillingTransactionsService, 'go').resolves(previousTransactions) }) - it('returns no transactions', async () => { + it('returns only the previous transactions', async () => { const result = await ProcessBillingTransactionsService.go( - calculatedTransactions, + [], billingInvoice, billingInvoiceLicence, billingPeriod ) - expect(result).to.be.empty() + expect(result).to.have.length(2) + expect(result[0].purposes).to.equal('I_WILL_NOT_BE_REMOVED_1') + expect(result[1].purposes).to.equal('I_WILL_NOT_BE_REMOVED_2') }) }) - describe('are empty', () => { + describe('partially match the previous transactions from the last billing batch', () => { beforeEach(() => { - previousTransactions = [allPreviousTransactions[0], allPreviousTransactions[1]] + previousTransactions = [ + _generatePreviousTransaction('4.10.1', 365, 'I_WILL_BE_REMOVED_1'), + _generatePreviousTransaction('5.11.2', 265, 'I_WILL_BE_REMOVED_2'), + _generatePreviousTransaction('9.9.9', 180, 'I_WILL_NOT_BE_REMOVED') + ] Sinon.stub(FetchPreviousBillingTransactionsService, 'go').resolves(previousTransactions) }) - it('returns only the previous transactions', async () => { + it('returns the unmatched calculated transactions and previous transactions (reversed)', async () => { const result = await ProcessBillingTransactionsService.go( - [], + calculatedTransactions, billingInvoice, billingInvoiceLicence, billingPeriod ) expect(result).to.have.length(2) - - // NOTE: We know the text says 'I_WILL_BE_REMOVED' but in this scenario they won't be! - expect(result[0].purposes).to.equal('I_WILL_BE_REMOVED_1') - expect(result[1].purposes).to.equal('I_WILL_BE_REMOVED_2') + expect(result[0].purposes).to.equal('CALCULATED_TRANSACTION_3') + expect(result[1].purposes).to.equal('I_WILL_NOT_BE_REMOVED') + expect(result[1].isCredit).to.be.true() }) }) }) @@ -213,4 +160,359 @@ describe('Process billing batch service', () => { }) }) }) + + describe('the service matches calculated to previous transactions', () => { + let calculatedTransactions + + beforeEach(() => { + calculatedTransactions = [_generateCalculatedTransaction('4.10.1', 365, 'CALCULATED_TRANSACTION')] + }) + + describe('when the charge type differs', () => { + beforeEach(() => { + const previousTransactions = [ + _generatePreviousTransaction( + '4.10.1', 365, 'PREVIOUS_TRANSACTION', { chargeType: 'compensation' } + ) + ] + + Sinon.stub(FetchPreviousBillingTransactionsService, 'go').resolves(previousTransactions) + }) + + it('does not match the transactions', async () => { + const result = await ProcessBillingTransactionsService.go( + calculatedTransactions, + billingInvoice, + billingInvoiceLicence, + billingPeriod + ) + + expect(result).to.have.length(2) + expect(result[0].purposes).to.equal('CALCULATED_TRANSACTION') + expect(result[1].purposes).to.equal('PREVIOUS_TRANSACTION') + }) + }) + + describe('when the charge category code differs', () => { + beforeEach(() => { + const previousTransactions = [_generatePreviousTransaction('5.10.1', 365, 'PREVIOUS_TRANSACTION')] + + Sinon.stub(FetchPreviousBillingTransactionsService, 'go').resolves(previousTransactions) + }) + + it('does not match the transactions', async () => { + const result = await ProcessBillingTransactionsService.go( + calculatedTransactions, + billingInvoice, + billingInvoiceLicence, + billingPeriod + ) + + expect(result).to.have.length(2) + expect(result[0].purposes).to.equal('CALCULATED_TRANSACTION') + expect(result[1].purposes).to.equal('PREVIOUS_TRANSACTION') + }) + }) + + describe('when the billable days differ', () => { + beforeEach(() => { + const previousTransactions = [_generatePreviousTransaction('4.10.1', 5, 'PREVIOUS_TRANSACTION')] + + Sinon.stub(FetchPreviousBillingTransactionsService, 'go').resolves(previousTransactions) + }) + + it('does not match the transactions', async () => { + const result = await ProcessBillingTransactionsService.go( + calculatedTransactions, + billingInvoice, + billingInvoiceLicence, + billingPeriod + ) + + expect(result).to.have.length(2) + expect(result[0].purposes).to.equal('CALCULATED_TRANSACTION') + expect(result[1].purposes).to.equal('PREVIOUS_TRANSACTION') + }) + }) + + describe('when the abatement agreement (section 126) differs', () => { + beforeEach(() => { + const previousTransactions = [ + _generatePreviousTransaction('4.10.1', 365, 'PREVIOUS_TRANSACTION', { section126Factor: 0.5 }) + ] + + Sinon.stub(FetchPreviousBillingTransactionsService, 'go').resolves(previousTransactions) + }) + + it('does not match the transactions', async () => { + const result = await ProcessBillingTransactionsService.go( + calculatedTransactions, + billingInvoice, + billingInvoiceLicence, + billingPeriod + ) + + expect(result).to.have.length(2) + expect(result[0].purposes).to.equal('CALCULATED_TRANSACTION') + expect(result[1].purposes).to.equal('PREVIOUS_TRANSACTION') + }) + }) + + describe('when the two-part tariff agreement (section 127) differs', () => { + beforeEach(() => { + const previousTransactions = [ + _generatePreviousTransaction('4.10.1', 365, 'PREVIOUS_TRANSACTION', { section127Agreement: true }) + ] + + Sinon.stub(FetchPreviousBillingTransactionsService, 'go').resolves(previousTransactions) + }) + + it('does not match the transactions', async () => { + const result = await ProcessBillingTransactionsService.go( + calculatedTransactions, + billingInvoice, + billingInvoiceLicence, + billingPeriod + ) + + expect(result).to.have.length(2) + expect(result[0].purposes).to.equal('CALCULATED_TRANSACTION') + expect(result[1].purposes).to.equal('PREVIOUS_TRANSACTION') + }) + }) + + describe('when the canal and river trust agreement (section 130) differs', () => { + beforeEach(() => { + const previousTransactions = [ + _generatePreviousTransaction('4.10.1', 365, 'PREVIOUS_TRANSACTION', { section130Agreement: true }) + ] + + Sinon.stub(FetchPreviousBillingTransactionsService, 'go').resolves(previousTransactions) + }) + + it('does not match the transactions', async () => { + const result = await ProcessBillingTransactionsService.go( + calculatedTransactions, + billingInvoice, + billingInvoiceLicence, + billingPeriod + ) + + expect(result).to.have.length(2) + expect(result[0].purposes).to.equal('CALCULATED_TRANSACTION') + expect(result[1].purposes).to.equal('PREVIOUS_TRANSACTION') + }) + }) + + describe('when the aggregate differs', () => { + beforeEach(() => { + const previousTransactions = [ + _generatePreviousTransaction('4.10.1', 365, 'PREVIOUS_TRANSACTION', { aggregateFactor: 0.5 }) + ] + + Sinon.stub(FetchPreviousBillingTransactionsService, 'go').resolves(previousTransactions) + }) + + it('does not match the transactions', async () => { + const result = await ProcessBillingTransactionsService.go( + calculatedTransactions, + billingInvoice, + billingInvoiceLicence, + billingPeriod + ) + + expect(result).to.have.length(2) + expect(result[0].purposes).to.equal('CALCULATED_TRANSACTION') + expect(result[1].purposes).to.equal('PREVIOUS_TRANSACTION') + }) + }) + + describe('when the charge adjustment differs', () => { + beforeEach(() => { + const previousTransactions = [ + _generatePreviousTransaction('4.10.1', 365, 'PREVIOUS_TRANSACTION', { adjustmentFactor: 0.5 }) + ] + + Sinon.stub(FetchPreviousBillingTransactionsService, 'go').resolves(previousTransactions) + }) + + it('does not match the transactions', async () => { + const result = await ProcessBillingTransactionsService.go( + calculatedTransactions, + billingInvoice, + billingInvoiceLicence, + billingPeriod + ) + + expect(result).to.have.length(2) + expect(result[0].purposes).to.equal('CALCULATED_TRANSACTION') + expect(result[1].purposes).to.equal('PREVIOUS_TRANSACTION') + }) + }) + + describe('when the winter discount differs', () => { + beforeEach(() => { + const previousTransactions = [ + _generatePreviousTransaction('4.10.1', 365, 'PREVIOUS_TRANSACTION', { isWinterOnly: true }) + ] + + Sinon.stub(FetchPreviousBillingTransactionsService, 'go').resolves(previousTransactions) + }) + + it('does not match the transactions', async () => { + const result = await ProcessBillingTransactionsService.go( + calculatedTransactions, + billingInvoice, + billingInvoiceLicence, + billingPeriod + ) + + expect(result).to.have.length(2) + expect(result[0].purposes).to.equal('CALCULATED_TRANSACTION') + expect(result[1].purposes).to.equal('PREVIOUS_TRANSACTION') + }) + }) + + describe('when if it is a supported source differs (additional charge)', () => { + beforeEach(() => { + const previousTransactions = [ + _generatePreviousTransaction('4.10.1', 365, 'PREVIOUS_TRANSACTION', { isSupportedSource: true }) + ] + + Sinon.stub(FetchPreviousBillingTransactionsService, 'go').resolves(previousTransactions) + }) + + it('does not match the transactions', async () => { + const result = await ProcessBillingTransactionsService.go( + calculatedTransactions, + billingInvoice, + billingInvoiceLicence, + billingPeriod + ) + + expect(result).to.have.length(2) + expect(result[0].purposes).to.equal('CALCULATED_TRANSACTION') + expect(result[1].purposes).to.equal('PREVIOUS_TRANSACTION') + }) + }) + + describe('when the supported source name differs (additional charge)', () => { + beforeEach(() => { + const previousTransactions = [ + _generatePreviousTransaction('4.10.1', 365, 'PREVIOUS_TRANSACTION', { supportedSourceName: 'source name' }) + ] + + Sinon.stub(FetchPreviousBillingTransactionsService, 'go').resolves(previousTransactions) + }) + + it('does not match the transactions', async () => { + const result = await ProcessBillingTransactionsService.go( + calculatedTransactions, + billingInvoice, + billingInvoiceLicence, + billingPeriod + ) + + expect(result).to.have.length(2) + expect(result[0].purposes).to.equal('CALCULATED_TRANSACTION') + expect(result[1].purposes).to.equal('PREVIOUS_TRANSACTION') + }) + }) + + describe('when the water company flag differs (additional charge)', () => { + beforeEach(() => { + const previousTransactions = [ + _generatePreviousTransaction('4.10.1', 365, 'PREVIOUS_TRANSACTION', { isWaterCompanyCharge: true }) + ] + + Sinon.stub(FetchPreviousBillingTransactionsService, 'go').resolves(previousTransactions) + }) + + it('does not match the transactions', async () => { + const result = await ProcessBillingTransactionsService.go( + calculatedTransactions, + billingInvoice, + billingInvoiceLicence, + billingPeriod + ) + + expect(result).to.have.length(2) + expect(result[0].purposes).to.equal('CALCULATED_TRANSACTION') + expect(result[1].purposes).to.equal('PREVIOUS_TRANSACTION') + }) + }) + + describe('when nothing differs', () => { + beforeEach(() => { + const previousTransactions = [_generatePreviousTransaction('4.10.1', 365, 'PREVIOUS_TRANSACTION')] + + Sinon.stub(FetchPreviousBillingTransactionsService, 'go').resolves(previousTransactions) + }) + + it('matches the transactions', async () => { + const result = await ProcessBillingTransactionsService.go( + calculatedTransactions, + billingInvoice, + billingInvoiceLicence, + billingPeriod + ) + + expect(result).to.be.empty() + }) + }) + }) }) + +function _generateCalculatedTransaction (chargeCategoryCode, billableDays, testReference, changes = {}) { + const defaultProperties = { + billingTransactionId: '61abdc15-7859-4783-9622-6cb8de7f2461', + billingInvoiceLicenceId: '110ab2e2-6076-4d5a-a56f-b17a048eb269', + isCredit: false, + status: 'candidate', + chargeType: 'standard', + chargeCategoryCode, + billableDays, + section126Factor: 1, + section127Agreement: false, + section130Agreement: false, + aggregateFactor: 1, + adjustmentFactor: 1, + isWinterOnly: false, + isSupportedSource: false, + supportedSourceName: null, + isWaterCompanyCharge: false, + purposes: testReference + } + + return { + ...defaultProperties, + ...changes + } +} + +function _generatePreviousTransaction (chargeCategoryCode, billableDays, testReference, changes = {}) { + const defaultProperties = { + billingTransactionId: '8d68eb26-d054-47a7-aee8-cd93a24fa860', + billingInvoiceLicenceId: 'a76b3ab3-d70d-4fb0-8d72-2e2cdd334729', + isCredit: false, + status: 'candidate', + chargeType: 'standard', + chargeCategoryCode, + billableDays, + section126Factor: 1, + section127Agreement: false, + section130Agreement: false, + aggregateFactor: 1, + adjustmentFactor: 1, + isWinterOnly: false, + isSupportedSource: false, + supportedSourceName: null, + isWaterCompanyCharge: false, + purposes: [testReference] + } + + return { + ...defaultProperties, + ...changes + } +}