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.

Sign up for GitHub

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

Jump to bottom

Rack::Utils.build_nested_query with integer values #557

Closed
cyx opened this issue May 15, 2013 · 5 comments
Closed

Rack::Utils.build_nested_query with integer values #557

cyx opened this issue May 15, 2013 · 5 comments
Milestone
Rack 2.0

Comments

@cyx
Copy link

cyx commented May 15, 2013

Hi,

Not sure if this is intended behavior, but if you try to use build_nested_query like so:

Rack::Utils.build_nested_query("foo" => 1)
# => "foo"

Rack::Utils.build_nested_query("foo" => { "bar" => 1})
# => "foo[bar]"

It drops the value silently. If you think this is a bug, I can do a pull request or something. Thanks!
@tizoc
Copy link

tizoc commented May 15, 2013

+1

Right now I just use my own wrapper to avoid having values silently discarded here, but it would be great if Rack did this check in the body of the else branch:

https://github.com/rack/rack/blob/master/lib/rack/utils.rb#L165

@spastorino
Copy link
Contributor

I agree on this one, I was talking with @raggi about it. I will wait for him just in case.
Anyway you can provide a PR, it's an easy fix and probably better for James to follow.

I did an sketch fix here https://gist.github.com/spastorino/7375c064765c35ec6b23 feel free to reuse that one if you want.

Thanks!

@tizoc
Copy link

tizoc commented May 16, 2013

Looks fine to me.

michaelsauter added a commit to sitepoint/the86-client that referenced this issue Jan 14, 2014
@michaelsauter
Use build_nested_query, of course
33be937 
Not quite sure why I assumed build_query would work. I guess
because the tests passed and the tests failed when I used
build_nested query. The issue then however was a bug in rack
(see rack/rack#557), for which this
commit provides a quick fix.
@aaronpk
Copy link

aaronpk commented Apr 5, 2014

+1 Ended up having to do the same workaround as @michaelsauter above.

@OpakAlex
Copy link

OpakAlex commented Jul 8, 2014

+1

@raggi raggi closed this as completed in c34b579 Aug 3, 2014
@raggi raggi mentioned this issue Nov 15, 2014
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
Rack 2.0
Development

No branches or pull requests
5 participants
@cyx @tizoc @spastorino @aaronpk @OpakAlex