properly handle URLs that already contain a query string

[jadlr]

before this, URLs that already contain a query string were not handled.

iex(3)> HTTPoison.get("https://httpbin.org/get?yolo=bolo", [], params: %{whut: "hello"})
{:ok,
 %HTTPoison.Response{body: "{\n  \"args\": {\n    \"yolo\": \"bolo?whut=hello\"\n  }, \n  \"headers\": {\n    \"Host\": \"httpbin.org\", \n    \"User-Agent\": \"hackney/1.6.5\"\n  }, \n  \"origin\": \"193.240.195.178\", \n  \"url\": \"https://httpbin.org/get?yolo=bolo%3Fwhut=hello\"\n}\n",
  headers: [{"Server", "nginx"}, {"Date", "Thu, 16 Feb 2017 10:29:15 GMT"},
   {"Content-Type", "application/json"}, {"Content-Length", "225"},
   {"Connection", "keep-alive"}, {"Access-Control-Allow-Origin", "*"},
   {"Access-Control-Allow-Credentials", "true"}], status_code: 200}}

note that the URL used was https://httpbin.org/get?yolo=bolo%3Fwhut=hello not https://httpbin.org/get?yolo=bolo&whut=hello

[jadlr]

This is a little more complex than I initially thought. Parameters should not be overwritten since a query string can contain a key multiple times.

EDIT: added a commit that fixes this

[edgurgel]

Thanks for the PR. I will have look soon. Sorry for the delay to give feedback

[jadlr]

No worries, thanks for considering it!

[edgurgel]

So there are now 3 cases here. I suspect we should have now 3 tests (1 for each case) right? How can we reach the true case?

[jadlr]

the true case is reached in the get with params test. The second condition is reached in the test I added and the first condition is reached in the get test. Maybe cond do isn't the best choice here, do you want me to change it into a nested if else?

[edgurgel]

Gotcha! Thanks for the explanation. Hmm I think cond is fine!

[jadlr]

You're welcome!

[edgurgel]

Thanks! I'm sorry for the delay 😶