Skip to content

Conversation

@dleavitt
Copy link

See failing tests below for more specifics.

To reproduce
In a params block, define an array param within a Hash or JSON param:

params do optional :data, type: Hash do optional :array, type: Array do optional :name, type: String end end end

When the request body looks like this: { data: {} }
I'd expect that calling declared(params) within the route would return something like this:
{ data: { array: [] } }
or maybe this:
{ data: {} }

What's actually returned is this:
{ data: { array: { name: nil } } }

A hash rather than an array.

@grape-bot
Copy link

grape-bot commented Feb 28, 2020

1 Warning
⚠️ Unless you’re refactoring existing code, please update CHANGELOG.md.
1 Message
📖 We really appreciate pull requests that demonstrate issues, even without a fix. That said, the next step is to try and fix the failing tests!

Here's an example of a CHANGELOG.md entry:

* [#2001](https://github.com/ruby-grape/grape/pull/2001): Params: optional arrays within hashes/json objects broken - [@dleavitt](https://github.com/dleavitt).

Generated by 🚫 danger

@myxoh
Copy link
Member

myxoh commented Mar 4, 2020

Makes sense, thanks for the test!

@dblock
Copy link
Member

dblock commented Mar 4, 2020

@dleavitt now that we have a test (thanks), try fixing it?

post '/nesty', { data: { } }
expect(last_response.status).to eq 201
# {"data"=>{"json"=>{"name"=>nil}}}
expect(JSON.parse(last_response.body)['data']['json']).not_to be_a Hash
Copy link
Member

Choose a reason for hiding this comment

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

I guess here we should expect the hash. @dleavitt could you verify this test?

@numbata
Copy link
Contributor

numbata commented Jul 23, 2024

I ran these tests and only params with nested JSON's seem to be broken. And it is fixable by calling declared(params, include_missed: false) instead of declared(params).

Also, looking at the rest of the specs from lib/grape/validations/params_scope.rb file, it looks like expected behavior and not a bug.

@dleavitt
Copy link
Author

Feel free to close if you're cleaning house!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

6 participants