search

TheFirstAvenger / elixir-ex_jira

Elixir wrapper for the JIRA REST API
MIT License
3 stars 8 forks source link

Change Request.get_more/5 to use responses "next" key as next `request_path` in recursion #4

Closed wolves closed 5 years ago

wolves commented 5 years ago

This addresses an issue that I ran into here: https://github.com/TheFirstAvenger/elixir-ex_jira/issues/3

I wanted to open first by saying thank you for creating this project. If any stylistic things in this PR are not to your liking I would be more than happy to adjust them. If you see something that I missed regarding these code changes and you have any suggestions for a refactor I encourage the corrective criticisms.

Obviously, this is totally your project and I have no intention of stepping on your toes. I have always believed that with Open-Source if you find something in a pkg you are using its important to not only report it but provide a possible solution if you are able. I digress...

*Apologies in advance for the Elixir formatter changes adding extra diffs

The changes essentially substitute an overloaded Request.get_more/5 function, that matches on the responses "next" => key, for the case statement that was being used

Since the response only has that key when there are more items to be fetched we can assume that if it isn't present the retrieved items have completed and are ready to be returned with the Request.get_more/5 function that does not require the "next" => key pattern.

In the process of this change I separated out the building of the url within the Request.request/4 function so that it handled the case where no additional query_params are passed in and removes any trailing "&".

I also removed the trailing "?"'s from the MockClient since they are not required and with the Request.build_request_url/1 change were causing failures.

All tests are passing and these changes resolve the issue I was seeing.

TheFirstAvenger commented 5 years ago

v0.0.5 was released with this PR