Skip to content

Commit c552813

Browse files
authored
Merge pull request #1950 from dnesteryuk/bugfix/1604
Consider the allow_blank option in the values validator
2 parents cf57d25 + 0cfc310 commit c552813

File tree

5 files changed

+35
-5
lines changed

5 files changed

+35
-5
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616

1717
#### Fixes
1818

19+
* [#1950](https://github.com/ruby-grape/grape/pull/1950): Consider the allow_blank option in the values validator - [@dnesteryuk](https://github.com/dnesteryuk).
1920
* [#1947](https://github.com/ruby-grape/grape/pull/1947): Careful check for empty params - [@dnesteryuk](https://github.com/dnesteryuk).
2021
* [#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).
2122

lib/grape/validations/params_scope.rb

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -282,9 +282,7 @@ def validates(attrs, validations)
282282
full_attrs = attrs.collect { |name| { name: name, full_name: full_name(name) } }
283283
@api.document_attribute(full_attrs, doc_attrs)
284284

285-
# slice out fail_fast attribute
286-
opts = {}
287-
opts[:fail_fast] = validations.delete(:fail_fast) || false
285+
opts = derive_validator_options(validations)
288286

289287
# Validate for presence before any other validators
290288
if validations.key?(:presence) && validations[:presence]
@@ -451,6 +449,17 @@ def options_key?(type, key, validations)
451449
def all_element_blank?(parameters)
452450
params(parameters).respond_to?(:all?) && params(parameters).all?(&:blank?)
453451
end
452+
453+
# Validators don't have access to each other and they don't need, however,
454+
# some validators might influence others, so their options should be shared
455+
def derive_validator_options(validations)
456+
allow_blank = validations[:allow_blank]
457+
458+
{
459+
allow_blank: allow_blank.is_a?(Hash) ? allow_blank[:value] : allow_blank,
460+
fail_fast: validations.delete(:fail_fast) || false
461+
}
462+
end
454463
end
455464
end
456465
end

lib/grape/validations/validators/base.rb

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ def initialize(attrs, options, required, scope, opts = {})
1919
@required = required
2020
@scope = scope
2121
@fail_fast = opts[:fail_fast] || false
22+
@allow_blank = opts[:allow_blank] || false
2223
end
2324

2425
# Validates a given request.

lib/grape/validations/validators/values.rb

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,9 +26,15 @@ def initialize(attrs, options, required, scope, opts = {})
2626

2727
def validate_param!(attr_name, params)
2828
return unless params.is_a?(Hash)
29-
return unless params[attr_name] || required_for_root_scope?
3029

31-
param_array = params[attr_name].nil? ? [nil] : Array.wrap(params[attr_name])
30+
val = params[attr_name]
31+
32+
return unless val || required_for_root_scope?
33+
34+
# don't forget that +false.blank?+ is true
35+
return if val != false && val.blank? && @allow_blank
36+
37+
param_array = val.nil? ? [nil] : Array.wrap(val)
3238

3339
raise Grape::Exceptions::Validation.new(params: [@scope.full_name(attr_name)], message: except_message) \
3440
unless check_excepts(param_array)

spec/grape/validations/validators/values_spec.rb

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -224,6 +224,11 @@ class API < Grape::API
224224
requires :type, values: { proc: ->(v) { ValuesModel.values.include? v }, message: 'failed check' }
225225
end
226226
get '/proc/message'
227+
228+
params do
229+
optional :name, type: String, values: %w[a b], allow_blank: true
230+
end
231+
get '/allow_blank'
227232
end
228233
end
229234
end
@@ -464,6 +469,14 @@ def app
464469
end.to raise_error Grape::Exceptions::IncompatibleOptionValues
465470
end
466471

472+
it 'allows a blank value when the allow_blank option is true' do
473+
get 'allow_blank', name: nil
474+
expect(last_response.status).to eq(200)
475+
476+
get 'allow_blank', name: ''
477+
expect(last_response.status).to eq(200)
478+
end
479+
467480
context 'with a lambda values' do
468481
subject do
469482
Class.new(Grape::API) do

0 commit comments

Comments
 (0)