Merge branch 'id-refactor-merge-to-ref-args' into 'master'

Refactor Repository#merge_to_ref number of args

See merge request gitlab-org/gitlab!45402

app/models/repository.rb

@@ -829,12 +829,6 @@ class Repository
     end
   end
 
-  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, allow_conflicts)
-  end
-
   def delete_refs(*ref_names)
     raw.delete_refs(*ref_names)
   end

app/services/merge_requests/merge_to_ref_service.rb

@@ -66,7 +66,13 @@ module MergeRequests
     end
 
     def commit
-      repository.merge_to_ref(current_user, source, merge_request, target_ref, commit_message, first_parent_ref, allow_conflicts)
+      repository.merge_to_ref(current_user,
+        source_sha: source,
+        branch: merge_request.target_branch,
+        target_ref: target_ref,
+        message: commit_message,
+        first_parent_ref: first_parent_ref,
+        allow_conflicts: allow_conflicts)
     rescue Gitlab::Git::PreReceiveError, Gitlab::Git::CommandError => error
       raise MergeError, error.message
     end

lib/gitlab/git/repository.rb

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

lib/gitlab/gitaly_client/operation_service.rb

@@ -103,7 +103,7 @@ module Gitlab
         end
       end
 
-      def user_merge_to_ref(user, source_sha, branch, target_ref, message, first_parent_ref, allow_conflicts)
+      def user_merge_to_ref(user, source_sha:, branch:, target_ref:, message:, first_parent_ref:, allow_conflicts: false)
         request = Gitaly::UserMergeToRefRequest.new(
           repository: @gitaly_repo,
           source_sha: source_sha,

spec/lib/gitlab/git/repository_spec.rb

@@ -1746,14 +1746,15 @@ 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, allow_conflicts)
+      repository.merge_to_ref(user,
+          source_sha: left_sha, branch: right_branch, target_ref: target_ref,
+          message: 'Merge message', first_parent_ref: first_parent_ref)
     end
 
     it 'generates a commit in the target_ref' do

spec/lib/gitlab/gitaly_client/operation_service_spec.rb

@@ -88,17 +88,29 @@ 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, allow_conflicts) }
+      let(:payload) do
+        { source_sha: source_sha, branch: 'branch', target_ref: ref,
+          message: message, first_parent_ref: first_parent_ref, allow_conflicts: true }
+      end
 
       it 'sends a user_merge_to_ref message' do
-        expect_any_instance_of(Gitaly::OperationService::Stub)
-          .to receive(:user_merge_to_ref).with(kind_of(Gitaly::UserMergeToRefRequest), kind_of(Hash))
-          .and_return(response)
-
-        subject
+        freeze_time do
+          expect_any_instance_of(Gitaly::OperationService::Stub).to receive(:user_merge_to_ref) do |_, request, options|
+            expect(options).to be_kind_of(Hash)
+            expect(request.to_h).to eq(
+              payload.merge({
+                repository: repository.gitaly_repository.to_h,
+                message: message.dup.force_encoding(Encoding::ASCII_8BIT),
+                user: Gitlab::Git::User.from_gitlab(user).to_gitaly.to_h,
+                timestamp: { nanos: 0, seconds: Time.current.to_i }
+              })
+            )
+          end.and_return(response)
+
+          client.user_merge_to_ref(user, **payload)
+        end
       end
     end
 

spec/models/repository_spec.rb

@@ -1698,12 +1698,13 @@ RSpec.describe Repository do
     end
 
     it 'writes merge of source SHA and first parent ref to MR merge_ref_path' do
-      merge_commit_id = repository.merge_to_ref(user,
-                                                merge_request.diff_head_sha,
-                                                merge_request,
-                                                merge_request.merge_ref_path,
-                                                'Custom message',
-                                                merge_request.target_branch_ref)
+      merge_commit_id =
+        repository.merge_to_ref(user,
+          source_sha: merge_request.diff_head_sha,
+          branch: merge_request.target_branch,
+          target_ref: merge_request.merge_ref_path,
+          message: 'Custom message',
+          first_parent_ref: merge_request.target_branch_ref)
 
       merge_commit = repository.commit(merge_commit_id)
 

spec/services/merge_requests/merge_to_ref_service_spec.rb

@@ -257,8 +257,9 @@ RSpec.describe MergeRequests::MergeToRefService 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)
+        expect(project.repository).to receive(:merge_to_ref) do |user, **kwargs|
+          expect(kwargs[:allow_conflicts]).to eq(true)
+        end.and_call_original
 
         service.execute(merge_request)
       end