diff options
author | Ash McKenzie <amckenzie@gitlab.com> | 2019-04-30 04:15:13 +0000 |
---|---|---|
committer | GitLab Release Tools Bot <robert+release-tools@gitlab.com> | 2019-04-30 23:54:56 +0000 |
commit | 1c63fc3ea8d1b038de6a121d9f084bea3986be7a (patch) | |
tree | eb570eb8dde1a63d033acdc7f5cb8a367eeb4fa4 | |
parent | 27e1e3dadbc9522b5f99ec3d189fea1d91dcd89b (diff) | |
download | gitlab-ce-1c63fc3ea8d1b038de6a121d9f084bea3986be7a.tar.gz |
Merge branch 'fix-ref-text-of-mr-pipelines' into 'master'
Fix pipelines for merge requests does not show pipeline page when source branch is removed
Closes #60432
See merge request gitlab-org/gitlab-ce!27803
(cherry picked from commit a2543ee29a97f61f960994d473291c7224c50c3d)
96b28d83 Fix ref_text of merge request pipelines
-rw-r--r-- | app/presenters/ci/pipeline_presenter.rb | 12 | ||||
-rw-r--r-- | app/presenters/merge_request_presenter.rb | 16 | ||||
-rw-r--r-- | changelogs/unreleased/fix-ref-text-of-mr-pipelines.yml | 6 | ||||
-rw-r--r-- | spec/features/projects/pipelines/pipeline_spec.rb | 38 | ||||
-rw-r--r-- | spec/presenters/merge_request_presenter_spec.rb | 46 |
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 55103c0a95c..99ea7849f0b 100644 --- a/app/presenters/merge_request_presenter.rb +++ b/app/presenters/merge_request_presenter.rb @@ -213,6 +213,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 4a0f91c4c7a..2d6b63cff85 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 |