Skip to content

Commit

Permalink
Save has_one associations only if record has changes
Browse files Browse the repository at this point in the history
Prevents save related callbacks such as `after_commit` being
triggered when `has_one` objects are already persisted and have no
changes.
  • Loading branch information
alan committed Oct 31, 2013
1 parent 6e5d409 commit f74a561
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 2 deletions.
6 changes: 6 additions & 0 deletions activerecord/CHANGELOG.md
@@ -1,3 +1,9 @@
* Only save has_one associations if record has changes.
Previously after save related callbacks, such as `#after_commit`, were triggered when the has_one
object did not get saved to the db.

*Alan Kennedy*

* ActiveRecord::Base#attribute_for_inspect now truncates long arrays (more than 10 elements)

*Jan Bernacki*
Expand Down
5 changes: 3 additions & 2 deletions activerecord/lib/active_record/autosave_association.rb
Expand Up @@ -377,14 +377,15 @@ def save_collection_association(reflection)
def save_has_one_association(reflection)
association = association_instance_get(reflection.name)
record = association && association.load_target

if record && !record.destroyed?
autosave = reflection.options[:autosave]

if autosave && record.marked_for_destruction?
record.destroy
else
elsif autosave != false
key = reflection.options[:primary_key] ? send(reflection.options[:primary_key]) : id
if autosave != false && (autosave || new_record? || record_changed?(reflection, record, key))
if (autosave && record.changed_for_autosave?) || new_record? || record_changed?(reflection, record, key)

unless reflection.through_reflection
record[reflection.foreign_key] = key
Expand Down
15 changes: 15 additions & 0 deletions activerecord/test/cases/autosave_association_test.rb
Expand Up @@ -626,10 +626,25 @@ def save(*args)
end
end

@ship.pirate.catchphrase = "Changed Catchphrase"

assert_raise(RuntimeError) { assert !@pirate.save }
assert_not_nil @pirate.reload.ship
end

def test_should_save_changed_has_one_changed_object_if_child_is_saved
@pirate.ship.name = "NewName"
@pirate.ship.expects(:save).once.returns(true)

assert @pirate.save
end

def test_should_not_save_changed_has_one_unchanged_object_if_child_is_saved
@pirate.ship.expects(:save).never

assert @pirate.save
end

# belongs_to
def test_should_destroy_a_parent_association_as_part_of_the_save_transaction_if_it_was_marked_for_destroyal
assert !@ship.pirate.marked_for_destruction?
Expand Down

0 comments on commit f74a561

Please sign in to comment.