Merge branch '30306-transaction-while-moving-issues-to-ghost-user' into 'master'

Add a transaction around `move_issues_to_ghost_user`

See merge request !10465

2 files changed, 70 insertions, 0 deletions

diff --git a/spec/services/users/migrate_to_ghost_user_service_spec.rb b/spec/services/users/migrate_to_ghost_user_service_spec.rb
index 8c5b7e41c15..9e1edf1ac30 100644
--- a/spec/services/users/migrate_to_ghost_user_service_spec.rb
+++ b/spec/services/users/migrate_to_ghost_user_service_spec.rb
@@ -60,5 +60,23 @@ describe Users::MigrateToGhostUserService, services: true do
         end
       end
     end
+
+    context "when record migration fails with a rollback exception" do
+      before do
+        expect_any_instance_of(MergeRequest::ActiveRecord_Associations_CollectionProxy)
+          .to receive(:update_all).and_raise(ActiveRecord::Rollback)
+      end
+
+      context "for records that were already migrated" do
+        let!(:issue) { create(:issue, project: project, author: user) }
+        let!(:merge_request) { create(:merge_request, source_project: project, author: user, target_branch: "first") }
+
+        it "reverses the migration" do
+          service.execute
+
+          expect(issue.reload.author).to eq(user)
+        end
+      end
+    end
   end
 end
diff --git a/spec/support/services/migrate_to_ghost_user_service_shared_examples.rb b/spec/support/services/migrate_to_ghost_user_service_shared_examples.rb
index 0eac587e973..dcc562c684b 100644
--- a/spec/support/services/migrate_to_ghost_user_service_shared_examples.rb
+++ b/spec/support/services/migrate_to_ghost_user_service_shared_examples.rb
@@ -35,5 +35,57 @@ shared_examples "migrating a deleted user's associated records to the ghost user
 
       expect(user).to be_blocked
     end
+
+    context "race conditions" do
+      context "when #{record_class_name} migration fails and is rolled back" do
+        before do
+          expect_any_instance_of(record_class::ActiveRecord_Associations_CollectionProxy)
+            .to receive(:update_all).and_raise(ActiveRecord::Rollback)
+        end
+
+        it 'rolls back the user block' do
+          service.execute
+
+          expect(user.reload).not_to be_blocked
+        end
+
+        it "doesn't unblock an previously-blocked user" do
+          user.block
+
+          service.execute
+
+          expect(user.reload).to be_blocked
+        end
+      end
+
+      context "when #{record_class_name} migration fails with a non-rollback exception" do
+        before do
+          expect_any_instance_of(record_class::ActiveRecord_Associations_CollectionProxy)
+            .to receive(:update_all).and_raise(ArgumentError)
+        end
+
+        it 'rolls back the user block' do
+          service.execute rescue nil
+
+          expect(user.reload).not_to be_blocked
+        end
+
+        it "doesn't unblock an previously-blocked user" do
+          user.block
+
+          service.execute rescue nil
+
+          expect(user.reload).to be_blocked
+        end
+      end
+
+      it "blocks the user before #{record_class_name} migration begins" do
+        expect(service).to receive("migrate_#{record_class_name.parameterize('_')}s".to_sym) do
+          expect(user.reload).to be_blocked
+        end
+
+        service.execute
+      end
+    end
   end
 end