Fixed edge case regarding Hash with inner param #1156
Conversation
lasseebert
force-pushed
the
fix_hash_with_validated_param
branch
from
3b8e0f6
to
11fd3dd
Compare
| @@ -7,6 +7,7 @@ def initialize(attrs, options, required, scope) | |||
| end | |||
|
|
|||
| def validate_param!(attr_name, params) | |||
| return unless params.is_a?(Hash) | |||
There was a problem hiding this comment.
We are skipping the error here, shouldn't it be a fail?
There was a problem hiding this comment.
Actually I take it back. The parent is the problem, we don't get deep enough to care about the values validator.
There was a problem hiding this comment.
This just skips the validation of the inner param if the outer param does not have a valid value.
The "real" error happens in the coerce validator that correctly invalidates a Hash param with string value.
This is the same thing happening in the allow_blank validator here: https://github.com/ruby-grape/grape/blob/master/lib/grape/validations/validators/allow_blank.rb#L5
There was a problem hiding this comment.
Yes I should first have coffee and then review code :)
There was a problem hiding this comment.
:) Always coffee first.
Fixed edge case regarding Hash with inner param
|
Merged, thanks. |
|
Thanks :) |
This fixes an edge case where the API would blow up with a
"no implicit conversion of Symbol into Integer".To trigger the error, make a param nested inside a
Hashparam and addvaluesvalidations to the param. Then send something that is not a hash to the outer param. (see spec)