Commit 96b28d83 authored by Shinya Maeda's avatar Shinya Maeda

Fix ref_text of merge request pipelines

Source branch can be removed after the merge and
we have to make sure to avoid rendering links if it's the case.
parent f109d24d
...@@ -63,19 +63,11 @@ module Ci ...@@ -63,19 +63,11 @@ module Ci
end end
def link_to_merge_request_source_branch def link_to_merge_request_source_branch
return unless merge_request_presenter merge_request_presenter&.source_branch_link
link_to(merge_request_presenter.source_branch,
merge_request_presenter.source_branch_commits_path,
class: 'ref-name')
end end
def link_to_merge_request_target_branch def link_to_merge_request_target_branch
return unless merge_request_presenter merge_request_presenter&.target_branch_link
link_to(merge_request_presenter.target_branch,
merge_request_presenter.target_branch_commits_path,
class: 'ref-name')
end end
private private
......
...@@ -216,6 +216,22 @@ class MergeRequestPresenter < Gitlab::View::Presenter::Delegated ...@@ -216,6 +216,22 @@ class MergeRequestPresenter < Gitlab::View::Presenter::Delegated
help_page_path('ci/merge_request_pipelines/index.md') help_page_path('ci/merge_request_pipelines/index.md')
end end
def source_branch_link
if source_branch_exists?
link_to(source_branch, source_branch_commits_path, class: 'ref-name')
else
content_tag(:span, source_branch, class: 'ref-name')
end
end
def target_branch_link
if target_branch_exists?
link_to(target_branch, target_branch_commits_path, class: 'ref-name')
else
content_tag(:span, target_branch, class: 'ref-name')
end
end
private private
def cached_can_be_reverted? def cached_can_be_reverted?
......
---
title: Fix pipelines for merge requests does not show pipeline page when source branch
is removed
merge_request: 27803
author:
type: fixed
...@@ -331,11 +331,9 @@ describe 'Pipeline', :js do ...@@ -331,11 +331,9 @@ describe 'Pipeline', :js do
merge_request.all_pipelines.last merge_request.all_pipelines.last
end end
before do it 'shows the pipeline information' do
visit_pipeline visit_pipeline
end
it 'shows the pipeline information' do
within '.pipeline-info' do within '.pipeline-info' do
expect(page).to have_content("#{pipeline.statuses.count} jobs " \ expect(page).to have_content("#{pipeline.statuses.count} jobs " \
"for !#{merge_request.iid} " \ "for !#{merge_request.iid} " \
...@@ -347,6 +345,21 @@ describe 'Pipeline', :js do ...@@ -347,6 +345,21 @@ describe 'Pipeline', :js do
end end
end end
context 'when source branch does not exist' do
before do
project.repository.rm_branch(user, merge_request.source_branch)
end
it 'does not link to the source branch commit path' do
visit_pipeline
within '.pipeline-info' do
expect(page).not_to have_link(merge_request.source_branch)
expect(page).to have_content(merge_request.source_branch)
end
end
end
context 'when source project is a forked project' do context 'when source project is a forked project' do
let(:source_project) { fork_project(project, user, repository: true) } let(:source_project) { fork_project(project, user, repository: true) }
...@@ -386,11 +399,11 @@ describe 'Pipeline', :js do ...@@ -386,11 +399,11 @@ describe 'Pipeline', :js do
before do before do
pipeline.update(user: user) pipeline.update(user: user)
visit_pipeline
end end
it 'shows the pipeline information' do it 'shows the pipeline information' do
visit_pipeline
within '.pipeline-info' do within '.pipeline-info' do
expect(page).to have_content("#{pipeline.statuses.count} jobs " \ expect(page).to have_content("#{pipeline.statuses.count} jobs " \
"for !#{merge_request.iid} " \ "for !#{merge_request.iid} " \
...@@ -405,6 +418,21 @@ describe 'Pipeline', :js do ...@@ -405,6 +418,21 @@ describe 'Pipeline', :js do
end end
end end
context 'when target branch does not exist' do
before do
project.repository.rm_branch(user, merge_request.target_branch)
end
it 'does not link to the target branch commit path' do
visit_pipeline
within '.pipeline-info' do
expect(page).not_to have_link(merge_request.target_branch)
expect(page).to have_content(merge_request.target_branch)
end
end
end
context 'when source project is a forked project' do context 'when source project is a forked project' do
let(:source_project) { fork_project(project, user, repository: true) } let(:source_project) { fork_project(project, user, repository: true) }
......
...@@ -439,6 +439,52 @@ describe MergeRequestPresenter do ...@@ -439,6 +439,52 @@ describe MergeRequestPresenter do
end end
end end
describe '#source_branch_link' do
subject { presenter.source_branch_link }
let(:presenter) { described_class.new(resource, current_user: user) }
context 'when source branch exists' do
it 'returns link' do
allow(resource).to receive(:source_branch_exists?) { true }
is_expected
.to eq("<a class=\"ref-name\" href=\"#{presenter.source_branch_commits_path}\">#{presenter.source_branch}</a>")
end
end
context 'when source branch does not exist' do
it 'returns text' do
allow(resource).to receive(:source_branch_exists?) { false }
is_expected.to eq("<span class=\"ref-name\">#{presenter.source_branch}</span>")
end
end
end
describe '#target_branch_link' do
subject { presenter.target_branch_link }
let(:presenter) { described_class.new(resource, current_user: user) }
context 'when target branch exists' do
it 'returns link' do
allow(resource).to receive(:target_branch_exists?) { true }
is_expected
.to eq("<a class=\"ref-name\" href=\"#{presenter.target_branch_commits_path}\">#{presenter.target_branch}</a>")
end
end
context 'when target branch does not exist' do
it 'returns text' do
allow(resource).to receive(:target_branch_exists?) { false }
is_expected.to eq("<span class=\"ref-name\">#{presenter.target_branch}</span>")
end
end
end
describe '#source_branch_with_namespace_link' do describe '#source_branch_with_namespace_link' do
subject do subject do
described_class.new(resource, current_user: user).source_branch_with_namespace_link described_class.new(resource, current_user: user).source_branch_with_namespace_link
......
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