Skip to content

Commit

Permalink
Remove deprecated force reload argument in association readers
Browse files Browse the repository at this point in the history
  • Loading branch information
rafaelfranca committed Dec 29, 2016
1 parent 00e3973 commit 09cac8c
Show file tree
Hide file tree
Showing 9 changed files with 12 additions and 67 deletions.
4 changes: 4 additions & 0 deletions activerecord/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
* Remove deprecated force reload argument in singular and collection association readers.

*Rafael Mendonça França*

* Remove deprecated `activerecord.errors.messages.restrict_dependent_destroy.one` and
`activerecord.errors.messages.restrict_dependent_destroy.many` i18n scopes.

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,16 +25,8 @@ module Associations
# +load_target+ and the +loaded+ flag are your friends.
class CollectionAssociation < Association #:nodoc:
# Implements the reader method, e.g. foo.items for Foo.has_many :items
def reader(force_reload = false)
if force_reload
ActiveSupport::Deprecation.warn(<<-MSG.squish)
Passing an argument to force an association to reload is now
deprecated and will be removed in Rails 5.1. Please call `reload`
on the result collection proxy instead.
MSG

klass.uncached { reload }
elsif stale_target?
def reader
if stale_target?
reload
end

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,8 @@ module ActiveRecord
module Associations
class SingularAssociation < Association #:nodoc:
# Implements the reader method, e.g. foo.bar for Foo.has_one :bar
def reader(force_reload = false)
if force_reload && klass
ActiveSupport::Deprecation.warn(<<-MSG.squish)
Passing an argument to force an association to reload is now
deprecated and will be removed in Rails 5.1. Please call `reload`
on the parent object instead.
MSG

klass.uncached { reload }
elsif !loaded? || stale_target?
def reader
if !loaded? || stale_target?
reload
end

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1131,12 +1131,6 @@ def test_reflect_the_most_recent_change
Column.create! record: record
assert_equal 1, Column.count
end

def test_association_force_reload_with_only_true_is_deprecated
client = Client.find(3)

assert_deprecated { client.firm(true) }
end
end

class BelongsToWithForeignKeyTest < ActiveRecord::TestCase
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -955,12 +955,6 @@ def test_with_constant_class_name
end
end

def test_association_force_reload_with_only_true_is_deprecated
developer = Developer.find(1)

assert_deprecated { developer.projects(true) }
end

def test_alternate_database
professor = Professor.create(name: "Plum")
course = Course.create(name: "Forensics")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2462,12 +2462,6 @@ def self.name
end
end

def test_association_force_reload_with_only_true_is_deprecated
company = Company.find(1)

assert_deprecated { company.clients_of_firm(true) }
end

class AuthorWithErrorDestroyingAssociation < ActiveRecord::Base
self.table_name = "authors"
has_many :posts_with_error_destroying,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1204,12 +1204,6 @@ def test_has_many_through_with_scope_that_should_not_be_fully_merged
assert_nil Club.new.special_favourites.distinct_value
end

def test_association_force_reload_with_only_true_is_deprecated
post = Post.find(1)

assert_deprecated { post.people(true) }
end

def test_has_many_through_do_not_cache_association_reader_if_the_though_method_has_default_scopes
member = Member.create!
club = Club.create!
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -645,12 +645,6 @@ def test_with_polymorphic_has_one_with_custom_columns_name
end
end

def test_association_force_reload_with_only_true_is_deprecated
firm = Firm.find(1)

assert_deprecated { firm.account(true) }
end

class SpecialBook < ActiveRecord::Base
self.table_name = "books"
belongs_to :author, class_name: "SpecialAuthor"
Expand Down
21 changes: 4 additions & 17 deletions activerecord/test/cases/associations_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -88,10 +88,10 @@ def test_force_reload
assert firm.clients.empty?, "New firm should have cached no client objects"
assert_equal 0, firm.clients.size, "New firm should have cached 0 clients count"

ActiveSupport::Deprecation.silence do
assert !firm.clients(true).empty?, "New firm should have reloaded client objects"
assert_equal 1, firm.clients(true).size, "New firm should have reloaded clients count"
end
firm.clients.reload

assert !firm.clients.empty?, "New firm should have reloaded client objects"
assert_equal 1, firm.clients.size, "New firm should have reloaded clients count"
end

def test_using_limitable_reflections_helper
Expand All @@ -104,19 +104,6 @@ def test_using_limitable_reflections_helper
assert !using_limitable_reflections.call(mixed_reflections), "No collection associations (has many style) should pass"
end

def test_force_reload_is_uncached
firm = Firm.create!("name" => "A New Firm, Inc")
Client.create!("name" => "TheClient.com", :firm => firm)

ActiveSupport::Deprecation.silence do
ActiveRecord::Base.cache do
firm.clients.each {}
assert_queries(0) { assert_not_nil firm.clients.each {} }
assert_queries(1) { assert_not_nil firm.clients(true).each {} }
end
end
end

def test_association_with_references
firm = companies(:first_firm)
assert_includes firm.association_with_references.references_values, "foo"
Expand Down

2 comments on commit 09cac8c

@heaven
Copy link

@heaven heaven commented on 09cac8c Aug 17, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That was pretty much useful #association(self.persisted?).

It was:

record.tags(record.persisted?).map(&:name)

It becomes:

(record.persisted? ? record.tags.reload : record.tags).map(&:name)

@BattleBrisket
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a gem that reintroduces the argument-based syntax for force-reloading associations called rails-force-reload.

Just a note for future travelers, since I ended up here while doing research.

Please sign in to comment.