Merge pull request #1963 from ruby-grape/bugfix/1577

The values validator must properly work with booleans

Author: dblock

CHANGELOG.md

@@ -16,6 +16,7 @@
 
 #### Fixes
 
+* [#1963](https://github.com/ruby-grape/grape/pull/1963): The values validator must properly work with booleans - [@dnesteryuk](https://github.com/dnesteryuk).
 * [#1950](https://github.com/ruby-grape/grape/pull/1950): Consider the allow_blank option in the values validator - [@dnesteryuk](https://github.com/dnesteryuk).
 * [#1947](https://github.com/ruby-grape/grape/pull/1947): Careful check for empty params - [@dnesteryuk](https://github.com/dnesteryuk).
 * [#1931](https://github.com/ruby-grape/grape/pull/1946): Fixes issue when using namespaces in `Grape::API::Instance` mounted directly - [@myxoh](https://github.com/myxoh).

lib/grape/validations/validators/values.rb

@@ -29,20 +29,20 @@ def validate_param!(attr_name, params)
 
         val = params[attr_name]
 
-        return unless val || required_for_root_scope?
+        return if val.nil? && !required_for_root_scope?
 
         # don't forget that +false.blank?+ is true
         return if val != false && val.blank? && @allow_blank
 
         param_array = val.nil? ? [nil] : Array.wrap(val)
 
-        raise Grape::Exceptions::Validation.new(params: [@scope.full_name(attr_name)], message: except_message) \
+        raise validation_exception(attr_name, except_message) \
           unless check_excepts(param_array)
 
-        raise Grape::Exceptions::Validation.new(params: [@scope.full_name(attr_name)], message: message(:values)) \
+        raise validation_exception(attr_name, message(:values)) \
           unless check_values(param_array, attr_name)
 
-        raise Grape::Exceptions::Validation.new(params: [@scope.full_name(attr_name)], message: message(:values)) \
+        raise validation_exception(attr_name, message(:values)) \
           if @proc && !param_array.all? { |param| @proc.call(param) }
       end
 
@@ -74,6 +74,10 @@ def except_message
       def required_for_root_scope?
         @required && @scope.root?
       end
+
+      def validation_exception(attr_name, message)
+        Grape::Exceptions::Validation.new(params: [@scope.full_name(attr_name)], message: message)
+      end
     end
   end
 end

spec/grape/validations/validators/values_spec.rb

@@ -438,11 +438,21 @@ def app
     end.to raise_error Grape::Exceptions::IncompatibleOptionValues
   end
 
-  it 'allows values to be true or false when setting the type to boolean' do
-    get('/values/optional_boolean', type: true)
-    expect(last_response.status).to eq 200
-    expect(last_response.body).to eq({ type: true }.to_json)
+  context 'boolean values' do
+    it 'allows a value from the list' do
+      get('/values/optional_boolean', type: true)
+
+      expect(last_response.status).to eq 200
+      expect(last_response.body).to eq({ type: true }.to_json)
+    end
+
+    it 'rejects a value which is not in the list' do
+      get('/values/optional_boolean', type: false)
+
+      expect(last_response.body).to eq({ error: 'type does not have a valid value' }.to_json)
+    end
   end
+
   it 'allows values to be a kind of the coerced type not just an instance of it' do
     get('/values/coercion', type: 10)
     expect(last_response.status).to eq 200