Skip to content

Commit

Permalink
Make validation consistent for multiple params per requires
Browse files Browse the repository at this point in the history
Add spec for multiple params per requires

fix validation inconsistency

update changelog

update UPGRADING.md
  • Loading branch information
dgasper authored and dblock committed Oct 21, 2016
1 parent eb1711a commit 813925e
Show file tree
Hide file tree
Showing 4 changed files with 50 additions and 5 deletions.
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@ 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).
* [#1512](https://github.com/ruby-grape/grape/pull/1512): Fix for deeply nested params TypeError situation - [@krbs](https://github.com/krbs).
* [#1512](https://github.com/ruby-grape/grape/pull/1512): Fix: deeply nested parameters are included within `#declared(params)` - [@krbs](https://github.com/krbs).
* [#1510](https://github.com/ruby-grape/grape/pull/1510): Fix: inconsistent validation for multiple parameters - [@dgasper](https://github.com/dgasper).
* Your contribution here.

#### Fixes
Expand Down
19 changes: 18 additions & 1 deletion UPGRADING.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,23 @@ When a request is made to the built-in `OPTIONS` handler, only the `before` and
callbacks associated with the resource will be run. The `before_validation` and
`after_validation` callbacks and parameter validations will be skipped.

See [#1505](https://github.com/ruby-grape/grape/pull/1505) for more information.

#### Changed endpoint params validation

Grape now correctly 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`.

See [#1510](https://github.com/ruby-grape/grape/pull/1510) for more information.

### Upgrading to >= 0.17.0

#### Removed official support for Ruby < 2.2.2
Expand All @@ -22,7 +39,7 @@ See [#1441](https://github.com/ruby-grape/grape/pull/1441) for nmore information
The `rescue_from` clauses declared inside a namespace would take a priority over ones declared in the root scope.
This could possibly affect those users who use different `rescue_from` clauses in root scope and in namespaces.

See [#1405](https://github.com/ruby-grape/grape/pull/1405) for more inforomation.
See [#1405](https://github.com/ruby-grape/grape/pull/1405) for more information.

#### Helper methods injected inside `rescue_from` in middleware

Expand Down
3 changes: 1 addition & 2 deletions lib/grape/validations/validators/base.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
30 changes: 29 additions & 1 deletion spec/grape/validations/validators/presence_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)
Expand Down

0 comments on commit 813925e

Please sign in to comment.