Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Rollback the transaction when one of the autosave associations fails …

…to save. [#3391 state:resolved]
  • Loading branch information...
commit f2aacd51405724cdf7cfd36a439c9dbfce16973a 1 parent 5193fe9
@alloy alloy authored
View
14 activerecord/lib/active_record/autosave_association.rb
@@ -296,13 +296,15 @@ def save_collection_association(reflection)
association.destroy(record)
elsif autosave != false && (@new_record_before_save || record.new_record?)
if autosave
- association.send(:insert_record, record, false, false)
+ saved = association.send(:insert_record, record, false, false)
else
association.send(:insert_record, record)
end
elsif autosave
- record.save(false)
+ saved = record.save(false)
end
+
+ raise ActiveRecord::Rollback if saved == false
end
end
@@ -329,7 +331,9 @@ def save_has_one_association(reflection)
key = reflection.options[:primary_key] ? send(reflection.options[:primary_key]) : id
if autosave != false && (new_record? || association.new_record? || association[reflection.primary_key_name] != key || autosave)
association[reflection.primary_key_name] = key
- association.save(!autosave)
+ saved = association.save(!autosave)
+ raise ActiveRecord::Rollback if !saved && autosave
+ saved
end
end
end
@@ -350,7 +354,7 @@ def save_belongs_to_association(reflection)
if autosave && association.marked_for_destruction?
association.destroy
elsif autosave != false
- association.save(!autosave) if association.new_record? || autosave
+ saved = association.save(!autosave) if association.new_record? || autosave
if association.updated?
association_id = association.send(reflection.options[:primary_key] || :id)
@@ -360,6 +364,8 @@ def save_belongs_to_association(reflection)
self[reflection.options[:foreign_type]] = association.class.base_class.name.to_s
end
end
+
+ saved if autosave
end
end
end
View
45 activerecord/test/cases/autosave_association_test.rb
@@ -830,6 +830,18 @@ def test_should_still_raise_an_ActiveRecordRecord_Invalid_exception_if_we_want_t
end
end
+ def test_should_not_save_and_return_false_if_a_callback_cancelled_saving
+ pirate = Pirate.new(:catchphrase => 'Arr')
+ ship = pirate.build_ship(:name => 'The Vile Insanity')
+ ship.cancel_save_from_callback = true
+
+ assert_no_difference 'Pirate.count' do
+ assert_no_difference 'Ship.count' do
+ assert !pirate.save
+ end
+ end
+ end
+
def test_should_rollback_any_changes_if_an_exception_occurred_while_saving
before = [@pirate.catchphrase, @pirate.ship.name]
@@ -913,6 +925,18 @@ def test_should_still_raise_an_ActiveRecordRecord_Invalid_exception_if_we_want_t
end
end
+ def test_should_not_save_and_return_false_if_a_callback_cancelled_saving
+ ship = Ship.new(:name => 'The Vile Insanity')
+ pirate = ship.build_pirate(:catchphrase => 'Arr')
+ pirate.cancel_save_from_callback = true
+
+ assert_no_difference 'Ship.count' do
+ assert_no_difference 'Pirate.count' do
+ assert !ship.save
+ end
+ end
+ end
+
def test_should_rollback_any_changes_if_an_exception_occurred_while_saving
before = [@ship.pirate.catchphrase, @ship.name]
@@ -928,7 +952,6 @@ def save(*args)
end
assert_raise(RuntimeError) { assert !@ship.save }
- # TODO: Why does using reload on @ship looses the associated pirate?
assert_equal before, [@ship.pirate.reload.catchphrase, @ship.reload.name]
end
@@ -1029,6 +1052,26 @@ def test_should_allow_to_bypass_validations_on_the_associated_models_on_create
end
end
+ def test_should_not_save_and_return_false_if_a_callback_cancelled_saving_in_either_create_or_update
+ @pirate.catchphrase = 'Changed'
+ @child_1.name = 'Changed'
+ @child_1.cancel_save_from_callback = true
+
+ assert !@pirate.save
+ assert_equal "Don' botharrr talkin' like one, savvy?", @pirate.reload.catchphrase
+ assert_equal "Posideons Killer", @child_1.reload.name
+
+ new_pirate = Pirate.new(:catchphrase => 'Arr')
+ new_child = new_pirate.send(@association_name).build(:name => 'Grace OMalley')
+ new_child.cancel_save_from_callback = true
+
+ assert_no_difference 'Pirate.count' do
+ assert_no_difference "#{new_child.class.name}.count" do
+ assert !new_pirate.save
+ end
+ end
+ end
+
def test_should_rollback_any_changes_if_an_exception_occurred_while_saving
before = [@pirate.catchphrase, *@pirate.send(@association_name).map(&:name)]
new_names = ['Grace OMalley', 'Privateers Greed']
View
6 activerecord/test/models/bird.rb
@@ -1,3 +1,9 @@
class Bird < ActiveRecord::Base
validates_presence_of :name
+
+ attr_accessor :cancel_save_from_callback
+ before_save :cancel_save_callback_method, :if => :cancel_save_from_callback
+ def cancel_save_callback_method
+ false
+ end
end
View
6 activerecord/test/models/parrot.rb
@@ -6,6 +6,12 @@ class Parrot < ActiveRecord::Base
alias_attribute :title, :name
validates_presence_of :name
+
+ attr_accessor :cancel_save_from_callback
+ before_save :cancel_save_callback_method, :if => :cancel_save_from_callback
+ def cancel_save_callback_method
+ false
+ end
end
class LiveParrot < Parrot
View
6 activerecord/test/models/pirate.rb
@@ -51,6 +51,12 @@ def reject_empty_ships_on_create(attributes)
attributes.delete('_reject_me_if_new').present? && new_record?
end
+ attr_accessor :cancel_save_from_callback
+ before_save :cancel_save_callback_method, :if => :cancel_save_from_callback
+ def cancel_save_callback_method
+ false
+ end
+
private
def log_before_add(record)
log(record, "before_adding_method")
View
6 activerecord/test/models/ship.rb
@@ -9,4 +9,10 @@ class Ship < ActiveRecord::Base
accepts_nested_attributes_for :update_only_pirate, :update_only => true
validates_presence_of :name
+
+ attr_accessor :cancel_save_from_callback
+ before_save :cancel_save_callback_method, :if => :cancel_save_from_callback
+ def cancel_save_callback_method
+ false
+ end
end
Please sign in to comment.
Something went wrong with that request. Please try again.