Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
* [#1252](https://github.com/ruby-grape/grape/pull/1252): Allow default to be a subset or equal to allowed values without raising IncompatibleOptionValues - [@jeradphelps](https://github.com/jeradphelps).
* [#1255](https://github.com/ruby-grape/grape/pull/1255): Allow param type definition in `route_param` - [@namusyaka](https://github.com/namusyaka).
* [#1257](https://github.com/ruby-grape/grape/pull/1257): Allow Proc, Symbol or String in `rescue_from with: ...` - [@namusyaka](https://github.com/namusyaka).
* [#1261](https://github.com/ruby-grape/grape/pull/1261): support array with index - [@itoufo](https://github.com/itoufo).
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Support with a capital S please.

* Your contribution here.

#### Fixes
Expand Down
25 changes: 25 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -912,6 +912,31 @@ params do
end
```

If type is Array, It support following kinds of parameter.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We never use curl in examples here, I don't think it's a good idea to introduce it. It's also fairly confusing TBH given that you POST to a JSON API, but don't actually send JSON data. I would explain this in Ruby, keeping it more like "documentation" and less like "example"?

#### Array
```sh
#without index
curl -X POST http://localhost:9292/api/v1/preference.json -d \
&preferences[][key]=foo \
&preferences[][value]=100 \
&preferences[][key]=bar \
&preferences[][value]=200

#with index
curl -X POST http://localhost:9292/api/v1/preference.json -d \
&preferences[0][key]=foo \
&preferences[1][key]=bar \
&preferences[0][value]=100 \
&preferences[1][value]=200
```

#### Hash
```sh
curl -X POST http://localhost:9292/api/v1/preference.json -d \
&preferences[key]=foo \
&preferences[value]=100 \
````

### Dependent Parameters

Suppose some of your parameters are only relevant if another parameter is given;
Expand Down
1 change: 1 addition & 0 deletions lib/grape.rb
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,7 @@ module Util
autoload :InheritableSetting
autoload :StrictHashConfiguration
autoload :FileResponse
autoload :HashParameter
end

module DSL
Expand Down
6 changes: 6 additions & 0 deletions lib/grape/dsl/parameters.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ module DSL
module Parameters
extend ActiveSupport::Concern

include Grape::Util::HashParameter

# Include reusable params rules among current.
# You can define reusable params with helpers method.
#
Expand Down Expand Up @@ -188,6 +190,7 @@ def declared_param?(param)
# @api private
def params(params)
params = @parent.params(params) if @parent

if @element
if params.is_a?(Array)
params = params.flat_map { |el| el[@element] || {} }
Expand All @@ -197,6 +200,9 @@ def params(params)
params = {}
end
end

params = params.values if params.is_a?(Hash) && deem_hash_array?(params)

params
end
end
Expand Down
20 changes: 20 additions & 0 deletions lib/grape/util/hash_parameter.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
require 'active_support/concern'
module Grape
module Util
module HashParameter
extend ActiveSupport::Concern

def deem_hash_array?(hash)
return false unless hash.is_a?(Hash) && hash.keys.any? { |key| integer_string?(key) }
true
end

def integer_string?(str)
Integer(str)
true
rescue ArgumentError, TypeError
false
end
end
end
end
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I am not in love with this. It brings an odd method, integer_string into whatever mixes it. I am not sure how to better refactor this, but at least not as a mixin maybe and just call class methods?

2 changes: 2 additions & 0 deletions lib/grape/validations/validators/base.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ module Validations
class Base
attr_reader :attrs

include Grape::Util::HashParameter

# Creates a new Validator from options specified
# by a +requires+ or +optional+ directive during
# parameter definition.
Expand Down
6 changes: 4 additions & 2 deletions lib/grape/validations/validators/coerce.rb
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,10 @@ def valid_type?(val)

# Allow nil, to ignore when a parameter is absent
return true if val.nil?

converter.value_coerced? val
unless !type.is_a?(Types::VariantCollectionCoercer) && type == Array && val.is_a?(Hash) && deem_hash_array?(val)
return converter.value_coerced? val
end
true
end

def coerce_value(val)
Expand Down
6 changes: 6 additions & 0 deletions spec/grape/validations/validators/default_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,12 @@ def app
expect(last_response.body).to eq({ array: [{ name: 'name', with_default: 'default' }, { name: 'name2', with_default: 'bar2' }] }.to_json)
end

it 'sets default values for grouped arrays with index' do
get('/array?array[0][name]=name&array[1][name]=name2&array[0][with_default]=bar1')
expect(last_response.status).to eq(200)
expect(last_response.body).to eq({ array: { 0 => { name: 'name', with_default: 'bar1' }, 1 => { name: 'name2', with_default: 'default' } } }.to_json)
end
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is a little weak on specs. Add separate tests for simpler scenarios as well, one for when the key is an Integer, another a string, then a mixed one, etc. So we see what's broken when it is as opposed as to "it doesn't work".


context 'optional group with defaults' do
subject do
Class.new(Grape::API) do
Expand Down