Skip to content

Conversation

@rvowles
Copy link
Contributor

@rvowles rvowles commented Mar 5, 2021

Most of the other request proxies support PUT
requests and we need this gem to also support it
for Action Controller. This addresses issue #180

It includes the modified Action Controller proxy
and fixes the tests.

@rvowles
Copy link
Contributor Author

rvowles commented Mar 5, 2021

It surprises me that DELETE is not also generically supported? We appear to not require it, but...

@rvowles
Copy link
Contributor Author

rvowles commented Mar 7, 2021

There was already test coverage for PUT that was IMHO wrong, I simply fixed it so I am not expecting the code coverage to change.

@xtagon xtagon self-requested a review March 28, 2021 20:02
reject { |kv| kv[0] == 'oauth_signature'}
end

def raw_post_signature
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO this should be raw_post_signature? with a ? for convention (it returns a boolean). Minor nitpick though

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, fixed!

request_proxy = request_proxy(:put, {'key'=>'value'}, {'key2'=>'value2'})

expected_parameters = [["key", "value"]]
expected_parameters = [["key", "value"],["key2", "value2"]]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I see what you mean, the original test looks wrong and this fixes it. LGTM.

Most of the other request proxies support PUT
requests and we need this gem to also support it
for Action Controller. This addresses issue #180

It includes the modified Action Controller proxy
and fixes the tests.
Copy link
Contributor

@xtagon xtagon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@xtagon xtagon merged commit 243cf06 into ruby-oauth:master Mar 29, 2021
@rvowles rvowles deleted the put-me-pull-you branch March 29, 2021 02:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants