Only force recheck when merge-ref is outdated

When recheck flag is true, we make sure the merge-ref
is indeed outdated. If it is, we update it along
the merge status.

2 files changed, 61 insertions, 3 deletions

diff --git a/app/services/merge_requests/mergeability_check_service.rb b/app/services/merge_requests/mergeability_check_service.rb
index a8b2cdd64d2..faa38eda61e 100644
--- a/app/services/merge_requests/mergeability_check_service.rb
+++ b/app/services/merge_requests/mergeability_check_service.rb
@@ -15,7 +15,7 @@ module MergeRequests
     # the merge-ref is refreshed.
     #
     # recheck - When given, it'll enforce a merge-ref refresh if the current merge_status is
-    # can_be_merged or cannot_be_merged.
+    # can_be_merged or cannot_be_merged and merge-ref is outdated.
     # Given MergeRequests::RefreshService is called async, it might happen that the target
     # branch gets updated, but the MergeRequest#merge_status lags behind. So in scenarios
     # where we need the current state of the merge ref in repository, the `recheck`
@@ -79,7 +79,22 @@ module MergeRequests
     end
 
     def recheck!
-      merge_request.mark_as_unchecked unless merge_request.recheck_merge_status?
+      if !merge_request.recheck_merge_status? && outdated_merge_ref?
+        merge_request.mark_as_unchecked
+      end
+    end
+
+    # Checks if the existing merge-ref is synced with the target branch.
+    #
+    # Returns true if the merge-ref does not exists or is out of sync.
+    def outdated_merge_ref?
+      return false unless merge_ref_auto_sync_enabled?
+
+      return true unless ref_head = merge_request.merge_ref_head
+      return true unless target_sha = merge_request.target_branch_sha
+      return true unless source_sha = merge_request.source_branch_sha
+
+      ref_head.parent_ids != [target_sha, source_sha]
     end
 
     def can_git_merge?
diff --git a/spec/services/merge_requests/mergeability_check_service_spec.rb b/spec/services/merge_requests/mergeability_check_service_spec.rb
index c5fe51b79f2..6c04e816b91 100644
--- a/spec/services/merge_requests/mergeability_check_service_spec.rb
+++ b/spec/services/merge_requests/mergeability_check_service_spec.rb
@@ -91,7 +91,7 @@ describe MergeRequests::MergeabilityCheckService do
         expect(result.payload).to be_empty
       end
 
-      it 'updates the merge_status' do
+      it 'ignores merge-ref and updates merge status' do
         expect { subject }.to change(merge_request, :merge_status).from('unchecked').to('can_be_merged')
       end
     end
@@ -206,6 +206,28 @@ describe MergeRequests::MergeabilityCheckService do
     context 'recheck enforced' do
       subject { described_class.new(merge_request).execute(recheck: true) }
 
+      context 'when MR is mergeable and merge-ref auto-sync is disabled' do
+        before do
+          stub_feature_flags(merge_ref_auto_sync: false)
+          merge_request.mark_as_mergeable!
+        end
+
+        it 'returns ServiceResponse.error' do
+          result = subject
+
+          expect(result).to be_a(ServiceResponse)
+          expect(result.error?).to be(true)
+          expect(result.message).to eq('Merge ref is outdated due to disabled feature')
+          expect(result.payload).to be_empty
+        end
+
+        it 'merge status is not changed' do
+          subject
+
+          expect(merge_request.merge_status).to eq('can_be_merged')
+        end
+      end
+
       context 'when MR is mergeable but merge-ref does not exists' do
         before do
           merge_request.mark_as_mergeable!
@@ -213,6 +235,27 @@ describe MergeRequests::MergeabilityCheckService do
 
         it_behaves_like 'mergeable merge request'
       end
+
+      context 'when MR is mergeable but merge-ref is already updated' do
+        before do
+          MergeRequests::MergeToRefService.new(project, merge_request.author).execute(merge_request)
+          merge_request.mark_as_mergeable!
+        end
+
+        it 'returns ServiceResponse.success' do
+          result = subject
+
+          expect(result).to be_a(ServiceResponse)
+          expect(result).to be_success
+          expect(result.payload[:merge_ref_head]).to be_present
+        end
+
+        it 'does not recreate the merge-ref' do
+          expect(MergeRequests::MergeToRefService).not_to receive(:new)
+
+          subject
+        end
+      end
     end
   end
 end