Merge branch 'make-create-ref-service-generic' into 'master'

Add MergeTrains::CreateRefService

See merge request https://gitlab.com/gitlab-org/gitlab/-/merge_requests/138185



Merged-by: Allison Browne <abrowne@gitlab.com>
Approved-by: Panos Kanellidis <pkanellidis@gitlab.com>
Approved-by: Allison Browne <abrowne@gitlab.com>
Reviewed-by: Allison Browne <abrowne@gitlab.com>
Co-authored-by: Hordur Freyr Yngvason <hfyngvason@gitlab.com>

app/services/merge_requests/create_ref_service.rb

@@ -35,8 +35,6 @@ def execute
       result = maybe_rebase!(**result)
       result = maybe_merge!(**result)
 
-      update_merge_request!(merge_request, result)
-
       ServiceResponse.success(payload: result)
     rescue CreateRefError => error
       ServiceResponse.error(message: error.message)
@@ -118,10 +116,6 @@ def maybe_merge!(commit_sha:, expected_old_oid:, **rest)
       ).compact
     end
 
-    def update_merge_request!(merge_request, result)
-      # overridden in EE
-    end
-
     def safe_gitaly_operation
       yield
     rescue Gitlab::Git::PreReceiveError, Gitlab::Git::CommandError, ArgumentError => error

ee/app/services/ee/merge_requests/create_ref_service.rb

-# frozen_string_literal: true
-
-module EE
-  module MergeRequests
-    module CreateRefService
-      extend ::Gitlab::Utils::Override
-
-      private
-
-      override :update_merge_request!
-      def update_merge_request!(merge_request, result)
-        merge_request.merge_params['train_ref'] =
-          result
-            .slice(:commit_sha, :merge_commit_sha, :squash_commit_sha)
-            .stringify_keys
-
-        merge_request.save!
-      rescue StandardError => e
-        ::Gitlab::ErrorTracking.track_exception(e)
-        raise ::MergeRequests::CreateRefService::CreateRefError, "Failed to update merge params"
-      end
-    end
-  end
-end

ee/app/services/merge_trains/create_pipeline_service.rb

@@ -25,10 +25,9 @@ def create_train_ref(merge_request, previous_ref, create_mergeable_ref)
       return error('previous ref is not specified') unless previous_ref
 
       if create_mergeable_ref
-        ::MergeRequests::CreateRefService.new(
+        ::MergeTrains::CreateRefService.new(
           current_user: merge_request.merge_user,
           merge_request: merge_request,
-          target_ref: merge_request.train_ref_path,
           source_sha: merge_request.diff_head_sha,
           first_parent_ref: previous_ref
         ).execute.to_h.transform_keys do |key|

ee/app/services/merge_trains/create_ref_service.rb

+# frozen_string_literal: true
+
+module MergeTrains
+  class CreateRefService
+    def initialize(current_user:, merge_request:, first_parent_ref:, source_sha: nil)
+      @current_user = current_user
+      @merge_request = merge_request
+      @source_sha = source_sha
+      @first_parent_ref = first_parent_ref
+    end
+
+    def execute
+      create_ref_result = MergeRequests::CreateRefService.new(
+        current_user: @current_user,
+        merge_request: @merge_request,
+        source_sha: @source_sha,
+        target_ref: @merge_request.train_ref_path,
+        first_parent_ref: @first_parent_ref
+      ).execute
+
+      return create_ref_result if update_merge_params_train_ref(create_ref_result)
+
+      ServiceResponse.error(message: "Failed to update merge params")
+    end
+
+    private
+
+    def update_merge_params_train_ref(create_ref_result)
+      @merge_request.merge_params['train_ref'] =
+        create_ref_result
+          .payload
+          .slice(:commit_sha, :merge_commit_sha, :squash_commit_sha)
+          .stringify_keys
+
+      @merge_request.save
+    end
+  end
+end

ee/spec/services/merge_trains/create_pipeline_service_spec.rb

@@ -170,12 +170,11 @@
 
   context 'when creating mergeable refs' do
     let(:create_mergeable_ref) { true }
-    let(:expected_ref_creation_service) { MergeRequests::CreateRefService }
+    let(:expected_ref_creation_service) { MergeTrains::CreateRefService }
     let(:expected_ref_creation_service_args) do
       {
         current_user: merge_request.merge_user,
         merge_request: merge_request,
-        target_ref: merge_request.train_ref_path,
         source_sha: merge_request.diff_head_sha,
         first_parent_ref: previous_ref
       }

ee/spec/services/ee/merge_requests/create_ref_service_spec.rb → ee/spec/services/merge_trains/create_ref_service_spec.rb

@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-RSpec.describe MergeRequests::CreateRefService, feature_category: :merge_trains do
+RSpec.describe MergeTrains::CreateRefService, feature_category: :merge_trains do
   using RSpec::Parameterized::TableSyntax
 
   describe '#execute' do
@@ -10,7 +10,6 @@
     let_it_be(:user) { project.creator }
     let_it_be(:first_parent_ref) { project.default_branch_or_main }
     let_it_be(:source_branch) { 'branch' }
-    let(:target_ref) { "refs/merge-requests/#{merge_request.iid}/train" }
     let(:source_sha) { project.commit(source_branch).sha }
     let(:squash) { false }
 
@@ -31,7 +30,6 @@
       described_class.new(
         current_user: user,
         merge_request: merge_request,
-        target_ref: target_ref,
         source_sha: source_sha,
         first_parent_ref: first_parent_ref
       ).execute
@@ -160,6 +158,14 @@
           it_behaves_like 'writing with a squash and no merge commit'
         end
       end
+
+      context 'when merge request fails to save' do
+        it 'returns an error', :aggregate_failures do
+          expect(merge_request).to receive(:save).and_return(false)
+          expect(result).to be_error
+          expect(result.message).to eq('Failed to update merge params')
+        end
+      end
     end
   end
 end