Permalink
Browse files

Fixed that ActiveRecord#Base.find_or_create/initialize would not hono…

…r attr_protected/accessible when used with a hash (closes #11422) [miloops]

git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@9090 5ecf4fe2-1ee6-0310-87b1-e25e094e27de
  • Loading branch information...
1 parent 4942e5b commit c10b2255b580de9b763bab28872ebf3434a16d8f @dhh dhh committed Mar 25, 2008
Showing with 46 additions and 4 deletions.
  1. +2 −0 activerecord/CHANGELOG
  2. +12 −4 activerecord/lib/active_record/base.rb
  3. +32 −0 activerecord/test/cases/finder_test.rb
View
@@ -1,5 +1,7 @@
*SVN*
+* Fixed that ActiveRecord#Base.find_or_create/initialize would not honor attr_protected/accessible when used with a hash #11422 [miloops]
+
* Added ActiveRecord#Base.all/first/last as aliases for find(:all/:first/:last) #11413 [nkallen, thechrisoshow]
* Merge the has_finder gem, renamed as 'named_scope'. #11404 [nkallen]
@@ -255,15 +255,18 @@ def initialize(errors)
# actually Person.find_by_user_name(user_name, options). So you could call <tt>Payment.find_all_by_amount(50, :order => "created_on")</tt>.
#
# The same dynamic finder style can be used to create the object if it doesn't already exist. This dynamic finder is called with
- # <tt>find_or_create_by_</tt> and will return the object if it already exists and otherwise creates it, then returns it. Example:
+ # <tt>find_or_create_by_</tt> and will return the object if it already exists and otherwise creates it, then returns it. Protected attributes won't be setted unless they are given in a block. For example:
#
# # No 'Summer' tag exists
# Tag.find_or_create_by_name("Summer") # equal to Tag.create(:name => "Summer")
#
# # Now the 'Summer' tag does exist
# Tag.find_or_create_by_name("Summer") # equal to Tag.find_by_name("Summer")
#
- # Use the <tt>find_or_initialize_by_</tt> finder if you want to return a new record without saving it first. Example:
+ # # Now 'Bob' exist and is an 'admin'
+ # User.find_or_create_by_name('Bob', :age => 40) { |u| u.admin = true }
+ #
+ # Use the <tt>find_or_initialize_by_</tt> finder if you want to return a new record without saving it first. Protected attributes won't be setted unless they are given in a block. For example:
#
# # No 'Winter' tag exists
# winter = Tag.find_or_initialize_by_name("Winter")
@@ -1591,7 +1594,10 @@ def self.#{method_id}(*args)
self.class_eval %{
def self.#{method_id}(*args)
+ guard_protected_attributes = false
+
if args[0].is_a?(Hash)
+ guard_protected_attributes = true
attributes = args[0].with_indifferent_access
find_attributes = attributes.slice(*[:#{attribute_names.join(',:')}])
else
@@ -1602,8 +1608,10 @@ def self.#{method_id}(*args)
set_readonly_option!(options)
record = find_initial(options)
- if record.nil?
- record = self.new { |r| r.send(:attributes=, attributes, false) }
+
+ if record.nil?
+ record = self.new { |r| r.send(:attributes=, attributes, guard_protected_attributes) }
+ #{'yield(record) if block_given?'}
#{'record.save' if instantiator == :create}
record
else
@@ -653,6 +653,22 @@ def test_find_or_initialize_from_one_aggregate_attribute
assert new_customer.new_record?
end
+ def test_find_or_initialize_from_one_attribute_should_not_set_attribute_even_when_protected
+ c = Company.find_or_initialize_by_name({:name => "Fortune 1000", :rating => 1000})
+ assert_equal "Fortune 1000", c.name
+ assert_not_equal 1000, c.rating
+ assert c.valid?
+ assert c.new_record?
+ end
+
+ def test_find_or_create_from_one_attribute_should_set_not_attribute_even_when_protected
+ c = Company.find_or_create_by_name({:name => "Fortune 1000", :rating => 1000})
+ assert_equal "Fortune 1000", c.name
+ assert_not_equal 1000, c.rating
+ assert c.valid?
+ assert !c.new_record?
+ end
+
def test_find_or_initialize_from_one_attribute_should_set_attribute_even_when_protected
c = Company.find_or_initialize_by_name_and_rating("Fortune 1000", 1000)
assert_equal "Fortune 1000", c.name
@@ -669,6 +685,22 @@ def test_find_or_create_from_one_attribute_should_set_attribute_even_when_protec
assert !c.new_record?
end
+ def test_find_or_initialize_should_set_protected_attributes_if_given_as_block
+ c = Company.find_or_initialize_by_name(:name => "Fortune 1000") { |f| f.rating = 1000 }
+ assert_equal "Fortune 1000", c.name
+ assert_equal 1000.to_f, c.rating.to_f
+ assert c.valid?
+ assert c.new_record?
+ end
+
+ def test_find_or_create_should_set_protected_attributes_if_given_as_block
+ c = Company.find_or_create_by_name(:name => "Fortune 1000") { |f| f.rating = 1000 }
+ assert_equal "Fortune 1000", c.name
+ assert_equal 1000.to_f, c.rating.to_f
+ assert c.valid?
+ assert !c.new_record?
+ end
+
def test_dynamic_find_or_initialize_from_one_attribute_caches_method
class << Company; self; end.send(:remove_method, :find_or_initialize_by_name) if Company.respond_to?(:find_or_initialize_by_name)
assert !Company.respond_to?(:find_or_initialize_by_name)

0 comments on commit c10b225

Please sign in to comment.