Skip to content

Commit 44913f7

Browse files
committed
Bugfix: Correctly handle given in Array params
Array parameters are handled as a parameter that opens a scope with `type: Array`; `given` opens up a new scope, but was always setting the type to Hash. This patch fixes that, as well as correctly labeling the error messages associated with array fields.
1 parent c644607 commit 44913f7

File tree

2 files changed

+41
-16
lines changed

2 files changed

+41
-16
lines changed

lib/grape/validations/params_scope.rb

Lines changed: 18 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -42,37 +42,39 @@ def should_validate?(parameters)
4242
return false if @optional && (params(parameters).blank? ||
4343
any_element_blank?(parameters))
4444

45-
@dependent_on.each do |dependency|
46-
if dependency.is_a?(Hash)
47-
dependency_key = dependency.keys[0]
48-
proc = dependency.values[0]
49-
return false unless proc.call(params(parameters).try(:[], dependency_key))
50-
elsif params(parameters).try(:[], dependency).blank?
51-
return false
45+
if @dependent_on && !params(parameters).is_a?(Array)
46+
@dependent_on.each do |dependency|
47+
if dependency.is_a?(Hash)
48+
dependency_key = dependency.keys[0]
49+
proc = dependency.values[0]
50+
return false unless proc.call(params(parameters).try(:[], dependency_key))
51+
elsif params(parameters).try(:[], dependency).blank?
52+
return false
53+
end
5254
end
53-
end if @dependent_on
55+
end
5456

5557
return true if parent.nil?
5658
parent.should_validate?(parameters)
5759
end
5860

5961
# @return [String] the proper attribute name, with nesting considered.
60-
def full_name(name)
62+
def full_name(name, index: nil)
6163
if nested?
6264
# Find our containing element's name, and append ours.
63-
"#{@parent.full_name(@element)}#{array_index}[#{name}]"
65+
[@parent.full_name(@element), [@index || index, name].map(&method(:brackets))].compact.join
6466
elsif lateral?
65-
# Find the name of the element as if it was at the
66-
# same nesting level as our parent.
67-
@parent.full_name(name)
67+
# Find the name of the element as if it was at the same nesting level
68+
# as our parent. We need to forward our index upward to achieve this.
69+
@parent.full_name(name, index: @index)
6870
else
6971
# We must be the root scope, so no prefix needed.
7072
name.to_s
7173
end
7274
end
7375

74-
def array_index
75-
"[#{@index}]" if @index.present?
76+
def brackets(val)
77+
"[#{val}]" if val
7678
end
7779

7880
# @return [Boolean] whether or not this scope is the root-level scope
@@ -187,7 +189,7 @@ def new_lateral_scope(options, &block)
187189
element: nil,
188190
parent: self,
189191
options: @optional,
190-
type: Hash,
192+
type: type == Array ? Array : Hash,
191193
dependent_on: options[:dependent_on],
192194
&block
193195
)

spec/grape/validations/params_scope_spec.rb

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -473,6 +473,29 @@ def initialize(value)
473473
end
474474
end
475475

476+
context 'when validations are dependent on a parameter within an array param' do
477+
before do
478+
subject.params do
479+
requires :foos, type: Array do
480+
optional :foo_type, :baz_type
481+
given :foo_type do
482+
requires :bar
483+
end
484+
end
485+
end
486+
subject.post('/test') { declared(params).to_json }
487+
end
488+
489+
it 'applies the constraint within each value' do
490+
post '/test',
491+
{ foos: [{ foo_type: 'a' }, { bar: 'not_needed', baz_type: 'c' }] }.to_json,
492+
'CONTENT_TYPE' => 'application/json'
493+
494+
expect(last_response.status).to eq(400)
495+
expect(last_response.body).to eq('foos[0][bar] is missing')
496+
end
497+
end
498+
476499
context 'when validations are dependent on a parameter with specific value' do
477500
# build test cases from all combinations of declarations and options
478501
a_decls = %i(optional requires)

0 commit comments

Comments
 (0)