Skip to content

Commit

Permalink
The values validator must properly work with booleans
Browse files Browse the repository at this point in the history
When we check whether a value is given or not, it is safer
to look for `nil`, there are cases when `false` is ok value.
  • Loading branch information
dnesteryuk committed Jan 10, 2020
1 parent 9feefdf commit bcf9e81
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 8 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -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).
Expand Down
12 changes: 8 additions & 4 deletions lib/grape/validations/validators/values.rb
Expand Up @@ -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

Expand Down Expand Up @@ -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
18 changes: 14 additions & 4 deletions spec/grape/validations/validators/values_spec.rb
Expand Up @@ -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
Expand Down

0 comments on commit bcf9e81

Please sign in to comment.