From eaf8ae2eddec233374693d67d3d3c9c75d180763 Mon Sep 17 00:00:00 2001
From: Jacob Schatz <jschatz@gitlab.com>
Date: Wed, 17 Aug 2016 16:49:42 +0000
Subject: [PATCH] Merge branch '18681-pipelines-merge-request' into 'master'
Resolve "Pipelines for merge request"
## What does this MR do?
Adds `Pipelines` tab in merge request view
## What are the relevant issue numbers?
Closes #18681
## Screenshots (if relevant)
See merge request !5485
---
app/assets/javascripts/merge_request_tabs.js | 22 ++++++++-
.../javascripts/merge_request_widget.js | 2 +-
app/assets/stylesheets/pages/pipelines.scss | 12 +++++
.../projects/merge_requests_controller.rb | 21 ++++++--
app/models/merge_request.rb | 11 +++++
.../projects/ci/pipelines/_pipeline.html.haml | 20 ++++----
.../projects/commit/_pipelines_list.haml | 17 +++++++
.../projects/merge_requests/_show.html.haml | 14 ++++--
.../merge_requests/show/_builds.html.haml | 1 -
.../merge_requests/show/_pipelines.html.haml | 1 +
.../merge_requests/widget/_show.html.haml | 3 +-
config/routes.rb | 1 +
db/fixtures/development/14_builds.rb | 32 ++++++++++---
.../features/merge_requests/pipelines_spec.rb | 48 +++++++++++++++++++
spec/models/merge_request_spec.rb | 27 +++++++++++
15 files changed, 205 insertions(+), 27 deletions(-)
create mode 100644 app/views/projects/commit/_pipelines_list.haml
create mode 100644 app/views/projects/merge_requests/show/_pipelines.html.haml
create mode 100644 spec/features/merge_requests/pipelines_spec.rb
diff --git a/app/assets/javascripts/merge_request_tabs.js b/app/assets/javascripts/merge_request_tabs.js
index 52c2ed61012..1bba69a255a 100644
--- a/app/assets/javascripts/merge_request_tabs.js
+++ b/app/assets/javascripts/merge_request_tabs.js
@@ -9,6 +9,8 @@
MergeRequestTabs.prototype.buildsLoaded = false;
+ MergeRequestTabs.prototype.pipelinesLoaded = false;
+
MergeRequestTabs.prototype.commitsLoaded = false;
function MergeRequestTabs(opts) {
@@ -50,6 +52,9 @@
} else if (action === 'builds') {
this.loadBuilds($target.attr('href'));
this.expandView();
+ } else if (action === 'pipelines') {
+ this.loadPipelines($target.attr('href'));
+ this.expandView();
} else {
this.expandView();
}
@@ -81,7 +86,7 @@
if (action === 'show') {
action = 'notes';
}
- new_state = this._location.pathname.replace(/\/(commits|diffs|builds)(\.html)?\/?$/, '');
+ new_state = this._location.pathname.replace(/\/(commits|diffs|builds|pipelines)(\.html)?\/?$/, '');
if (action !== 'notes') {
new_state += "/" + action;
}
@@ -177,6 +182,21 @@
});
};
+ MergeRequestTabs.prototype.loadPipelines = function(source) {
+ if (this.pipelinesLoaded) {
+ return;
+ }
+ return this._get({
+ url: source + ".json",
+ success: function(data) {
+ $('#pipelines').html(data.html);
+ gl.utils.localTimeAgo($('.js-timeago', '#pipelines'));
+ this.pipelinesLoaded = true;
+ return this.scrollToElement("#pipelines");
+ }.bind(this)
+ });
+ };
+
MergeRequestTabs.prototype.toggleLoading = function(status) {
return $('.mr-loading-status .loading').toggle(status);
};
diff --git a/app/assets/javascripts/merge_request_widget.js b/app/assets/javascripts/merge_request_widget.js
index 362aaa906d0..659bd37c388 100644
--- a/app/assets/javascripts/merge_request_widget.js
+++ b/app/assets/javascripts/merge_request_widget.js
@@ -28,7 +28,7 @@
MergeRequestWidget.prototype.addEventListeners = function() {
var allowedPages;
- allowedPages = ['show', 'commits', 'builds', 'changes'];
+ allowedPages = ['show', 'commits', 'builds', 'pipelines', 'changes'];
return $(document).on('page:change.merge_request', (function(_this) {
return function() {
var page;
diff --git a/app/assets/stylesheets/pages/pipelines.scss b/app/assets/stylesheets/pages/pipelines.scss
index 21919fe4d73..50ac4d8449b 100644
--- a/app/assets/stylesheets/pages/pipelines.scss
+++ b/app/assets/stylesheets/pages/pipelines.scss
@@ -229,3 +229,15 @@
box-shadow: none;
}
}
+
+.pipelines.tab-pane {
+
+ .content-list.pipelines {
+ overflow: scroll;
+ }
+
+ .stage {
+ max-width: 60px;
+ width: 60px;
+ }
+}
diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb
index 7ca2dd2276a..7f331ba1dcd 100644
--- a/app/controllers/projects/merge_requests_controller.rb
+++ b/app/controllers/projects/merge_requests_controller.rb
@@ -9,15 +9,15 @@ class Projects::MergeRequestsController < Projects::ApplicationController
before_action :module_enabled
before_action :merge_request, only: [
- :edit, :update, :show, :diffs, :commits, :builds, :merge, :merge_check,
+ :edit, :update, :show, :diffs, :commits, :builds, :pipelines, :merge, :merge_check,
:ci_status, :toggle_subscription, :cancel_merge_when_build_succeeds, :remove_wip
]
- before_action :validates_merge_request, only: [:show, :diffs, :commits, :builds]
- before_action :define_show_vars, only: [:show, :diffs, :commits, :builds]
+ before_action :validates_merge_request, only: [:show, :diffs, :commits, :builds, :pipelines]
+ before_action :define_show_vars, only: [:show, :diffs, :commits, :builds, :pipelines]
before_action :define_widget_vars, only: [:merge, :cancel_merge_when_build_succeeds, :merge_check]
before_action :define_commit_vars, only: [:diffs]
before_action :define_diff_comment_vars, only: [:diffs]
- before_action :ensure_ref_fetched, only: [:show, :diffs, :commits, :builds]
+ before_action :ensure_ref_fetched, only: [:show, :diffs, :commits, :builds, :pipelines]
# Allow read any merge_request
before_action :authorize_read_merge_request!
@@ -141,6 +141,19 @@ class Projects::MergeRequestsController < Projects::ApplicationController
end
end
+ def pipelines
+ @pipelines = @merge_request.all_pipelines
+
+ respond_to do |format|
+ format.html do
+ define_discussion_vars
+
+ render 'show'
+ end
+ format.json { render json: { html: view_to_html_string('projects/merge_requests/show/_pipelines') } }
+ end
+ end
+
def new
apply_diff_view_cookie!
diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb
index fe799382fd0..d6a6a9a11ae 100644
--- a/app/models/merge_request.rb
+++ b/app/models/merge_request.rb
@@ -674,10 +674,21 @@ class MergeRequest < ActiveRecord::Base
diverged_commits_count > 0
end
+ def commits_sha
+ commits.map(&:sha)
+ end
+
def pipeline
@pipeline ||= source_project.pipeline(diff_head_sha, source_branch) if diff_head_sha && source_project
end
+ def all_pipelines
+ @all_pipelines ||=
+ if diff_head_sha && source_project
+ source_project.pipelines.order(id: :desc).where(sha: commits_sha, ref: source_branch)
+ end
+ end
+
def merge_commit
@merge_commit ||= project.commit(merge_commit_sha) if merge_commit_sha
end
diff --git a/app/views/projects/ci/pipelines/_pipeline.html.haml b/app/views/projects/ci/pipelines/_pipeline.html.haml
index 78709a92aed..be387201f8d 100644
--- a/app/views/projects/ci/pipelines/_pipeline.html.haml
+++ b/app/views/projects/ci/pipelines/_pipeline.html.haml
@@ -2,19 +2,21 @@
%tr.commit
%td.commit-link
= link_to namespace_project_pipeline_path(@project.namespace, @project, pipeline.id) do
- = ci_status_with_icon(status)
-
-
+ - if defined?(status_icon_only) && status_icon_only
+ = ci_icon_for_status(status)
+ - else
+ = ci_status_with_icon(status)
%td
.branch-commit
= link_to namespace_project_pipeline_path(@project.namespace, @project, pipeline.id) do
%span ##{pipeline.id}
- if pipeline.ref
- .icon-container
- = pipeline.tag? ? icon('tag') : icon('code-fork')
- = link_to pipeline.ref, namespace_project_commits_path(@project.namespace, @project, pipeline.ref), class: "monospace branch-name"
- .icon-container
- = custom_icon("icon_commit")
+ - unless defined?(hide_branch) && hide_branch
+ .icon-container
+ = pipeline.tag? ? icon('tag') : icon('code-fork')
+ = link_to pipeline.ref, namespace_project_commits_path(@project.namespace, @project, pipeline.ref), class: "monospace branch-name"
+ .icon-container
+ = custom_icon("icon_commit")
= link_to pipeline.short_sha, namespace_project_commit_path(@project.namespace, @project, pipeline.sha), class: "commit-id monospace"
- if pipeline.latest?
%span.label.label-success.has-tooltip{ title: 'Latest build for this branch' } latest
@@ -53,7 +55,7 @@
- if pipeline.finished_at
%p.finished-at
= icon("calendar")
- #{time_ago_with_tooltip(pipeline.finished_at, short_format: true, skip_js: true)}
+ #{time_ago_with_tooltip(pipeline.finished_at, short_format: false, skip_js: true)}
%td.pipeline-actions
.controls.hidden-xs.pull-right
diff --git a/app/views/projects/commit/_pipelines_list.haml b/app/views/projects/commit/_pipelines_list.haml
new file mode 100644
index 00000000000..29f4ef8f49e
--- /dev/null
+++ b/app/views/projects/commit/_pipelines_list.haml
@@ -0,0 +1,17 @@
+%ul.content-list.pipelines
+ - if pipelines.blank?
+ %li
+ .nothing-here-block No pipelines to show
+ - else
+ .table-holder
+ %table.table.builds
+ %tbody
+ %th Status
+ %th Commit
+ - pipelines.stages.each do |stage|
+ %th.stage
+ %span.has-tooltip{ title: "#{stage.titleize}" }
+ = stage.titleize
+ %th
+ %th
+ = render pipelines, commit_sha: true, stage: true, allow_retry: true, stages: pipelines.stages, status_icon_only: true, hide_branch: true
diff --git a/app/views/projects/merge_requests/_show.html.haml b/app/views/projects/merge_requests/_show.html.haml
index 269198adf91..a1313064725 100644
--- a/app/views/projects/merge_requests/_show.html.haml
+++ b/app/views/projects/merge_requests/_show.html.haml
@@ -45,20 +45,24 @@
- if @commits_count.nonzero?
%ul.merge-request-tabs.nav-links.no-top.no-bottom
%li.notes-tab
- = link_to namespace_project_merge_request_path(@project.namespace, @project, @merge_request), data: {target: 'div#notes', action: 'notes', toggle: 'tab'} do
+ = link_to namespace_project_merge_request_path(@project.namespace, @project, @merge_request), data: { target: 'div#notes', action: 'notes', toggle: 'tab' } do
Discussion
%span.badge= @merge_request.mr_and_commit_notes.user.count
%li.commits-tab
- = link_to commits_namespace_project_merge_request_path(@project.namespace, @project, @merge_request), data: {target: 'div#commits', action: 'commits', toggle: 'tab'} do
+ = link_to commits_namespace_project_merge_request_path(@project.namespace, @project, @merge_request), data: { target: 'div#commits', action: 'commits', toggle: 'tab' } do
Commits
%span.badge= @commits_count
- if @pipeline
+ %li.pipelines-tab
+ = link_to pipelines_namespace_project_merge_request_path(@project.namespace, @project, @merge_request), data: { target: '#pipelines', action: 'pipelines', toggle: 'tab' } do
+ Pipelines
+ %span.badge= @merge_request.all_pipelines.size
%li.builds-tab
- = link_to builds_namespace_project_merge_request_path(@project.namespace, @project, @merge_request), data: {target: '#builds', action: 'builds', toggle: 'tab'} do
+ = link_to builds_namespace_project_merge_request_path(@project.namespace, @project, @merge_request), data: { target: '#builds', action: 'builds', toggle: 'tab' } do
Builds
%span.badge= @statuses.size
%li.diffs-tab
- = link_to diffs_namespace_project_merge_request_path(@project.namespace, @project, @merge_request), data: {target: 'div#diffs', action: 'diffs', toggle: 'tab'} do
+ = link_to diffs_namespace_project_merge_request_path(@project.namespace, @project, @merge_request), data: { target: 'div#diffs', action: 'diffs', toggle: 'tab' } do
Changes
%span.badge= @merge_request.diff_size
@@ -76,6 +80,8 @@
- # This tab is always loaded via AJAX
#builds.builds.tab-pane
- # This tab is always loaded via AJAX
+ #pipelines.pipelines.tab-pane
+ - # This tab is always loaded via AJAX
#diffs.diffs.tab-pane
- # This tab is always loaded via AJAX
diff --git a/app/views/projects/merge_requests/show/_builds.html.haml b/app/views/projects/merge_requests/show/_builds.html.haml
index 81de60f116c..808ef7fed27 100644
--- a/app/views/projects/merge_requests/show/_builds.html.haml
+++ b/app/views/projects/merge_requests/show/_builds.html.haml
@@ -1,2 +1 @@
= render "projects/commit/pipeline", pipeline: @pipeline, link_to_commit: true
-
diff --git a/app/views/projects/merge_requests/show/_pipelines.html.haml b/app/views/projects/merge_requests/show/_pipelines.html.haml
new file mode 100644
index 00000000000..afe3f3430c6
--- /dev/null
+++ b/app/views/projects/merge_requests/show/_pipelines.html.haml
@@ -0,0 +1 @@
+= render "projects/commit/pipelines_list", pipelines: @pipelines, link_to_commit: true
diff --git a/app/views/projects/merge_requests/widget/_show.html.haml b/app/views/projects/merge_requests/widget/_show.html.haml
index d9efe81701f..ea618263a4a 100644
--- a/app/views/projects/merge_requests/widget/_show.html.haml
+++ b/app/views/projects/merge_requests/widget/_show.html.haml
@@ -23,7 +23,8 @@
preparing: "{{status}} build",
normal: "Build {{status}}"
},
- builds_path: "#{builds_namespace_project_merge_request_path(@project.namespace, @project, @merge_request)}"
+ builds_path: "#{builds_namespace_project_merge_request_path(@project.namespace, @project, @merge_request)}",
+ pipelines_path: "#{pipelines_namespace_project_merge_request_path(@project.namespace, @project, @merge_request)}"
};
if (typeof merge_request_widget !== 'undefined') {
diff --git a/config/routes.rb b/config/routes.rb
index 293d9e69813..f2c51706e25 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -723,6 +723,7 @@ Rails.application.routes.draw do
get :commits
get :diffs
get :builds
+ get :pipelines
get :merge_check
post :merge
post :cancel_merge_when_build_succeeds
diff --git a/db/fixtures/development/14_builds.rb b/db/fixtures/development/14_builds.rb
index 6441a036e75..0d493fa1c3c 100644
--- a/db/fixtures/development/14_builds.rb
+++ b/db/fixtures/development/14_builds.rb
@@ -26,24 +26,44 @@ class Gitlab::Seeder::Builds
begin
BUILDS.each { |opts| build_create!(pipeline, opts) }
commit_status_create!(pipeline, name: 'jenkins', status: :success)
-
print '.'
rescue ActiveRecord::RecordInvalid
print 'F'
+ ensure
+ pipeline.build_updated
end
end
end
def pipelines
- commits = @project.repository.commits('master', limit: 5)
- commits_sha = commits.map { |commit| commit.raw.id }
- commits_sha.map do |sha|
- @project.ensure_pipeline(sha, 'master')
- end
+ master_pipelines + merge_request_pipelines
+ end
+
+ def master_pipelines
+ create_pipelines_for(@project, 'master')
rescue
[]
end
+ def merge_request_pipelines
+ @project.merge_requests.last(5).map do |merge_request|
+ create_pipelines(merge_request.source_project, merge_request.source_branch, merge_request.commits.last(5))
+ end.flatten
+ rescue
+ []
+ end
+
+ def create_pipelines_for(project, ref)
+ commits = project.repository.commits(ref, limit: 5)
+ create_pipelines(project, ref, commits)
+ end
+
+ def create_pipelines(project, ref, commits)
+ commits.map do |commit|
+ project.pipelines.create(sha: commit.id, ref: ref)
+ end
+ end
+
def build_create!(pipeline, opts = {})
attributes = build_attributes_for(pipeline, opts)
diff --git a/spec/features/merge_requests/pipelines_spec.rb b/spec/features/merge_requests/pipelines_spec.rb
new file mode 100644
index 00000000000..9c4c0525267
--- /dev/null
+++ b/spec/features/merge_requests/pipelines_spec.rb
@@ -0,0 +1,48 @@
+require 'spec_helper'
+
+feature 'Pipelines for Merge Requests', feature: true, js: true do
+ include WaitForAjax
+
+ given(:user) { create(:user) }
+ given(:merge_request) { create(:merge_request) }
+ given(:project) { merge_request.target_project }
+
+ before do
+ project.team << [user, :master]
+ login_as user
+ end
+
+ context 'with pipelines' do
+ let!(:pipeline) do
+ create(:ci_empty_pipeline,
+ project: merge_request.source_project,
+ ref: merge_request.source_branch,
+ sha: merge_request.diff_head_sha)
+ end
+
+ before do
+ visit namespace_project_merge_request_path(project.namespace, project, merge_request)
+ end
+
+ scenario 'user visits merge request pipelines tab' do
+ page.within('.merge-request-tabs') do
+ click_link('Pipelines')
+ end
+ wait_for_ajax
+
+ expect(page).to have_selector('.pipeline-actions')
+ end
+ end
+
+ context 'without pipelines' do
+ before do
+ visit namespace_project_merge_request_path(project.namespace, project, merge_request)
+ end
+
+ scenario 'user visits merge request page' do
+ page.within('.merge-request-tabs') do
+ expect(page).to have_no_link('Pipelines')
+ end
+ end
+ end
+end
diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb
index 35a4418ebb3..acb75ec21a9 100644
--- a/spec/models/merge_request_spec.rb
+++ b/spec/models/merge_request_spec.rb
@@ -456,6 +456,20 @@ describe MergeRequest, models: true do
subject { create :merge_request, :simple }
end
+ describe '#commits_sha' do
+ let(:commit0) { double('commit0', sha: 'sha1') }
+ let(:commit1) { double('commit1', sha: 'sha2') }
+ let(:commit2) { double('commit2', sha: 'sha3') }
+
+ before do
+ allow(subject.merge_request_diff).to receive(:commits).and_return([commit0, commit1, commit2])
+ end
+
+ it 'returns sha of commits' do
+ expect(subject.commits_sha).to contain_exactly('sha1', 'sha2', 'sha3')
+ end
+ end
+
describe '#pipeline' do
describe 'when the source project exists' do
it 'returns the latest pipeline' do
@@ -480,6 +494,19 @@ describe MergeRequest, models: true do
end
end
+ describe '#all_pipelines' do
+ let!(:pipelines) do
+ subject.merge_request_diff.commits.map do |commit|
+ create(:ci_empty_pipeline, project: subject.source_project, sha: commit.id, ref: subject.source_branch)
+ end
+ end
+
+ it 'returns a pipelines from source projects with proper ordering' do
+ expect(subject.all_pipelines).not_to be_empty
+ expect(subject.all_pipelines).to eq(pipelines.reverse)
+ end
+ end
+
describe '#participants' do
let(:project) { create(:project, :public) }
--
2.30.9