Closed jeevatkm closed 11 months ago
Patch coverage: 100.00%
and no project coverage change.
Comparison is base (
4801bed
) 95.65% compared to head (28012cc
) 95.65%. Report is 1 commits behind head on master.
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
@jeevatkm, the previous implementation was correct.
Let's say we have url example.com/{foo}/{bar}/
And have the parameters:
Client: {"foo": "123"}
Request: {"foo": "456", "bar": "789"}
r.URL = "example.com/{foo}/{bar}/"
the current code will apply the parameters from Client
first:
"foo"
:
r.URL = r.URL = strings.Replace(r.URL, "{foo}", url.PathEscape("123"), -1)`
---
r.URL = "example.com/123/{bar}/"
then the request parameters:
"foo"
:
r.URL = r.URL = strings.Replace(r.URL, "{foo}", url.PathEscape("456"), -1)`
---
r.URL = "example.com/123/{bar}/"
no changes because there is no {foo}
placeholder in the r.URL
"bar"
:
r.URL = r.URL = strings.Replace(r.URL, "{bar}", url.PathEscape("789"), -1)`
---
r.URL = "example.com/123/789/"
so the request parameters must be applied first, then the client, or we need to change the logic to use a map and strings.Replacer
@SVilgelm Thanks for reminding me. Damn it, last night, how did I forget this!! 🤦 I will revert the changes.
We need to add the unit tests to prevent such thing in the future, I can do it later today
Closes #519