From c10b2255b580de9b763bab28872ebf3434a16d8f Mon Sep 17 00:00:00 2001 From: David Heinemeier Hansson Date: Tue, 25 Mar 2008 23:56:48 +0000 Subject: Fixed that ActiveRecord#Base.find_or_create/initialize would not honor 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 --- activerecord/CHANGELOG | 2 ++ activerecord/lib/active_record/base.rb | 16 ++++++++++++---- activerecord/test/cases/finder_test.rb | 32 ++++++++++++++++++++++++++++++++ 3 files changed, 46 insertions(+), 4 deletions(-) diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG index d4dccb08b1..7b218900a7 100644 --- a/activerecord/CHANGELOG +++ b/activerecord/CHANGELOG @@ -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] diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index 4f9fd71ffc..eeb728f60d 100755 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -255,7 +255,7 @@ module ActiveRecord #:nodoc: # actually Person.find_by_user_name(user_name, options). So you could call Payment.find_all_by_amount(50, :order => "created_on"). # # The same dynamic finder style can be used to create the object if it doesn't already exist. This dynamic finder is called with - # find_or_create_by_ and will return the object if it already exists and otherwise creates it, then returns it. Example: + # find_or_create_by_ 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") @@ -263,7 +263,10 @@ module ActiveRecord #:nodoc: # # Now the 'Summer' tag does exist # Tag.find_or_create_by_name("Summer") # equal to Tag.find_by_name("Summer") # - # Use the find_or_initialize_by_ 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 find_or_initialize_by_ 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 @@ module ActiveRecord #:nodoc: 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 @@ module ActiveRecord #:nodoc: 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 diff --git a/activerecord/test/cases/finder_test.rb b/activerecord/test/cases/finder_test.rb index 8cb5c9206a..79907c9c64 100644 --- a/activerecord/test/cases/finder_test.rb +++ b/activerecord/test/cases/finder_test.rb @@ -653,6 +653,22 @@ class FinderTest < ActiveRecord::TestCase 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 @@ class FinderTest < ActiveRecord::TestCase 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) -- cgit v1.2.3