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

Tidy supplementary billing code #197

Merged
merged 19 commits into from
Apr 26, 2023
Merged

Tidy supplementary billing code #197

merged 19 commits into from
Apr 26, 2023

Conversation

StuAA78
Copy link
Contributor

@StuAA78 StuAA78 commented Apr 24, 2023

We've been working quickly to get supplementary billing completed on time. This means we haven't always been able to spend as much time as we'd like on some of the finer details -- documentation, consistency of standards, naming conventions, etc. This PR is a first pass at spotting these things and amending them.

We've been working quickly to get supplementary billing completed on time. This means we haven't always been able to spend as much time as we'd like on some of the finer details -- documentation, consistency of standards, naming conventions, etc. This PR is a first pass at spotting these things and amending them.
@StuAA78 StuAA78 added the housekeeping Refactoring, tidying up or other work which supports the project label Apr 24, 2023
@StuAA78 StuAA78 self-assigned this Apr 24, 2023
StuAA78 added 15 commits April 24, 2023 11:32
We no longer require the `/check/supplementary` endpoint so we delete it along with its related controller, service and presenter.
The docs for `GenerateBillingInvoiceLicenceService` refer to us passing in and returning an array of billing invoice licence objects. This was true when the service was originally written but we now pass in and return a single billing invoice licence object. We therefore update the docs to reflect this
As with `GenerateBillingInvoiceLicenceService`, the docs for `GenerateBillingInvoiceService` referred to receiving and returning an array instead of a single instance of an object, so we update the docs to reflect this
We would normally put constants at the top of the file. We also rename it to make its meaning a little clearer
Since we only care about the number of results returned, we can use the `.select(1)` optimisation
@StuAA78 StuAA78 marked this pull request as ready for review April 25, 2023 13:45
StuAA78 added 3 commits April 26, 2023 11:41
We had a `TODO` to check if legacy code was correct in flagging minimum charge for a charge period starting April 1st. Since our new code is tested working, we are confident we can remove it.
A previous refactor meant we no longer needed `ProcessPreviousBillingTransactionsService`. We therefore delete it.
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.

@StuAA78 StuAA78 merged commit 19d1680 into main Apr 26, 2023
@StuAA78 StuAA78 deleted the tidy-of-supplementary-code branch April 26, 2023 13:26
StuAA78 added a commit that referenced this pull request May 9, 2023
We've been working quickly to get supplementary billing completed on time. This means we haven't always been able to spend as much time as we'd like on some of the finer details -- documentation, consistency of standards, naming conventions, etc. [Following our first pass](#197), this PR is a second pass at spotting these things and amending them.

Note that while refactoring `ProcessBillingBatchService` we identified a couple of ways we can restructure the process. This will be handled in a separate PR; for now, we have simply refactored code into helper functions which more accurately describe the flow of the process.
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