diff --git a/CHANGELOG.md b/CHANGELOG.md index e3527613cb..bdf574f80b 100644 --- a/CHANGELOG.md +++ b/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). diff --git a/lib/grape/validations/validators/values.rb b/lib/grape/validations/validators/values.rb index 0f01af1e10..7889f3badc 100644 --- a/lib/grape/validations/validators/values.rb +++ b/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 diff --git a/spec/grape/validations/validators/values_spec.rb b/spec/grape/validations/validators/values_spec.rb index 81cfe6000e..b975c3feb0 100644 --- a/spec/grape/validations/validators/values_spec.rb +++ b/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