revert ignore params in path commit #32
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
removed URI query params from authorization calculation. This behavior is not compatible with mgomes api_auth. This PR restores this with a small refactoring.
zfletch
suggested changes
Jan 17, 2018
|
|
||
| case query do |
There was a problem hiding this comment.
I think this case statement can be replaced with "#{value_for(path)}#{query}".
There was a problem hiding this comment.
You could have "#{value_for(path)}?#{query}" (note the ?); however that would append the ? to paths that don't have query params. I believe you will have to conditionally determine if there is a query string to know whether or not to include it, as done here with the case statement.
Let me know if you were thinking of something different, or if there's another control structure you'd like to use to conditionally add the ?.
There was a problem hiding this comment.
It looks like I missed the ? in "#{path}?#{query}". I think the code here makes sense then.
zfletch
approved these changes
Feb 2, 2018
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The commit ignore params in path removed URI query params from authorization calculation. This behavior is not compatible with
mgomes api_auth.
This PR restores this with a small refactoring.