Migrate Charging Module services to *.request.js #797
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-4375 > Part of a series of changes related to replacing the create bill run journey to incorporate changes for two-part tariff We know as part of the 2pT create bill run journey changes that we'll now be responsible for triggering any bill runs in the legacy service. This means we'll need to make a new call to the legacy service. We already make one to each legacy app's `/health/info` endpoint and one to the `/refresh` bill run in[water-abstraction-service](https://github.com/DEFRA/water-abstraction-service). We were just going to copy and paste what we'd done before when we realised we had conflicting conventions. Any calls to the [Charging Module API](https://github.com/DEFRA/sroc-charging-module.api) are wrapped in their own dedicated module. This removes duplication, simplifies testing and makes the intent much clearer. All calls using the `LegacyRequestLib` are being done directly. This means we already have duplication in the billing engines and a conflicting pattern. We want to make the Legacy stuff consistent with the Charging Module. Problem is, we're already seeing an explosion in services in the `app/services` as this is going to add another sub grouping. Plus, these services are different to the others as they are dedicated to making API requests to other services. So, rather than moving Legacy to the Charging Module pattern, we're going to move the CHA to a new top level grouping called `app/api`. Files will be renamed from `*.service.js` to `*.api.js` and the call will be renamed from `go()` to `send()`. Hopefully, that will make the intent of these types of modules clearer.
Cruikshanks
added
the
housekeeping
Cruikshanks
changed the title
Would have been nice to break this up but it broke so much stuff in the process it was clearer and easier just to do it en-masse and then confirm app was running and tests were passing again. Also includes some cleanup of the JSDoc comments to bring them all to the same level of consistency.
Cruikshanks
added a commit
that referenced
this pull request
Mar 9, 2024
https://eaflood.atlassian.net/browse/WATER-4375 > Part of a series of changes related to replacing the create bill run journey to incorporate changes for two-part tariff In [Migrate Charging Module services to *.request.js](#797) we made structural changes to where the code that sends requests to 3rd party apps sit. This was in preparation for this change. For each request type we send to the [Charging Module API](https://github.com/DEFRA/sroc-charging-mdoule-api) we have a module. This removes duplication, centralises how that type of request is made and makes the intent clearer. But legacy requests we're just using `Legacy.request.js` directly. We want a consistent pattern for how these things are done, especially as we are on the cusp of adding a new legacy request type. So, having moved the Charging Module requests to the pattern we want to adopt, it is now time to move how we are sending requests to the Legacy apps to refresh a bill run.
Cruikshanks
added a commit
that referenced
this pull request
Mar 9, 2024
https://eaflood.atlassian.net/browse/WATER-4375 > Part of a series of changes related to replacing the create bill run journey to incorporate changes for two-part tariff In [Migrate Charging Module services to *.request.js](#797) we made structural changes to where the code that sends requests to 3rd party apps sits. This was in preparation for this change. For each request type, we send to the [Charging Module API](https://github.com/DEFRA/sroc-charging-mdoule-api) we have a module. This removes duplication, centralises how that type of request is made and makes the intent clearer. But for legacy requests, we're just using `Legacy.request.js` directly. We want a consistent pattern for how these things are done, especially as we are on the cusp of adding a new legacy request type. So, having moved the Charging Module requests to the pattern we want to adopt, it is now time to move how we send requests to the Legacy apps to refresh a bill run.
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-4375
We know as part of the 2pT create bill run journey changes that we'll now be responsible for triggering any bill runs in the legacy service. This means we'll need to make a new call to the legacy service.
We already make one to each legacy app's
/health/infoendpoint and one to the/refreshbill run in water-abstraction-service. We were just going to copy and paste what we'd done before when we realised we had conflicting conventions.Any calls to the Charging Module API are wrapped in a dedicated module. This removes duplication, simplifies testing and makes the intent much clearer.
All calls using the
LegacyRequestLibare being done directly. This means we already have duplication in the billing engines and a conflicting pattern.We want to make the Legacy stuff consistent with the Charging Module. The problem is, that we're already seeing an explosion in services in the
app/servicesas this is going to add another sub-grouping. Plus, these services are different to the others as they are dedicated to making API requests to other services.So, rather than moving Legacy to the Charging Module pattern, we're going to move the CHA to a new top-level grouping called
app/requests. Files will be renamed from*.service.jsto*.request.jsand the call will be renamed fromgo()tosend().Hopefully, that will make the intent of these types of modules clearer.