diff --git a/CHANGELOG.md b/CHANGELOG.md index 592b2aee42..bba067a869 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ Next Release * [#1503](https://github.com/ruby-grape/grape/pull/1503): Allow to use regexp validator with arrays - [@akoltun](https://github.com/akoltun). * [#1507](https://github.com/ruby-grape/grape/pull/1507): Add group attributes for parameter definitions - [@304](https://github.com/304). +* [#1510](https://github.com/ruby-grape/grape/pull/1510): Fix inconsistent validation - [@dgasper](https://github.com/dgasper). * Your contribution here. 0.18.0 (10/7/2016) diff --git a/UPGRADING.md b/UPGRADING.md index 71eff88e23..1d26f3b99c 100644 --- a/UPGRADING.md +++ b/UPGRADING.md @@ -1,6 +1,21 @@ Upgrading Grape =============== +### Upgrading to >= 0.18.1 + +#### Changed endpoint params validation + +Grape now returns validation errors for all params when multiple params are passed to a requires. +The following code will return `one is missing, two is missing` when calling the endpoint without parameters. + +```ruby +params do + requires :one, :two +end +``` + +Prior to this version the response would be `one is missing`. + ### Upgrading to >= 0.17.0 #### Removed official support for Ruby < 2.2.2 diff --git a/lib/grape/validations/validators/base.rb b/lib/grape/validations/validators/base.rb index 1315054f2d..7b3a2e9655 100644 --- a/lib/grape/validations/validators/base.rb +++ b/lib/grape/validations/validators/base.rb @@ -39,13 +39,12 @@ def validate(request) def validate!(params) attributes = AttributesIterator.new(self, @scope, params) array_errors = [] - attributes.each do |resource_params, attr_name, inside_array| + attributes.each do |resource_params, attr_name| next unless @required || (resource_params.respond_to?(:key?) && resource_params.key?(attr_name)) begin validate_param!(attr_name, resource_params) rescue Grape::Exceptions::Validation => e - raise e unless inside_array # we collect errors inside array because # there may be more than one error per field array_errors << e diff --git a/spec/grape/validations/validators/presence_spec.rb b/spec/grape/validations/validators/presence_spec.rb index 6f287b69c3..d00163ead8 100644 --- a/spec/grape/validations/validators/presence_spec.rb +++ b/spec/grape/validations/validators/presence_spec.rb @@ -105,6 +105,34 @@ def app end end + context 'with multiple parameters per requires' do + before do + subject.params do + requires :one, :two + end + subject.get '/single-requires' do + 'Hello' + end + + subject.params do + requires :one + requires :two + end + subject.get '/multiple-requires' do + 'Hello' + end + end + it 'validates for all defined params' do + get '/single-requires' + expect(last_response.status).to eq(400) + single_requires_error = last_response.body + + get '/multiple-requires' + expect(last_response.status).to eq(400) + expect(last_response.body).to eq(single_requires_error) + end + end + context 'with required parameters and no type' do before do subject.params do @@ -117,7 +145,7 @@ def app it 'validates name, company' do get '/' expect(last_response.status).to eq(400) - expect(last_response.body).to eq('{"error":"name is missing"}') + expect(last_response.body).to eq('{"error":"name is missing, company is missing"}') get '/', name: 'Bob' expect(last_response.status).to eq(400)