From d5526218e43072a3b9b4a55568f4b29b3a2c0445 Mon Sep 17 00:00:00 2001 From: Jean-Francois Turcot Date: Wed, 7 Dec 2011 22:50:01 -0500 Subject: ParamsWrapper only wrap the accessible attributes when they were set --- .../lib/action_controller/metal/params_wrapper.rb | 9 ++++++- actionpack/test/controller/params_wrapper_test.rb | 29 +++++++++++++++++++++- 2 files changed, 36 insertions(+), 2 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_controller/metal/params_wrapper.rb b/actionpack/lib/action_controller/metal/params_wrapper.rb index 9dcea86253..5c28a8074f 100644 --- a/actionpack/lib/action_controller/metal/params_wrapper.rb +++ b/actionpack/lib/action_controller/metal/params_wrapper.rb @@ -43,6 +43,11 @@ module ActionController # wrap_parameters :person, :include => [:username, :password] # end # + # On ActiveRecord models with no +:include+ or +:exclude+ option set, + # if attr_accessible is set on that model, it will only wrap the accessible + # parameters, else it will only wrap the parameters returned by the class + # method attribute_names. + # # If you're going to pass the parameters to an +ActiveModel+ object (such as # +User.new(params[:user])+), you might consider passing the model class to # the method instead. The +ParamsWrapper+ will actually try to determine the @@ -162,7 +167,9 @@ module ActionController unless options[:include] || options[:exclude] model ||= _default_wrap_model - if model.respond_to?(:attribute_names) && model.attribute_names.present? + if model.respond_to?(:accessible_attributes) && model.accessible_attributes.present? + options[:include] = model.accessible_attributes.to_a + elsif model.respond_to?(:attribute_names) && model.attribute_names.present? options[:include] = model.attribute_names end end diff --git a/actionpack/test/controller/params_wrapper_test.rb b/actionpack/test/controller/params_wrapper_test.rb index 0102f66dfe..a4c6c08abb 100644 --- a/actionpack/test/controller/params_wrapper_test.rb +++ b/actionpack/test/controller/params_wrapper_test.rb @@ -26,7 +26,7 @@ class ParamsWrapperTest < ActionController::TestCase self.class.last_parameters = request.params.except(:controller, :action) head :ok end - end +end class User; end class Person; end @@ -147,6 +147,7 @@ class ParamsWrapperTest < ActionController::TestCase end def test_derived_wrapped_keys_from_matching_model + User.expects(:respond_to?).with(:accessible_attributes).returns(false) User.expects(:respond_to?).with(:attribute_names).returns(true) User.expects(:attribute_names).twice.returns(["username"]) @@ -159,6 +160,7 @@ class ParamsWrapperTest < ActionController::TestCase def test_derived_wrapped_keys_from_specified_model with_default_wrapper_options do + Person.expects(:respond_to?).with(:accessible_attributes).returns(false) Person.expects(:respond_to?).with(:attribute_names).returns(true) Person.expects(:attribute_names).twice.returns(["username"]) @@ -169,8 +171,33 @@ class ParamsWrapperTest < ActionController::TestCase assert_parameters({ 'username' => 'sikachu', 'title' => 'Developer', 'person' => { 'username' => 'sikachu' }}) end end + + def test_accessible_wrapped_keys_from_matching_model + User.expects(:respond_to?).with(:accessible_attributes).returns(true) + User.expects(:accessible_attributes).twice.returns(["username"]) + + with_default_wrapper_options do + @request.env['CONTENT_TYPE'] = 'application/json' + post :parse, { 'username' => 'sikachu', 'title' => 'Developer' } + assert_parameters({ 'username' => 'sikachu', 'title' => 'Developer', 'user' => { 'username' => 'sikachu' }}) + end + end + + def test_accessible_wrapped_keys_from_specified_model + with_default_wrapper_options do + Person.expects(:respond_to?).with(:accessible_attributes).returns(true) + Person.expects(:accessible_attributes).twice.returns(["username"]) + + UsersController.wrap_parameters Person + + @request.env['CONTENT_TYPE'] = 'application/json' + post :parse, { 'username' => 'sikachu', 'title' => 'Developer' } + assert_parameters({ 'username' => 'sikachu', 'title' => 'Developer', 'person' => { 'username' => 'sikachu' }}) + end + end def test_not_wrapping_abstract_model + User.expects(:respond_to?).with(:accessible_attributes).returns(false) User.expects(:respond_to?).with(:attribute_names).returns(true) User.expects(:attribute_names).returns([]) -- cgit v1.2.3 From 677f968b771c837ae9bf4d9117372717e1bb6c11 Mon Sep 17 00:00:00 2001 From: Jean-Francois Turcot Date: Thu, 8 Dec 2011 04:03:55 -0500 Subject: Add information to the changelog about the changes to ActionController::ParamsWrapper --- actionpack/CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'actionpack') diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index 8844d6e6f6..473115dc6d 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -106,6 +106,11 @@ persistent between requests so if you need to manipulate the environment for your test you need to do it before the cookie jar is created. +* ActionController::ParamsWrapper on ActiveRecord models now only wrap + attr_accessible attributes if they were set, if not, only the attributes + returned by the class method attribute_names will be wrapped. This fixes + the wrapping of nested attributes by adding them to attr_accessible. + ## Rails 3.1.4 (unreleased) ## * Allow to use asset_path on named_routes aliasing RailsHelper's -- cgit v1.2.3