Skip to content

Commit fe0f78b

Browse files
authored
Fix ordering issues between as: and default: validations (#2177)
1 parent 613f9f1 commit fe0f78b

File tree

3 files changed

+46
-7
lines changed

3 files changed

+46
-7
lines changed

CHANGELOG.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,8 @@
66

77
#### Fixes
88

9-
* [#2176](https://github.com/ruby-grape/grape/pull/2176): Fixes issue-2175 - options call failing if matching all routes - [@myxoh](https://github.com/myxoh).
9+
* [#2176](https://github.com/ruby-grape/grape/pull/2176): Fix: OPTIONS fails if matching all routes - [@myxoh](https://github.com/myxoh).
10+
* [#2177](https://github.com/ruby-grape/grape/pull/2177): Fix: `default` validator fails if preceded by `as` validator - [@Catsuko](https://github.com/Catsuko).
1011
* Your contribution here.
1112
### 1.5.3 (2021/03/07)
1213

lib/grape/validations/params_scope.rb

Lines changed: 22 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -283,16 +283,15 @@ def validates(attrs, validations)
283283

284284
doc_attrs[:documentation] = validations.delete(:documentation) if validations.key?(:documentation)
285285

286-
full_attrs = attrs.collect { |name| { name: name, full_name: full_name(name) } }
287-
@api.document_attribute(full_attrs, doc_attrs)
286+
document_attribute(attrs, doc_attrs)
288287

289288
opts = derive_validator_options(validations)
290289

290+
order_specific_validations = Set[:as]
291+
291292
# Validate for presence before any other validators
292-
if validations.key?(:presence) && validations[:presence]
293-
validate('presence', validations[:presence], attrs, doc_attrs, opts)
294-
validations.delete(:presence)
295-
validations.delete(:message) if validations.key?(:message)
293+
validates_presence(validations, attrs, doc_attrs, opts) do |validation_type|
294+
order_specific_validations << validation_type
296295
end
297296

298297
# Before we run the rest of the validators, let's handle
@@ -301,8 +300,13 @@ def validates(attrs, validations)
301300
coerce_type validations, attrs, doc_attrs, opts
302301

303302
validations.each do |type, options|
303+
next if order_specific_validations.include?(type)
304304
validate(type, options, attrs, doc_attrs, opts)
305305
end
306+
307+
# Apply as validator last so other validations are applied to
308+
# renamed param
309+
validate(:as, validations[:as], attrs, doc_attrs, opts) if validations.key?(:as)
306310
end
307311

308312
# Validate and comprehend the +:type+, +:types+, and +:coerce_with+
@@ -464,6 +468,18 @@ def derive_validator_options(validations)
464468
fail_fast: validations.delete(:fail_fast) || false
465469
}
466470
end
471+
472+
def validates_presence(validations, attrs, doc_attrs, opts)
473+
return unless validations.key?(:presence) && validations[:presence]
474+
validate(:presence, validations[:presence], attrs, doc_attrs, opts)
475+
yield :presence
476+
yield :message if validations.key?(:message)
477+
end
478+
479+
def document_attribute(attrs, doc_attrs)
480+
full_attrs = attrs.collect { |name| { name: name, full_name: full_name(name) } }
481+
@api.document_attribute(full_attrs, doc_attrs)
482+
end
467483
end
468484
end
469485
end

spec/grape/validations/params_scope_spec.rb

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -180,6 +180,28 @@ def initialize(value)
180180
expect(last_response.status).to eq(200)
181181
expect(last_response.body).to eq('{"baz":{"qux":"any"}}')
182182
end
183+
184+
it 'renaming can be defined before default' do
185+
subject.params do
186+
optional :foo, as: :bar, default: 'before'
187+
end
188+
subject.get('/rename-before-default') { params[:bar] }
189+
get '/rename-before-default'
190+
191+
expect(last_response.status).to eq(200)
192+
expect(last_response.body).to eq('before')
193+
end
194+
195+
it 'renaming can be defined after default' do
196+
subject.params do
197+
optional :foo, default: 'after', as: :bar
198+
end
199+
subject.get('/rename-after-default') { params[:bar] }
200+
get '/rename-after-default'
201+
202+
expect(last_response.status).to eq(200)
203+
expect(last_response.body).to eq('after')
204+
end
183205
end
184206

185207
context 'array without coerce type explicitly given' do

0 commit comments

Comments
 (0)