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

Move and make generic some billing services #304

Merged
merged 14 commits into from
Jul 12, 2023

Conversation

Cruikshanks
Copy link
Member

@Cruikshanks Cruikshanks commented Jul 11, 2023

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

Continuing our theme of getting things ready for two-part tariff (2PT) bill runs we have already spotted that app/services/billing/supplementary/fetch-region.service.js will be needed by both types.

This PR either moves 'generic' billing services to the root app/services/billing/ level, updates them to be generic, or both!

Primarily, they are the services that deal with validating, then initialising and creating the new Billing Batch records. It's when you get to ProcessBillingBatchService that the specifics of each bill run type will determine what happens next.

So, included in this change is a 'shell' two-part tariff ProcessBillingBatchService to demonstrate how that initial flow can be generic but then hand off to the specific services.

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

Continuing our theme of getting things ready for two-part tariff (2PT) bill runs we have already spotted that `app/services/billing/supplementary/fetch-region.service.js` will be needed by both types.

So, this PR either moves 'generic' billing services to the root `app/services/billing/` level, updates them to be generic, or both!
@Cruikshanks Cruikshanks added the housekeeping Refactoring, tidying up or other work which supports the project label Jul 11, 2023
@Cruikshanks Cruikshanks self-assigned this Jul 11, 2023
We update the validator to accept both supplementary and 2PT arguments (based on what the UI will fire at us).

In the [charging-module-api](https://github.com/DEFRA/sroc-charging-module-api) we used the convention of keeping any static list of values in a central place.

This seemed an apt time to bring in that concept to this project and make use of it in the validator.
If we were following the pattern in services they really should be in `app/presenters/billing/supplementary`! But we need to create the same data structure for creating bill runs and their events for all types of bill run so actually we can rename the folder as `/billing`.
Again, another service that will be generic amongst the bill run types. They might not use all the periods like supplementary but they do need the service to confirm what the current period is.

Whilst we're doing this we take the opportunity to rename it. `BillingPeriodsService` was the only one that didn't follow a `VerbNounService` naming convention. It's the little things!
We are going to need to always check if there is an existing live bill run before we create a new one, irrespective of whether it's supplementary or 2PT.

We do need to make a change to the service because it was hard coded to check for live supplementary bill runs only. Now we pass the billing batch type as an argument.
The initiate service deals with creating the charging module bill run, then merging its result with the WRLS billing batch record we are trying to create. It also creates a new billing batch event (though goodness knows why!)

Again, these are all things we need to do irrespective of the billing batch type so we make it generic and move it out of `supplementary/`.
We will need to make some changes to support the move.
This will be the start point when we come to implement our 2PT bill run engine. Obviously we have nothing as yet so we just have it throw. But it means the generic stuff of creating the billing batch, billing batch event, the charging module bill run, and providing the appropriate response is all done ready (after a few more changes!)
Plus follows the same naming/wording conventions we used in 2PT process billing batch.
Now our NewBillingBatchService is truly generic and is ready to handle both supplementary and 2PT bill runs. Plus it can easily be extended to handle Annual in future.
Now the journey is complete and we are ready to start handling 2PT requests (just not do anything about them!)
@Cruikshanks Cruikshanks marked this pull request as ready for review July 12, 2023 08:49
@Cruikshanks
Copy link
Member Author

This looks bigger than it is. With files moving you need to dip into both the main an test files of others to update the references. Also, git has decided there was enough change to warrant a couple of services to appear deleted and then added.

But I'm happy to walk through it if anyone has concerns/is struggling. I just felt making the create bill run journey generic to the point of kicking off processing the billing batch was better done as one change.

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.

@Cruikshanks Cruikshanks merged commit ffb37fd into main Jul 12, 2023
@Cruikshanks Cruikshanks deleted the move-generic-billing-services branch July 12, 2023 15:04
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.

2 participants