diff --git a/app/models/bulk_update_request.rb b/app/models/bulk_update_request.rb index 83e5ac313..2cfcd50bc 100644 --- a/app/models/bulk_update_request.rb +++ b/app/models/bulk_update_request.rb @@ -90,10 +90,12 @@ class BulkUpdateRequest < ApplicationRecord end def approve!(approver) - CurrentUser.scoped(approver) do - AliasAndImplicationImporter.new(script, forum_topic_id, "1", true).process! - update(status: "approved", approver: CurrentUser.user, skip_secondary_validations: true) - forum_updater.update("The #{bulk_update_request_link} (forum ##{forum_post.id}) has been approved by @#{approver.name}.", "APPROVED") + transaction do + CurrentUser.scoped(approver) do + AliasAndImplicationImporter.new(script, forum_topic_id, "1", true).process! + update(status: "approved", approver: CurrentUser.user, skip_secondary_validations: true) + forum_updater.update("The #{bulk_update_request_link} (forum ##{forum_post.id}) has been approved by @#{approver.name}.", "APPROVED") + end end rescue AliasAndImplicationImporter::Error => x @@ -118,8 +120,10 @@ class BulkUpdateRequest < ApplicationRecord end def reject!(rejector) - forum_updater.update("The #{bulk_update_request_link} (forum ##{forum_post.id}) has been rejected by @#{rejector.name}.", "REJECTED") - update_attribute(:status, "rejected") + transaction do + update(status: "rejected") + forum_updater.update("The #{bulk_update_request_link} (forum ##{forum_post.id}) has been rejected by @#{rejector.name}.", "REJECTED") + end end def bulk_update_request_link diff --git a/test/unit/bulk_update_request_test.rb b/test/unit/bulk_update_request_test.rb index b25a67b80..3d507dbb1 100644 --- a/test/unit/bulk_update_request_test.rb +++ b/test/unit/bulk_update_request_test.rb @@ -112,15 +112,21 @@ class BulkUpdateRequestTest < ActiveSupport::TestCase @req = FactoryGirl.create(:bulk_update_request, :script => "create alias AAA -> BBB", :forum_topic_id => @topic.id, :forum_post_id => @post.id, :title => "[bulk] hoge") end - should "handle errors gracefully" do + should "gracefully handle validation errors during approval" do @req.stubs(:update).raises(AliasAndImplicationImporter::Error.new("blah")) assert_difference("ForumPost.count", 1) do @req.approve!(@admin) end - @topic.reload - @post.reload - assert_match(/\[FAILED\]/, @topic.title) + assert_equal("pending", @req.reload.status) + assert_match(/\[FAILED\]/, @topic.reload.title) + end + + should "leave the BUR pending if there is an unexpected error during approval" do + @req.forum_updater.stubs(:update).raises(RuntimeError.new("blah")) + assert_raises(RuntimeError) { @req.approve!(@admin) } + + assert_equal("pending", @req.reload.status) end should "downcase the text" do