From b79fc882470b3c55a095f99d9edc61813666b79a Mon Sep 17 00:00:00 2001
From: Santiago Pastorino <santiago@wyeworks.com>
Date: Thu, 6 May 2010 23:47:23 -0300
Subject: [PATCH] Make find_or_create and find_or_initialize work mixing explicit parameters and a hash [#4457 state:committed]

---
 .../lib/active_record/relation/finder_methods.rb   |   22 +++++++++++--------
 activerecord/test/cases/finder_test.rb             |   18 +++++++++++++++-
 2 files changed, 30 insertions(+), 10 deletions(-)

diff --git a/activerecord/lib/active_record/relation/finder_methods.rb b/activerecord/lib/active_record/relation/finder_methods.rb
index d6144dc..f69acb6 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 77b2b74..e78db89 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
-- 
1.7.0