Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add 'query' to available attribute writers so it can be set with 'new' #133

Merged
merged 1 commit into from
Oct 16, 2014
Merged

Add 'query' to available attribute writers so it can be set with 'new' #133

merged 1 commit into from
Oct 16, 2014

Conversation

msievers
Copy link
Contributor

At the the moment, HTTPI::Request.new ignores query as a parameter.

HTTPI::Request.new(url: "http://example.org", query: { foo: "bar" })
=> #<HTTPI::Request:0x02b05598 @url=#<URI::HTTP:0x02b05188 URL:http://example.org>>

Expected behaviour would be

HTTPI::Request.new(url: "http://example.org", query: { foo: "bar" })
=> #<HTTPI::Request:0x0579cbd8 @url=#<URI::HTTP:0x0579c890 URL:http://example.org?foo=bar>>

This PR adds query to Request::Attributes so that it is called in Request#mass_assign.

@msievers msievers changed the title Added 'query' to available attribute writers so it can be set during with 'new' Add 'query' to available attribute writers so it can be set with 'new' Oct 16, 2014
@rogerleite
Copy link
Member

Thanks!

rogerleite added a commit that referenced this pull request Oct 16, 2014
…t_new

Add 'query' to available attribute writers so it can be set with 'new'
@rogerleite rogerleite merged commit f783a9c into savonrb:master Oct 16, 2014
@rogerleite
Copy link
Member

@msievers we can wait for more improvements/fixes or do you need a new version for now?

@msievers
Copy link
Contributor Author

@d-konovalov As far as I can see, this PR does not change Savons behaviour in your case, because

response = client.call(:auto_complete, message: {'query' => {'key1' => 'value1', 'key2' => 'value2'}})

message: { ... } only effects the request body, whereas the change introduced by this PR permits you to initialize a HTTPI::Request with a url query parameter, which only effects the url, not the request body.

@msievers
Copy link
Contributor Author

@rogerleite My code actually circumvents this issue by first creating a request and setting the query afterwards. So it's ok for me to wait some time for a new release.

@rogerleite
Copy link
Member

@msievers 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

3 participants