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

Group param with a required param defaults to Array #722

Closed
jdp opened this issue Aug 16, 2014 · 4 comments
Closed

Group param with a required param defaults to Array #722

jdp opened this issue Aug 16, 2014 · 4 comments

Comments

@jdp
Copy link

jdp commented Aug 16, 2014

I'm trying to use a required param inside of a group, like this:

params do
  group :a do
    requires :b
  end
end

As an aside, my actual use case looks more like:

params do
  group :a do
    requires :b
  end
  group :c do
    requires :d
  end
  exactly_one_of :a, :c
end

But if I send a request that should satisfy that first example, like one of these:

$ curl "localhost:9292/case" -d "a[b]=c"
$ curl "localhost:9292/case" -d '{"a":{"b":"c"}}' -H "Content-Type: application/json"

I get a response saying a is invalid:

{"error":"a is invalid"}

I made an example here that reproduces the bug. I'm using grape version 0.8.0.

@dm1try
Copy link
Member

dm1try commented Aug 17, 2014

Hello, seems like you should provide Hash type for your groups.

group :a, type: Hash do
  requires :b
end

from readme:
... With a block, group, requires and optional accept an additional option type which can be either Array or Hash, and defaults to Array...

BTW, I'd like Hash as default value ;)

@dblock
Copy link
Member

dblock commented Aug 17, 2014

Lets not change the default, but I would be open to raising an error when the type is not provided in this case.

@dblock dblock changed the title Group param with a required param is always invalid Group param with a required param defaults to Array Aug 17, 2014
@jrichter1
Copy link
Contributor

I'm going to give it a shot to make it raise an error if no type is specified. I'll issue a PR when done and we'll see what comes out of it.

jrichter1 added a commit to jrichter1/grape that referenced this issue Jan 10, 2015
jrichter1 added a commit to jrichter1/grape that referenced this issue Jan 24, 2015
jrichter1 added a commit to jrichter1/grape that referenced this issue Jan 26, 2015
jrichter1 added a commit to jrichter1/grape that referenced this issue Jan 26, 2015
dblock added a commit that referenced this issue Jan 26, 2015
Make the type in group of required params required (#722)
@dnesteryuk
Copy link
Member

The solution was implemented in the attached PR.

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

No branches or pull requests

5 participants