-
Notifications
You must be signed in to change notification settings - Fork 0
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
Update "create bill run" endpoint to create a bill run #56
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
Cruikshanks
added a commit
that referenced
this pull request
Dec 16, 2022
It was spotted whilst working on [Update "create bill run" endpoint to create a bill run](#56) that we had an issue with our [Objection models](https://vincit.github.io/objection.js/). The previous team didn't use the convention of just calling the unique identifier in each table `id`. Instead they incorporate the table name, for example, `billing_batch_id`. **Objection** is expecting the identifier column to be `id` and is failing when in some cases. Because of this we need to go back and specify [idColumn](https://vincit.github.io/objection.js/api/model/static-properties.html#static-idcolumn) in all our models.
Cruikshanks
added a commit
that referenced
this pull request
Dec 16, 2022
It was spotted whilst working on [Update "create bill run" endpoint to create a bill run](#56) that we had an issue with our [Objection models](https://vincit.github.io/objection.js/). The previous team didn't use the convention of just calling the unique identifier in each table `id`. Instead they incorporate the table name, for example, `billing_batch_id`. **Objection** is expecting the identifier column to be `id` and is failing when in some cases. Because of this we need to go back and specify [idColumn](https://vincit.github.io/objection.js/api/model/static-properties.html#static-idcolumn) in all our models.
Cruikshanks
added a commit
that referenced
this pull request
Dec 19, 2022
It was spotted whilst working on [Update "create bill run" endpoint to create a bill run](#56) that we had an issue with our [Objection models](https://vincit.github.io/objection.js/). The previous team didn't use the convention of just calling the unique identifier in each table `id`. Instead, they incorporate the table name, for example, `billing_batch_id`. **Objection** is expecting the identifier column to be `id` and is failing when in some cases. Because of this, we need to go back and specify [idColumn](https://vincit.github.io/objection.js/api/model/static-properties.html#static-idcolumn) in all our models.
Cruikshanks
added a commit
that referenced
this pull request
Dec 20, 2022
https://eaflood.atlassian.net/browse/WATER-3854 We are about to [Update "create bill run" endpoint to create a bill run](#56). Currently, everything is happening in the controller whereas our convention is to do everything in a service and keep our controllers ['skinny'](https://blog.makersacademy.com/forget-fat-models-its-time-for-skinny-controllers-and-skinny-models-a9b84ec481b7). So, we need a service, something like `SupplementaryService`. But oh no! It already exists. This change is about refactoring the code currently in `SupplementaryService` to a more explicit testing version so we can free up the namespace. While we're at it, we also rename our `/test` path to `/qa` because everyone is getting confused when they come across a `test/` folder in the repo!
Cruikshanks
added a commit
that referenced
this pull request
Dec 21, 2022
https://eaflood.atlassian.net/browse/WATER-3854 We are about to [Update "create bill run" endpoint to create a bill run](#56). Currently, everything is happening in the controller whereas our convention is to do everything in a service and keep our controllers ['skinny'](https://blog.makersacademy.com/forget-fat-models-its-time-for-skinny-controllers-and-skinny-models-a9b84ec481b7). So, we need a service, something like `SupplementaryService`. But oh no! It already exists. This change is about refactoring the code currently in `SupplementaryService` to a more explicit testing version so we can free up the namespace. Whilst we're at it we also rename the `/test` path to `check/` as everyone was getting confused by seeing `test/` folders in various places in the code 😁
Cruikshanks
force-pushed
the
update-endpoint-to-create-bill-run
branch
from
December 23, 2022 14:59
818543e
to
f2e219b
Compare
Cruikshanks
force-pushed
the
update-endpoint-to-create-bill-run
branch
2 times, most recently
from
January 2, 2023 22:08
12a5a6a
to
121661b
Compare
https://eaflood.atlassian.net/browse/WATER-3854 When we set up our "create bill run" endpoint, we had it simply respond to requests with a dummy response. We now update it to actually create a bill run and an event, and return the required details in the same format as the dummy response.
As we start to look at creating entries in the db, we run into an issue when Objection errors because it can't find an `id` column. On investigation it turns out that if the primary id column isn't called `id`, it needs to be specified in the model. We therefore add an `idColumn` static method to our billing batch model
We update our `createBillRun` controller to create a record for the bill run in the db. We initially hardcode the financial year fields just to get it working, with the intention of pulling these through properly from the request in a later commit
We try and follow that the path you see in the url matches where the controller sits in the repo.
We need something that handles formatting the response for the POST `/bill-runs` endpoint. We use what we did in DEFRA/water-abstraction-ui#2247 as a spec.
Cruikshanks
force-pushed
the
update-endpoint-to-create-bill-run
branch
from
January 2, 2023 22:58
534bb73
to
e005c3a
Compare
At the start of a new bill run we need to - create the initial billing batch record - create the associated event record - send a request to the SROC Charging Module API and link it to our billing batch With those 3 things done we can return control back to the requester (which will be the UI) whilst we get on with the rest of the bill run process. This adds the service that will handle those 3 steps. We're waiting on additional functionality before we can do the last one. But for now steps 1 & 2 are done and we'll have a home for step 3 when it's ready!
Now we've created the InitiateBillingBatchService we refactor the bill runs controller to use it.
Cruikshanks
approved these changes
Jan 3, 2023
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
😁
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-3854
When we set up our "create bill run" endpoint, we had it simply respond to requests with a dummy response. We now update it to actually create a bill run and an event, and return the required details in the same format as the dummy response.