Fix #3554: approving BUR with nil forum_post_id doesn't update forum.

Wrap `approve!` and `reject!` in transactions so that if there's an
error in approving or rejecting a BUR, it leaves the BUR's status
unchanged instead of updating the BUR but not updating the forum.

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

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