Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ParamsWrapper only wrap the accessible attributes when they were set #3900

Merged
merged 2 commits into from Dec 8, 2011
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 5 additions & 0 deletions actionpack/CHANGELOG.md
Expand Up @@ -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
Expand Down
9 changes: 8 additions & 1 deletion actionpack/lib/action_controller/metal/params_wrapper.rb
Expand Up @@ -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
Expand Down Expand Up @@ -162,7 +167,9 @@ def _set_wrapper_defaults(options, model=nil)

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
Expand Down
29 changes: 28 additions & 1 deletion actionpack/test/controller/params_wrapper_test.rb
Expand Up @@ -26,7 +26,7 @@ def parse
self.class.last_parameters = request.params.except(:controller, :action)
head :ok
end
end
end

class User; end
class Person; end
Expand Down Expand Up @@ -147,6 +147,7 @@ def test_nested_params
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"])

Expand All @@ -159,6 +160,7 @@ def test_derived_wrapped_keys_from_matching_model

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"])

Expand All @@ -169,8 +171,33 @@ def test_derived_wrapped_keys_from_specified_model
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([])

Expand Down