Skip to content

Commit

Permalink
Merge branch '9b4a0b9-with-reverts-and-fixes' into 5813c82-with-rever…
Browse files Browse the repository at this point in the history
…ts-and-fixes
  • Loading branch information
adrianna-chang-shopify committed Jan 17, 2025
2 parents 5813c82 + 267fdf4 commit 2e59171
Show file tree
Hide file tree
Showing 16 changed files with 44 additions and 179 deletions.
14 changes: 0 additions & 14 deletions activerecord/lib/active_record/associations/association.rb
Original file line number Diff line number Diff line change
Expand Up @@ -188,16 +188,6 @@ def extensions
# not reraised. The proxy is \reset and +nil+ is the return value.
def load_target
@target = find_target(async: false) if (@stale_state && stale_target?) || find_target?
if !@target && set_through_target_for_new_record?
reflections = reflection.chain
reflections.pop
reflections.reverse!

@target = reflections.reduce(through_association.target) do |middle_target, through_reflection|
break unless middle_target
middle_target.association(through_reflection.source_reflection_name).load_target
end
end

loaded! unless loaded?
target
Expand Down Expand Up @@ -331,10 +321,6 @@ def find_target?
!loaded? && (!owner.new_record? || foreign_key_present?) && klass
end

def set_through_target_for_new_record?
owner.new_record? && reflection.through_reflection? && through_association.target
end

# Returns true if there is a foreign key present on the owner which
# references the target. This is used to determine whether we can load
# the target if the owner is currently a new record (and therefore
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -272,18 +272,6 @@ def include?(record)
def load_target
if find_target?
@target = merge_target_lists(find_target, target)
elsif target.empty? && set_through_target_for_new_record?
reflections = reflection.chain.reverse!

@target = reflections.each_cons(2).reduce(through_association.target) do |middle_target, (middle_reflection, through_reflection)|
if middle_target.nil? || (middle_reflection.collection? && middle_target.empty?)
break []
elsif middle_reflection.collection?
middle_target.flat_map { |record| record.association(through_reflection.source_reflection_name).load_target }
else
middle_target.association(through_reflection.source_reflection_name).load_target
end
end
end

loaded!
Expand Down
2 changes: 1 addition & 1 deletion activerecord/lib/active_record/autosave_association.rb
Original file line number Diff line number Diff line change
Expand Up @@ -376,7 +376,7 @@ def association_valid?(association, record)
# If the associated record is unchanged we shouldn't auto validate it.
# Even if a record is invalid you should still be able to create new references
# to it.
return true if !record.new_record? && !record.changed?
return true if self.new_record? && !record.new_record? && !record.changed?
end

unless valid = record.valid?(context)
Expand Down
4 changes: 0 additions & 4 deletions activerecord/lib/active_record/reflection.rb
Original file line number Diff line number Diff line change
Expand Up @@ -655,10 +655,6 @@ def source_reflection
self
end

def source_reflection_name
source_reflection.name
end

# A chain of reflections from this one back to the owner. For more see the explanation in
# ThroughReflection.
def collect_join_chain
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@
require "models/zine"
require "models/interest"
require "models/human"
require "models/account"

class HasManyThroughAssociationsTest < ActiveRecord::TestCase
fixtures :posts, :readers, :people, :comments, :authors, :categories, :taggings, :tags,
Expand All @@ -57,84 +56,6 @@ def setup
Reader.create person_id: 0, post_id: 0
end

def test_setting_has_many_through_one_association_on_new_record_sets_through_records
account_1, account_2 = Account.create!(credit_limit: 100), Account.create!(credit_limit: 100)
firm = Firm.new(accounts: [account_1, account_2])
client = Client.new
client.firm = firm

assert_predicate account_1, :persisted?
assert_predicate account_2, :persisted?
assert_predicate client, :new_record?
assert_predicate client.firm, :new_record?
assert_no_queries { assert_equal [account_1, account_2].sort, client.accounts.sort }
end

def test_setting_has_many_through_many_association_on_new_record_sets_through_records
subscriber_1, subscriber_2 = Subscriber.create!(nick: "nick 1"), Subscriber.create!(nick: "nick 2")
subscription_1 = Subscription.new(subscriber: subscriber_1)
subscription_2 = Subscription.new(subscriber: subscriber_2)
book = Book.new
book.subscriptions = [subscription_1, subscription_2]

assert_predicate subscriber_1, :persisted?
assert_predicate subscriber_2, :persisted?
assert_predicate book, :new_record?
book.subscriptions.each { |subscription| assert_predicate subscription, :new_record? }
assert_no_queries { assert_equal [subscriber_1, subscriber_2].sort, book.subscribers.sort }
end

def test_setting_nested_has_many_through_one_association_on_new_record_sets_nested_through_records
post_tagging_1, post_tagging_2 = Tagging.create!, Tagging.create!
post = Post.create!(title: "Tagged", body: "Post", taggings: [post_tagging_1, post_tagging_2])
author = Author.new(name: "Josh")
author.posts = [post]
categorization = Categorization.new
categorization.author = author

assert_predicate post_tagging_1, :persisted?
assert_predicate post_tagging_2, :persisted?
assert_predicate post, :persisted?
assert_predicate categorization, :new_record?
assert_predicate categorization.author, :new_record?
assert_no_queries { assert_equal [post_tagging_1, post_tagging_2].sort, categorization.post_taggings.sort }
end

def test_setting_nested_has_many_through_one_association_on_new_record_sets_targetless_nested_through_records
post = Post.create!(title: "Tagged", body: "Post")
post_tagging_1, post_tagging_2 = Tagging.create!(taggable: post), Tagging.create!(taggable: post)
author = Author.new(name: "Josh")
author.posts = [post]
categorization = Categorization.new
categorization.author = author

assert_predicate post_tagging_1, :persisted?
assert_predicate post_tagging_2, :persisted?
assert_predicate post, :persisted?
assert_predicate categorization, :new_record?
assert_predicate categorization.author, :new_record?
assert_queries_count(1) { assert_equal [post_tagging_1, post_tagging_2].sort, categorization.post_taggings.sort }
end

def test_setting_nested_has_many_through_many_association_on_new_record_sets_nested_through_records
account_1 = Account.create!(firm_name: "account 1", credit_limit: 100)
subscriber_1 = Subscriber.create!(nick: "nick 1", account: account_1)
account_2 = Account.create!(firm_name: "account 2", credit_limit: 100)
subscriber_2 = Subscriber.create!(nick: "nick 2", account: account_2)
subscription_1 = Subscription.new(subscriber: subscriber_1)
subscription_2 = Subscription.new(subscriber: subscriber_2)
book = Book.new
book.subscriptions = [subscription_1, subscription_2]

assert_predicate subscriber_1, :persisted?
assert_predicate subscriber_2, :persisted?
assert_predicate account_1, :persisted?
assert_predicate account_2, :persisted?
assert_predicate book, :new_record?
book.subscriptions.each { |subscription| assert_predicate subscription, :new_record? }
assert_no_queries { assert_equal [account_1, account_2].sort, book.subscriber_accounts.sort }
end

def test_has_many_through_create_record
assert books(:awdr).subscribers.create!(nick: "bob")
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
require "models/shop_account"
require "models/customer_carrier"
require "models/cpk"
require "models/rating"

class HasOneThroughAssociationsTest < ActiveRecord::TestCase
fixtures :member_types, :members, :clubs, :memberships, :sponsors, :organizations, :minivans,
Expand All @@ -44,63 +43,6 @@ def test_has_one_through_executes_limited_query
end
end

def test_setting_association_on_new_record_sets_through_record
club = Club.create!
membership = CurrentMembership.new(club: club)
member = Member.new
member.current_membership = membership

assert_predicate club, :persisted?
assert_predicate member, :new_record?
assert_predicate member.current_membership, :new_record?
assert_no_queries { assert_equal club, member.club }
end

def test_setting_association_on_new_record_sets_nested_through_record
category = Category.create!(name: "General")
club = Club.create!(category: category)
membership = CurrentMembership.new(club: club)
member = Member.new
member.current_membership = membership

assert_predicate category, :persisted?
assert_predicate club, :persisted?
assert_predicate member, :new_record?
assert_predicate member.current_membership, :new_record?
assert_no_queries { assert_equal club.category, member.club_category }
end

def test_setting_association_on_new_record_sets_not_loaded_nested_through_record
category = Category.create!(name: "General")
club = Club.create!(category: category)
club.reload
membership = CurrentMembership.new(club: club)
member = Member.new
member.current_membership = membership

assert_predicate category, :persisted?
assert_predicate club, :persisted?
assert_predicate member, :new_record?
assert_predicate member.current_membership, :new_record?
assert_queries_count(1) { assert_equal club.category, member.club_category }
end

def test_setting_association_on_new_record_sets_nested_through_record_with_belongs_to
essay = Essay.create!
author = Author.create!(name: "Josh", owned_essay: essay)
post = Post.create!(title: "Foo", body: "Bar", author: author)
comment = Comment.new(post: post)
rating = Rating.new
rating.comment = comment

assert_predicate essay, :persisted?
assert_predicate author, :persisted?
assert_predicate post, :persisted?
assert_predicate rating, :new_record?
assert_predicate rating.comment, :new_record?
assert_no_queries { assert_equal essay, rating.owned_essay }
end

def test_creating_association_creates_through_record
new_member = Member.create(name: "Chris")
new_member.club = Club.create(name: "LRUG")
Expand Down
41 changes: 41 additions & 0 deletions activerecord/test/cases/autosave_association_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
require "models/tagging"
require "models/treasure"
require "models/eye"
require "models/liquid"
require "models/electron"
require "models/molecule"
require "models/member"
Expand Down Expand Up @@ -1501,6 +1502,46 @@ def test_should_skip_validation_on_habtm_if_persisted_and_unchanged
new_pirate.save!
end

def test_should_not_skip_validation_on_deeply_nested_attributes_if_unpersisted_and_unchanged
liquid = Liquid.new
molecule = liquid.molecules.build

liquid.save!

input_attributes = {
molecules_attributes: {
"0" => {
id: molecule.id,
electrons_attributes: { id: nil, name: "" }
}
}
}

assert_not liquid.update(input_attributes), "Liquid should not have been updated"
assert_includes liquid.errors.full_messages, "Molecules electrons name can't be blank"

liquid.reload

electron = molecule.electrons.build(name: "electron")
electron.save!

assert_predicate liquid, :valid?
assert_predicate electron, :persisted?
assert_predicate electron, :valid?

input_attributes = {
molecules_attributes: {
"0" => {
id: molecule.id,
electrons_attributes: { id: electron.id, name: "" }
}
}
}

assert_not liquid.update(input_attributes), "Liquid should not have been updated"
assert_includes liquid.errors.full_messages, "Molecules electrons name can't be blank"
end

def test_a_child_marked_for_destruction_should_not_be_destroyed_twice_while_saving_habtm
@pirate.parrots.create!(name: "parrots_1")

Expand Down
1 change: 0 additions & 1 deletion activerecord/test/models/book.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ class Book < ActiveRecord::Base

has_many :subscriptions
has_many :subscribers, through: :subscriptions
has_many :subscriber_accounts, through: :subscribers, source: :account

has_one :essay

Expand Down
2 changes: 0 additions & 2 deletions activerecord/test/models/family_tree.rb
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
# frozen_string_literal: true

class FamilyTree < ActiveRecord::Base
belongs_to :user

belongs_to :member, class_name: "User", foreign_key: "member_id"
belongs_to :family
end
2 changes: 2 additions & 0 deletions activerecord/test/models/liquid.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,6 @@
class Liquid < ActiveRecord::Base
self.table_name = :liquid
has_many :molecules, -> { distinct }

accepts_nested_attributes_for :molecules
end
1 change: 0 additions & 1 deletion activerecord/test/models/member_detail.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ class MemberDetail < ActiveRecord::Base
belongs_to :organization
has_one :member_type, through: :member
has_one :membership, through: :member
has_one :sponsor, through: :membership
has_one :admittable, through: :member, source_type: "Member"

has_many :organization_member_details, through: :organization, source: :member_details
Expand Down
1 change: 0 additions & 1 deletion activerecord/test/models/membership.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ class Membership < ActiveRecord::Base
enum :type, %i(Membership CurrentMembership SuperMembership SelectedMembership TenantMembership)
belongs_to :member
belongs_to :club
has_one :sponsor, through: :club
end

class CurrentMembership < Membership
Expand Down
1 change: 0 additions & 1 deletion activerecord/test/models/post.rb
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ def greeting
}

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

belongs_to :author_with_posts, -> { includes(:posts) }, class_name: "Author", foreign_key: :author_id
Expand Down
2 changes: 0 additions & 2 deletions activerecord/test/models/rating.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@

class Rating < ActiveRecord::Base
belongs_to :comment
has_one :post, through: :comment
has_one :owned_essay, through: :post
has_many :taggings, as: :taggable
has_many :taggings_without_tag, -> { left_joins(:tag).where("tags.id": [nil, 0]) }, as: :taggable, class_name: "Tagging"
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"
Expand Down
2 changes: 0 additions & 2 deletions activerecord/test/models/subscriber.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@
class Subscriber < ActiveRecord::Base
self.primary_key = "nick"

belongs_to :account

has_many :subscriptions
has_many :books, through: :subscriptions
end
Expand Down
1 change: 0 additions & 1 deletion activerecord/test/schema/schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1173,7 +1173,6 @@
t.integer :books_count, null: false, default: 0
t.integer :update_count, null: false, default: 0
t.index :nick, unique: true
t.references :account
end

create_table :subscriptions, force: true do |t|
Expand Down

0 comments on commit 2e59171

Please sign in to comment.