diff options
| -rw-r--r-- | activerecord/CHANGELOG | 2 | ||||
| -rw-r--r-- | activerecord/lib/active_record/relation/finder_methods.rb | 22 | ||||
| -rw-r--r-- | activerecord/test/cases/finder_test.rb | 18 |
3 files changed, 32 insertions, 10 deletions
diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG index ac5bd8e635..3f42fa34ef 100644 --- a/activerecord/CHANGELOG +++ b/activerecord/CHANGELOG @@ -1,5 +1,7 @@ *Rails 3.0.0 [beta 4/release candidate] (unreleased)* +* find_or_create_by_attr(value, ...) works when attr is protected. #4457 [Santiago Pastorino, Marc-André Lafortune] + * New callbacks: after_commit and after_rollback. Do expensive operations like image thumbnailing after_commit instead of after_save. #2991 [Brian Durand] * Serialized attributes are not converted to YAML if they are any of the formats that can be serialized to XML (like Hash, Array and Strings). [José Valim] diff --git a/activerecord/lib/active_record/relation/finder_methods.rb b/activerecord/lib/active_record/relation/finder_methods.rb index d6144dc206..f69acb6877 100644 --- a/activerecord/lib/active_record/relation/finder_methods.rb +++ b/activerecord/lib/active_record/relation/finder_methods.rb @@ -234,20 +234,24 @@ module ActiveRecord end def find_or_instantiator_by_attributes(match, attributes, *args) - guard_protected_attributes = false - - if args[0].is_a?(Hash) - guard_protected_attributes = true - attributes_for_create = args[0].with_indifferent_access - conditions = attributes_for_create.slice(*attributes).symbolize_keys - else - attributes_for_create = conditions = attributes.inject({}) {|h, a| h[a] = args[attributes.index(a)]; h} + protected_attributes_for_create = unprotected_attributes_for_create = {} + args.each_with_index do |arg, i| + if arg.is_a?(Hash) + protected_attributes_for_create = args[i].with_indifferent_access + else + unprotected_attributes_for_create[attributes[i]] = args[i] + end end + conditions = (protected_attributes_for_create.merge(unprotected_attributes_for_create)).slice(*attributes).symbolize_keys + record = where(conditions).first unless record - record = @klass.new { |r| r.send(:attributes=, attributes_for_create, guard_protected_attributes) } + record = @klass.new do |r| + r.send(:attributes=, protected_attributes_for_create, true) unless protected_attributes_for_create.empty? + r.send(:attributes=, unprotected_attributes_for_create, false) unless unprotected_attributes_for_create.empty? + end yield(record) if block_given? record.save if match.instantiator == :create end diff --git a/activerecord/test/cases/finder_test.rb b/activerecord/test/cases/finder_test.rb index 77b2b748b1..e78db8969d 100644 --- a/activerecord/test/cases/finder_test.rb +++ b/activerecord/test/cases/finder_test.rb @@ -840,7 +840,7 @@ class FinderTest < ActiveRecord::TestCase assert c.new_record? end - def test_find_or_create_from_one_attribute_should_set_not_attribute_even_when_protected + def test_find_or_create_from_one_attribute_should_not_set_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 @@ -864,6 +864,22 @@ class FinderTest < ActiveRecord::TestCase assert !c.new_record? end + def test_find_or_initialize_from_one_attribute_should_set_attribute_even_when_protected_and_also_set_the_hash + c = Company.find_or_initialize_by_rating(1000, {:name => "Fortune 1000"}) + assert_equal "Fortune 1000", c.name + assert_equal 1000, c.rating + assert c.valid? + assert c.new_record? + end + + def test_find_or_create_from_one_attribute_should_set_attribute_even_when_protected_and_also_set_the_hash + c = Company.find_or_create_by_rating(1000, {:name => "Fortune 1000"}) + assert_equal "Fortune 1000", c.name + assert_equal 1000, c.rating + assert c.valid? + 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 |