Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Modify strong_parameters to be more compliant with accepts_nested_attributes_for #87

Open
wants to merge 2 commits into from

2 participants

@HurricaneJames

This is a follow up to #86

I wanted to make strong_parameters work with accepts_nested_attributes_for when fields_for was not the method used to generate fields. In the previous discussion there was some argument about having to dynamically define the permitted fields. However, that is not necessary because the keys do not matter, they do not mean anything, accepts_nested_attributes_for ignores them. In fact, the current strong_parameters code takes this into account by looking for any hash with all numeric keys, assuming that hash is going to be used with accepts_nested_attributes_for, and throwing out the keys.

So, I spent a couple hours learning how strong_parameters works and realized that my previous solution was not particularly good (read pretty bad). It didn't cause any problems with my code, but it did cause some with the gem's tests. Anyway, I went back and did some thinking. I realized that what we really need to look for isn't a regex on every value in the hash, but the name of the hash. If its name ends in _attributes then it should be treated as an attributes hash where the keys are ignored and the values are processed.

This pull request addresses changes the each_element method to do a single regex check on the original key name from hash_filter method. I also tossed in a couple of happy pass tests to make sure that strong_parameters was processing _attributes hashes with random keys and no keys (which are actually arrays not hashes) in addition to the numeric keys fields_for generates.

HurricaneJames added some commits
@HurricaneJames HurricaneJames Changed the assumption that accepts_nested_attributes_for will only b…
…e used with fields_for and the arbitrary decimal number keys. Instead, the code now looks to see if the key ends in _attributes, and deals with such hashes in a more acceptes_nested_attributes_for compliant way. Added a test to check that fields_attributes with random keys are processed properly.
aa40d67
@HurricaneJames HurricaneJames added test for accepts_nested_attributes_for with no keys 943b14b
@oivoodoo

Hi, @HurricaneJames

I've created the gist probably it could help you to deal with the none numeric keys in the nested attributes.

gist

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Jan 22, 2013
  1. @HurricaneJames

    Changed the assumption that accepts_nested_attributes_for will only b…

    HurricaneJames authored
    …e used with fields_for and the arbitrary decimal number keys. Instead, the code now looks to see if the key ends in _attributes, and deals with such hashes in a more acceptes_nested_attributes_for compliant way. Added a test to check that fields_attributes with random keys are processed properly.
  2. @HurricaneJames
This page is out of date. Refresh to see the latest.
View
8 lib/action_controller/parameters.rb
@@ -167,7 +167,7 @@ def hash_filter(params, filter)
array_of_permitted_scalars_filter(params, key)
else
# Declaration {:user => :name} or {:user => [:name, :age, {:adress => ...}]}.
- params[key] = each_element(value) do |element|
+ params[key] = each_element(key, value) do |element|
if element.is_a?(Hash)
element = self.class.new(element) unless element.respond_to?(:permit)
element.permit(*Array.wrap(filter[key]))
@@ -177,11 +177,11 @@ def hash_filter(params, filter)
end
end
- def each_element(value)
+ def each_element(key, value)
if value.is_a?(Array)
value.map { |el| yield el }.compact
- # fields_for on an array of records uses numeric hash keys.
- elsif value.is_a?(Hash) && value.keys.all? { |k| k =~ /\A-?\d+\z/ }
+ # fields_for on an array of records will label the key as fields_attributes
+ elsif value.is_a?(Hash) && key =~ /_attributes$/
hash = value.class.new
value.each { |k,v| hash[k] = yield v }
hash
View
42 test/parameters_permit_test.rb
@@ -290,4 +290,46 @@ def assert_filtered_out(params, key)
assert_filtered_out permitted[:book][:authors_attributes]['-1'], :age_of_death
end
+
+ test "acceptes_nested_attributes_for_style_nested_params random keys" do
+ params = ActionController::Parameters.new({
+ :entry_form => {
+ :user_responses_attributes => {
+ :'first_name' => { :response => 'William', :entry_field_id => '52', :stip_me => 'Dali' },
+ :'last_name' => { :response => 'Shakespeare', :entry_field_id => '27' },
+ :'phone' => { :response => %w(this bad phone number should be stripped), :entry_field_id => '16' }
+ }
+ }
+ })
+ permitted = params.permit :entry_form => { :user_responses_attributes => [ :response, :entry_field_id ] }
+
+ assert_not_nil permitted[:entry_form][:user_responses_attributes]['first_name']
+ assert_not_nil permitted[:entry_form][:user_responses_attributes]['last_name']
+ assert_equal 'William', permitted[:entry_form][:user_responses_attributes]['first_name'][:response]
+ assert_equal 'Shakespeare', permitted[:entry_form][:user_responses_attributes]['last_name'][:response]
+
+ assert_filtered_out permitted[:entry_form][:user_responses_attributes]['phone'], :response
+ assert_filtered_out permitted[:entry_form][:user_responses_attributes]['first_name'], :strip_me
+ end
+
+ test "acceptes_nested_attributes_for_style_nested_params no keys" do
+ params = ActionController::Parameters.new({
+ :entry_form => {
+ :user_responses_attributes => [
+ { :response => 'William', :entry_field_id => '52', :stip_me => 'Dali' },
+ { :response => 'Shakespeare', :entry_field_id => '27' },
+ { :response => %w(this bad phone number should be stripped), :entry_field_id => '16' }
+ ]
+ }
+ })
+ permitted = params.permit :entry_form => { :user_responses_attributes => [ :response, :entry_field_id ] }
+
+ assert_not_nil permitted[:entry_form][:user_responses_attributes][0]
+ assert_not_nil permitted[:entry_form][:user_responses_attributes][1]
+ assert_equal 'William', permitted[:entry_form][:user_responses_attributes][0][:response]
+ assert_equal 'Shakespeare', permitted[:entry_form][:user_responses_attributes][1][:response]
+
+ assert_filtered_out permitted[:entry_form][:user_responses_attributes][2], :response
+ assert_filtered_out permitted[:entry_form][:user_responses_attributes][0], :strip_me
+ end
end
Something went wrong with that request. Please try again.