Permalink
Browse files

Merge branch 'master' into adequaterecord

* master:
  passing an instance of an AR object to `find` is deprecated
  passing an ActiveRecord object to `exists?` is deprecated.
  • Loading branch information...
2 parents d99974b + d35f003 commit 7658dc37a40077444fee1eafcf4a59fae1b441a5 @tenderlove tenderlove committed Mar 13, 2014
@@ -1,3 +1,9 @@
+* Passing an Active Record object to `find` is now deprecated. Call `.id`
+ on the object first.
+
+* Passing an Active Record object to `exists?` is now deprecated. Call `.id`
+ on the object first.
+
* Only use BINARY for mysql case sensitive uniqueness check when column has a case insensitive collation.
*Ryuta Kamizono*
@@ -370,7 +370,7 @@ def include?(record)
if record.new_record?
include_in_memory?(record)
else
- loaded? ? target.include?(record) : scope.exists?(record)
+ loaded? ? target.include?(record) : scope.exists?(record.id)
end
else
false
@@ -1,3 +1,5 @@
+require 'active_support/deprecation'
+
module ActiveRecord
module FinderMethods
ONE_AS_ONE = '1 AS one'
@@ -280,7 +282,12 @@ def forty_two!
# Person.exists?(false)
# Person.exists?
def exists?(conditions = :none)
- conditions = conditions.id if Base === conditions
+ if Base === conditions
+ conditions = conditions.id
+ ActiveSupport::Deprecation.warn "You are passing an instance of ActiveRecord::Base to `exists?`." \
+ "Please pass the id of the object by calling `.id`"
+ end
+
return false if !conditions
relation = apply_join_dependency(self, construct_join_dependency)
@@ -412,7 +419,11 @@ def find_with_ids(*ids)
end
def find_one(id)
- id = id.id if ActiveRecord::Base === id
+ if ActiveRecord::Base === id
+ id = id.id
+ ActiveSupport::Deprecation.warn "You are passing an instance of ActiveRecord::Base to `find`." \
+ "Please pass the id of the object by calling `.id`"
+ end
column = columns_hash[primary_key]
substitute = connection.substitute_at(column, bind_values.length)
@@ -407,19 +407,19 @@ def test_eager_load_belongs_to_quotes_table_and_column_names
end
def test_eager_load_has_one_quotes_table_and_column_names
- michael = Person.all.merge!(:includes => :favourite_reference).find(people(:michael))
+ michael = Person.all.merge!(:includes => :favourite_reference).find(people(:michael).id)
references(:michael_unicyclist)
assert_no_queries{ assert_equal references(:michael_unicyclist), michael.favourite_reference}
end
def test_eager_load_has_many_quotes_table_and_column_names
- michael = Person.all.merge!(:includes => :references).find(people(:michael))
+ michael = Person.all.merge!(:includes => :references).find(people(:michael).id)
references(:michael_magician,:michael_unicyclist)
assert_no_queries{ assert_equal references(:michael_magician,:michael_unicyclist), michael.references.sort_by(&:id) }
end
def test_eager_load_has_many_through_quotes_table_and_column_names
- michael = Person.all.merge!(:includes => :jobs).find(people(:michael))
+ michael = Person.all.merge!(:includes => :jobs).find(people(:michael).id)
jobs(:magician, :unicyclist)
assert_no_queries{ assert_equal jobs(:unicyclist, :magician), michael.jobs.sort_by(&:id) }
end
@@ -56,7 +56,7 @@ def test_exists
assert_equal true, Topic.exists?(id: [1, 9999])
assert_equal false, Topic.exists?(45)
- assert_equal false, Topic.exists?(Topic.new)
+ assert_equal false, Topic.exists?(Topic.new.id)
assert_raise(NoMethodError) { Topic.exists?([1,2]) }
end
@@ -637,7 +637,7 @@ def test_where_with_ar_object
def test_find_with_list_of_ar
author = Author.first
- authors = Author.find([author])
+ authors = Author.find([author.id])
assert_equal author, authors.first
end
@@ -769,7 +769,7 @@ def test_exists
assert ! davids.exists?(authors(:mary).id)
assert ! davids.exists?("42")
assert ! davids.exists?(42)
- assert ! davids.exists?(davids.new)
+ assert ! davids.exists?(davids.new.id)
fake = Author.where(:name => 'fake author')
assert ! fake.exists?
@@ -223,7 +223,7 @@ def test_validate_case_insensitive_uniqueness
assert t_utf8.save, "Should save t_utf8 as unique"
# If database hasn't UTF-8 character set, this test fails
- if Topic.all.merge!(:select => 'LOWER(title) AS title').find(t_utf8).title == "я тоже уникальный!"
+ if Topic.all.merge!(:select => 'LOWER(title) AS title').find(t_utf8.id).title == "я тоже уникальный!"
t2_utf8 = Topic.new("title" => "я тоже УНИКАЛЬНЫЙ!")
assert !t2_utf8.valid?, "Shouldn't be valid"
assert !t2_utf8.save, "Shouldn't save t2_utf8 as unique"

6 comments on commit 7658dc3

Owner

rafaelfranca replied Jan 4, 2015

If I remove the deprecation code the behavior will be the same because the predicate builder already handle these cases.

Should we deprecate BaseHandler before removing these deprecations? cc @tenderlove @sgrif

Member

sgrif replied Jan 4, 2015

I don't think we deprecated passing them to where

Owner

rafaelfranca replied Jan 4, 2015

Yeah, we don't. But what is the point of deprecating in find and exists? if it still work on where? Also, if we don't deprecate on where we still need the code to handle Base instances inside find and exists? or it will just work as before the deprecation.

Member

sgrif replied Jan 4, 2015

My assumption is to improve caching, but I'm not sure. I don't the we can remove BaseHandler without a deprecation cycle there, though?

Owner

rafaelfranca replied Jan 4, 2015

Yeah, we will need to have a deprecation cycle. My plan is to keep these deprecations for one more release and deprecate BaseHandler too. In the next release we remove all three deprecations together.

Member

sgrif replied Jan 4, 2015

👍

Please sign in to comment.