mirror of
https://github.com/rails/rails.git
synced 2022-11-09 12:12:34 -05:00
Allow autosave: true
to be used with inverse of
With the changes in #25337, double save bugs are pretty much impossible, so we can just lift this restriction with pretty much no change. There were a handful of cases where we were relying on specific quirks in tests that had to be updated. The change to has_one associations was due to a particularly interesting test where an autosaved has_one association was replaced with a new child, where the child failed to save but the test wanted to check that the parent id persisted to `nil`. I think this is almost certainly the wrong behavior, and I may change that behavior later. But ultimately the root cause was because we never remove the parent in memory when nullifying the child. This makes #23197 no longer needed, but it is what we'll do to fix some issues on 5.0 Close #23197
This commit is contained in:
parent
29b3b5dd8e
commit
c7adc610f0
5 changed files with 14 additions and 12 deletions
|
@ -112,6 +112,15 @@ module ActiveRecord
|
|||
record
|
||||
end
|
||||
|
||||
# Remove the inverse association, if possible
|
||||
def remove_inverse_instance(record)
|
||||
if invertible_for?(record)
|
||||
inverse = record.association(inverse_reflection_for(record).name)
|
||||
inverse.target = nil
|
||||
inverse.inversed = false
|
||||
end
|
||||
end
|
||||
|
||||
# Returns the class of the target. belongs_to polymorphic overrides this to look at the
|
||||
# polymorphic_type field on the owner.
|
||||
def klass
|
||||
|
|
|
@ -86,8 +86,9 @@ module ActiveRecord
|
|||
target.delete
|
||||
when :destroy
|
||||
target.destroy
|
||||
else
|
||||
else
|
||||
nullify_owner_attributes(target)
|
||||
remove_inverse_instance(target)
|
||||
|
||||
if target.persisted? && owner.persisted? && !target.save
|
||||
set_owner_attributes(target)
|
||||
|
|
|
@ -282,11 +282,6 @@ module ActiveRecord
|
|||
end
|
||||
|
||||
def autosave=(autosave)
|
||||
# autosave and inverse_of do not get along together nowadays. They may
|
||||
# for example cause double saves. Thus, we disable this flag. If in the
|
||||
# future those two flags are known to work well together, this could be
|
||||
# removed.
|
||||
@automatic_inverse_of = false
|
||||
@options[:autosave] = autosave
|
||||
parent_reflection = self.parent_reflection
|
||||
if parent_reflection
|
||||
|
@ -544,11 +539,7 @@ module ActiveRecord
|
|||
# nil.
|
||||
def inverse_name
|
||||
options.fetch(:inverse_of) do
|
||||
if @automatic_inverse_of == false
|
||||
nil
|
||||
else
|
||||
@automatic_inverse_of ||= automatic_inverse_of
|
||||
end
|
||||
@automatic_inverse_of ||= automatic_inverse_of
|
||||
end
|
||||
end
|
||||
|
||||
|
|
|
@ -601,7 +601,7 @@ class HasOneAssociationsTest < ActiveRecord::TestCase
|
|||
|
||||
new_ship = Ship.create(name: "new name")
|
||||
assert_queries(2) do
|
||||
# One query for updating name and second query for updating pirate_id
|
||||
# One query to nullify the old ship, one query to update the new ship
|
||||
pirate.ship = new_ship
|
||||
end
|
||||
|
||||
|
|
|
@ -792,6 +792,7 @@ class TestDestroyAsPartOfAutosaveAssociation < ActiveRecord::TestCase
|
|||
end
|
||||
|
||||
@ship.pirate.catchphrase = "Changed Catchphrase"
|
||||
@ship.name_will_change!
|
||||
|
||||
assert_raise(RuntimeError) { assert !@pirate.save }
|
||||
assert_not_nil @pirate.reload.ship
|
||||
|
|
Loading…
Reference in a new issue