Skip to content

Commit

Permalink
consider the allow_blank option in the values validator
Browse files Browse the repository at this point in the history
When `allow_blank` is true, the values validator shouldn't
raise `Grape::Exceptions::Validation` in case of a blank value.
  • Loading branch information
dnesteryuk committed Dec 31, 2019
1 parent cf57d25 commit 0cfc310
Show file tree
Hide file tree
Showing 5 changed files with 35 additions and 5 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

#### Fixes

* [#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
15 changes: 12 additions & 3 deletions lib/grape/validations/params_scope.rb
Original file line number Diff line number Diff line change
Expand Up @@ -282,9 +282,7 @@ def validates(attrs, validations)
full_attrs = attrs.collect { |name| { name: name, full_name: full_name(name) } }
@api.document_attribute(full_attrs, doc_attrs)

# slice out fail_fast attribute
opts = {}
opts[:fail_fast] = validations.delete(:fail_fast) || false
opts = derive_validator_options(validations)

# Validate for presence before any other validators
if validations.key?(:presence) && validations[:presence]
Expand Down Expand Up @@ -451,6 +449,17 @@ def options_key?(type, key, validations)
def all_element_blank?(parameters)
params(parameters).respond_to?(:all?) && params(parameters).all?(&:blank?)
end

# Validators don't have access to each other and they don't need, however,
# some validators might influence others, so their options should be shared
def derive_validator_options(validations)
allow_blank = validations[:allow_blank]

{
allow_blank: allow_blank.is_a?(Hash) ? allow_blank[:value] : allow_blank,
fail_fast: validations.delete(:fail_fast) || false
}
end
end
end
end
1 change: 1 addition & 0 deletions lib/grape/validations/validators/base.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ def initialize(attrs, options, required, scope, opts = {})
@required = required
@scope = scope
@fail_fast = opts[:fail_fast] || false
@allow_blank = opts[:allow_blank] || false
end

# Validates a given request.
Expand Down
10 changes: 8 additions & 2 deletions lib/grape/validations/validators/values.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,15 @@ def initialize(attrs, options, required, scope, opts = {})

def validate_param!(attr_name, params)
return unless params.is_a?(Hash)
return unless params[attr_name] || required_for_root_scope?

param_array = params[attr_name].nil? ? [nil] : Array.wrap(params[attr_name])
val = params[attr_name]

return unless val || 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) \
unless check_excepts(param_array)
Expand Down
13 changes: 13 additions & 0 deletions spec/grape/validations/validators/values_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,11 @@ class API < Grape::API
requires :type, values: { proc: ->(v) { ValuesModel.values.include? v }, message: 'failed check' }
end
get '/proc/message'

params do
optional :name, type: String, values: %w[a b], allow_blank: true
end
get '/allow_blank'
end
end
end
Expand Down Expand Up @@ -464,6 +469,14 @@ def app
end.to raise_error Grape::Exceptions::IncompatibleOptionValues
end

it 'allows a blank value when the allow_blank option is true' do
get 'allow_blank', name: nil
expect(last_response.status).to eq(200)

get 'allow_blank', name: ''
expect(last_response.status).to eq(200)
end

context 'with a lambda values' do
subject do
Class.new(Grape::API) do
Expand Down

0 comments on commit 0cfc310

Please sign in to comment.