mirror of
https://github.com/rails/rails.git
synced 2022-11-09 12:12:34 -05:00
Fix has_many_inversing with touch: true on a belongs_to association with inverse
`has_many_inversing` adds records to a has_many association. It does so with destroyed records, too. So if a child was destroyed with a `touch: true` association on the parent *and* the parent was not loaded, it tried to load the parent to touch it. While loading the parent it added the child record to the parent's has_many association. The logic doing this always set the child's parent id – even if it was correct/the same already. But since the child is destroyed, it resulted in a `FrozenError`. This commit prevents doing the unnecessary setting of the identical id and therefore fixes this error. Fixes #40943 by not doing an unneeded attribute set. Closes #40969. [Markus Doits + Rafael Mendonça França]
This commit is contained in:
parent
46165a0fe9
commit
2dd81877a4
6 changed files with 46 additions and 6 deletions
|
@ -80,7 +80,7 @@ module ActiveRecord
|
|||
@updated = true
|
||||
end
|
||||
|
||||
replace_keys(record)
|
||||
replace_keys(record, force: true)
|
||||
|
||||
self.target = record
|
||||
end
|
||||
|
@ -108,8 +108,12 @@ module ActiveRecord
|
|||
reflection.counter_cache_column && owner.persisted?
|
||||
end
|
||||
|
||||
def replace_keys(record)
|
||||
owner[reflection.foreign_key] = record ? record._read_attribute(primary_key(record.class)) : nil
|
||||
def replace_keys(record, force: false)
|
||||
target_key = record ? record._read_attribute(primary_key(record.class)) : nil
|
||||
|
||||
if force || owner[reflection.foreign_key] != target_key
|
||||
owner[reflection.foreign_key] = target_key
|
||||
end
|
||||
end
|
||||
|
||||
def primary_key(klass)
|
||||
|
|
|
@ -14,9 +14,14 @@ module ActiveRecord
|
|||
end
|
||||
|
||||
private
|
||||
def replace_keys(record)
|
||||
def replace_keys(record, force: false)
|
||||
super
|
||||
owner[reflection.foreign_type] = record ? record.class.polymorphic_name : nil
|
||||
|
||||
target_type = record ? record.class.polymorphic_name : nil
|
||||
|
||||
if force || owner[reflection.foreign_type] != target_type
|
||||
owner[reflection.foreign_type] = target_type
|
||||
end
|
||||
end
|
||||
|
||||
def inverse_reflection_for(record)
|
||||
|
|
|
@ -25,6 +25,8 @@ require "models/admin/user"
|
|||
require "models/ship"
|
||||
require "models/treasure"
|
||||
require "models/parrot"
|
||||
require "models/book"
|
||||
require "models/citation"
|
||||
|
||||
class BelongsToAssociationsTest < ActiveRecord::TestCase
|
||||
fixtures :accounts, :companies, :developers, :projects, :topics,
|
||||
|
@ -1177,6 +1179,17 @@ class BelongsToAssociationsTest < ActiveRecord::TestCase
|
|||
assert_equal companies(:another_firm), client.firm_with_condition
|
||||
end
|
||||
|
||||
def test_destroying_child_with_unloaded_parent_and_foreign_key_and_touch_is_possible_with_has_many_inversing
|
||||
with_has_many_inversing do
|
||||
book = Book.create!
|
||||
citation = book.citations.create!
|
||||
|
||||
assert_difference "Citation.count", -1 do
|
||||
Citation.find(citation.id).destroy
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
def test_polymorphic_reassignment_of_associated_id_updates_the_object
|
||||
sponsor = sponsors(:moustache_club_sponsor_for_groucho)
|
||||
|
||||
|
@ -1359,6 +1372,21 @@ class BelongsToAssociationsTest < ActiveRecord::TestCase
|
|||
assert_equal toy, sponsor.reload.sponsorable
|
||||
end
|
||||
|
||||
class SponsorWithTouchInverse < Sponsor
|
||||
belongs_to :sponsorable, polymorphic: true, inverse_of: :sponsors, touch: true
|
||||
end
|
||||
|
||||
def test_destroying_polymorphic_child_with_unloaded_parent_and_touch_is_possible_with_has_many_inversing
|
||||
with_has_many_inversing do
|
||||
toy = Toy.create!
|
||||
sponsor = toy.sponsors.create!
|
||||
|
||||
assert_difference "Sponsor.count", -1 do
|
||||
SponsorWithTouchInverse.find(sponsor.id).destroy
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
def test_polymorphic_with_false
|
||||
assert_nothing_raised do
|
||||
Class.new(ActiveRecord::Base) do
|
||||
|
|
|
@ -3,7 +3,7 @@
|
|||
class Book < ActiveRecord::Base
|
||||
belongs_to :author
|
||||
|
||||
has_many :citations, foreign_key: "book1_id"
|
||||
has_many :citations, foreign_key: "book1_id", inverse_of: :book
|
||||
has_many :references, -> { distinct }, through: :citations, source: :reference_of
|
||||
|
||||
has_many :subscriptions
|
||||
|
|
|
@ -1,6 +1,7 @@
|
|||
# frozen_string_literal: true
|
||||
|
||||
class Citation < ActiveRecord::Base
|
||||
belongs_to :book, foreign_key: :book1_id, inverse_of: :citations, touch: true
|
||||
belongs_to :reference_of, class_name: "Book", foreign_key: :book2_id
|
||||
has_many :citations
|
||||
end
|
||||
|
|
|
@ -4,5 +4,7 @@ class Toy < ActiveRecord::Base
|
|||
self.primary_key = :toy_id
|
||||
belongs_to :pet
|
||||
|
||||
has_many :sponsors, as: :sponsorable, inverse_of: :sponsorable
|
||||
|
||||
scope :with_pet, -> { joins(:pet) }
|
||||
end
|
||||
|
|
Loading…
Reference in a new issue