Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix inconsistent validation #1510

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
15 changes: 15 additions & 0 deletions UPGRADING.md
Original file line number Diff line number Diff line change
@@ -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.
Copy link
Member

@dblock dblock Oct 21, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would add "correctly" otherwise it seems like this is some other kind of behavior change.

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