Create error bill run when CM fails
#104
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-3833 When we currently initiate a new bill run, a fail response from the Charging Module means we throw an error and no bill run is created. We want to revise this so that it does create a new bill run and gives it `error` status.
We immediately spot that the JSDoc comment is outdated and should reflect the fact that we now create the bill run record in the CM
…status` We want to add a `status` param to `CreateBillingBatchService`, but it's starting to accmulate too many optional params. We therefore refactor it so that all of these are passed in an `options` object as the third param, and add `status` to the `options` object as we do this
We modify the existing 'CM fails' test to check for a bill run being created with `error` status
Since we're no longer throwing errors for failed CM requests, we can delete the "error doesn't include a message" test
Cruikshanks
reviewed
Jan 31, 2023
There was a problem hiding this comment.
Unfortunately, I have a further requirement!
When the legacy code errors when trying to create the CHA bill run it will also apply an errorCode to the billing batch.
The codes are
// water-abstraction-service - src/lib/models/batch.js
const BATCH_ERROR_CODE = {
failedToPopulateChargeVersions: 10,
failedToProcessChargeVersions: 20,
failedToPrepareTransactions: 30,
failedToCreateCharge: 40,
failedToCreateBillRun: 50,
failedToDeleteInvoice: 60,
failedToProcessTwoPartTariff: 70,
failedToGetChargeModuleBillRunSummary: 80,
failedToProcessRebilling: 90
}If you look at the table you will see a field for it. Could I ask you to also do the same (use code 50)? Happy for the change to be included in this PR. Else, if you'd prefer to do it separately ping me and I'll approve this so you can get on 😁
We have an additional requirement to record the error code for a bill run created with status `error`. For our unit tests we therefore need to create a migration that adds the `error_code` column to the `billing_batches` table
When the CM fails to create a bill run, we want to save our bill run with an error code of `50`. We therefore create a failing unit test to check for this
SonarCloud is yelling at us that a comment line is too long, and that we should cluster shorthand properties of an object either at the start or the end of the the object definition
Cruikshanks
approved these changes
Jan 31, 2023
Cruikshanks
added a commit
that referenced
this pull request
Mar 13, 2023
https://eaflood.atlassian.net/browse/WATER-3924 Should an error occur during the supplementary bill run process it brings down the service. We're not handling the exception and because we moved the process to the background in our bill runs controller, there is nothing to deal with it when thrown. So, we need to protect the service from _any_ errors during the process, to prevent the service going down. But we also need to mimic the legacy service and use an 'error code' to provide extra information about what the error was. We've already used this concept with [Create error bill run when CM fails](#104) where we apply a code to the `billing_batch` record which the UI then uses to indicate what happened to the user. This change deals with both; ensuring we handle any exceptions raised and where possible, set a relevant error code when updating the `billing_batch` status to 'errored'.
Cruikshanks
added a commit
that referenced
this pull request
Mar 13, 2023
https://eaflood.atlassian.net/browse/WATER-3924 Should an error occur during the supplementary bill run process it brings down the service. We're not handling the exception and because we moved the process to the background in our bill runs controller, there is nothing to deal with it when thrown. So, we need to protect the service from _any_ errors during the process, to prevent the service from going down. But we also need to mimic the legacy service and use an 'error code' to provide extra information about the error. We've already used this concept with [Create error bill run when CM fails](#104) where we apply a code to the `billing_batch` record which the UI then uses to indicate what happened to the user. This change deals with both; ensuring we handle any exceptions raised and where possible, setting a relevant error code when updating the `billing_batch` status to 'errored'.
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-3833
When we currently initiate a new bill run, a fail response from the Charging Module means we throw an error and no bill run is created. We want to revise this so that it does create a new bill run but with the status
errorand witherrorCodeset to50.As part of this we update
BillingBatchModelwith a static gettererrorCodes, which allows us to consistently use the correct error codes by referring to (for example)BillingBatchModel.errorCodes.failedToCreateBillRun.