diff --git a/CHANGELOG.md b/CHANGELOG.md index c94600fadc..0b015154bf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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). diff --git a/lib/grape/validations.rb b/lib/grape/validations.rb index 5bc13267ab..3f50f24420 100644 --- a/lib/grape/validations.rb +++ b/lib/grape/validations.rb @@ -174,6 +174,10 @@ def full_name(name) name.to_s end + def root? + !@parent + end + protected def push_declared_params(attrs) diff --git a/lib/grape/validations/values.rb b/lib/grape/validations/values.rb index 2b5644573c..84f87eed2c 100644 --- a/lib/grape/validations/values.rb +++ b/lib/grape/validations/values.rb @@ -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 diff --git a/spec/grape/validations/values_spec.rb b/spec/grape/validations/values_spec.rb index a5d8606c59..7c483c4144 100644 --- a/spec/grape/validations/values_spec.rb +++ b/spec/grape/validations/values_spec.rb @@ -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 @@ -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