Merge branch 'fix-pipeline-link' into 'master'

Fix issue with merge_pipeline when using ff merges

See merge request gitlab-org/gitlab!39396

app/models/merge_request.rb

@@ -333,7 +333,11 @@ class MergeRequest < ApplicationRecord
   def merge_pipeline
     return unless merged?
 
-    target_project.pipeline_for(target_branch, merge_commit_sha)
+    # When the merge_method is :merge there will be a merge_commit_sha, however
+    # when it is fast-forward there is no merge commit, so we must fall back to
+    # either the squash commit (if the MR was squashed) or the diff head commit.
+    sha = merge_commit_sha || squash_commit_sha || diff_head_sha
+    target_project.pipeline_for(target_branch, sha)
   end
 
   # Pattern used to extract `!123` merge request references from text

changelogs/unreleased/fix-pipeline-link.yml

+---
+title: Fixes wrong MR pipeline link when FF-merge strategy is used
+merge_request: 39396
+author:
+type: fixed

spec/models/environment_status_spec.rb

@@ -90,22 +90,6 @@ RSpec.describe EnvironmentStatus do
     end
   end
 
-  describe '.after_merge_request' do
-    let(:admin)    { create(:admin) }
-    let(:pipeline) { create(:ci_pipeline, sha: sha) }
-
-    before do
-      merge_request.mark_as_merged!
-    end
-
-    it 'is based on merge_request.merge_commit_sha' do
-      expect(merge_request).to receive(:merge_commit_sha)
-      expect(merge_request).not_to receive(:diff_head_sha)
-
-      described_class.after_merge_request(merge_request, admin)
-    end
-  end
-
   describe '.for_deployed_merge_request' do
     context 'when a merge request has no explicitly linked deployments' do
       it 'returns the statuses based on the CI pipelines' do
@@ -191,7 +175,7 @@ RSpec.describe EnvironmentStatus do
     let(:environment) { build.deployment.environment }
     let(:user) { project.owner }
 
-    context 'when environment is created on a forked project' do
+    context 'when environment is created on a forked project', :sidekiq_inline do
       let(:project) { create(:project, :repository) }
       let(:forked) { fork_project(project, user, repository: true) }
       let(:sha) { forked.commit.sha }
@@ -205,7 +189,7 @@ RSpec.describe EnvironmentStatus do
                head_pipeline: pipeline)
       end
 
-      it 'returns environment status', :sidekiq_might_not_need_inline do
+      it 'returns environment status' do
         expect(subject.count).to eq(1)
         expect(subject[0].environment).to eq(environment)
         expect(subject[0].merge_request).to eq(merge_request)

spec/models/merge_request_spec.rb

@@ -1575,11 +1575,36 @@ RSpec.describe MergeRequest do
 
       before do
         subject.mark_as_merged!
-        subject.update_attribute(:merge_commit_sha, pipeline.sha)
       end
 
-      it 'returns the post-merge pipeline' do
-        expect(subject.merge_pipeline).to eq(pipeline)
+      context 'and there is a merge commit' do
+        before do
+          subject.update_attribute(:merge_commit_sha, pipeline.sha)
+        end
+
+        it 'returns the pipeline associated with that merge request' do
+          expect(subject.merge_pipeline).to eq(pipeline)
+        end
+      end
+
+      context 'and there is no merge commit, but there is a diff head' do
+        before do
+          allow(subject).to receive(:diff_head_sha).and_return(pipeline.sha)
+        end
+
+        it 'returns the pipeline associated with that merge request' do
+          expect(subject.merge_pipeline).to eq(pipeline)
+        end
+      end
+
+      context 'and there is no merge commit, but there is a squash commit' do
+        before do
+          subject.update_attribute(:squash_commit_sha, pipeline.sha)
+        end
+
+        it 'returns the pipeline associated with that merge request' do
+          expect(subject.merge_pipeline).to eq(pipeline)
+        end
       end
     end
   end