Skip to content

Commit afff083

Browse files
committed
Make validation consistent for multiple params per requires
Add spec for multiple params per requires fix validation inconsistency update changelog update UPGRADING.md
1 parent 3a34ca2 commit afff083

File tree

4 files changed

+46
-3
lines changed

4 files changed

+46
-3
lines changed

CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ Next Release
33

44
* [#1503](https://github.com/ruby-grape/grape/pull/1503): Allow to use regexp validator with arrays - [@akoltun](https://github.com/akoltun).
55
* [#1507](https://github.com/ruby-grape/grape/pull/1507): Add group attributes for parameter definitions - [@304](https://github.com/304).
6+
* [#1510](https://github.com/ruby-grape/grape/pull/1510): Fix inconsistent validation - [@dgasper](https://github.com/dgasper).
67
* Your contribution here.
78

89
0.18.0 (10/7/2016)

UPGRADING.md

+15
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,21 @@
11
Upgrading Grape
22
===============
33

4+
### Upgrading to >= 0.18.1
5+
6+
#### Changed endpoint params validation
7+
8+
Grape now returns validation errors for all params when multiple params are passed to a requires.
9+
The following code will return `one is missing, two is missing` when calling the endpoint without parameters.
10+
11+
```ruby
12+
params do
13+
requires :one, :two
14+
end
15+
```
16+
17+
Prior to this version the response would be `one is missing`.
18+
419
### Upgrading to >= 0.17.0
520

621
#### Removed official support for Ruby < 2.2.2

lib/grape/validations/validators/base.rb

+1-2
Original file line numberDiff line numberDiff line change
@@ -39,13 +39,12 @@ def validate(request)
3939
def validate!(params)
4040
attributes = AttributesIterator.new(self, @scope, params)
4141
array_errors = []
42-
attributes.each do |resource_params, attr_name, inside_array|
42+
attributes.each do |resource_params, attr_name|
4343
next unless @required || (resource_params.respond_to?(:key?) && resource_params.key?(attr_name))
4444

4545
begin
4646
validate_param!(attr_name, resource_params)
4747
rescue Grape::Exceptions::Validation => e
48-
raise e unless inside_array
4948
# we collect errors inside array because
5049
# there may be more than one error per field
5150
array_errors << e

spec/grape/validations/validators/presence_spec.rb

+29-1
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,34 @@ def app
105105
end
106106
end
107107

108+
context 'with multiple parameters per requires' do
109+
before do
110+
subject.params do
111+
requires :one, :two
112+
end
113+
subject.get '/single-requires' do
114+
'Hello'
115+
end
116+
117+
subject.params do
118+
requires :one
119+
requires :two
120+
end
121+
subject.get '/multiple-requires' do
122+
'Hello'
123+
end
124+
end
125+
it 'validates for all defined params' do
126+
get '/single-requires'
127+
expect(last_response.status).to eq(400)
128+
single_requires_error = last_response.body
129+
130+
get '/multiple-requires'
131+
expect(last_response.status).to eq(400)
132+
expect(last_response.body).to eq(single_requires_error)
133+
end
134+
end
135+
108136
context 'with required parameters and no type' do
109137
before do
110138
subject.params do
@@ -117,7 +145,7 @@ def app
117145
it 'validates name, company' do
118146
get '/'
119147
expect(last_response.status).to eq(400)
120-
expect(last_response.body).to eq('{"error":"name is missing"}')
148+
expect(last_response.body).to eq('{"error":"name is missing, company is missing"}')
121149

122150
get '/', name: 'Bob'
123151
expect(last_response.status).to eq(400)

0 commit comments

Comments
 (0)