summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorShinya Maeda <shinya@gitlab.com>2019-04-29 14:29:54 +0700
committerShinya Maeda <shinya@gitlab.com>2019-04-30 10:14:19 +0700
commit96b28d8387b97afae5a77eb6a4b240b67b1ced7f (patch)
tree3fb6882e8927e19cfeac866136cdc8157c22b4a1
parentf109d24ddca8d15c315d0283dd54e0be44ce2c42 (diff)
downloadgitlab-ce-96b28d8387b97afae5a77eb6a4b240b67b1ced7f.tar.gz
Fix ref_text of merge request pipelinesfix-ref-text-of-mr-pipelines
Source branch can be removed after the merge and we have to make sure to avoid rendering links if it's the case.
-rw-r--r--app/presenters/ci/pipeline_presenter.rb12
-rw-r--r--app/presenters/merge_request_presenter.rb16
-rw-r--r--changelogs/unreleased/fix-ref-text-of-mr-pipelines.yml6
-rw-r--r--spec/features/projects/pipelines/pipeline_spec.rb38
-rw-r--r--spec/presenters/merge_request_presenter_spec.rb46
5 files changed, 103 insertions, 15 deletions
diff --git a/app/presenters/ci/pipeline_presenter.rb b/app/presenters/ci/pipeline_presenter.rb
index 1c1347c5a57..944895904fe 100644
--- a/app/presenters/ci/pipeline_presenter.rb
+++ b/app/presenters/ci/pipeline_presenter.rb
@@ -63,19 +63,11 @@ module Ci
end
def link_to_merge_request_source_branch
- return unless merge_request_presenter
-
- link_to(merge_request_presenter.source_branch,
- merge_request_presenter.source_branch_commits_path,
- class: 'ref-name')
+ merge_request_presenter&.source_branch_link
end
def link_to_merge_request_target_branch
- return unless merge_request_presenter
-
- link_to(merge_request_presenter.target_branch,
- merge_request_presenter.target_branch_commits_path,
- class: 'ref-name')
+ merge_request_presenter&.target_branch_link
end
private
diff --git a/app/presenters/merge_request_presenter.rb b/app/presenters/merge_request_presenter.rb
index 3f7b5bebb74..ba0711ca867 100644
--- a/app/presenters/merge_request_presenter.rb
+++ b/app/presenters/merge_request_presenter.rb
@@ -216,6 +216,22 @@ class MergeRequestPresenter < Gitlab::View::Presenter::Delegated
help_page_path('ci/merge_request_pipelines/index.md')
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
def cached_can_be_reverted?
diff --git a/changelogs/unreleased/fix-ref-text-of-mr-pipelines.yml b/changelogs/unreleased/fix-ref-text-of-mr-pipelines.yml
new file mode 100644
index 00000000000..8803f9b52a4
--- /dev/null
+++ b/changelogs/unreleased/fix-ref-text-of-mr-pipelines.yml
@@ -0,0 +1,6 @@
+---
+title: Fix pipelines for merge requests does not show pipeline page when source branch
+ is removed
+merge_request: 27803
+author:
+type: fixed
diff --git a/spec/features/projects/pipelines/pipeline_spec.rb b/spec/features/projects/pipelines/pipeline_spec.rb
index cf334e1e4da..4ec44cb05b3 100644
--- a/spec/features/projects/pipelines/pipeline_spec.rb
+++ b/spec/features/projects/pipelines/pipeline_spec.rb
@@ -331,11 +331,9 @@ describe 'Pipeline', :js do
merge_request.all_pipelines.last
end
- before do
+ it 'shows the pipeline information' do
visit_pipeline
- end
- it 'shows the pipeline information' do
within '.pipeline-info' do
expect(page).to have_content("#{pipeline.statuses.count} jobs " \
"for !#{merge_request.iid} " \
@@ -347,6 +345,21 @@ describe 'Pipeline', :js do
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
let(:source_project) { fork_project(project, user, repository: true) }
@@ -386,11 +399,11 @@ describe 'Pipeline', :js do
before do
pipeline.update(user: user)
-
- visit_pipeline
end
it 'shows the pipeline information' do
+ visit_pipeline
+
within '.pipeline-info' do
expect(page).to have_content("#{pipeline.statuses.count} jobs " \
"for !#{merge_request.iid} " \
@@ -405,6 +418,21 @@ describe 'Pipeline', :js do
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
let(:source_project) { fork_project(project, user, repository: true) }
diff --git a/spec/presenters/merge_request_presenter_spec.rb b/spec/presenters/merge_request_presenter_spec.rb
index e5f08aeb1fa..451dc88880c 100644
--- a/spec/presenters/merge_request_presenter_spec.rb
+++ b/spec/presenters/merge_request_presenter_spec.rb
@@ -439,6 +439,52 @@ describe MergeRequestPresenter do
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
subject do
described_class.new(resource, current_user: user).source_branch_with_namespace_link