Mark merge request as unchecked when MR is prepared for mergeability

We currently only require a pipeline to be created (if there's any)
before we mark a merge request to be ready for mergeability check.

Before this fix, we are also waiting for other stuff to be done
before we set a merge request ready for mergeability.

In this fix, we split the logic in AfterCreateService so we only
wait for the pipeline to be created. If it's done, we set the
`merge_status` to `unchecked`.

This is behind the `early_prepare_for_mergeability` feature flag.

app/services/merge_requests/after_create_service.rb

@@ -3,12 +3,19 @@
 module MergeRequests
   class AfterCreateService < MergeRequests::BaseService
     def execute(merge_request)
+      prepare_for_mergeability(merge_request) if early_prepare_for_mergeability?(merge_request)
       prepare_merge_request(merge_request)
-      merge_request.mark_as_unchecked if merge_request.preparing?
+      mark_as_unchecked(merge_request) unless early_prepare_for_mergeability?(merge_request)
     end
 
     private
 
+    def prepare_for_mergeability(merge_request)
+      create_pipeline_for(merge_request, current_user)
+      merge_request.update_head_pipeline
+      mark_as_unchecked(merge_request)
+    end
+
     def prepare_merge_request(merge_request)
       event_service.open_mr(merge_request, current_user)
 
@@ -17,8 +24,10 @@ module MergeRequests
 
       notification_service.new_merge_request(merge_request, current_user)
 
-      create_pipeline_for(merge_request, current_user)
-      merge_request.update_head_pipeline
+      unless early_prepare_for_mergeability?(merge_request)
+        create_pipeline_for(merge_request, current_user)
+        merge_request.update_head_pipeline
+      end
 
       merge_request.diffs(include_stats: false).write_cache
       merge_request.create_cross_references!(current_user)
@@ -37,6 +46,14 @@ module MergeRequests
     def link_lfs_objects(merge_request)
       LinkLfsObjectsService.new(project: merge_request.target_project).execute(merge_request)
     end
+
+    def early_prepare_for_mergeability?(merge_request)
+      Feature.enabled?(:early_prepare_for_mergeability, merge_request.target_project)
+    end
+
+    def mark_as_unchecked(merge_request)
+      merge_request.mark_as_unchecked if merge_request.preparing?
+    end
   end
 end
 

config/feature_flags/development/early_prepare_for_mergeability.yml

+---
+name: early_prepare_for_mergeability
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/75402
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/346667
+milestone: '14.6'
+type: development
+group: group::code review
+default_enabled: false

spec/services/merge_requests/after_create_service_spec.rb

@@ -85,13 +85,67 @@ RSpec.describe MergeRequests::AfterCreateService do
 
     context 'when merge request is in preparing state' do
       before do
+        merge_request.mark_as_unchecked! unless merge_request.unchecked?
         merge_request.mark_as_preparing!
-        execute_service
       end
 
       it 'marks the merge request as unchecked' do
+        execute_service
+
         expect(merge_request.reload).to be_unchecked
       end
+
+      context 'when preparing for mergeability fails' do
+        before do
+          # This is only one of the possible cases that can fail. This is to
+          # simulate a failure that happens during the service call.
+          allow(merge_request)
+            .to receive(:update_head_pipeline)
+            .and_raise(StandardError)
+        end
+
+        it 'does not mark the merge request as unchecked' do
+          expect { execute_service }.to raise_error(StandardError)
+          expect(merge_request.reload).to be_preparing
+        end
+
+        context 'when early_prepare_for_mergeability feature flag is disabled' do
+          before do
+            stub_feature_flags(early_prepare_for_mergeability: false)
+          end
+
+          it 'does not mark the merge request as unchecked' do
+            expect { execute_service }.to raise_error(StandardError)
+            expect(merge_request.reload).to be_preparing
+          end
+        end
+      end
+
+      context 'when preparing merge request fails' do
+        before do
+          # This is only one of the possible cases that can fail. This is to
+          # simulate a failure that happens during the service call.
+          allow(merge_request)
+            .to receive_message_chain(:diffs, :write_cache)
+            .and_raise(StandardError)
+        end
+
+        it 'still marks the merge request as unchecked' do
+          expect { execute_service }.to raise_error(StandardError)
+          expect(merge_request.reload).to be_unchecked
+        end
+
+        context 'when early_prepare_for_mergeability feature flag is disabled' do
+          before do
+            stub_feature_flags(early_prepare_for_mergeability: false)
+          end
+
+          it 'does not mark the merge request as unchecked' do
+            expect { execute_service }.to raise_error(StandardError)
+            expect(merge_request.reload).to be_preparing
+          end
+        end
+      end
     end
 
     it 'increments the usage data counter of create event' do