From 58c5261efa6ca0134ebfac8a701915d5e36d2c25 Mon Sep 17 00:00:00 2001
From: Godfrey Chan <godfreykfc@gmail.com>
Date: Sun, 24 Aug 2014 04:42:12 -0700
Subject: [PATCH] Fixed find_by("sql fragment without bindings") on master

* Also duplicated find_by tests from relations_test.rb to finder_test.rb now
	that we have a completely different implementation on the class (in core.rb
	with AST caching stuff).

* Also removed a (failing) test that used mocks. Now that we have tests for the
  behavior, there's no point having another test that tests the implementation
	(that it delegates). Further, what the test was implying is nolonger true with
	the current implementation, because Class.find_by is a real method now.
---
 activerecord/lib/active_record/core.rb    |  2 +-
 activerecord/test/cases/base_test.rb      | 14 --------------
 activerecord/test/cases/finder_test.rb    | 20 ++++++++++++++++++++
 activerecord/test/cases/relations_test.rb |  4 ++--
 4 files changed, 23 insertions(+), 17 deletions(-)

diff --git a/activerecord/lib/active_record/core.rb b/activerecord/lib/active_record/core.rb
index d22806fbdf..2cbb8442a1 100644
--- a/activerecord/lib/active_record/core.rb
+++ b/activerecord/lib/active_record/core.rb
@@ -151,7 +151,7 @@ module ActiveRecord
       end
 
       def find_by(*args)
-        return super if current_scope || args.length > 1 || reflect_on_all_aggregations.any?
+        return super if current_scope || !(Hash === args.first) || reflect_on_all_aggregations.any?
 
         hash = args.first
 
diff --git a/activerecord/test/cases/base_test.rb b/activerecord/test/cases/base_test.rb
index 4c0b0c868a..fb535e74fc 100644
--- a/activerecord/test/cases/base_test.rb
+++ b/activerecord/test/cases/base_test.rb
@@ -1540,20 +1540,6 @@ class BasicsTest < ActiveRecord::TestCase
     assert_equal "", Company.new.description
   end
 
-  ["find_by", "find_by!"].each do |meth|
-    test "#{meth} delegates to scoped" do
-      record = stub
-
-      scope = mock
-      scope.expects(meth).with(:foo, :bar).returns(record)
-
-      klass = Class.new(ActiveRecord::Base)
-      klass.stubs(:all => scope)
-
-      assert_equal record, klass.public_send(meth, :foo, :bar)
-    end
-  end
-
   test "scoped can take a values hash" do
     klass = Class.new(ActiveRecord::Base)
     assert_equal ['foo'], klass.all.merge!(select: 'foo').select_values
diff --git a/activerecord/test/cases/finder_test.rb b/activerecord/test/cases/finder_test.rb
index b42a60fea5..9129dbaf63 100644
--- a/activerecord/test/cases/finder_test.rb
+++ b/activerecord/test/cases/finder_test.rb
@@ -1027,6 +1027,26 @@ class FinderTest < ActiveRecord::TestCase
     assert_nothing_raised(ActiveRecord::StatementInvalid) { Topic.offset("3").to_a }
   end
 
+  test "find_by with hash conditions returns the first matching record" do
+    assert_equal posts(:eager_other), Post.find_by(id: posts(:eager_other).id)
+  end
+
+  test "find_by with non-hash conditions returns the first matching record" do
+    assert_equal posts(:eager_other), Post.find_by("id = #{posts(:eager_other).id}")
+  end
+
+  test "find_by with multi-arg conditions returns the first matching record" do
+    assert_equal posts(:eager_other), Post.find_by('id = ?', posts(:eager_other).id)
+  end
+
+  test "find_by returns nil if the record is missing" do
+    assert_equal nil, Post.find_by("1 = 0")
+  end
+
+  test "find_by doesn't have implicit ordering" do
+    assert_sql(/^((?!ORDER).)*$/) { Post.find_by(id: posts(:eager_other).id) }
+  end
+
   protected
     def bind(statement, *vars)
       if vars.first.is_a?(Hash)
diff --git a/activerecord/test/cases/relations_test.rb b/activerecord/test/cases/relations_test.rb
index 88df997a2f..cc6a3888e3 100644
--- a/activerecord/test/cases/relations_test.rb
+++ b/activerecord/test/cases/relations_test.rb
@@ -1570,7 +1570,7 @@ class RelationTest < ActiveRecord::TestCase
   end
 
   test "find_by doesn't have implicit ordering" do
-    assert_sql(/^((?!ORDER).)*$/) { Post.find_by(author_id: 2) }
+    assert_sql(/^((?!ORDER).)*$/) { Post.all.find_by(author_id: 2) }
   end
 
   test "find_by! with hash conditions returns the first matching record" do
@@ -1586,7 +1586,7 @@ class RelationTest < ActiveRecord::TestCase
   end
 
   test "find_by! doesn't have implicit ordering" do
-    assert_sql(/^((?!ORDER).)*$/) { Post.find_by!(author_id: 2) }
+    assert_sql(/^((?!ORDER).)*$/) { Post.all.find_by!(author_id: 2) }
   end
 
   test "find_by! raises RecordNotFound if the record is missing" do