Skip to content

Commit 24bae89

Browse files
Revert "Handle nested middle belongs_to, not-loaded target when setting through records"
This reverts commit a0a8dd1.
1 parent b9ad920 commit 24bae89

File tree

10 files changed

+18
-106
lines changed

10 files changed

+18
-106
lines changed

activerecord/lib/active_record/associations/association.rb

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -193,9 +193,9 @@ def load_target
193193
reflections.pop
194194
reflections.reverse!
195195

196-
@target = reflections.reduce(through_association.target) do |middle_target, through_reflection|
196+
@target = reflections.reduce(through_association.target) do |middle_target, reflection|
197197
break unless middle_target
198-
middle_target.association(through_reflection.source_reflection_name).load_target
198+
middle_target.association(reflection.source_reflection_name).target
199199
end
200200
end
201201

activerecord/lib/active_record/associations/collection_association.rb

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -273,15 +273,17 @@ def load_target
273273
if find_target?
274274
@target = merge_target_lists(find_target, target)
275275
elsif target.empty? && set_through_target_for_new_record?
276-
reflections = reflection.chain.reverse!
276+
reflections = reflection.chain
277+
reflections.pop
278+
reflections.reverse!
277279

278-
@target = reflections.each_cons(2).reduce(through_association.target) do |middle_target, (middle_reflection, through_reflection)|
279-
if middle_target.nil? || (middle_reflection.collection? && middle_target.empty?)
280+
@target = reflections.reduce(through_association.target) do |middle_target, reflection|
281+
if middle_target.empty?
280282
break []
281-
elsif middle_reflection.collection?
282-
middle_target.flat_map { |record| record.association(through_reflection.source_reflection_name).load_target }
283+
elsif reflection.collection?
284+
middle_target.flat_map { |record| record.association(reflection.source_reflection_name).target }
283285
else
284-
middle_target.association(through_reflection.source_reflection_name).load_target
286+
middle_target.association(reflection.source_reflection_name).target
285287
end
286288
end
287289
end

activerecord/lib/active_record/reflection.rb

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -655,10 +655,6 @@ def source_reflection
655655
self
656656
end
657657

658-
def source_reflection_name
659-
source_reflection.name
660-
end
661-
662658
# A chain of reflections from this one back to the owner. For more see the explanation in
663659
# ThroughReflection.
664660
def collect_join_chain

activerecord/test/cases/associations/has_many_through_associations_test.rb

Lines changed: 6 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -57,20 +57,7 @@ def setup
5757
Reader.create person_id: 0, post_id: 0
5858
end
5959

60-
def test_setting_has_many_through_one_association_on_new_record_sets_through_records
61-
account_1, account_2 = Account.create!(credit_limit: 100), Account.create!(credit_limit: 100)
62-
firm = Firm.new(accounts: [account_1, account_2])
63-
client = Client.new
64-
client.firm = firm
65-
66-
assert_predicate account_1, :persisted?
67-
assert_predicate account_2, :persisted?
68-
assert_predicate client, :new_record?
69-
assert_predicate client.firm, :new_record?
70-
assert_no_queries { assert_equal [account_1, account_2].sort, client.accounts.sort }
71-
end
72-
73-
def test_setting_has_many_through_many_association_on_new_record_sets_through_records
60+
def test_setting_association_on_new_record_sets_through_records
7461
subscriber_1, subscriber_2 = Subscriber.create!(nick: "nick 1"), Subscriber.create!(nick: "nick 2")
7562
subscription_1 = Subscription.new(subscriber: subscriber_1)
7663
subscription_2 = Subscription.new(subscriber: subscriber_2)
@@ -81,42 +68,10 @@ def test_setting_has_many_through_many_association_on_new_record_sets_through_re
8168
assert_predicate subscriber_2, :persisted?
8269
assert_predicate book, :new_record?
8370
book.subscriptions.each { |subscription| assert_predicate subscription, :new_record? }
84-
assert_no_queries { assert_equal [subscriber_1, subscriber_2].sort, book.subscribers.sort }
85-
end
86-
87-
def test_setting_nested_has_many_through_one_association_on_new_record_sets_nested_through_records
88-
post_tagging_1, post_tagging_2 = Tagging.create!, Tagging.create!
89-
post = Post.create!(title: "Tagged", body: "Post", taggings: [post_tagging_1, post_tagging_2])
90-
author = Author.new(name: "Josh")
91-
author.posts = [post]
92-
categorization = Categorization.new
93-
categorization.author = author
94-
95-
assert_predicate post_tagging_1, :persisted?
96-
assert_predicate post_tagging_2, :persisted?
97-
assert_predicate post, :persisted?
98-
assert_predicate categorization, :new_record?
99-
assert_predicate categorization.author, :new_record?
100-
assert_no_queries { assert_equal [post_tagging_1, post_tagging_2].sort, categorization.post_taggings.sort }
101-
end
102-
103-
def test_setting_nested_has_many_through_one_association_on_new_record_sets_targetless_nested_through_records
104-
post = Post.create!(title: "Tagged", body: "Post")
105-
post_tagging_1, post_tagging_2 = Tagging.create!(taggable: post), Tagging.create!(taggable: post)
106-
author = Author.new(name: "Josh")
107-
author.posts = [post]
108-
categorization = Categorization.new
109-
categorization.author = author
110-
111-
assert_predicate post_tagging_1, :persisted?
112-
assert_predicate post_tagging_2, :persisted?
113-
assert_predicate post, :persisted?
114-
assert_predicate categorization, :new_record?
115-
assert_predicate categorization.author, :new_record?
116-
assert_queries_count(1) { assert_equal [post_tagging_1, post_tagging_2].sort, categorization.post_taggings.sort }
117-
end
118-
119-
def test_setting_nested_has_many_through_many_association_on_new_record_sets_nested_through_records
71+
assert_equal [subscriber_1, subscriber_2].sort, book.subscribers.sort
72+
end
73+
74+
def test_setting_association_on_new_record_sets_nested_through_records
12075
account_1 = Account.create!(firm_name: "account 1", credit_limit: 100)
12176
subscriber_1 = Subscriber.create!(nick: "nick 1", account: account_1)
12277
account_2 = Account.create!(firm_name: "account 2", credit_limit: 100)
@@ -128,11 +83,9 @@ def test_setting_nested_has_many_through_many_association_on_new_record_sets_nes
12883

12984
assert_predicate subscriber_1, :persisted?
13085
assert_predicate subscriber_2, :persisted?
131-
assert_predicate account_1, :persisted?
132-
assert_predicate account_2, :persisted?
13386
assert_predicate book, :new_record?
13487
book.subscriptions.each { |subscription| assert_predicate subscription, :new_record? }
135-
assert_no_queries { assert_equal [account_1, account_2].sort, book.subscriber_accounts.sort }
88+
assert_equal [account_1, account_2].sort, book.subscriber_accounts.sort
13689
end
13790

13891
def test_has_many_through_create_record

activerecord/test/cases/associations/has_one_through_associations_test.rb

Lines changed: 2 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@
2323
require "models/shop_account"
2424
require "models/customer_carrier"
2525
require "models/cpk"
26-
require "models/rating"
2726

2827
class HasOneThroughAssociationsTest < ActiveRecord::TestCase
2928
fixtures :member_types, :members, :clubs, :memberships, :sponsors, :organizations, :minivans,
@@ -53,7 +52,7 @@ def test_setting_association_on_new_record_sets_through_record
5352
assert_predicate club, :persisted?
5453
assert_predicate member, :new_record?
5554
assert_predicate member.current_membership, :new_record?
56-
assert_no_queries { assert_equal club, member.club }
55+
assert_equal club, member.club
5756
end
5857

5958
def test_setting_association_on_new_record_sets_nested_through_record
@@ -67,38 +66,7 @@ def test_setting_association_on_new_record_sets_nested_through_record
6766
assert_predicate club, :persisted?
6867
assert_predicate member, :new_record?
6968
assert_predicate member.current_membership, :new_record?
70-
assert_no_queries { assert_equal club.category, member.club_category }
71-
end
72-
73-
def test_setting_association_on_new_record_sets_not_loaded_nested_through_record
74-
category = Category.create!(name: "General")
75-
club = Club.create!(category: category)
76-
club.reload
77-
membership = CurrentMembership.new(club: club)
78-
member = Member.new
79-
member.current_membership = membership
80-
81-
assert_predicate category, :persisted?
82-
assert_predicate club, :persisted?
83-
assert_predicate member, :new_record?
84-
assert_predicate member.current_membership, :new_record?
85-
assert_queries_count(1) { assert_equal club.category, member.club_category }
86-
end
87-
88-
def test_setting_association_on_new_record_sets_nested_through_record_with_belongs_to
89-
essay = Essay.create!
90-
author = Author.create!(name: "Josh", owned_essay: essay)
91-
post = Post.create!(title: "Foo", body: "Bar", author: author)
92-
comment = Comment.new(post: post)
93-
rating = Rating.new
94-
rating.comment = comment
95-
96-
assert_predicate essay, :persisted?
97-
assert_predicate author, :persisted?
98-
assert_predicate post, :persisted?
99-
assert_predicate rating, :new_record?
100-
assert_predicate rating.comment, :new_record?
101-
assert_no_queries { assert_equal essay, rating.owned_essay }
69+
assert_equal club.category, member.club_category
10270
end
10371

10472
def test_creating_association_creates_through_record
Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,6 @@
11
# frozen_string_literal: true
22

33
class FamilyTree < ActiveRecord::Base
4-
belongs_to :user
5-
64
belongs_to :member, class_name: "User", foreign_key: "member_id"
75
belongs_to :family
86
end

activerecord/test/models/member_detail.rb

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ class MemberDetail < ActiveRecord::Base
55
belongs_to :organization
66
has_one :member_type, through: :member
77
has_one :membership, through: :member
8-
has_one :sponsor, through: :membership
98
has_one :admittable, through: :member, source_type: "Member"
109

1110
has_many :organization_member_details, through: :organization, source: :member_details

activerecord/test/models/membership.rb

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ class Membership < ActiveRecord::Base
44
enum :type, %i(Membership CurrentMembership SuperMembership SelectedMembership TenantMembership)
55
belongs_to :member
66
belongs_to :club
7-
has_one :sponsor, through: :club
87
end
98

109
class CurrentMembership < Membership

activerecord/test/models/post.rb

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,6 @@ def greeting
4646
}
4747

4848
belongs_to :author
49-
has_one :owned_essay, through: :author
5049
belongs_to :readonly_author, -> { readonly }, class_name: "Author", foreign_key: :author_id
5150

5251
belongs_to :author_with_posts, -> { includes(:posts) }, class_name: "Author", foreign_key: :author_id

activerecord/test/models/rating.rb

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,6 @@
22

33
class Rating < ActiveRecord::Base
44
belongs_to :comment
5-
has_one :post, through: :comment
6-
has_one :owned_essay, through: :post
75
has_many :taggings, as: :taggable
86
has_many :taggings_without_tag, -> { left_joins(:tag).where("tags.id": [nil, 0]) }, as: :taggable, class_name: "Tagging"
97
has_many :taggings_with_no_tag, -> { joins("LEFT OUTER JOIN tags ON tags.id = taggings.tag_id").where("tags.id": nil) }, as: :taggable, class_name: "Tagging"

0 commit comments

Comments
 (0)