Merge branch 'id-display-conflicts-on-diff' into 'master'

Allow conflicts to be merged in diff

See merge request gitlab-org/gitlab!45008

Gemfile

@@ -466,7 +466,7 @@ group :ed25519 do
 end
 
 # Gitaly GRPC protocol definitions
-gem 'gitaly', '~> 13.5.0-rc1'
+gem 'gitaly', '~> 13.5.0-rc2'
 
 gem 'grpc', '~> 1.30.2'
 

Gemfile.lock

@@ -416,7 +416,7 @@ GEM
       rails (>= 3.2.0)
     git (1.7.0)
       rchardet (~> 1.8)
-    gitaly (13.5.0.pre.rc1)
+    gitaly (13.5.0.pre.rc2)
       grpc (~> 1.0)
     github-markup (1.7.0)
     gitlab-chronic (0.10.5)
@@ -1327,7 +1327,7 @@ DEPENDENCIES
   gettext (~> 3.3)
   gettext_i18n_rails (~> 1.8.0)
   gettext_i18n_rails_js (~> 1.3)
-  gitaly (~> 13.5.0.pre.rc1)
+  gitaly (~> 13.5.0.pre.rc2)
   github-markup (~> 1.7.0)
   gitlab-chronic (~> 0.10.5)
   gitlab-fog-azure-rm (~> 1.0)

app/models/merge_request.rb

@@ -933,7 +933,7 @@ class MergeRequest < ApplicationRecord
   # rubocop: enable CodeReuse/ServiceClass
 
   def diffable_merge_ref?
-    can_be_merged? && merge_ref_head.present?
+    merge_ref_head.present? && (Feature.enabled?(:display_merge_conflicts_in_diff, project) || can_be_merged?)
   end
 
   # Returns boolean indicating the merge_status should be rechecked in order to

app/models/repository.rb

@@ -860,10 +860,10 @@ class Repository
     end
   end
 
-  def merge_to_ref(user, source_sha, merge_request, target_ref, message, first_parent_ref)
+  def merge_to_ref(user, source_sha, merge_request, target_ref, message, first_parent_ref, allow_conflicts = false)
     branch = merge_request.target_branch
 
-    raw.merge_to_ref(user, source_sha, branch, target_ref, message, first_parent_ref)
+    raw.merge_to_ref(user, source_sha, branch, target_ref, message, first_parent_ref, allow_conflicts)
   end
 
   def delete_refs(*ref_names)

app/services/merge_requests/merge_to_ref_service.rb

@@ -58,8 +58,15 @@ module MergeRequests
       params[:first_parent_ref] || merge_request.target_branch_ref
     end
 
+    ##
+    # The parameter `allow_conflicts` is a flag whether merge conflicts should be merged into diff
+    # Default is false
+    def allow_conflicts
+      params[:allow_conflicts] || false
+    end
+
     def commit
-      repository.merge_to_ref(current_user, source, merge_request, target_ref, commit_message, first_parent_ref)
+      repository.merge_to_ref(current_user, source, merge_request, target_ref, commit_message, first_parent_ref, allow_conflicts)
     rescue Gitlab::Git::PreReceiveError, Gitlab::Git::CommandError => error
       raise MergeError, error.message
     end

app/services/merge_requests/mergeability_check_service.rb

@@ -115,10 +115,14 @@ module MergeRequests
 
     def update_merge_status
       return unless merge_request.recheck_merge_status?
+      return merge_request.mark_as_unmergeable if merge_request.broken?
 
-      if can_git_merge? && merge_to_ref
+      merge_to_ref_success = merge_to_ref
+
+      update_diff_discussion_positions! if merge_to_ref_success
+
+      if merge_to_ref_success && can_git_merge?
         merge_request.mark_as_mergeable
-        update_diff_discussion_positions!
       else
         merge_request.mark_as_unmergeable
       end
@@ -149,13 +153,14 @@ module MergeRequests
     end
 
     def can_git_merge?
-      !merge_request.broken? && repository.can_be_merged?(merge_request.diff_head_sha, merge_request.target_branch)
+      repository.can_be_merged?(merge_request.diff_head_sha, merge_request.target_branch)
     end
 
     def merge_to_ref
       return true unless merge_ref_auto_sync_enabled?
 
-      result = MergeRequests::MergeToRefService.new(project, merge_request.author).execute(merge_request)
+      params = { allow_conflicts: Feature.enabled?(:display_merge_conflicts_in_diff, project) }
+      result = MergeRequests::MergeToRefService.new(project, merge_request.author, params).execute(merge_request)
       result[:status] == :success
     end
 

config/feature_flags/development/display_merge_conflicts_in_diff.yml

+---
+name: display_merge_conflicts_in_diff
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/45008
+rollout_issue_url:
+type: development
+group: group::source code
+default_enabled: false

lib/gitlab/git/repository.rb

@@ -587,9 +587,9 @@ module Gitlab
         tags.find { |tag| tag.name == name }
       end
 
-      def merge_to_ref(user, source_sha, branch, target_ref, message, first_parent_ref)
+      def merge_to_ref(user, source_sha, branch, target_ref, message, first_parent_ref, allow_conflicts)
         wrapped_gitaly_errors do
-          gitaly_operation_client.user_merge_to_ref(user, source_sha, branch, target_ref, message, first_parent_ref)
+          gitaly_operation_client.user_merge_to_ref(user, source_sha, branch, target_ref, message, first_parent_ref, allow_conflicts)
         end
       end
 

lib/gitlab/gitaly_client/operation_service.rb

@@ -102,7 +102,7 @@ module Gitlab
         end
       end
 
-      def user_merge_to_ref(user, source_sha, branch, target_ref, message, first_parent_ref)
+      def user_merge_to_ref(user, source_sha, branch, target_ref, message, first_parent_ref, allow_conflicts)
         request = Gitaly::UserMergeToRefRequest.new(
           repository: @gitaly_repo,
           source_sha: source_sha,
@@ -110,7 +110,8 @@ module Gitlab
           target_ref: encode_binary(target_ref),
           user: Gitlab::Git::User.from_gitlab(user).to_gitaly,
           message: encode_binary(message),
-          first_parent_ref: encode_binary(first_parent_ref)
+          first_parent_ref: encode_binary(first_parent_ref),
+          allow_conflicts: allow_conflicts
         )
 
         response = GitalyClient.call(@repository.storage, :operation_service,

spec/lib/gitlab/git/repository_spec.rb

@@ -1650,13 +1650,14 @@ RSpec.describe Gitlab::Git::Repository, :seed_helper do
     let(:right_branch) { 'test-master' }
     let(:first_parent_ref) { 'refs/heads/test-master' }
     let(:target_ref) { 'refs/merge-requests/999/merge' }
+    let(:allow_conflicts) { false }
 
     before do
       repository.create_branch(right_branch, branch_head) unless repository.ref_exists?(first_parent_ref)
     end
 
     def merge_to_ref
-      repository.merge_to_ref(user, left_sha, right_branch, target_ref, 'Merge message', first_parent_ref)
+      repository.merge_to_ref(user, left_sha, right_branch, target_ref, 'Merge message', first_parent_ref, allow_conflicts)
     end
 
     it 'generates a commit in the target_ref' do

spec/lib/gitlab/gitaly_client/operation_service_spec.rb

@@ -88,9 +88,10 @@ RSpec.describe Gitlab::GitalyClient::OperationService do
       let(:source_sha) { 'cfe32cf61b73a0d5e9f13e774abde7ff789b1660' }
       let(:ref) { 'refs/merge-requests/x/merge' }
       let(:message) { 'validación' }
+      let(:allow_conflicts) { false }
       let(:response) { Gitaly::UserMergeToRefResponse.new(commit_id: 'new-commit-id') }
 
-      subject { client.user_merge_to_ref(user, source_sha, nil, ref, message, first_parent_ref) }
+      subject { client.user_merge_to_ref(user, source_sha, nil, ref, message, first_parent_ref, allow_conflicts) }
 
       it 'sends a user_merge_to_ref message' do
         expect_any_instance_of(Gitaly::OperationService::Stub)

spec/models/merge_request_spec.rb

@@ -4228,17 +4228,29 @@ RSpec.describe MergeRequest, factory_default: :keep do
         it 'returns true' do
           expect(subject.diffable_merge_ref?).to eq(true)
         end
-      end
-    end
 
         context 'merge request cannot be merged' do
-      it 'returns false' do
+          before do
             subject.mark_as_unchecked!
+          end
+
+          it 'returns false' do
+            expect(subject.diffable_merge_ref?).to eq(true)
+          end
+
+          context 'display_merge_conflicts_in_diff is disabled' do
+            before do
+              stub_feature_flags(display_merge_conflicts_in_diff: false)
+            end
 
+            it 'returns false' do
               expect(subject.diffable_merge_ref?).to eq(false)
             end
           end
         end
+      end
+    end
+  end
 
   describe '#predefined_variables' do
     let(:merge_request) { create(:merge_request) }

spec/services/merge_requests/merge_to_ref_service_spec.rb

@@ -252,5 +252,16 @@ RSpec.describe MergeRequests::MergeToRefService do
         end
       end
     end
+
+    context 'allow conflicts to be merged in diff' do
+      let(:params) { { allow_conflicts: true } }
+
+      it 'calls merge_to_ref with allow_conflicts param' do
+        expect(project.repository).to receive(:merge_to_ref)
+          .with(anything, anything, anything, anything, anything, anything, true)
+
+        service.execute(merge_request)
+      end
+    end
   end
 end

spec/services/merge_requests/mergeability_check_service_spec.rb

@@ -211,11 +211,18 @@ RSpec.describe MergeRequests::MergeabilityCheckService, :clean_gitlab_redis_shar
                target_branch: 'conflict-start')
       end
 
-      it_behaves_like 'unmergeable merge request'
+      it 'does not change the merge ref HEAD' do
+        expect(merge_request.merge_ref_head).to be_nil
 
-      it 'returns ServiceResponse.error' do
+        subject
+
+        expect(merge_request.reload.merge_ref_head).not_to be_nil
+      end
+
+      it 'returns ServiceResponse.error and keeps merge status as cannot_be_merged' do
         result = subject
 
+        expect(merge_request.merge_status).to eq('cannot_be_merged')
         expect(result).to be_a(ServiceResponse)
         expect(result.error?).to be(true)
         expect(result.message).to eq('Merge request is not mergeable')
@@ -373,5 +380,27 @@ RSpec.describe MergeRequests::MergeabilityCheckService, :clean_gitlab_redis_shar
         end
       end
     end
+
+    context 'merge with conflicts' do
+      it 'calls MergeToRefService with true allow_conflicts param' do
+        expect(MergeRequests::MergeToRefService).to receive(:new)
+          .with(project, merge_request.author, { allow_conflicts: true }).and_call_original
+
+        subject
+      end
+
+      context 'when display_merge_conflicts_in_diff is disabled' do
+        before do
+          stub_feature_flags(display_merge_conflicts_in_diff: false)
+        end
+
+        it 'calls MergeToRefService with false allow_conflicts param' do
+          expect(MergeRequests::MergeToRefService).to receive(:new)
+            .with(project, merge_request.author, { allow_conflicts: false }).and_call_original
+
+          subject
+        end
+      end
+    end
   end
 end