Implement new preparing internal merge_status

When we check if MR is mergeable we also check the CI pipeline.
Since we are creating the pipeline asynchronously via
`MergeRequests::AfterCreateService`, there's a possible race
condition wherein the pipeline isn't created yet but we already
checked if MR is mergeable. That can result to inaccurate outcome.

To ensure that whenever we check if MR is mergeable, we introduced
a new internal state called `preparing`. When the MR is in this
state, it means it's still being prepared (e.g. creating a pipeline
) and it's not mergeable.

Once the preparation is finished, we mark the MR as unchecked so
we can check its mergeability when the user views the MR page.

app/models/merge_request.rb

@@ -191,8 +191,12 @@ class MergeRequest < ApplicationRecord
   end
 
   state_machine :merge_status, initial: :unchecked do
+    event :mark_as_preparing do
+      transition unchecked: :preparing
+    end
+
     event :mark_as_unchecked do
-      transition [:can_be_merged, :checking, :unchecked] => :unchecked
+      transition [:preparing, :can_be_merged, :checking, :unchecked] => :unchecked
       transition [:cannot_be_merged, :cannot_be_merged_rechecking, :cannot_be_merged_recheck] => :cannot_be_merged_recheck
     end
 
@@ -237,7 +241,7 @@ class MergeRequest < ApplicationRecord
   # Returns current merge_status except it returns `cannot_be_merged_rechecking` as `checking`
   # to avoid exposing unnecessary internal state
   def public_merge_status
-    cannot_be_merged_rechecking? ? 'checking' : merge_status
+    cannot_be_merged_rechecking? || preparing? ? 'checking' : merge_status
   end
 
   validates :source_project, presence: true, unless: [:allow_broken, :importing?, :closed_or_merged_without_fork?]
@@ -1054,6 +1058,8 @@ class MergeRequest < ApplicationRecord
   end
 
   def mergeable?(skip_ci_check: false, skip_discussions_check: false)
+    return false if preparing?
+
     return false unless mergeable_state?(skip_ci_check: skip_ci_check,
                                          skip_discussions_check: skip_discussions_check)
 

app/services/merge_requests/after_create_service.rb

@@ -3,6 +3,13 @@
 module MergeRequests
   class AfterCreateService < MergeRequests::BaseService
     def execute(merge_request)
+      prepare_merge_request(merge_request)
+      merge_request.mark_as_unchecked! if merge_request.preparing?
+    end
+
+    private
+
+    def prepare_merge_request(merge_request)
       event_service.open_mr(merge_request, current_user)
       merge_request_activity_counter.track_create_mr_action(user: current_user)
       notification_service.new_merge_request(merge_request, current_user)

app/services/merge_requests/create_service.rb

@@ -14,6 +14,8 @@ module MergeRequests
     end
 
     def after_create(issuable)
+      issuable.mark_as_preparing
+
       # Add new items to MergeRequests::AfterCreateService if they can
       # be performed in Sidekiq
       NewMergeRequestWorker.perform_async(issuable.id, current_user.id)

changelogs/unreleased/291012-preparing-mr-state.yml

+---
+title: Implement new preparing internal merge_status
+merge_request: 54900
+author:
+type: other

ee/app/services/ee/merge_requests/after_create_service.rb

@@ -5,8 +5,8 @@ module EE
     module AfterCreateService
       extend ::Gitlab::Utils::Override
 
-      override :execute
-      def execute(merge_request)
+      override :prepare_merge_request
+      def prepare_merge_request(merge_request)
         super
 
         schedule_sync_for(merge_request.head_pipeline_id)

spec/models/merge_request_spec.rb

@@ -2885,6 +2885,11 @@ RSpec.describe MergeRequest, factory_default: :keep do
   describe '#mergeable?' do
     subject { build_stubbed(:merge_request) }
 
+    it 'returns false if still preparing' do
+      expect(subject).to receive(:preparing?) { true }
+      expect(subject.mergeable?).to be_falsey
+    end
+
     it 'returns false if #mergeable_state? is false' do
       expect(subject).to receive(:mergeable_state?) { false }
 
@@ -3075,6 +3080,7 @@ RSpec.describe MergeRequest, factory_default: :keep do
     subject { build(:merge_request, merge_status: status) }
 
     where(:status, :public_status) do
+      'preparing' | 'checking'
       'cannot_be_merged_rechecking' | 'checking'
       'checking'                    | 'checking'
       'cannot_be_merged'            | 'cannot_be_merged'

spec/services/merge_requests/after_create_service_spec.rb

@@ -67,5 +67,27 @@ RSpec.describe MergeRequests::AfterCreateService do
     it_behaves_like 'records an onboarding progress action', :merge_request_created do
       let(:namespace) { merge_request.target_project.namespace }
     end
+
+    context 'when merge request is in unchecked state' do
+      before do
+        merge_request.mark_as_unchecked!
+        execute_service
+      end
+
+      it 'does not change its state' do
+        expect(merge_request.reload).to be_unchecked
+      end
+    end
+
+    context 'when merge request is in preparing state' do
+      before do
+        merge_request.mark_as_preparing!
+        execute_service
+      end
+
+      it 'marks the merge request as unchecked' do
+        expect(merge_request.reload).to be_unchecked
+      end
+    end
   end
 end

spec/services/merge_requests/create_service_spec.rb

@@ -67,6 +67,10 @@ RSpec.describe MergeRequests::CreateService, :clean_gitlab_redis_shared_state do
         expect(Event.where(attributes).count).to eq(1)
       end
 
+      it 'sets the merge_status to preparing' do
+        expect(merge_request.reload).to be_preparing
+      end
+
       describe 'when marked with /wip' do
         context 'in title and in description' do
           let(:opts) do