Merge pull request #671 from dm1try/fix_required_params_inside_optional_group

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

Author: dblock

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).

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)

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

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