Skip to content

Commit

Permalink
update_attributes and update_attributes! are now wrapped in a transac…
Browse files Browse the repository at this point in the history
…tion

[#922 state:resovled]

Signed-off-by: José Valim <jose.valim@gmail.com>
  • Loading branch information
Neeraj Singh authored and josevalim committed Jul 13, 2010
1 parent e35e617 commit f4fbc2c
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 5 deletions.
12 changes: 8 additions & 4 deletions activerecord/lib/active_record/persistence.rb
Expand Up @@ -121,15 +121,19 @@ def update_attribute(name, value)
# Updates all the attributes from the passed-in Hash and saves the record.
# If the object is invalid, the saving will fail and false will be returned.
def update_attributes(attributes)
self.attributes = attributes
save
with_transaction_returning_status do
self.attributes = attributes
save
end
end

# Updates an object just like Base.update_attributes but calls save! instead
# of save so an exception is raised if the record is invalid.
def update_attributes!(attributes)
self.attributes = attributes
save!
with_transaction_returning_status do
self.attributes = attributes
save!
end
end

# Initializes +attribute+ to zero if +nil+ and adds the value passed as +by+ (default is 1).
Expand Down
23 changes: 22 additions & 1 deletion activerecord/test/cases/transactions_test.rb
Expand Up @@ -3,10 +3,12 @@
require 'models/reply'
require 'models/developer'
require 'models/book'
require 'models/author'
require 'models/post'

class TransactionTest < ActiveRecord::TestCase
self.use_transactional_fixtures = false
fixtures :topics, :developers
fixtures :topics, :developers, :authors, :posts

def setup
@first, @second = Topic.find(1, 2).sort_by { |t| t.id }
Expand Down Expand Up @@ -103,6 +105,25 @@ def test_raising_exception_in_callback_rollbacks_in_save
end
end

def test_update_attributes_should_rollback_on_failure
author = Author.find(1)
posts_count = author.posts.size
assert posts_count > 0
status = author.update_attributes(:name => nil, :post_ids => [])
assert !status
assert_equal posts_count, author.posts(true).size
end

def test_update_attributes_should_rollback_on_failure!
author = Author.find(1)
posts_count = author.posts.size
assert posts_count > 0
assert_raise(ActiveRecord::RecordInvalid) do
author.update_attributes!(:name => nil, :post_ids => [])
end
assert_equal posts_count, author.posts(true).size
end

def test_cancellation_from_before_destroy_rollbacks_in_destroy
add_cancelling_before_destroy_with_db_side_effect_to_topic
begin
Expand Down
2 changes: 2 additions & 0 deletions activerecord/test/models/author.rb
Expand Up @@ -108,6 +108,8 @@ def social
%w(twitter github)
end

validates_presence_of :name

private
def log_before_adding(object)
@post_log << "before_adding#{object.id || '<new>'}"
Expand Down

2 comments on commit f4fbc2c

@nesquena
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My friend Thomas and I (2Collegebums) posted this ticket in August 2008. I have been tracking the progress ever since on LH: https://rails.lighthouseapp.com/projects/8994/tickets/922-has_many-through-transaction-rollback . Just wanted to thank you guys for committing this fix once and for all just short of 2 years later :)

@neerajsingh0101
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

well rails wanted to ensure that 2 collegebums successfully graduated from college before the fix is applied :-)

Please sign in to comment.