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

Refactor how we create transactions #157

Merged
merged 15 commits into from
Mar 9, 2023
Merged

Conversation

Cruikshanks
Copy link
Member

@Cruikshanks Cruikshanks commented Mar 9, 2023

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

Having implemented our Fix error calculating auth. and billable days we clocked that we were calculating charge periods, option flags, and the authorised and billable days multiple times, when they do not change between iterations of the charge elements in a charge version.

We only want to be executing our services when needed as it reduces complexity and the chance of an error occurring. It also means we get a slight performance bump.

So, this change refactors how the ProcessBillingBatchService and the FormatSrocTransactionLineService interact together to create the transaction lines for a billing batch.

Dropping the SROC because SROC is always assumed. Should we take on PRESROC functionality _then_ we'll label it as such.

Replacing format with generate as we are 'generating' the transaction data from other data elements. It is also becoming our convention where a service is responsible for extracting data from other things to create a new object, but not persist it.
The charge period is based on the charge version and the billing period. So, when called within the transaction service we are just getting the same result.

To avoid executing unnecessary code, we create the charge period outside the service and pass it in. This also means we can drop charge version as an arg to the service.
Until we are ready to implement the functionality and understand how the flag is determined, we'll just set it to false to simplify things.
We were already handling this one outside of the service. But as the number of options dwindles it's more verbose to specify it as an argument.
Trying to do everything just once which meant a whole re-jig of everything.
@Cruikshanks Cruikshanks added the housekeeping Refactoring, tidying up or other work which supports the project label Mar 9, 2023
@Cruikshanks Cruikshanks self-assigned this Mar 9, 2023
Group shorthand properties together at the top of an object declaration.
@Cruikshanks Cruikshanks marked this pull request as ready for review March 9, 2023 12:55
@Cruikshanks Cruikshanks requested review from Jozzey and StuAA78 March 9, 2023 12:55
Jozzey
Jozzey previously approved these changes Mar 9, 2023
Copy link
Contributor

@Jozzey Jozzey left a comment

Choose a reason for hiding this comment

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

👍🏼

As suggested by the team.
@Cruikshanks Cruikshanks force-pushed the refactor-format-transaction branch from c238bd5 to 5f50dfc Compare March 9, 2023 14:06
@Cruikshanks Cruikshanks merged commit 7b18e8d into main Mar 9, 2023
@Cruikshanks Cruikshanks deleted the refactor-format-transaction branch March 9, 2023 14:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
housekeeping Refactoring, tidying up or other work which supports the project
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants