Dynamic finders should use the ActiveRecord::Base::find method instead of ::find_initial, :find_last, and ::find_all.
This is so when people override ActiveRecord::Base::find, the new ::find method will also be invoked by the dynamic finders. Associations for instance do go through ::find, so this makes it more consistent. Also removed the unnecessary deprecation silence blocks. Signed-off-by: Michael Koziarski <michael@koziarski.com> [#1162 state:committed]
This commit is contained in:
parent
e69b506abd
commit
8d337e9ec2
@ -1724,10 +1724,10 @@ def self.#{method_id}(*args)
|
||||
|
||||
#{'result = ' if bang}if options[:conditions]
|
||||
with_scope(:find => finder_options) do
|
||||
ActiveSupport::Deprecation.silence { send(:#{finder}, options) }
|
||||
find(:#{finder}, options)
|
||||
end
|
||||
else
|
||||
ActiveSupport::Deprecation.silence { send(:#{finder}, options.merge(finder_options)) }
|
||||
find(:#{finder}, options.merge(finder_options))
|
||||
end
|
||||
#{'result || raise(RecordNotFound)' if bang}
|
||||
end
|
||||
@ -1750,9 +1750,9 @@ def self.#{method_id}(*args)
|
||||
options = { :conditions => find_attributes }
|
||||
set_readonly_option!(options)
|
||||
|
||||
record = find_initial(options)
|
||||
record = find(:first, options)
|
||||
|
||||
if record.nil?
|
||||
if record.nil?
|
||||
record = self.new { |r| r.send(:attributes=, attributes, guard_protected_attributes) }
|
||||
#{'yield(record) if block_given?'}
|
||||
#{'record.save' if instantiator == :create}
|
||||
|
@ -6,11 +6,11 @@ def self.match(method)
|
||||
end
|
||||
|
||||
def initialize(method)
|
||||
@finder = :find_initial
|
||||
@finder = :first
|
||||
case method.to_s
|
||||
when /^find_(all_by|last_by|by)_([_a-zA-Z]\w*)$/
|
||||
@finder = :find_last if $1 == 'last_by'
|
||||
@finder = :find_every if $1 == 'all_by'
|
||||
@finder = :last if $1 == 'last_by'
|
||||
@finder = :all if $1 == 'all_by'
|
||||
names = $2
|
||||
when /^find_by_([_a-zA-Z]\w*)\!$/
|
||||
@bang = true
|
||||
@ -31,7 +31,7 @@ def finder?
|
||||
end
|
||||
|
||||
def instantiator?
|
||||
@finder == :find_initial && !@instantiator.nil?
|
||||
@finder == :first && !@instantiator.nil?
|
||||
end
|
||||
|
||||
def bang?
|
||||
|
@ -21,7 +21,7 @@ def test_find_by
|
||||
match = ActiveRecord::DynamicFinderMatch.match("find_by_age_and_sex_and_location")
|
||||
assert_not_nil match
|
||||
assert match.finder?
|
||||
assert_equal :find_initial, match.finder
|
||||
assert_equal :first, match.finder
|
||||
assert_equal %w(age sex location), match.attribute_names
|
||||
end
|
||||
|
||||
@ -30,7 +30,7 @@ def find_by_bang
|
||||
assert_not_nil match
|
||||
assert match.finder?
|
||||
assert match.bang?
|
||||
assert_equal :find_initial, match.finder
|
||||
assert_equal :first, match.finder
|
||||
assert_equal %w(age sex location), match.attribute_names
|
||||
end
|
||||
|
||||
@ -38,7 +38,7 @@ def test_find_all_by
|
||||
match = ActiveRecord::DynamicFinderMatch.match("find_all_by_age_and_sex_and_location")
|
||||
assert_not_nil match
|
||||
assert match.finder?
|
||||
assert_equal :find_every, match.finder
|
||||
assert_equal :all, match.finder
|
||||
assert_equal %w(age sex location), match.attribute_names
|
||||
end
|
||||
|
||||
@ -47,7 +47,7 @@ def test_find_or_initialize_by
|
||||
assert_not_nil match
|
||||
assert !match.finder?
|
||||
assert match.instantiator?
|
||||
assert_equal :find_initial, match.finder
|
||||
assert_equal :first, match.finder
|
||||
assert_equal :new, match.instantiator
|
||||
assert_equal %w(age sex location), match.attribute_names
|
||||
end
|
||||
@ -57,7 +57,7 @@ def test_find_or_create_by
|
||||
assert_not_nil match
|
||||
assert !match.finder?
|
||||
assert match.instantiator?
|
||||
assert_equal :find_initial, match.finder
|
||||
assert_equal :first, match.finder
|
||||
assert_equal :create, match.instantiator
|
||||
assert_equal %w(age sex location), match.attribute_names
|
||||
end
|
||||
@ -500,6 +500,23 @@ def test_count_by_sql
|
||||
assert_equal(2, Entrant.count_by_sql(["SELECT COUNT(*) FROM entrants WHERE id > ?", 1]))
|
||||
end
|
||||
|
||||
uses_mocha('test_dynamic_finder_should_go_through_the_find_class_method') do
|
||||
def test_dynamic_finders_should_go_through_the_find_class_method
|
||||
Topic.expects(:find).with(:first, :conditions => { :title => 'The First Topic!' })
|
||||
Topic.find_by_title("The First Topic!")
|
||||
|
||||
Topic.expects(:find).with(:last, :conditions => { :title => 'The Last Topic!' })
|
||||
Topic.find_last_by_title("The Last Topic!")
|
||||
|
||||
Topic.expects(:find).with(:all, :conditions => { :title => 'A Topic.' })
|
||||
Topic.find_all_by_title("A Topic.")
|
||||
|
||||
Topic.expects(:find).with(:first, :conditions => { :title => 'Does not exist yet for sure!' }).times(2)
|
||||
Topic.find_or_initialize_by_title('Does not exist yet for sure!')
|
||||
Topic.find_or_create_by_title('Does not exist yet for sure!')
|
||||
end
|
||||
end
|
||||
|
||||
def test_find_by_one_attribute
|
||||
assert_equal topics(:first), Topic.find_by_title("The First Topic")
|
||||
assert_nil Topic.find_by_title("The First Topic!")
|
||||
|
Loading…
Reference in New Issue
Block a user