diff --git a/app/presenters/ci/pipeline_presenter.rb b/app/presenters/ci/pipeline_presenter.rb
index 57daf04efc6..1c1347c5a57 100644
--- a/app/presenters/ci/pipeline_presenter.rb
+++ b/app/presenters/ci/pipeline_presenter.rb
@@ -3,6 +3,7 @@
module Ci
class PipelinePresenter < Gitlab::View::Presenter::Delegated
include Gitlab::Utils::StrongMemoize
+ include ActionView::Helpers::UrlHelper
# We use a class method here instead of a constant, allowing EE to redefine
# the returned `Hash` more easily.
@@ -32,5 +33,57 @@ module Ci
"Pipeline is redundant and is auto-canceled by Pipeline ##{auto_canceled_by_id}"
end
end
+
+ def ref_text
+ if pipeline.detached_merge_request_pipeline?
+ _("for %{link_to_merge_request} with %{link_to_merge_request_source_branch}").html_safe % { link_to_merge_request: link_to_merge_request, link_to_merge_request_source_branch: link_to_merge_request_source_branch }
+ elsif pipeline.merge_request_pipeline?
+ _("for %{link_to_merge_request} with %{link_to_merge_request_source_branch} into %{link_to_merge_request_target_branch}").html_safe % { link_to_merge_request: link_to_merge_request, link_to_merge_request_source_branch: link_to_merge_request_source_branch, link_to_merge_request_target_branch: link_to_merge_request_target_branch }
+ elsif pipeline.ref
+ if pipeline.ref_exists?
+ _("for %{link_to_pipeline_ref}").html_safe % { link_to_pipeline_ref: link_to_pipeline_ref }
+ else
+ _("for %{ref}") % { ref: content_tag(:span, pipeline.ref, class: 'ref-name') }
+ end
+ end
+ end
+
+ def link_to_pipeline_ref
+ link_to(pipeline.ref,
+ project_commits_path(pipeline.project, pipeline.ref),
+ class: "ref-name")
+ end
+
+ def link_to_merge_request
+ return unless merge_request_presenter
+
+ link_to(merge_request_presenter.to_reference,
+ project_merge_request_path(merge_request_presenter.project, merge_request_presenter),
+ class: 'mr-iid')
+ 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')
+ 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')
+ end
+
+ private
+
+ def merge_request_presenter
+ return unless pipeline.triggered_by_merge_request?
+
+ @merge_request_presenter ||= pipeline.merge_request.present(current_user: current_user)
+ end
end
end
diff --git a/app/presenters/merge_request_presenter.rb b/app/presenters/merge_request_presenter.rb
index af164858408..284b1ad9b55 100644
--- a/app/presenters/merge_request_presenter.rb
+++ b/app/presenters/merge_request_presenter.rb
@@ -104,6 +104,12 @@ class MergeRequestPresenter < Gitlab::View::Presenter::Delegated
end
end
+ def source_branch_commits_path
+ if source_branch_exists?
+ project_commits_path(source_project, source_branch)
+ end
+ end
+
def source_branch_path
if source_branch_exists?
project_branch_path(source_project, source_branch)
diff --git a/app/serializers/merge_request_for_pipeline_entity.rb b/app/serializers/merge_request_for_pipeline_entity.rb
index 7779ddfd65a..17a5c4ebbf9 100644
--- a/app/serializers/merge_request_for_pipeline_entity.rb
+++ b/app/serializers/merge_request_for_pipeline_entity.rb
@@ -11,7 +11,7 @@ class MergeRequestForPipelineEntity < Grape::Entity
expose :title
expose :source_branch
- expose :source_branch_path
+ expose :source_branch_commits_path, as: :source_branch_path
expose :target_branch
- expose :target_branch_path
+ expose :target_branch_commits_path, as: :target_branch_path
end
diff --git a/app/views/projects/pipelines/_info.html.haml b/app/views/projects/pipelines/_info.html.haml
index 55adeb345ab..5d307d6a70d 100644
--- a/app/views/projects/pipelines/_info.html.haml
+++ b/app/views/projects/pipelines/_info.html.haml
@@ -10,13 +10,7 @@
.icon-container
= icon('clock-o')
= pluralize @pipeline.total_size, "job"
- - if @pipeline.ref
- from
- - if @pipeline.ref_exists?
- = link_to @pipeline.ref, project_ref_path(@project, @pipeline.ref), class: "ref-name"
- - else
- %span.ref-name
- = @pipeline.ref
+ = @pipeline.ref_text
- if @pipeline.duration
in
= time_interval_in_words(@pipeline.duration)
@@ -48,9 +42,9 @@
content: "#{popover_content_text}",
} }
Auto DevOps
- - if @pipeline.merge_request_event?
- %span.js-pipeline-url-mergerequest.badge.badge-info.has-tooltip{ title: "This pipeline is run in a merge request context" }
- merge request
+ - if @pipeline.detached_merge_request_pipeline?
+ %span.js-pipeline-url-mergerequest.badge.badge-info.has-tooltip{ title: "This pipeline is run on the source branch" }
+ detached
- if @pipeline.stuck?
%span.js-pipeline-url-stuck.badge.badge-warning
stuck
diff --git a/changelogs/unreleased/nfriend-update-pipeline-detail-view.yml b/changelogs/unreleased/nfriend-update-pipeline-detail-view.yml
new file mode 100644
index 00000000000..a24325c4eb6
--- /dev/null
+++ b/changelogs/unreleased/nfriend-update-pipeline-detail-view.yml
@@ -0,0 +1,5 @@
+---
+title: Update pipeline detail view to accommodate post-merge pipelines
+merge_request: 25775
+author:
+type: added
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index c0c7041500d..47f201526d3 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -9126,6 +9126,18 @@ msgstr ""
msgid "estimateCommand|%{slash_command} will update the estimated time with the latest command."
msgstr ""
+msgid "for %{link_to_merge_request} with %{link_to_merge_request_source_branch}"
+msgstr ""
+
+msgid "for %{link_to_merge_request} with %{link_to_merge_request_source_branch} into %{link_to_merge_request_target_branch}"
+msgstr ""
+
+msgid "for %{link_to_pipeline_ref}"
+msgstr ""
+
+msgid "for %{ref}"
+msgstr ""
+
msgid "for this project"
msgstr ""
diff --git a/spec/features/projects/pipelines/pipeline_spec.rb b/spec/features/projects/pipelines/pipeline_spec.rb
index 36b8c15b8b6..5d2a99f40b7 100644
--- a/spec/features/projects/pipelines/pipeline_spec.rb
+++ b/spec/features/projects/pipelines/pipeline_spec.rb
@@ -1,6 +1,9 @@
require 'spec_helper'
describe 'Pipeline', :js do
+ include RoutesHelpers
+ include ProjectForksHelper
+
let(:project) { create(:project) }
let(:user) { create(:user) }
let(:role) { :developer }
@@ -72,6 +75,15 @@ describe 'Pipeline', :js do
expect(page).to have_link(pipeline.ref)
end
+ it 'shows the pipeline information' do
+ within '.pipeline-info' do
+ expect(page).to have_content("#{pipeline.statuses.count} jobs " \
+ "for #{pipeline.ref} ")
+ expect(page).to have_link(pipeline.ref,
+ href: project_commits_path(pipeline.project, pipeline.ref))
+ end
+ end
+
it_behaves_like 'showing user status' do
let(:user_with_status) { pipeline.user }
@@ -254,6 +266,113 @@ describe 'Pipeline', :js do
expect(page).to have_content(pipeline.ref)
end
end
+
+ context 'when pipeline is detached merge request pipeline' do
+ let(:source_project) { project }
+ let(:target_project) { project }
+
+ let(:merge_request) do
+ create(:merge_request,
+ :with_detached_merge_request_pipeline,
+ source_project: source_project,
+ target_project: target_project)
+ end
+
+ let(:pipeline) do
+ merge_request.all_pipelines.last
+ end
+
+ it 'shows the pipeline information' do
+ within '.pipeline-info' do
+ expect(page).to have_content("#{pipeline.statuses.count} jobs " \
+ "for !#{merge_request.iid} " \
+ "with #{merge_request.source_branch}")
+ expect(page).to have_link("!#{merge_request.iid}",
+ href: project_merge_request_path(project, merge_request))
+ expect(page).to have_link(merge_request.source_branch,
+ href: project_commits_path(merge_request.source_project, merge_request.source_branch))
+ end
+ end
+
+ context 'when source project is a forked project' do
+ let(:source_project) { fork_project(project, user, repository: true) }
+
+ before do
+ visit project_pipeline_path(source_project, 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} " \
+ "with #{merge_request.source_branch}")
+ expect(page).to have_link("!#{merge_request.iid}",
+ href: project_merge_request_path(project, merge_request))
+ expect(page).to have_link(merge_request.source_branch,
+ href: project_commits_path(merge_request.source_project, merge_request.source_branch))
+ end
+ end
+ end
+ end
+
+ context 'when pipeline is merge request pipeline' do
+ let(:source_project) { project }
+ let(:target_project) { project }
+
+ let(:merge_request) do
+ create(:merge_request,
+ :with_merge_request_pipeline,
+ source_project: source_project,
+ target_project: target_project,
+ merge_sha: project.commit.id)
+ end
+
+ let(:pipeline) do
+ merge_request.all_pipelines.last
+ end
+
+ before do
+ pipeline.update(user: user)
+ end
+
+ it 'shows the pipeline information' do
+ within '.pipeline-info' do
+ expect(page).to have_content("#{pipeline.statuses.count} jobs " \
+ "for !#{merge_request.iid} " \
+ "with #{merge_request.source_branch} " \
+ "into #{merge_request.target_branch}")
+ expect(page).to have_link("!#{merge_request.iid}",
+ href: project_merge_request_path(project, merge_request))
+ expect(page).to have_link(merge_request.source_branch,
+ href: project_commits_path(merge_request.source_project, merge_request.source_branch))
+ expect(page).to have_link(merge_request.target_branch,
+ href: project_commits_path(merge_request.target_project, merge_request.target_branch))
+ end
+ end
+
+ context 'when source project is a forked project' do
+ let(:source_project) { fork_project(project, user, repository: true) }
+
+ before do
+ visit project_pipeline_path(source_project, 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} " \
+ "with #{merge_request.source_branch} " \
+ "into #{merge_request.target_branch}")
+ expect(page).to have_link("!#{merge_request.iid}",
+ href: project_merge_request_path(project, merge_request))
+ expect(page).to have_link(merge_request.source_branch,
+ href: project_commits_path(merge_request.source_project, merge_request.source_branch))
+ expect(page).to have_link(merge_request.target_branch,
+ href: project_commits_path(merge_request.target_project, merge_request.target_branch))
+ end
+ end
+ end
+ end
end
context 'when user does not have access to read jobs' do
@@ -686,9 +805,9 @@ describe 'Pipeline', :js do
visit project_pipeline_path(project, pipeline)
end
- it 'contains badge that indicates merge request pipeline' do
+ it 'contains badge that indicates detached merge request pipeline' do
page.within(all('.well-segment')[1]) do
- expect(page).to have_content 'merge request'
+ expect(page).to have_content 'detached'
end
end
end
diff --git a/spec/presenters/ci/pipeline_presenter_spec.rb b/spec/presenters/ci/pipeline_presenter_spec.rb
index f7ceaf844be..cda07a0ae09 100644
--- a/spec/presenters/ci/pipeline_presenter_spec.rb
+++ b/spec/presenters/ci/pipeline_presenter_spec.rb
@@ -1,6 +1,9 @@
require 'spec_helper'
describe Ci::PipelinePresenter do
+ include Gitlab::Routing
+
+ let(:user) { create(:user) }
let(:project) { create(:project) }
let(:pipeline) { create(:ci_pipeline, project: project) }
@@ -8,6 +11,11 @@ describe Ci::PipelinePresenter do
described_class.new(pipeline)
end
+ before do
+ project.add_developer(user)
+ allow(presenter).to receive(:current_user) { user }
+ end
+
it 'inherits from Gitlab::View::Presenter::Delegated' do
expect(described_class.superclass).to eq(Gitlab::View::Presenter::Delegated)
end
@@ -68,4 +76,130 @@ describe Ci::PipelinePresenter do
end
end
end
+
+ describe '#ref_text' do
+ subject { presenter.ref_text }
+
+ context 'when pipeline is detached merge request pipeline' do
+ let(:merge_request) { create(:merge_request, :with_detached_merge_request_pipeline) }
+ let(:pipeline) { merge_request.all_pipelines.last }
+
+ it 'returns a correct ref text' do
+ is_expected.to eq("for #{merge_request.to_reference} " \
+ "with #{merge_request.source_branch}")
+ end
+ end
+
+ context 'when pipeline is merge request pipeline' do
+ let(:merge_request) { create(:merge_request, :with_merge_request_pipeline) }
+ let(:pipeline) { merge_request.all_pipelines.last }
+
+ it 'returns a correct ref text' do
+ is_expected.to eq("for #{merge_request.to_reference} " \
+ "with #{merge_request.source_branch} " \
+ "into #{merge_request.target_branch}")
+ end
+ end
+
+ context 'when pipeline is branch pipeline' do
+ let(:pipeline) { create(:ci_pipeline, project: project) }
+
+ context 'when ref exists in the repository' do
+ before do
+ allow(pipeline).to receive(:ref_exists?) { true }
+ end
+
+ it 'returns a correct ref text' do
+ is_expected.to eq("for #{pipeline.ref}")
+ end
+
+ context 'when ref contains malicious script' do
+ let(:pipeline) { create(:ci_pipeline, ref: "", project: project) }
+
+ it 'does not include the malicious script' do
+ is_expected.not_to include("")
+ end
+ end
+ end
+
+ context 'when ref exists in the repository' do
+ before do
+ allow(pipeline).to receive(:ref_exists?) { false }
+ end
+
+ it 'returns a correct ref text' do
+ is_expected.to eq("for #{pipeline.ref}")
+ end
+
+ context 'when ref contains malicious script' do
+ let(:pipeline) { create(:ci_pipeline, ref: "", project: project) }
+
+ it 'does not include the malicious script' do
+ is_expected.not_to include("")
+ end
+ end
+ end
+ end
+ end
+
+ describe '#link_to_merge_request' do
+ subject { presenter.link_to_merge_request }
+
+ let(:merge_request) { create(:merge_request, :with_detached_merge_request_pipeline) }
+ let(:pipeline) { merge_request.all_pipelines.last }
+
+ it 'returns a correct link' do
+ is_expected
+ .to include(project_merge_request_path(merge_request.project, merge_request))
+ end
+
+ context 'when pipeline is branch pipeline' do
+ let(:pipeline) { create(:ci_pipeline, project: project) }
+
+ it 'returns nothing' do
+ is_expected.to be_nil
+ end
+ end
+ end
+
+ describe '#link_to_merge_request_source_branch' do
+ subject { presenter.link_to_merge_request_source_branch }
+
+ let(:merge_request) { create(:merge_request, :with_detached_merge_request_pipeline) }
+ let(:pipeline) { merge_request.all_pipelines.last }
+
+ it 'returns a correct link' do
+ is_expected
+ .to include(project_commits_path(merge_request.source_project,
+ merge_request.source_branch))
+ end
+
+ context 'when pipeline is branch pipeline' do
+ let(:pipeline) { create(:ci_pipeline, project: project) }
+
+ it 'returns nothing' do
+ is_expected.to be_nil
+ end
+ end
+ end
+
+ describe '#link_to_merge_request_target_branch' do
+ subject { presenter.link_to_merge_request_target_branch }
+
+ let(:merge_request) { create(:merge_request, :with_merge_request_pipeline) }
+ let(:pipeline) { merge_request.all_pipelines.last }
+
+ it 'returns a correct link' do
+ is_expected
+ .to include(project_commits_path(merge_request.target_project, merge_request.target_branch))
+ end
+
+ context 'when pipeline is branch pipeline' do
+ let(:pipeline) { create(:ci_pipeline, project: project) }
+
+ it 'returns nothing' do
+ is_expected.to be_nil
+ end
+ end
+ end
end
diff --git a/spec/presenters/merge_request_presenter_spec.rb b/spec/presenters/merge_request_presenter_spec.rb
index 02cefcbc916..fd03f594c35 100644
--- a/spec/presenters/merge_request_presenter_spec.rb
+++ b/spec/presenters/merge_request_presenter_spec.rb
@@ -345,6 +345,30 @@ describe MergeRequestPresenter do
end
end
+ describe '#source_branch_commits_path' do
+ subject do
+ described_class.new(resource, current_user: user)
+ .source_branch_commits_path
+ end
+
+ context 'when source branch exists' do
+ it 'returns path' do
+ allow(resource).to receive(:source_branch_exists?) { true }
+
+ is_expected
+ .to eq("/#{resource.source_project.full_path}/commits/#{resource.source_branch}")
+ end
+ end
+
+ context 'when source branch does not exist' do
+ it 'returns nil' do
+ allow(resource).to receive(:source_branch_exists?) { false }
+
+ is_expected.to be_nil
+ end
+ end
+ end
+
describe '#target_branch_tree_path' do
subject do
described_class.new(resource, current_user: user)
diff --git a/spec/serializers/pipeline_entity_spec.rb b/spec/serializers/pipeline_entity_spec.rb
index c8308a0ae85..1d992e8a483 100644
--- a/spec/serializers/pipeline_entity_spec.rb
+++ b/spec/serializers/pipeline_entity_spec.rb
@@ -159,13 +159,13 @@ describe PipelineEntity do
expect(subject[:merge_request][:source_branch])
.to eq(merge_request.source_branch)
- expect(project_branch_path(project, merge_request.source_branch))
+ expect(project_commits_path(project, merge_request.source_branch))
.to include(subject[:merge_request][:source_branch_path])
expect(subject[:merge_request][:target_branch])
.to eq(merge_request.target_branch)
- expect(project_branch_path(project, merge_request.target_branch))
+ expect(project_commits_path(project, merge_request.target_branch))
.to include(subject[:merge_request][:target_branch_path])
end
end