mirror of
https://github.com/github/rails.git
synced 2026-01-10 07:07:54 -05:00
Use nil? instead of blank? to check dynamic finder result
It's safe to use `nil?` instead of `blank?` because it's impossible to get an array on finder with bang; `all_by` finder matches against regex without bang: `when /^find_(all_|last_)?by_([_a-zA-Z]\w*)$/`. Fixes #7238
This commit is contained in:
@@ -1,5 +1,10 @@
|
||||
## Rails 3.2.10 (unreleased)
|
||||
|
||||
* Use `nil?` instead of `blank?` to check whether dynamic finder with a bang
|
||||
should raise RecordNotFound. Fixes #7238.
|
||||
|
||||
*Nikita Afanasenko*
|
||||
|
||||
* Use query cache/uncache when using ENV["DATABASE_URL"].
|
||||
Fixes #6951. [Backport #8074]
|
||||
|
||||
|
||||
@@ -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?
|
||||
|
||||
@@ -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')
|
||||
|
||||
@@ -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'
|
||||
|
||||
Reference in New Issue
Block a user