Skip to content

Commit

Permalink
Correctly ignore mark_for_destruction without autosave
Browse files Browse the repository at this point in the history
As per the docs, `mark_for_destruction` should do nothing if `autosave`
is not set to true. We normally persist associations on a record no
matter what if the record is a new record, but we were always skipping
records which were `marked_for_destruction?`.

Fixes rails#20882
  • Loading branch information
sgrif committed Jul 20, 2015
1 parent 9bd6e39 commit c0ef95a
Show file tree
Hide file tree
Showing 4 changed files with 16 additions and 1 deletion.
7 changes: 7 additions & 0 deletions activerecord/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,10 @@
* Correctly ignore `mark_for_destruction` when `autosave` isn't set to `true`
when validating associations.

Fixes #20882.

*Sean Griffin*

* Fix a bug where counter_cache doesn't always work with polymorphic
relations.

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 @@ -325,7 +325,7 @@ def validate_collection_association(reflection)
# the parent, <tt>self</tt>, if it wasn't. Skips any <tt>:autosave</tt>
# enabled records if they're marked_for_destruction? or destroyed.
def association_valid?(reflection, record)
return true if record.destroyed? || record.marked_for_destruction?
return true if record.destroyed? || (reflection.options[:autosave] && record.marked_for_destruction?)

validation_context = self.validation_context unless [:create, :update].include?(self.validation_context)
unless valid = record.valid?(validation_context)
Expand Down
7 changes: 7 additions & 0 deletions activerecord/test/cases/autosave_association_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1154,6 +1154,13 @@ def save(*args)
def test_should_not_load_the_associated_model
assert_queries(1) { @pirate.catchphrase = 'Arr'; @pirate.save! }
end

def test_mark_for_destruction_is_ignored_without_autosave_true
ship = ShipWithoutNestedAttributes.new(name: "The Black Flag")
ship.parts.build.mark_for_destruction

assert_not ship.valid?
end
end

class TestAutosaveAssociationOnAHasOneThroughAssociation < ActiveRecord::TestCase
Expand Down
1 change: 1 addition & 0 deletions activerecord/test/models/ship.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ def cancel_save_callback_method
class ShipWithoutNestedAttributes < ActiveRecord::Base
self.table_name = "ships"
has_many :prisoners, inverse_of: :ship, foreign_key: :ship_id
has_many :parts, class_name: "ShipPart", foreign_key: :ship_id

validates :name, presence: true
end
Expand Down

0 comments on commit c0ef95a

Please sign in to comment.