Skip to content

Commit 9a08358

Browse files
ipkesdblock
authored andcommitted
Fix parameter validation with an empty optional nested Array. Closes #723.
1 parent a711cab commit 9a08358

File tree

3 files changed

+53
-1
lines changed

3 files changed

+53
-1
lines changed

CHANGELOG.md

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

1212
* [#1365](https://github.com/ruby-grape/grape/pull/1365): Fix finding exception handler in error middleware - [@ktimothy](https://github.com/ktimothy).
1313
* [#1380](https://github.com/ruby-grape/grape/pull/1380): Fix `allow_blank: false` for `Time` attributes with valid values causes `NoMethodError` - [@ipkes](https://github.com/ipkes).
14+
* [#1384](https://github.com/ruby-grape/grape/pull/1384): Fix parameter validation with an empty optional nested `Array` - [@ipkes](https://github.com/ipkes).
1415

1516
0.16.2 (4/12/2016)
1617
==================

lib/grape/validations/params_scope.rb

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,10 +35,13 @@ def initialize(opts, &block)
3535
# @return [Boolean] whether or not this entire scope needs to be
3636
# validated
3737
def should_validate?(parameters)
38-
return false if @optional && params(parameters).respond_to?(:all?) && params(parameters).all?(&:blank?)
38+
return false if @optional && (params(parameters).blank? ||
39+
any_element_blank?(parameters))
40+
3941
@dependent_on.each do |dependency|
4042
return false if params(parameters).try(:[], dependency).blank?
4143
end if @dependent_on
44+
4245
return true if parent.nil?
4346
parent.should_validate?(parameters)
4447
end
@@ -357,6 +360,10 @@ def extract_message_option(attrs)
357360
def options_key?(type, key, validations)
358361
validations[type].respond_to?(:key?) && validations[type].key?(key) && !validations[type][key].nil?
359362
end
363+
364+
def any_element_blank?(parameters)
365+
params(parameters).respond_to?(:any?) && params(parameters).any?(&:blank?)
366+
end
360367
end
361368
end
362369
end

spec/grape/validations/validators/default_spec.rb

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,32 @@ class API < Grape::API
4949
get '/array' do
5050
{ array: params[:array] }
5151
end
52+
53+
params do
54+
requires :thing1
55+
optional :more_things, type: Array do
56+
requires :nested_thing
57+
requires :other_thing, default: 1
58+
end
59+
end
60+
get '/optional_array' do
61+
{ thing1: params[:thing1] }
62+
end
63+
64+
params do
65+
requires :root, type: Hash do
66+
optional :some_things, type: Array do
67+
requires :foo
68+
optional :options, type: Array do
69+
requires :name, type: String
70+
requires :value, type: String
71+
end
72+
end
73+
end
74+
end
75+
get '/nested_optional_array' do
76+
{ root: params[:root] }
77+
end
5278
end
5379
end
5480
end
@@ -57,6 +83,24 @@ def app
5783
ValidationsSpec::DefaultValidatorSpec::API
5884
end
5985

86+
it 'lets you leave required values nested inside an optional blank' do
87+
get '/optional_array', thing1: 'stuff'
88+
expect(last_response.status).to eq(200)
89+
expect(last_response.body).to eq({ thing1: 'stuff' }.to_json)
90+
end
91+
92+
it 'allows optional arrays to be omitted' do
93+
params = { some_things:
94+
[{ foo: 'one', options: [{ name: 'wat', value: 'nope' }] },
95+
{ foo: 'two' },
96+
{ foo: 'three', options: [{ name: 'wooop', value: 'yap' }] }
97+
]
98+
}
99+
get '/nested_optional_array', root: params
100+
expect(last_response.status).to eq(200)
101+
expect(last_response.body).to eq({ root: params }.to_json)
102+
end
103+
60104
it 'set default value for optional param' do
61105
get('/')
62106
expect(last_response.status).to eq(200)

0 commit comments

Comments
 (0)