Bugfix: Correctly handle given in Array params (ruby-grape#1625)

* 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.

* Code review fix

* Add CHANGELOG entry

Author: rnubel

CHANGELOG.md

@@ -1,3 +1,18 @@
+### 1.0.0 (Next)
+
+#### Features
+
+* [#1594](https://github.com/ruby-grape/grape/pull/1594): Replace `Hashie::Mash` parameters with `ActiveSupport::HashWithIndifferentAccess` - [@james2m](https://github.com/james2m), [@dblock](https://github.com/dblock).
+* [#1622](https://github.com/ruby-grape/grape/pull/1622): Add `except_values` validator to replace `except` option of `values` validator - [@jlfaber](https://github.com/jlfaber).
+* [#1635](https://github.com/ruby-grape/grape/pull/1635): Instrument validators with ActiveSupport::Notifications - [@ktimothy](https://github.com/ktimothy).
+* Your contribution here.
+
+#### 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)
 
 #### Features

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
         )

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)

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)