Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix assigning through records when using polymorphic has many through association #47534

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -59,9 +59,10 @@ def build_through_record(record)

attributes = through_scope_attributes
attributes[source_reflection.name] = record
attributes[source_reflection.foreign_type] = options[:source_type] if options[:source_type]

through_association.build(attributes)
through_association.build(attributes).tap do |new_record|
new_record.send("#{source_reflection.foreign_type}=", options[:source_type]) if options[:source_type]
end
end
end

Expand Down
17 changes: 16 additions & 1 deletion activerecord/test/cases/reflection_test.rb
Expand Up @@ -27,6 +27,7 @@
require "models/drink_designer"
require "models/recipe"
require "models/user_with_invalid_relation"
require "models/hardback"

class ReflectionTest < ActiveRecord::TestCase
include ActiveRecord::Reflection
Expand Down Expand Up @@ -324,7 +325,7 @@ def test_scope_chain_does_not_interfere_with_hmt_with_polymorphic_case
assert_equal 2, hotel.chefs.count
end

def test_scope_chain_does_not_interfere_with_hmt_with_polymorphic_case_and_sti
def test_scope_chain_does_not_interfere_with_hmt_with_polymorphic_case_and_subclass_source
hotel = Hotel.create!
hotel.mocktail_designers << MocktailDesigner.create!

Expand All @@ -341,6 +342,20 @@ def test_scope_chain_does_not_interfere_with_hmt_with_polymorphic_case_and_sti
assert_equal 0, hotel.chef_lists.count
end

def test_scope_chain_does_not_interfere_with_hmt_with_polymorphic_and_subclass_source_2
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Test failure without the fix:

ReflectionTest#test_scope_chain_does_not_interfere_with_hmt_with_polymorphic_and_subclass_source_2 [/home/spin/src/github.com/Shopify/rails/activerecord/test/cases/reflection_test.rb:350]:
--- expected
+++ actual
@@ -1 +1 @@
-[#<BestHardback id: 1>]
+#<ActiveRecord::Associations::CollectionProxy []>

author = Author.create!(name: "John Doe")
hardback = BestHardback.create!
author.best_hardbacks << hardback

assert_equal [hardback], author.best_hardbacks
assert_equal [hardback], author.reload.best_hardbacks

author.best_hardbacks = []

assert_empty author.best_hardbacks
assert_empty author.reload.best_hardbacks
end

def test_scope_chain_of_polymorphic_association_does_not_leak_into_other_hmt_associations
hotel = Hotel.create!
department = hotel.departments.create!
Expand Down
1 change: 1 addition & 0 deletions activerecord/test/models/author.rb
Expand Up @@ -168,6 +168,7 @@ def ratings
has_many :tags_with_primary_key, through: :posts

has_many :books
has_many :best_hardbacks, through: :books, source: :format_record, source_type: "BestHardback"
has_many :published_books, class_name: "PublishedBook"
has_many :unpublished_books, -> { where(status: [:proposed, :written]) }, class_name: "Book"
has_many :subscriptions, through: :books
Expand Down
1 change: 1 addition & 0 deletions activerecord/test/models/book.rb
Expand Up @@ -2,6 +2,7 @@

class Book < ActiveRecord::Base
belongs_to :author
belongs_to :format_record, polymorphic: true

has_many :citations, foreign_key: "book1_id", inverse_of: :book
has_many :references, -> { distinct }, through: :citations, source: :reference_of
Expand Down
7 changes: 7 additions & 0 deletions activerecord/test/models/hardback.rb
@@ -0,0 +1,7 @@
# frozen_string_literal: true

class Hardback < ActiveRecord::Base
end

class BestHardback < Hardback
end
5 changes: 5 additions & 0 deletions activerecord/test/schema/schema.rb
Expand Up @@ -125,6 +125,8 @@
default_zero = { default: 0 }
t.references :author
t.string :format
t.integer :format_record_id
t.string :format_record_type
t.column :name, :string
t.column :status, :integer, **default_zero
t.column :last_read, :integer, **default_zero
Expand Down Expand Up @@ -165,6 +167,9 @@
t.datetime :updated_at
end

create_table :hardbacks, force: true do |t|
end

create_table :booleans, force: true do |t|
t.boolean :value
t.boolean :has_fun, null: false, default: false
Expand Down