Provide reliable source and target IDs

Returns the source and target IDs used to create
the merge commit on Gitaly.

4 files changed, 71 insertions, 46 deletions

diff --git a/app/services/merge_requests/merge_to_ref_service.rb b/app/services/merge_requests/merge_to_ref_service.rb
index d62d9cba3f6..69cc441f1bb 100644
--- a/app/services/merge_requests/merge_to_ref_service.rb
+++ b/app/services/merge_requests/merge_to_ref_service.rb
@@ -20,7 +20,12 @@ module MergeRequests
 
       raise_error('Conflicts detected during merge') unless commit_id
 
-      success(commit_id: commit_id, source_id: source)
+      commit = project.commit(commit_id)
+      target_id, source_id = commit.parent_ids
+
+      success(commit_id: commit.id,
+              target_id: target_id,
+              source_id: source_id)
     rescue MergeError => error
       error(error.message)
     end
diff --git a/changelogs/unreleased/osw-merge-to-ref-changes-for-ci-team.yml b/changelogs/unreleased/osw-merge-to-ref-changes-for-ci-team.yml
new file mode 100644
index 00000000000..dfccd6194d4
--- /dev/null
+++ b/changelogs/unreleased/osw-merge-to-ref-changes-for-ci-team.yml
@@ -0,0 +1,5 @@
+---
+title: Make merge to refs/merge-requests/:iid/merge not raise when FF-only enabled
+merge_request: 25653
+author:
+type: fixed
diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb
index 6272bb38d59..05b904ab59c 100644
--- a/spec/requests/api/merge_requests_spec.rb
+++ b/spec/requests/api/merge_requests_spec.rb
@@ -1060,18 +1060,6 @@ describe API::MergeRequests do
 
       expect(response).to have_gitlab_http_status(404)
     end
-
-    it "returns 400 when merge method is not supported" do
-      merge_request.project.update!(merge_method: 'ff')
-
-      put api(url, user)
-
-      expected_error =
-        'Fast-forward to refs/merge-requests/1/merge is currently not supported.'
-
-      expect(response).to have_gitlab_http_status(400)
-      expect(json_response['message']).to eq(expected_error)
-    end
   end
 
   describe "PUT /projects/:id/merge_requests/:merge_request_iid" do
diff --git a/spec/services/merge_requests/merge_to_ref_service_spec.rb b/spec/services/merge_requests/merge_to_ref_service_spec.rb
index a7075aa97da..fabca8f6b4a 100644
--- a/spec/services/merge_requests/merge_to_ref_service_spec.rb
+++ b/spec/services/merge_requests/merge_to_ref_service_spec.rb
@@ -19,27 +19,7 @@ describe MergeRequests::MergeToRefService do
     end
   end
 
-  set(:user) { create(:user) }
-  let(:merge_request) { create(:merge_request, :simple) }
-  let(:project) { merge_request.project }
-
-  before do
-    project.add_maintainer(user)
-  end
-
-  describe '#execute' do
-    let(:service) do
-      described_class.new(project, user,
-                          commit_message: 'Awesome message',
-                          'should_remove_source_branch' => true)
-    end
-
-    def process_merge_to_ref
-      perform_enqueued_jobs do
-        service.execute(merge_request)
-      end
-    end
-
+  shared_examples_for 'successfully merges to ref with merge method' do
     it 'writes commit to merge ref' do
       repository = project.repository
       target_ref = merge_request.merge_ref_path
@@ -53,9 +33,30 @@ describe MergeRequests::MergeToRefService do
       expect(result[:status]).to eq(:success)
       expect(result[:commit_id]).to be_present
       expect(result[:source_id]).to eq(merge_request.source_branch_sha)
+      expect(result[:target_id]).to eq(merge_request.target_branch_sha)
       expect(repository.ref_exists?(target_ref)).to be(true)
       expect(ref_head.id).to eq(result[:commit_id])
     end
+  end
+
+  shared_examples_for 'successfully evaluates pre-condition checks' do
+    it 'returns error when feature is disabled' do
+      stub_feature_flags(merge_to_tmp_merge_ref_path: false)
+
+      result = service.execute(merge_request)
+
+      expect(result[:status]).to eq(:error)
+      expect(result[:message]).to eq('Feature is not enabled')
+    end
+
+    it 'returns an error when the failing to process the merge' do
+      allow(project.repository).to receive(:merge_to_ref).and_return(nil)
+
+      result = service.execute(merge_request)
+
+      expect(result[:status]).to eq(:error)
+      expect(result[:message]).to eq('Conflicts detected during merge')
+    end
 
     it 'does not send any mail' do
       expect { process_merge_to_ref }.not_to change { ActionMailer::Base.deliveries.count }
@@ -74,25 +75,31 @@ describe MergeRequests::MergeToRefService do
 
       process_merge_to_ref
     end
+  end
 
-    it 'returns error when feature is disabled' do
-      stub_feature_flags(merge_to_tmp_merge_ref_path: false)
+  set(:user) { create(:user) }
+  let(:merge_request) { create(:merge_request, :simple) }
+  let(:project) { merge_request.project }
 
-      result = service.execute(merge_request)
+  before do
+    project.add_maintainer(user)
+  end
 
-      expect(result[:status]).to eq(:error)
-      expect(result[:message]).to eq('Feature is not enabled')
+  describe '#execute' do
+    let(:service) do
+      described_class.new(project, user, commit_message: 'Awesome message',
+                                         should_remove_source_branch: true)
     end
 
-    it 'returns an error when the failing to process the merge' do
-      allow(project.repository).to receive(:merge_to_ref).and_return(nil)
-
-      result = service.execute(merge_request)
-
-      expect(result[:status]).to eq(:error)
-      expect(result[:message]).to eq('Conflicts detected during merge')
+    def process_merge_to_ref
+      perform_enqueued_jobs do
+        service.execute(merge_request)
+      end
     end
 
+    it_behaves_like 'successfully merges to ref with merge method'
+    it_behaves_like 'successfully evaluates pre-condition checks'
+
     context 'commit history comparison with regular MergeService' do
       let(:merge_ref_service) do
         described_class.new(project, user, {})
@@ -116,7 +123,27 @@ describe MergeRequests::MergeToRefService do
     end
 
     context 'merge pre-condition checks' do
+      before do
+        merge_request.project.update!(merge_method: merge_method)
+      end
+
+      context 'when semi-linear merge method' do
+        let(:merge_method) { :rebase_merge }
+
+        it_behaves_like 'successfully merges to ref with merge method'
+        it_behaves_like 'successfully evaluates pre-condition checks'
+      end
+
+      context 'when fast-forward merge method' do
+        let(:merge_method) { :ff }
+
+        it_behaves_like 'successfully merges to ref with merge method'
+        it_behaves_like 'successfully evaluates pre-condition checks'
+      end
+
       context 'when MR is not mergeable to ref' do
+        let(:merge_method) { :merge }
+
         it 'returns error' do
           allow(merge_request).to receive(:mergeable_to_ref?) { false }