Skip to content

Commit

Permalink
Add regression test for has_many_through associations failing to crea…
Browse files Browse the repository at this point in the history
…te join models

This commit adds a test that demonstrates a regression in has_many_through associations
where multiple has_many_through associations associate the same parent with the same
child relation.

The regression is ultimately caused by rails/rails@0d38f2a, which avoids saving a through association target
if we expect the target to be persisted by another (direct) association.

The problem is that if we have multiple has_many_through associations that associate the same parent with the same
child, we will autosave the first set of associations from the parent, but avoid saving the through target a second
time (once it's persisted by the first association), and fail to create the join model for the second association chain.
  • Loading branch information
adrianna-chang-shopify committed Jan 17, 2025
1 parent b680ec7 commit 4199eeb
Show file tree
Hide file tree
Showing 8 changed files with 67 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,9 @@
require "models/member"
require "models/membership"
require "models/club"
require "models/program"
require "models/program_offering"
require "models/enrollment"
require "models/organization"
require "models/user"
require "models/family"
Expand Down Expand Up @@ -1548,6 +1551,21 @@ def test_has_many_through_update_ids_with_conditions
assert_equal [], author.nonspecial_categories_with_condition_ids
end

def test_has_many_through_from_same_parent_to_same_child_creates_join_models
club = Club.new(name: "Awesome Rails Club")
member = club.simple_members.build(name: "Jane Doe")

program = Program.new(name: "Learn Ruby on Rails")
program.members << member

club.programs << program

club.save!

assert_equal(1, program.enrollments.size)
assert_equal(1, club.simple_memberships.size)
end

def test_single_has_many_through_association_with_unpersisted_parent_instance
post_with_single_has_many_through = Class.new(Post) do
def self.name; "PostWithSingleHasManyThrough"; end
Expand Down
6 changes: 6 additions & 0 deletions activerecord/test/models/club.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,12 @@ class Club < ActiveRecord::Base

scope :general, -> { left_joins(:category).where(categories: { name: "General" }).unscope(:limit) }

has_many :program_offerings
has_many :programs, through: :program_offerings

has_many :simple_memberships, class_name: "Membership"
has_many :simple_members, through: :simple_memberships, foreign_key: "member_id"

accepts_nested_attributes_for :membership

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

class Enrollment < ActiveRecord::Base
belongs_to :program
belongs_to :member, class_name: "SimpleMember"
end
7 changes: 7 additions & 0 deletions activerecord/test/models/member.rb
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,13 @@ class Member < ActiveRecord::Base
scope :with_member_type_id, -> (id) { where(member_type_id: id) }
end

class SimpleMember < ActiveRecord::Base
self.table_name = "members"

has_many :memberships
has_many :clubs, through: :memberships
end

class SelfMember < ActiveRecord::Base
self.table_name = "members"
has_and_belongs_to_many :friends, class_name: "SelfMember", join_table: "member_friends"
Expand Down
3 changes: 3 additions & 0 deletions activerecord/test/models/membership.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@ class Membership < ActiveRecord::Base
enum :type, %i(Membership CurrentMembership SuperMembership SelectedMembership TenantMembership)
belongs_to :member
belongs_to :club
has_one :sponsor, through: :club

belongs_to :simple_member, foreign_key: "member_id"
end

class CurrentMembership < Membership
Expand Down
6 changes: 6 additions & 0 deletions activerecord/test/models/program.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
# frozen_string_literal: true

class Program < ActiveRecord::Base
has_many :enrollments
has_many :members, through: :enrollments, class_name: "SimpleMember"
end
6 changes: 6 additions & 0 deletions activerecord/test/models/program_offering.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
# frozen_string_literal: true

class ProgramOffering < ActiveRecord::Base
belongs_to :club
belongs_to :program
end
15 changes: 15 additions & 0 deletions activerecord/test/schema/schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -588,6 +588,11 @@
t.references :car, index: false
end

create_table :enrollments, force: true do |t|
t.integer :program_id
t.integer :member_id
end

create_table :entrants, force: true do |t|
t.string :name, null: false
t.integer :course_id, null: false
Expand Down Expand Up @@ -1018,6 +1023,16 @@
t.decimal :discounted_price
end

create_table :program_offerings, force: true do |t|
t.integer :club_id
t.integer :program_id
t.datetime :start_date
end

create_table :programs, force: true do |t|
t.string :name
end

add_check_constraint :products, "price > discounted_price", name: "products_price_check"

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

0 comments on commit 4199eeb

Please sign in to comment.