Merge branch 'ee-issue_39773_be' into 'master'

[EE-Compatibiliy] Prevent merge when fast-forward is required

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

app/services/merge_requests/merge_service.rb

@@ -6,6 +6,8 @@ module MergeRequests
   # Executed when you do merge via GitLab UI
   #
   class MergeService < MergeRequests::BaseService
+    prepend EE::MergeRequests::MergeService
+
     MergeError = Class.new(StandardError)
 
     attr_reader :merge_request, :source
@@ -18,17 +20,7 @@ module MergeRequests
 
       @merge_request = merge_request
 
-      unless @merge_request.mergeable?
-        return handle_merge_error(log_message: 'Merge request is not mergeable', save_message_on_model: true)
-      end
-
-      check_size_limit
-
-      @source = find_merge_source
-
-      unless @source
-        return handle_merge_error(log_message: 'No source for merge', save_message_on_model: true)
-      end
+      error_check!
 
       merge_request.in_locked_state do
         if commit
@@ -65,6 +57,19 @@ module MergeRequests
 
     private
 
+    def error_check!
+      error =
+        if @merge_request.should_be_rebased?
+          'Only fast-forward merge is allowed for your project. Please update your source branch'
+        elsif !@merge_request.mergeable?
+          'Merge request is not mergeable'
+        elsif !source
+          'No source for merge'
+        end
+
+      raise MergeError, error if error
+    end
+
     def commit
       message = params[:commit_message] || merge_request.merge_commit_message
 
@@ -115,25 +120,8 @@ module MergeRequests
       merge_request.to_reference(full: true)
     end
 
-    def check_size_limit
-      if @merge_request.target_project.above_size_limit?
-        message = Gitlab::RepositorySizeError.new(@merge_request.target_project).merge_error
-
-        raise MergeError, message
-      end
-    end
-
-    def find_merge_source
-      return merge_request.diff_head_sha unless merge_request.squash
-
-      squash_result = SquashService.new(project, current_user, params).execute(merge_request)
-
-      case squash_result[:status]
-      when :success
-        squash_result[:squash_sha]
-      when :error
-        raise MergeError, squash_result[:message]
-      end
+    def source
+      @source ||= @merge_request.diff_head_sha
     end
   end
 end

changelogs/unreleased/issue_39773_be.yml

+---
+title: Show error message when fast-forward merge is not possible
+merge_request:
+author:
+type: fixed

ee/app/services/ee/merge_requests/merge_service.rb

+module EE
+  module MergeRequests
+    module MergeService
+      def error_check!
+        raise NotImplementedError unless defined?(super)
+
+        check_size_limit
+
+        super
+      end
+
+      def source
+        return merge_request.diff_head_sha unless merge_request.squash
+
+        squash_result = ::MergeRequests::SquashService.new(project, current_user, params).execute(merge_request)
+
+        case squash_result[:status]
+        when :success
+          squash_result[:squash_sha]
+        when :error
+          raise ::MergeRequests::MergeService::MergeError, squash_result[:message]
+        end
+      end
+
+      private
+
+      def check_size_limit
+        if @merge_request.target_project.above_size_limit?
+          message = ::Gitlab::RepositorySizeError.new(@merge_request.target_project).merge_error
+
+          raise ::MergeRequests::MergeService::MergeError, message
+        end
+      end
+    end
+  end
+end

spec/ee/spec/services/ee/merge_requests/merge_service_spec.rb

+require 'spec_helper'
+
+describe MergeRequests::MergeService do
+  let(:user) { create(:user) }
+  let(:merge_request) { create(:merge_request, :simple) }
+  let(:project) { merge_request.project }
+
+  before do
+    project.add_master(user)
+  end
+
+  describe '#execute' do
+    context 'project has exceeded size limit' do
+      let(:service) { described_class.new(project, user, commit_message: 'Awesome message') }
+
+      before do
+        allow(project).to receive(:above_size_limit?).and_return(true)
+
+        perform_enqueued_jobs do
+          service.execute(merge_request)
+        end
+      end
+
+      it 'returns the correct error message' do
+        expect(merge_request.merge_error).to include('This merge request cannot be merged')
+      end
+    end
+  end
+end

spec/services/merge_requests/merge_service_spec.rb

@@ -38,22 +38,6 @@ describe MergeRequests::MergeService do
       end
     end
 
-    context 'project has exceeded size limit' do
-      let(:service) { described_class.new(project, user, commit_message: 'Awesome message') }
-
-      before do
-        allow(project).to receive(:above_size_limit?).and_return(true)
-
-        perform_enqueued_jobs do
-          service.execute(merge_request)
-        end
-      end
-
-      it 'returns the correct error message' do
-        expect(merge_request.merge_error).to include('This merge request cannot be merged')
-      end
-    end
-
     context 'closes related issues' do
       let(:service) { described_class.new(project, user, commit_message: 'Awesome message') }
 
@@ -297,6 +281,28 @@ describe MergeRequests::MergeService do
           expect(merge_request.merge_error).to include(error_message)
           expect(Rails.logger).to have_received(:error).with(a_string_matching(error_message))
         end
+
+        context "when fast-forward merge is not allowed" do
+          before do
+            allow_any_instance_of(Repository).to receive(:ancestor?).and_return(nil)
+          end
+
+          %w(semi-linear ff).each do |merge_method|
+            it "logs and saves error if merge is #{merge_method} only" do
+              merge_method = 'rebase_merge' if merge_method == 'semi-linear'
+              merge_request.project.update(merge_method: merge_method)
+              error_message = 'Only fast-forward merge is allowed for your project. Please update your source branch'
+              allow(service).to receive(:execute_hooks)
+
+              service.execute(merge_request)
+
+              expect(merge_request).to be_open
+              expect(merge_request.merge_commit_sha).to be_nil
+              expect(merge_request.merge_error).to include(error_message)
+              expect(Rails.logger).to have_received(:error).with(a_string_matching(error_message))
+            end
+          end
+        end
       end
     end
   end