Merge pull request #8204 from nikitug/fix_dynamic_finder_result_check

Use nil? instead of blank? to check dynamic finder result

Conflicts:
	activerecord/CHANGELOG.md

4 files changed, 21 insertions, 2 deletions

diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
index b918bf66b3..7e6a3df956 100644
--- a/activerecord/CHANGELOG.md
+++ b/activerecord/CHANGELOG.md
@@ -1,6 +1,14 @@
 ## Rails 3.2.10 (unreleased)
 
-*   Fix deleting from a HABTM join table upon destroying an object of a model with optimistic locking enabled. Fixes #5332.
+*   Use `nil?` instead of `blank?` to check whether dynamic finder with a bang
+    should raise RecordNotFound.
+    Fixes #7238.
+
+    *Nikita Afanasenko*
+
+*   Fix deleting from a HABTM join table upon destroying an object of a model
+    with optimistic locking enabled.
+    Fixes #5332.
 
     *Nick Rogers*
 
diff --git a/activerecord/lib/active_record/relation/finder_methods.rb b/activerecord/lib/active_record/relation/finder_methods.rb
index abc67d9c15..44aeb309d6 100644
--- a/activerecord/lib/active_record/relation/finder_methods.rb
+++ b/activerecord/lib/active_record/relation/finder_methods.rb
@@ -263,7 +263,7 @@ module ActiveRecord
       conditions = Hash[attributes.map {|a| [a, args[attributes.index(a)]]}]
       result = where(conditions).send(match.finder)
 
-      if match.bang? && result.blank?
+      if match.bang? && result.nil?
         raise RecordNotFound, "Couldn't find #{@klass.name} with #{conditions.to_a.collect {|p| p.join(' = ')}.join(', ')}"
       else
         yield(result) if block_given?
diff --git a/activerecord/test/cases/finder_test.rb b/activerecord/test/cases/finder_test.rb
index 5d72e35c60..e50a334958 100644
--- a/activerecord/test/cases/finder_test.rb
+++ b/activerecord/test/cases/finder_test.rb
@@ -652,6 +652,11 @@ class FinderTest < ActiveRecord::TestCase
     assert_raise(ActiveRecord::RecordNotFound) { Topic.find_by_title!("The First Topic!") }
   end
 
+  def test_find_by_one_attribute_bang_with_blank_defined
+    blank_topic = BlankTopic.create(:title => "The Blank One")
+    assert_equal blank_topic, BlankTopic.find_by_title!("The Blank One")
+  end
+
   def test_find_by_one_attribute_with_order_option
     assert_equal accounts(:signals37), Account.find_by_credit_limit(50, :order => 'id')
     assert_equal accounts(:rails_core_account), Account.find_by_credit_limit(50, :order => 'id DESC')
diff --git a/activerecord/test/models/topic.rb b/activerecord/test/models/topic.rb
index 1a1a18166a..5166fefe81 100644
--- a/activerecord/test/models/topic.rb
+++ b/activerecord/test/models/topic.rb
@@ -112,6 +112,12 @@ class ImportantTopic < Topic
   serialize :important, Hash
 end
 
+class BlankTopic < Topic
+  def blank?
+    true
+  end
+end
+
 module Web
   class Topic < ActiveRecord::Base
     has_many :replies, :dependent => :destroy, :foreign_key => "parent_id", :class_name => 'Web::Reply'