Merge branch '13376-fix-mr-block-validation-checks' into 'master'

Respect merge request block validations

Closes #13376

See merge request gitlab-org/gitlab-ee!15123

ee/app/models/merge_request_block.rb

@@ -14,23 +14,6 @@ class MergeRequestBlock < ApplicationRecord
     where(blocking_merge_request_id: ids).includes(:blocking_merge_request)
   end
 
-  # Add a number of blocks at once. Pass a hash of blocking_id => blocked_id, or
-  # an Array of [blocking_id, blocked_id] tuples
-  def self.bulk_insert(pairs)
-    now = Time.current
-
-    rows = pairs.map do |blocking, blocked|
-      {
-        blocking_merge_request_id: blocking,
-        blocked_merge_request_id: blocked,
-        created_at: now,
-        updated_at: now
-      }
-    end
-
-    ::Gitlab::Database.bulk_insert(table_name, rows)
-  end
-
   private
 
   def check_block_constraints

ee/app/services/merge_requests/update_blocks_service.rb

@@ -38,9 +38,13 @@ module MergeRequests
         .with_blocking_mr_ids(ids_to_del)
         .delete_all
 
-      ::MergeRequestBlock.bulk_insert(
-        ids_to_add.map { |blocking_id| [blocking_id, merge_request.id] }
-      )
+      # If the block is invalid, silently fail to add it
+      ids_to_add.each do |blocking_id|
+        ::MergeRequestBlock.create(
+          blocking_merge_request_id: blocking_id,
+          blocked_merge_request_id: merge_request.id
+        )
+      end
 
       true
     end

ee/spec/models/merge_request_block_spec.rb

@@ -69,31 +69,6 @@ describe MergeRequestBlock do
     end
   end
 
-  describe '.bulk_insert' do
-    let(:mrs) { create_list(:merge_request, 4) }
-
-    it 'inserts multiple blocks specified as a Hash' do
-      described_class.bulk_insert(
-        mrs[0].id => mrs[1].id,
-        mrs[2].id => mrs[3].id
-      )
-
-      expect(described_class.all).to contain_exactly(
-        having_attributes(blocking_merge_request_id: mrs[0].id, blocked_merge_request_id: mrs[1].id),
-        having_attributes(blocking_merge_request_id: mrs[2].id, blocked_merge_request_id: mrs[3].id)
-      )
-    end
-
-    it 'inserts multiple blocks specified as an Array' do
-      described_class.bulk_insert([[mrs[0].id, mrs[1].id], [mrs[2].id, mrs[3].id]])
-
-      expect(described_class.all).to contain_exactly(
-        having_attributes(blocking_merge_request_id: mrs[0].id, blocked_merge_request_id: mrs[1].id),
-        having_attributes(blocking_merge_request_id: mrs[2].id, blocked_merge_request_id: mrs[3].id)
-      )
-    end
-  end
-
   describe '.with_blocking_mr_ids' do
     let!(:block) { create(:merge_request_block) }
     let!(:other_block) { create(:merge_request_block) }

ee/spec/services/merge_requests/update_blocks_service_spec.rb

@@ -84,6 +84,16 @@ describe MergeRequests::UpdateBlocksService do
             expect(merge_request.blocking_merge_requests)
               .to contain_exactly(mr_to_add, mr_to_keep, hidden_mr)
           end
+
+          context 'with a self-referential block' do
+            let(:mr_to_add) { merge_request }
+
+            it 'ignores the addition' do
+              service.execute
+
+              expect(merge_request.blocking_merge_requests).not_to include(mr_to_add)
+            end
+          end
         end
 
         context 'with remove_hidden: true' do