diff --git a/CHANGELOG.md b/CHANGELOG.md index 6dccef1db0..304347ad7a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ #### Fixes * [#1615](https://github.com/ruby-grape/grape/pull/1615): Fix default and type validator when values is a Hash with no value attribute - [@jlfaber](https://github.com/jlfaber). +* [#1625](https://github.com/ruby-grape/grape/pull/1625): Handle `given` correctly when nested in Array params - [@rnubel](https://github.com/rnubel), [@avellable](https://github.com/avellable). * Your contribution here. ### 0.19.2 (4/12/2017) diff --git a/lib/grape/validations/params_scope.rb b/lib/grape/validations/params_scope.rb index c57ca5f142..c4c109688a 100644 --- a/lib/grape/validations/params_scope.rb +++ b/lib/grape/validations/params_scope.rb @@ -42,37 +42,45 @@ def should_validate?(parameters) return false if @optional && (params(parameters).blank? || any_element_blank?(parameters)) + return true if parent.nil? + parent.should_validate?(parameters) + end + + def meets_dependency?(params) + return true unless @dependent_on + + params = params.with_indifferent_access + @dependent_on.each do |dependency| if dependency.is_a?(Hash) dependency_key = dependency.keys[0] proc = dependency.values[0] - return false unless proc.call(params(parameters).try(:[], dependency_key)) - elsif params(parameters).try(:[], dependency).blank? + return false unless proc.call(params.try(:[], dependency_key)) + elsif params.respond_to?(:key?) && params.try(:[], dependency).blank? return false end - end if @dependent_on + end - return true if parent.nil? - parent.should_validate?(parameters) + true end # @return [String] the proper attribute name, with nesting considered. - def full_name(name) + def full_name(name, index: nil) if nested? # Find our containing element's name, and append ours. - "#{@parent.full_name(@element)}#{array_index}[#{name}]" + [@parent.full_name(@element), [@index || index, name].map(&method(:brackets))].compact.join elsif lateral? - # Find the name of the element as if it was at the - # same nesting level as our parent. - @parent.full_name(name) + # Find the name of the element as if it was at the same nesting level + # as our parent. We need to forward our index upward to achieve this. + @parent.full_name(name, index: @index) else # We must be the root scope, so no prefix needed. name.to_s end end - def array_index - "[#{@index}]" if @index.present? + def brackets(val) + "[#{val}]" if val end # @return [Boolean] whether or not this scope is the root-level scope @@ -187,7 +195,7 @@ def new_lateral_scope(options, &block) element: nil, parent: self, options: @optional, - type: Hash, + type: type == Array ? Array : Hash, dependent_on: options[:dependent_on], &block ) diff --git a/lib/grape/validations/validators/base.rb b/lib/grape/validations/validators/base.rb index 7b3a2e9655..ad3b440fa7 100644 --- a/lib/grape/validations/validators/base.rb +++ b/lib/grape/validations/validators/base.rb @@ -41,6 +41,7 @@ def validate!(params) array_errors = [] attributes.each do |resource_params, attr_name| next unless @required || (resource_params.respond_to?(:key?) && resource_params.key?(attr_name)) + next unless @scope.meets_dependency?(resource_params) begin validate_param!(attr_name, resource_params) diff --git a/spec/grape/validations/params_scope_spec.rb b/spec/grape/validations/params_scope_spec.rb index 5b1c4e41a5..7b659e5cee 100644 --- a/spec/grape/validations/params_scope_spec.rb +++ b/spec/grape/validations/params_scope_spec.rb @@ -473,6 +473,29 @@ def initialize(value) end end + context 'when validations are dependent on a parameter within an array param' do + before do + subject.params do + requires :foos, type: Array do + optional :foo_type, :baz_type + given :foo_type do + requires :bar + end + end + end + subject.post('/test') { declared(params).to_json } + end + + it 'applies the constraint within each value' do + post '/test', + { foos: [{ foo_type: 'a' }, { baz_type: 'c' }] }.to_json, + 'CONTENT_TYPE' => 'application/json' + + expect(last_response.status).to eq(400) + expect(last_response.body).to eq('foos[0][bar] is missing') + end + end + context 'when validations are dependent on a parameter with specific value' do # build test cases from all combinations of declarations and options a_decls = %i(optional requires)