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

Add PATCH support to our request lib modules #142

Merged
merged 5 commits into from
Mar 4, 2023

Conversation

Cruikshanks
Copy link
Member

@Cruikshanks Cruikshanks commented Mar 4, 2023

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

In order to call the /generate bill run endpoint in the Charging Module API we need to be able to make PATCH HTTP requests.

This change adds the functionality to app/lib/request.lib.js and app/lib/charging-module-request.lib.js.


Housekeeping

It is a personal preference that has become a convention in our tests. Executing the thing under test should be done within the test rather than a before() block.

It's our opinion when this isn't done you lose sight of what a test is testing as you add more and more unit tests. It does mean you get some duplication. But we think it makes for more readable code.

As we were adding new tests to test/lib/charging-module-request.lib.test.js we spotted this deviation and corrected it as part of this change.

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

In order to call the [/generate](https://defra.github.io/sroc-charging-module-api-docs/#/bill-run/GenerateBillRun) bill run endpoint in the Charging Module API we need to be able to make `PATCH` http requests.

This change adds the functionality to `app/lib/request.lib.js` and `app/lib/charging-module-request.lib.js`.
@Cruikshanks Cruikshanks added the enhancement New feature or request label Mar 4, 2023
@Cruikshanks Cruikshanks self-assigned this Mar 4, 2023
That is a little harsh. It does work, but it has now retry support from Got unlike the other types of requests.

So, our retry tests fail. Good to know they are working as expected!
@Cruikshanks Cruikshanks force-pushed the add-patch-support-to-requests branch from 70fc645 to e096055 Compare March 4, 2023 10:42
It is a personal preference that has become a convention in our tests. Executing the thing under test is done within the test rather than a `before()` block.

IMHO when this isn't done it is easy to lose sight of what a test is testing as you add more and more unit tests. It does mean you get some duplication. But we think it makes for more readable code.
@Cruikshanks Cruikshanks marked this pull request as ready for review March 4, 2023 11:20
@Cruikshanks Cruikshanks merged commit c2d9ac4 into main Mar 4, 2023
@Cruikshanks Cruikshanks deleted the add-patch-support-to-requests branch March 4, 2023 11:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant