Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Merge pull request #8313 from alan/only_save_changed_has_one_objects

Save has_one associations only if record has changes

Conflicts:
	activerecord/CHANGELOG.md

Conflicts:
	activerecord/CHANGELOG.md
  • Loading branch information...
commit 3fa487d9279ad7515e887c1fecd369917f09604c 1 parent f31a035
@rafaelfranca rafaelfranca authored
View
6 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*
+
* `includes` is able to detect the right preloading strategy when string
joins are involved.
View
5 activerecord/lib/active_record/autosave_association.rb
@@ -377,15 +377,16 @@ 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
end
View
15 activerecord/test/cases/autosave_association_test.rb
@@ -687,10 +687,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?
Please sign in to comment.
Something went wrong with that request. Please try again.