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

allow required param with predefined set of values to be nil inside optional group #671

Merged
merged 1 commit into from
Jul 4, 2014
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ Next Release

#### Fixes

* [#671](https://github.com/intridea/grape/pull/671): Allow required param with predefined set of values to be nil inside optional group - [@dm1try](https://github.com/dm1try).
* [#651](https://github.com/intridea/grape/pull/651): The `rescue_from` keyword now properly defaults to rescuing subclasses of exceptions - [@xevix](https://github.com/xevix).
* [#614](https://github.com/intridea/grape/pull/614): Params with `nil` value are now refused by `RegexpValidator` - [@dm1try](https://github.com/dm1try).
* [#494](https://github.com/intridea/grape/issues/494): Fixed performance issue with requests carrying a large payload - [@dblock](https://github.com/dblock).
Expand Down
4 changes: 4 additions & 0 deletions lib/grape/validations.rb
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,10 @@ def full_name(name)
name.to_s
end

def root?
!@parent
end

protected

def push_declared_params(attrs)
Expand Down
8 changes: 7 additions & 1 deletion lib/grape/validations/values.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,16 @@ def initialize(attrs, options, required, scope)
end

def validate_param!(attr_name, params)
if (params[attr_name] || @required) && !(@values.is_a?(Proc) ? @values.call : @values).include?(params[attr_name])
if (params[attr_name] || required_for_root_scope?) && !(@values.is_a?(Proc) ? @values.call : @values).include?(params[attr_name])
raise Grape::Exceptions::Validation, param: @scope.full_name(attr_name), message_key: :values
end
end

private

def required_for_root_scope?
@required && @scope.root?
end
end
end
end
22 changes: 18 additions & 4 deletions spec/grape/validations/values_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,13 @@ class API < Grape::API
get '/values/coercion' do
{ type: params[:type] }
end

params do
optional :optional do
requires :type, values: ["a", "b"]
end
end
get '/optional_with_required_values'
end
end
end
Expand All @@ -69,10 +76,17 @@ def app
expect(last_response.body).to eq({ error: "type does not have a valid value" }.to_json)
end

it 'does not allow nil value for a parameter' do
get("/", type: nil)
expect(last_response.status).to eq 400
expect(last_response.body).to eq({ error: "type does not have a valid value" }.to_json)
context 'nil value for a parameter' do
it 'does not allow for root params scope' do
get("/", type: nil)
expect(last_response.status).to eq 400
expect(last_response.body).to eq({ error: "type does not have a valid value" }.to_json)
end

it 'allows for a required param in child scope' do
get('/optional_with_required_values')
expect(last_response.status).to eq 200
end
end

it 'allows a valid default value' do
Expand Down