-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Optional nested array validation #1724
Conversation
…resent Added specs
Added rubocop_todo
Refactor specs
We do not validate if all empty We skip validation if parents is optional
CHANGELOG.md
Outdated
@@ -6,6 +6,7 @@ | |||
* [#1688](https://github.com/ruby-grape/grape/pull/1688): Removes yard docs - [@ramkumar-kr](https://github.com/ramkumar-kr). | |||
* [#1702](https://github.com/ruby-grape/grape/pull/1702): Added danger-toc, verify correct TOC in README - [@dblock](https://github.com/dblock). | |||
* [#1711](https://github.com/ruby-grape/grape/pull/1711): Automatically coerce arrays and sets of types that implement a `parse` method - [@dslh](https://github.com/dslh). | |||
* [#1724](https://github.com/ruby-grape/grape/pull/1724): Optional nested array validation - [@ericproulx](https://github.com/ericproulx). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a fix, no?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is, my bad
{ foo: 'three' } | ||
] } | ||
get '/nested_optional_array', root: params | ||
expect(last_response.status).to eq(400) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Test that the actual response contains the correct value being wrong. Maybe break this one up more, too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In this case the status can be 400 for all kinds of reasons. Check last_response.body
that it says something about the parameter being incorrect.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
got it 👍
Merged, thank you. |
Fix #1636 Merry 🎄