Permitting arrays within a hash #121

Merged
merged 1 commit into from Mar 27, 2013

Conversation

Projects
None yet
8 participants
Contributor

simonc commented Mar 26, 2013

Hi,

After digging the strong_parameters code and trying several filters I can't find a way to permit something like this:

{
  modification: {
    opening_hours: {
      "0" => ["08:00 18:00"],
      "1" => ["08:00 18:00"]
    }
  }
}

Is there a specific filter that would allow this ? I tried the following:

params.require(:modification).permit(:opening_hours)
params.require(:modification).permit(opening_hours: [])
params.require(:modification).permit(opening_hours: {})
params.require(:modification).permit(opening_hours: ['0'])
params.require(:modification).permit(opening_hours: { '0' => [] })

The only thing I get so far is this:

{
  opening_hours: {
    "0" => nil,
    "1" => nil
  }
}

Is there a solution here ?
Thanks :)

👍 this is really frustrating. Right now we have to resource to some really ugly hacks.

Same problem

Owner

dhh commented Mar 24, 2013

params.require(:modification).permit(opening_hours: { '0' => [] }) is the one that should work. It won't work with a variable number of keys, though. To allow your entire bit to work, it should be: params.require(:modification).permit(opening_hours: { '0' => [], '1' => [] })

If that's not working, it's a bug. Please do dig in!

Owner

dhh commented Mar 24, 2013

Worth asking, though. Why is the value an array? Looks like there's no need for that?

Contributor

simonc commented Mar 24, 2013

Yeah sorry, not a really good example, the value could also be:

{
  modification: {
    opening_hours: {
      "0" => ["08:00 12:00", "14:00 19:00"],
      "1" => ["08:00 18:00"]
    }
  }
}
Owner

dhh commented Mar 24, 2013

Gotcha. The code I supplied is how it's supposed to be able to allow those parameters. If it's not working, there's a bug. Please do dig in and see if you can fix it and attach the PR to this ticket. Thanks!

On Mar 24, 2013, at 3:23 PM, Simon COURTOIS notifications@github.com wrote:

Yeah sorry, not a really good example, the value could also be:

{

modification: {

opening_hours: {

"0" => ["08:00 12:00", "14:00 19:00"],

"1" => ["08:00 18:00"]

}

}
}

Reply to this email directly or view it on GitHub.

Contributor

simonc commented Mar 24, 2013

I checked with this code:

params = ActionController::Parameters.new({
  modification: {
    opening_hours: {
      "0" => ["08:00 12:00", "14:00 19:00"],
      "1" => ["08:00 18:00"]
    }
  }
})

params.require(:modification).permit(opening_hours: { '0' => [], '1' => [] })
#=> {"opening_hours"=>{"0"=>nil, "1"=>nil}}

So I guess it's a bug. I'll try to find a way to fix this and PR.
Thanks for your feedback.

@simonc simonc Allowing arrays to be nested in numeric-key hashes
When params was of the form

    {
      :book => {
        :authors_attributes => {
          :'0' => ['William Shakespeare', '52'],
          :'1' => ['Unattributed Assistant']
        }
      }
    }

And its corresponding filter was

    {
      :book => {
        :authors_attributes => {
          :'0' => [],
          :'1' => []
        }
      }
    }

It resulted in params looking like this

    {
      :book => {
        :authors_attributes => {
          :'0' => nil,
          :'1' => nil
        }
      }
    }

This was due to the way hashes with all numeric keys are treated.
I just added a special case when a corresponding filter exists and is an empty
array.
ad2c982
Contributor

simonc commented Mar 26, 2013

I found a solution. It may not be the prettiest but it works and passes the tests.

@dhh dhh added a commit that referenced this pull request Mar 27, 2013

@dhh dhh Merge pull request #121 from simonc/121-arrays-within-a-hash
Permitting arrays within a hash
5e351d2

@dhh dhh merged commit 5e351d2 into rails:master Mar 27, 2013

1 check failed

default The Travis build failed
Details
Member

arthurnn commented Apr 1, 2013

👍

Owner

dhh commented Apr 2, 2013

@simonc, can you create a PR for rails/rails as well so we can get parity? Thanks!

Member

arthurnn commented Apr 2, 2013

lol @dhh , I was about to PR myself, as today I was having hard time with this on rails/rails 4 ...

Contributor

simonc commented Apr 4, 2013

@dhh I'll try but I'm on hollidays in California right now and the wifi in hotels is not so great, a simple bundle install is taking forever. I do it as soon as possible.

simonc deleted the simonc:121-arrays-within-a-hash branch Apr 4, 2013

Owner

fxn commented Apr 24, 2013

This solution seems suspicious to me at first sight.

Hashes whose keys are numeric in principle should be transparent. Point is you in general do not want to declare all possible indexes. Wouldn't

params.require(:modification).permit(opening_hours: [])

be the expected declaration? The same way today you declare

params.permit(:book => { :authors_attributes => [ :name ] })

to match

ActionController::Parameters.new({
  :book => {
    :authors_attributes => {
      :'0' => { :name => 'William Shakespeare', :age_of_death => '52' },
      :'1' => { :name => 'Unattributed Assistant' },
      :'2' => { :name => %w(injected names)}
    }
  }
})

?

Contributor

simonc commented Apr 24, 2013

It depends of the use cases I guess. In mine it's a limited set of indexes and they should be considered as any string indexes just as "name".
I guess allowing undefined numerical keys should be another subject but maybe I missing something :)

Owner

dhh commented Apr 24, 2013

Xavier, that looks reasonable to me.

On Apr 23, 2013, at 8:37 PM, Xavier Noria notifications@github.com wrote:

This solution seems suspicious to me at first sight.

Hashes whose keys are numeric in principle should be transparent. Point is you in general do not want to declare all possible indexes. Wouldn't

params.require(:modification).permit(opening_hours: [])
be the expected declaration? The same way today you declare

params.permit(:book => { :authors_attributes => [ :name ] })
to match

ActionController::Parameters.new({

:book => {

:authors_attributes => {

:'0' => { :name => 'William Shakespeare', :age_of_death => '52' },

:'1' => { :name => 'Unattributed Assistant' },

:'2' => { :name => %w(injected names)}

}

}
})
?


Reply to this email directly or view it on GitHub.

Owner

fxn commented Apr 24, 2013

Excellent David, I'll have a stab at it.

superp commented May 20, 2013

It stay have a bug at 0.2.1 version

# controller
params.require(:specialist).permit(:skills_attributes => [:id, :price, :subcategory_ids => {} ])

# incoming params
"specialist" => {"skills_attributes" => {"0"=>{"_destroy"=>"false", "subcategory_ids"=>{"22"=>"1", "23"=>"0", "24"=>"1"}

# in model
subcategory_ids # {"22"=>nil, "23"=>nil, "24"=>nil}
Contributor

simonc commented Jul 6, 2013

@superp I don't see how your example is supposed to work :/
skill_attributes is a hash and you're trying to permit it as an array.

Contributor

simonc commented Jul 7, 2013

@superp sorry I read it wrong. But still, I'm not sure it's supposed to work. I may be wrong though.

laffy commented on ad2c982 Aug 16, 2013

Was this ever pulled into rails?

Owner

simonc replied Aug 16, 2013

Not that I know of. There is a Pull Request though. No activity for some time :(

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