Commit 252518a7 authored by Amy Troschinetz's avatar Amy Troschinetz Committed by Stan Hu

Fix issue with merge_pipeline when using ff merges

**app/models/merge_request.rb**:

- Fix how `MergeRequest#merge_pipeline` is found to account for FF merge

**changelogs/unreleased/fix-pipeline-link.yml**:

- Changelog entry for this fix

**spec/models/environment_status_spec.rb**:

- Code this test was exercising is better covered in merge_request_spec

**spec/models/merge_request_spec.rb**:

- Test that new logic works in the face of FF merge and squash FF merge
parent e77d0d12
......@@ -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
......
---
title: Fixes wrong MR pipeline link when FF-merge strategy is used
merge_request: 39396
author:
type: fixed
......@@ -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)
......
......@@ -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
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment