Support getAllCountryIdentifiers endpoints#110
Conversation
HMS-Victory
left a comment
HMS-Victory
left a comment
There was a problem hiding this comment.
All tests in the LocationSpec.groovy pass successfully, overall this looks well done.
HMS-Victory
left a comment
HMS-Victory
left a comment
There was a problem hiding this comment.
it might be a little easier to use these two methods if we rename getAllCountryIdentifiersWithPathVar and getAllCountryIdentifiers as 'getUnsupportedCountryIdentifiers' and 'getAllCountryIdentifiers' instead.
Good idea. I have made that change. |
Jonathan-Zollinger
left a comment
Jonathan-Zollinger
left a comment
There was a problem hiding this comment.
after rebasing the last PR onto this one, I'm getting several dozen test failures.
…ryIdentifiers This change is because a future endpoint will get country identifiers for all countries, including unsupported countries, whereas this method only gets country identifiers for supported countries.
This test was accidentally removed in #104.
…ifiersWithPathVar endpoints The getALlCountryIdentifiersWithPathVar is the same as getAllCountryIdentifiers, but one variable is a path variable instead of a query parameter. The name of this method is subject to change.
…iersWithPathVar methods
…dCountryIdentifiers
6f498af to
a5d7cc2
Compare
|
Yup, seems I forgot to rename the method in one of the tests in 35da4df |
…ntifiers method The method name was changed in 35da4df and that change was not reflected in the test.
|
Should be working now. I figured the edit was a simple refactor and didn't bother cleaning and rebuilding before running tests, I'll be sure to do that in the future. |
|
Changing PR to draft. I want to review my tests because they are unnecessarily complicated, and could cover more. |
…t English This greatly reduces the number of tests. Not all languages need to be tested for this method.
3 participants
This PR adds support for the following endpoints:
GET api/v1/locations/{language}/countries/allGET api/v1/locations/{language}/countries/all/{excludeSupportedCountriesIt should be noted, however, that both of these endpoints are essentially the same. The first endpoint has two query parameters for excluding supported countries, or only returning supported countries. The second endpoint has a path variable for whether supported countries should be included, and a query parameter to only return supported countries.
The method names are "getAllCountryIdentifiers" and "getAllCountryIdentifiersWithPathVar". The latter name is subject to change.
To my knowledge, I can't have a method with two endpoints because the OpenAPI generator will make a getAllCountryIdentifiers and a getAllCountryIdentifiers_1 method instead of assigning two endpoints to a single method. Neither can I make one method call the other. So I went with having one method have a more verbose name, and testing each method individually.
When testing, not all tests completed. It should be mentioned that I am trying each supported language, both authenticated and unauthenticated clients, and combinations of the two parameters. Therefore there are 8 tests, for each method, for each language, so I think the API throttled the 112 tests.
This PR also adds a test that was removed when merging #104 into main.