From db19e72027f21e573eb7889791e37ffa14a9a925 Mon Sep 17 00:00:00 2001 From: Annabel Dunstone Date: Fri, 22 Jul 2016 17:28:39 -0700 Subject: [PATCH 01/12] Add route, controller action, and views for MR pipelines --- .../javascripts/merge_request_tabs.js.coffee | 266 ++++++++++++++++++ .../merge_request_widget.js.coffee | 143 ++++++++++ .../projects/merge_requests_controller.rb | 19 +- .../projects/commit/_pipelines_list.haml | 54 ++++ .../projects/merge_requests/_show.html.haml | 6 + .../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 + 9 files changed, 488 insertions(+), 6 deletions(-) create mode 100644 app/assets/javascripts/merge_request_tabs.js.coffee create mode 100644 app/assets/javascripts/merge_request_widget.js.coffee create mode 100644 app/views/projects/commit/_pipelines_list.haml create mode 100644 app/views/projects/merge_requests/show/_pipelines.html.haml diff --git a/app/assets/javascripts/merge_request_tabs.js.coffee b/app/assets/javascripts/merge_request_tabs.js.coffee new file mode 100644 index 00000000000..ccdfcf895a3 --- /dev/null +++ b/app/assets/javascripts/merge_request_tabs.js.coffee @@ -0,0 +1,266 @@ +# MergeRequestTabs +# +# Handles persisting and restoring the current tab selection and lazily-loading +# content on the MergeRequests#show page. +# +#= require jquery.cookie +# +# ### Example Markup +# +# +# +#
+#
+# Notes Content +#
+#
+# Commits Content +#
+#
+# Diffs Content +#
+#
+# +#
+#
+# Loading Animation +#
+#
+# +class @MergeRequestTabs + diffsLoaded: false + buildsLoaded: false + commitsLoaded: false + + constructor: (@opts = {}) -> + # Store the `location` object, allowing for easier stubbing in tests + @_location = location + + @bindEvents() + @activateTab(@opts.action) + + bindEvents: -> + $(document).on 'shown.bs.tab', '.merge-request-tabs a[data-toggle="tab"]', @tabShown + $(document).on 'click', '.js-show-tab', @showTab + + showTab: (event) => + event.preventDefault() + + @activateTab $(event.target).data('action') + + tabShown: (event) => + $target = $(event.target) + action = $target.data('action') + + if action == 'commits' + @loadCommits($target.attr('href')) + @expandView() + else if action == 'diffs' + @loadDiff($target.attr('href')) + if bp? and bp.getBreakpointSize() isnt 'lg' + @shrinkView() + + navBarHeight = $('.navbar-gitlab').outerHeight() + $.scrollTo(".merge-request-details .merge-request-tabs", offset: -navBarHeight) + else if action == 'builds' + @loadBuilds($target.attr('href')) + @expandView() + else if action == 'pipelines' + @loadPipelines($target.attr('href')) + @expandView() + else + @expandView() + + @setCurrentAction(action) + + scrollToElement: (container) -> + if window.location.hash + navBarHeight = $('.navbar-gitlab').outerHeight() + $('.layout-nav').outerHeight() + + $el = $("#{container} #{window.location.hash}:not(.match)") + $.scrollTo("#{container} #{window.location.hash}:not(.match)", offset: -navBarHeight) if $el.length + + # Activate a tab based on the current action + activateTab: (action) -> + action = 'notes' if action == 'show' + $(".merge-request-tabs a[data-action='#{action}']").tab('show') + + # Replaces the current Merge Request-specific action in the URL with a new one + # + # If the action is "notes", the URL is reset to the standard + # `MergeRequests#show` route. + # + # Examples: + # + # location.pathname # => "/namespace/project/merge_requests/1" + # setCurrentAction('diffs') + # location.pathname # => "/namespace/project/merge_requests/1/diffs" + # + # location.pathname # => "/namespace/project/merge_requests/1/diffs" + # setCurrentAction('notes') + # location.pathname # => "/namespace/project/merge_requests/1" + # + # location.pathname # => "/namespace/project/merge_requests/1/diffs" + # setCurrentAction('commits') + # location.pathname # => "/namespace/project/merge_requests/1/commits" + # + # Returns the new URL String + setCurrentAction: (action) => + # Normalize action, just to be safe + action = 'notes' if action == 'show' + + # Remove a trailing '/commits' or '/diffs' + new_state = @_location.pathname.replace(/\/(commits|diffs|builds|pipelines)(\.html)?\/?$/, '') + + # Append the new action if we're on a tab other than 'notes' + unless action == 'notes' + new_state += "/#{action}" + + # Ensure parameters and hash come along for the ride + new_state += @_location.search + @_location.hash + + # Replace the current history state with the new one without breaking + # Turbolinks' history. + # + # See https://github.com/rails/turbolinks/issues/363 + history.replaceState {turbolinks: true, url: new_state}, document.title, new_state + + new_state + + loadCommits: (source) -> + return if @commitsLoaded + + @_get + url: "#{source}.json" + success: (data) => + document.querySelector("div#commits").innerHTML = data.html + gl.utils.localTimeAgo($('.js-timeago', 'div#commits')) + @commitsLoaded = true + @scrollToElement("#commits") + + loadDiff: (source) -> + return if @diffsLoaded + @_get + url: "#{source}.json" + @_location.search + success: (data) => + $('#diffs').html data.html + gl.utils.localTimeAgo($('.js-timeago', 'div#diffs')) + $('#diffs .js-syntax-highlight').syntaxHighlight() + $('#diffs .diff-file').singleFileDiff() + @expandViewContainer() if @diffViewType() is 'parallel' + @diffsLoaded = true + @scrollToElement("#diffs") + @highlighSelectedLine() + @filesCommentButton = $('.files .diff-file').filesCommentButton() + + $(document) + .off 'click', '.diff-line-num a' + .on 'click', '.diff-line-num a', (e) => + e.preventDefault() + window.location.hash = $(e.currentTarget).attr 'href' + @highlighSelectedLine() + @scrollToElement("#diffs") + + highlighSelectedLine: -> + $('.hll').removeClass 'hll' + locationHash = window.location.hash + + if locationHash isnt '' + hashClassString = ".#{locationHash.replace('#', '')}" + $diffLine = $("#{locationHash}:not(.match)", $('#diffs')) + + if not $diffLine.is 'tr' + $diffLine = $('#diffs').find("td#{locationHash}, td#{hashClassString}") + else + $diffLine = $diffLine.find('td') + + if $diffLine.length + $diffLine.addClass 'hll' + diffLineTop = $diffLine.offset().top + navBarHeight = $('.navbar-gitlab').outerHeight() + + loadBuilds: (source) -> + return if @buildsLoaded + + @_get + url: "#{source}.json" + success: (data) => + document.querySelector("div#builds").innerHTML = data.html + gl.utils.localTimeAgo($('.js-timeago', 'div#builds')) + @buildsLoaded = true + @scrollToElement("#builds") + + loadPipelines: (source) -> + return if @pipelinesLoaded + + @_get + url: "#{source}.json" + success: (data) => + document.querySelector("div#pipelines").innerHTML = data.html + gl.utils.localTimeAgo($('.js-timeago', 'div#pipelines')) + @pipelinesLoaded = true + @scrollToElement("#pipelines") + + # Show or hide the loading spinner + # + # status - Boolean, true to show, false to hide + toggleLoading: (status) -> + $('.mr-loading-status .loading').toggle(status) + + _get: (options) -> + defaults = { + beforeSend: => @toggleLoading(true) + complete: => @toggleLoading(false) + dataType: 'json' + type: 'GET' + } + + options = $.extend({}, defaults, options) + + $.ajax(options) + + # Returns diff view type + diffViewType: -> + $('.inline-parallel-buttons a.active').data('view-type') + + expandViewContainer: -> + $('.container-fluid').removeClass('container-limited') + + shrinkView: -> + $gutterIcon = $('.js-sidebar-toggle i:visible') + + # Wait until listeners are set + setTimeout( -> + # Only when sidebar is expanded + if $gutterIcon.is('.fa-angle-double-right') + $gutterIcon.closest('a').trigger('click', [true]) + , 0) + + # Expand the issuable sidebar unless the user explicitly collapsed it + expandView: -> + return if $.cookie('collapsed_gutter') == 'true' + + $gutterIcon = $('.js-sidebar-toggle i:visible') + + # Wait until listeners are set + setTimeout( -> + # Only when sidebar is collapsed + if $gutterIcon.is('.fa-angle-double-left') + $gutterIcon.closest('a').trigger('click', [true]) + , 0) diff --git a/app/assets/javascripts/merge_request_widget.js.coffee b/app/assets/javascripts/merge_request_widget.js.coffee new file mode 100644 index 00000000000..63a12582ddc --- /dev/null +++ b/app/assets/javascripts/merge_request_widget.js.coffee @@ -0,0 +1,143 @@ +class @MergeRequestWidget + # Initialize MergeRequestWidget behavior + # + # check_enable - Boolean, whether to check automerge status + # merge_check_url - String, URL to use to check automerge status + # ci_status_url - String, URL to use to check CI status + # + + constructor: (@opts) -> + $('#modal_merge_info').modal(show: false) + @firstCICheck = true + @readyForCICheck = false + @cancel = false + clearInterval @fetchBuildStatusInterval + + @clearEventListeners() + @addEventListeners() + @getCIStatus(false) + @pollCIStatus() + notifyPermissions() + + clearEventListeners: -> + $(document).off 'page:change.merge_request' + + cancelPolling: -> + @cancel = true + + addEventListeners: -> + allowedPages = ['show', 'commits', 'builds', 'pipelines', 'changes'] + $(document).on 'page:change.merge_request', => + page = $('body').data('page').split(':').last() + if allowedPages.indexOf(page) < 0 + clearInterval @fetchBuildStatusInterval + @cancelPolling() + @clearEventListeners() + + mergeInProgress: (deleteSourceBranch = false)-> + $.ajax + type: 'GET' + url: $('.merge-request').data('url') + success: (data) => + if data.state == "merged" + urlSuffix = if deleteSourceBranch then '?delete_source=true' else '' + + window.location.href = window.location.pathname + urlSuffix + else if data.merge_error + $('.mr-widget-body').html("

" + data.merge_error + "

") + else + callback = -> merge_request_widget.mergeInProgress(deleteSourceBranch) + setTimeout(callback, 2000) + dataType: 'json' + + getMergeStatus: -> + $.get @opts.merge_check_url, (data) -> + $('.mr-state-widget').replaceWith(data) + + ciLabelForStatus: (status) -> + switch status + when 'success' + 'passed' + when 'success_with_warnings' + 'passed with warnings' + else + status + + pollCIStatus: -> + @fetchBuildStatusInterval = setInterval ( => + return if not @readyForCICheck + + @getCIStatus(true) + + @readyForCICheck = false + ), 10000 + + getCIStatus: (showNotification) -> + _this = @ + $('.ci-widget-fetching').show() + + $.getJSON @opts.ci_status_url, (data) => + return if @cancel + @readyForCICheck = true + + if data.status is '' + return + + if @firstCICheck || data.status isnt @opts.ci_status and data.status? + @opts.ci_status = data.status + @showCIStatus data.status + if data.coverage + @showCICoverage data.coverage + + # The first check should only update the UI, a notification + # should only be displayed on status changes + if showNotification and not @firstCICheck + status = @ciLabelForStatus(data.status) + + if status is "preparing" + title = @opts.ci_title.preparing + status = status.charAt(0).toUpperCase() + status.slice(1); + message = @opts.ci_message.preparing.replace('{{status}}', status) + else + title = @opts.ci_title.normal + message = @opts.ci_message.normal.replace('{{status}}', status) + + title = title.replace('{{status}}', status) + message = message.replace('{{sha}}', data.sha) + message = message.replace('{{title}}', data.title) + + notify( + title, + message, + @opts.gitlab_icon, + -> + @close() + Turbolinks.visit _this.opts.builds_path + ) + @firstCICheck = false + + showCIStatus: (state) -> + return if not state? + $('.ci_widget').hide() + allowed_states = ["failed", "canceled", "running", "pending", "success", "success_with_warnings", "skipped", "not_found"] + if state in allowed_states + $('.ci_widget.ci-' + state).show() + switch state + when "failed", "canceled", "not_found" + @setMergeButtonClass('btn-danger') + when "running" + @setMergeButtonClass('btn-warning') + when "success", "success_with_warnings" + @setMergeButtonClass('btn-create') + else + $('.ci_widget.ci-error').show() + @setMergeButtonClass('btn-danger') + + showCICoverage: (coverage) -> + text = 'Coverage ' + coverage + '%' + $('.ci_widget:visible .ci-coverage').text(text) + + setMergeButtonClass: (css_class) -> + $('.js-merge-button,.accept-action .dropdown-toggle') + .removeClass('btn-danger btn-warning btn-create') + .addClass(css_class) diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index 594a61464b9..5c6396fba9f 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -7,15 +7,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! @@ -136,6 +136,17 @@ class Projects::MergeRequestsController < Projects::ApplicationController end end + def 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 build_merge_request @noteable = @merge_request diff --git a/app/views/projects/commit/_pipelines_list.haml b/app/views/projects/commit/_pipelines_list.haml new file mode 100644 index 00000000000..2c2feced657 --- /dev/null +++ b/app/views/projects/commit/_pipelines_list.haml @@ -0,0 +1,54 @@ +- status = pipeline.status + +%ul.content-list.pipelines + + .table-holder + %table.table.builds + %tbody + %th Status + %th Commit + %th.stage + %th + %th + %tr.commit + %td.commit-link + = link_to namespace_project_pipeline_path(@project.namespace, @project, pipeline.id) do + = 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") + = 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 + - if pipeline.triggered? + %span.label.label-primary triggered + - if pipeline.yaml_errors.present? + %span.label.label-danger.has-tooltip{ title: "#{pipeline.yaml_errors}" } yaml invalid + - if pipeline.builds.any?(&:stuck?) + %span.label.label-warning stuck + + %p.commit-title + - if commit = pipeline.commit + = author_avatar(commit, size: 20) + = link_to_gfm truncate(commit.title, length: 60), namespace_project_commit_path(@project.namespace, @project, commit.id), class: "commit-row-message" + - else + Cant find HEAD commit for this branch + + -# - stages_status = pipeline.statuses.latest.stages_status + -# - stages.each do |stage| + -# %td.stage-cell + -# - status = stages_status[stage] + -# - tooltip = "#{stage.titleize}: #{status || 'not found'}" + -# - if status + -# = link_to namespace_project_pipeline_path(@project.namespace, @project, pipeline.id, anchor: stage), class: "has-tooltip ci-status-icon-#{status}", title: tooltip do + -# = ci_icon_for_status(status) + -# - else + -# .light.has-tooltip{ title: tooltip } + -# \- diff --git a/app/views/projects/merge_requests/_show.html.haml b/app/views/projects/merge_requests/_show.html.haml index 873ed9b59ee..a78407f26ea 100644 --- a/app/views/projects/merge_requests/_show.html.haml +++ b/app/views/projects/merge_requests/_show.html.haml @@ -53,6 +53,10 @@ 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= @statuses.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 Builds @@ -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..bcaec137371 --- /dev/null +++ b/app/views/projects/merge_requests/show/_pipelines.html.haml @@ -0,0 +1 @@ += render "projects/commit/pipelines_list", pipeline: @pipeline, 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 21f3585bacd..84a89200111 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -704,6 +704,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 From d71d3b8c2baaf0527c14d649896590a1e0a9b218 Mon Sep 17 00:00:00 2001 From: Annabel Dunstone Date: Mon, 25 Jul 2016 16:14:14 -0500 Subject: [PATCH 02/12] Convert to javascript --- app/assets/javascripts/merge_request_tabs.js | 24 +- .../javascripts/merge_request_tabs.js.coffee | 266 ------------------ .../javascripts/merge_request_widget.js | 2 +- .../merge_request_widget.js.coffee | 143 ---------- 4 files changed, 24 insertions(+), 411 deletions(-) delete mode 100644 app/assets/javascripts/merge_request_tabs.js.coffee delete mode 100644 app/assets/javascripts/merge_request_widget.js.coffee diff --git a/app/assets/javascripts/merge_request_tabs.js b/app/assets/javascripts/merge_request_tabs.js index 52c2ed61012..a21cdbc8a10 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,23 @@ }); }; + MergeRequestTabs.prototype.loadPipelines = function(source) { + if (this.pipelinesLoaded) { + return; + } + return this._get({ + url: source + ".json", + success: (function(_this) { + return function(data) { + document.querySelector("div#pipelines").innerHTML = data.html; + gl.utils.localTimeAgo($('.js-timeago', 'div#pipelines')); + _this.pipelinesLoaded = true; + return _this.scrollToElement("#pipelines"); + }; + })(this) + }); + }; + MergeRequestTabs.prototype.toggleLoading = function(status) { return $('.mr-loading-status .loading').toggle(status); }; diff --git a/app/assets/javascripts/merge_request_tabs.js.coffee b/app/assets/javascripts/merge_request_tabs.js.coffee deleted file mode 100644 index ccdfcf895a3..00000000000 --- a/app/assets/javascripts/merge_request_tabs.js.coffee +++ /dev/null @@ -1,266 +0,0 @@ -# MergeRequestTabs -# -# Handles persisting and restoring the current tab selection and lazily-loading -# content on the MergeRequests#show page. -# -#= require jquery.cookie -# -# ### Example Markup -# -# -# -#
-#
-# Notes Content -#
-#
-# Commits Content -#
-#
-# Diffs Content -#
-#
-# -#
-#
-# Loading Animation -#
-#
-# -class @MergeRequestTabs - diffsLoaded: false - buildsLoaded: false - commitsLoaded: false - - constructor: (@opts = {}) -> - # Store the `location` object, allowing for easier stubbing in tests - @_location = location - - @bindEvents() - @activateTab(@opts.action) - - bindEvents: -> - $(document).on 'shown.bs.tab', '.merge-request-tabs a[data-toggle="tab"]', @tabShown - $(document).on 'click', '.js-show-tab', @showTab - - showTab: (event) => - event.preventDefault() - - @activateTab $(event.target).data('action') - - tabShown: (event) => - $target = $(event.target) - action = $target.data('action') - - if action == 'commits' - @loadCommits($target.attr('href')) - @expandView() - else if action == 'diffs' - @loadDiff($target.attr('href')) - if bp? and bp.getBreakpointSize() isnt 'lg' - @shrinkView() - - navBarHeight = $('.navbar-gitlab').outerHeight() - $.scrollTo(".merge-request-details .merge-request-tabs", offset: -navBarHeight) - else if action == 'builds' - @loadBuilds($target.attr('href')) - @expandView() - else if action == 'pipelines' - @loadPipelines($target.attr('href')) - @expandView() - else - @expandView() - - @setCurrentAction(action) - - scrollToElement: (container) -> - if window.location.hash - navBarHeight = $('.navbar-gitlab').outerHeight() + $('.layout-nav').outerHeight() - - $el = $("#{container} #{window.location.hash}:not(.match)") - $.scrollTo("#{container} #{window.location.hash}:not(.match)", offset: -navBarHeight) if $el.length - - # Activate a tab based on the current action - activateTab: (action) -> - action = 'notes' if action == 'show' - $(".merge-request-tabs a[data-action='#{action}']").tab('show') - - # Replaces the current Merge Request-specific action in the URL with a new one - # - # If the action is "notes", the URL is reset to the standard - # `MergeRequests#show` route. - # - # Examples: - # - # location.pathname # => "/namespace/project/merge_requests/1" - # setCurrentAction('diffs') - # location.pathname # => "/namespace/project/merge_requests/1/diffs" - # - # location.pathname # => "/namespace/project/merge_requests/1/diffs" - # setCurrentAction('notes') - # location.pathname # => "/namespace/project/merge_requests/1" - # - # location.pathname # => "/namespace/project/merge_requests/1/diffs" - # setCurrentAction('commits') - # location.pathname # => "/namespace/project/merge_requests/1/commits" - # - # Returns the new URL String - setCurrentAction: (action) => - # Normalize action, just to be safe - action = 'notes' if action == 'show' - - # Remove a trailing '/commits' or '/diffs' - new_state = @_location.pathname.replace(/\/(commits|diffs|builds|pipelines)(\.html)?\/?$/, '') - - # Append the new action if we're on a tab other than 'notes' - unless action == 'notes' - new_state += "/#{action}" - - # Ensure parameters and hash come along for the ride - new_state += @_location.search + @_location.hash - - # Replace the current history state with the new one without breaking - # Turbolinks' history. - # - # See https://github.com/rails/turbolinks/issues/363 - history.replaceState {turbolinks: true, url: new_state}, document.title, new_state - - new_state - - loadCommits: (source) -> - return if @commitsLoaded - - @_get - url: "#{source}.json" - success: (data) => - document.querySelector("div#commits").innerHTML = data.html - gl.utils.localTimeAgo($('.js-timeago', 'div#commits')) - @commitsLoaded = true - @scrollToElement("#commits") - - loadDiff: (source) -> - return if @diffsLoaded - @_get - url: "#{source}.json" + @_location.search - success: (data) => - $('#diffs').html data.html - gl.utils.localTimeAgo($('.js-timeago', 'div#diffs')) - $('#diffs .js-syntax-highlight').syntaxHighlight() - $('#diffs .diff-file').singleFileDiff() - @expandViewContainer() if @diffViewType() is 'parallel' - @diffsLoaded = true - @scrollToElement("#diffs") - @highlighSelectedLine() - @filesCommentButton = $('.files .diff-file').filesCommentButton() - - $(document) - .off 'click', '.diff-line-num a' - .on 'click', '.diff-line-num a', (e) => - e.preventDefault() - window.location.hash = $(e.currentTarget).attr 'href' - @highlighSelectedLine() - @scrollToElement("#diffs") - - highlighSelectedLine: -> - $('.hll').removeClass 'hll' - locationHash = window.location.hash - - if locationHash isnt '' - hashClassString = ".#{locationHash.replace('#', '')}" - $diffLine = $("#{locationHash}:not(.match)", $('#diffs')) - - if not $diffLine.is 'tr' - $diffLine = $('#diffs').find("td#{locationHash}, td#{hashClassString}") - else - $diffLine = $diffLine.find('td') - - if $diffLine.length - $diffLine.addClass 'hll' - diffLineTop = $diffLine.offset().top - navBarHeight = $('.navbar-gitlab').outerHeight() - - loadBuilds: (source) -> - return if @buildsLoaded - - @_get - url: "#{source}.json" - success: (data) => - document.querySelector("div#builds").innerHTML = data.html - gl.utils.localTimeAgo($('.js-timeago', 'div#builds')) - @buildsLoaded = true - @scrollToElement("#builds") - - loadPipelines: (source) -> - return if @pipelinesLoaded - - @_get - url: "#{source}.json" - success: (data) => - document.querySelector("div#pipelines").innerHTML = data.html - gl.utils.localTimeAgo($('.js-timeago', 'div#pipelines')) - @pipelinesLoaded = true - @scrollToElement("#pipelines") - - # Show or hide the loading spinner - # - # status - Boolean, true to show, false to hide - toggleLoading: (status) -> - $('.mr-loading-status .loading').toggle(status) - - _get: (options) -> - defaults = { - beforeSend: => @toggleLoading(true) - complete: => @toggleLoading(false) - dataType: 'json' - type: 'GET' - } - - options = $.extend({}, defaults, options) - - $.ajax(options) - - # Returns diff view type - diffViewType: -> - $('.inline-parallel-buttons a.active').data('view-type') - - expandViewContainer: -> - $('.container-fluid').removeClass('container-limited') - - shrinkView: -> - $gutterIcon = $('.js-sidebar-toggle i:visible') - - # Wait until listeners are set - setTimeout( -> - # Only when sidebar is expanded - if $gutterIcon.is('.fa-angle-double-right') - $gutterIcon.closest('a').trigger('click', [true]) - , 0) - - # Expand the issuable sidebar unless the user explicitly collapsed it - expandView: -> - return if $.cookie('collapsed_gutter') == 'true' - - $gutterIcon = $('.js-sidebar-toggle i:visible') - - # Wait until listeners are set - setTimeout( -> - # Only when sidebar is collapsed - if $gutterIcon.is('.fa-angle-double-left') - $gutterIcon.closest('a').trigger('click', [true]) - , 0) 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/javascripts/merge_request_widget.js.coffee b/app/assets/javascripts/merge_request_widget.js.coffee deleted file mode 100644 index 63a12582ddc..00000000000 --- a/app/assets/javascripts/merge_request_widget.js.coffee +++ /dev/null @@ -1,143 +0,0 @@ -class @MergeRequestWidget - # Initialize MergeRequestWidget behavior - # - # check_enable - Boolean, whether to check automerge status - # merge_check_url - String, URL to use to check automerge status - # ci_status_url - String, URL to use to check CI status - # - - constructor: (@opts) -> - $('#modal_merge_info').modal(show: false) - @firstCICheck = true - @readyForCICheck = false - @cancel = false - clearInterval @fetchBuildStatusInterval - - @clearEventListeners() - @addEventListeners() - @getCIStatus(false) - @pollCIStatus() - notifyPermissions() - - clearEventListeners: -> - $(document).off 'page:change.merge_request' - - cancelPolling: -> - @cancel = true - - addEventListeners: -> - allowedPages = ['show', 'commits', 'builds', 'pipelines', 'changes'] - $(document).on 'page:change.merge_request', => - page = $('body').data('page').split(':').last() - if allowedPages.indexOf(page) < 0 - clearInterval @fetchBuildStatusInterval - @cancelPolling() - @clearEventListeners() - - mergeInProgress: (deleteSourceBranch = false)-> - $.ajax - type: 'GET' - url: $('.merge-request').data('url') - success: (data) => - if data.state == "merged" - urlSuffix = if deleteSourceBranch then '?delete_source=true' else '' - - window.location.href = window.location.pathname + urlSuffix - else if data.merge_error - $('.mr-widget-body').html("

" + data.merge_error + "

") - else - callback = -> merge_request_widget.mergeInProgress(deleteSourceBranch) - setTimeout(callback, 2000) - dataType: 'json' - - getMergeStatus: -> - $.get @opts.merge_check_url, (data) -> - $('.mr-state-widget').replaceWith(data) - - ciLabelForStatus: (status) -> - switch status - when 'success' - 'passed' - when 'success_with_warnings' - 'passed with warnings' - else - status - - pollCIStatus: -> - @fetchBuildStatusInterval = setInterval ( => - return if not @readyForCICheck - - @getCIStatus(true) - - @readyForCICheck = false - ), 10000 - - getCIStatus: (showNotification) -> - _this = @ - $('.ci-widget-fetching').show() - - $.getJSON @opts.ci_status_url, (data) => - return if @cancel - @readyForCICheck = true - - if data.status is '' - return - - if @firstCICheck || data.status isnt @opts.ci_status and data.status? - @opts.ci_status = data.status - @showCIStatus data.status - if data.coverage - @showCICoverage data.coverage - - # The first check should only update the UI, a notification - # should only be displayed on status changes - if showNotification and not @firstCICheck - status = @ciLabelForStatus(data.status) - - if status is "preparing" - title = @opts.ci_title.preparing - status = status.charAt(0).toUpperCase() + status.slice(1); - message = @opts.ci_message.preparing.replace('{{status}}', status) - else - title = @opts.ci_title.normal - message = @opts.ci_message.normal.replace('{{status}}', status) - - title = title.replace('{{status}}', status) - message = message.replace('{{sha}}', data.sha) - message = message.replace('{{title}}', data.title) - - notify( - title, - message, - @opts.gitlab_icon, - -> - @close() - Turbolinks.visit _this.opts.builds_path - ) - @firstCICheck = false - - showCIStatus: (state) -> - return if not state? - $('.ci_widget').hide() - allowed_states = ["failed", "canceled", "running", "pending", "success", "success_with_warnings", "skipped", "not_found"] - if state in allowed_states - $('.ci_widget.ci-' + state).show() - switch state - when "failed", "canceled", "not_found" - @setMergeButtonClass('btn-danger') - when "running" - @setMergeButtonClass('btn-warning') - when "success", "success_with_warnings" - @setMergeButtonClass('btn-create') - else - $('.ci_widget.ci-error').show() - @setMergeButtonClass('btn-danger') - - showCICoverage: (coverage) -> - text = 'Coverage ' + coverage + '%' - $('.ci_widget:visible .ci-coverage').text(text) - - setMergeButtonClass: (css_class) -> - $('.js-merge-button,.accept-action .dropdown-toggle') - .removeClass('btn-danger btn-warning btn-create') - .addClass(css_class) From 46cfd642ec78f9f5f0a174d3f5f6330acb0587dc Mon Sep 17 00:00:00 2001 From: Annabel Dunstone Date: Mon, 25 Jul 2016 18:29:53 -0500 Subject: [PATCH 03/12] Update MR pipeline view --- .../projects/commit/_pipelines_list.haml | 69 +++++-------------- .../merge_requests/show/_pipelines.html.haml | 2 +- 2 files changed, 17 insertions(+), 54 deletions(-) diff --git a/app/views/projects/commit/_pipelines_list.haml b/app/views/projects/commit/_pipelines_list.haml index 2c2feced657..0d6de6dfa2e 100644 --- a/app/views/projects/commit/_pipelines_list.haml +++ b/app/views/projects/commit/_pipelines_list.haml @@ -1,54 +1,17 @@ -- status = pipeline.status - %ul.content-list.pipelines - - .table-holder - %table.table.builds - %tbody - %th Status - %th Commit - %th.stage - %th - %th - %tr.commit - %td.commit-link - = link_to namespace_project_pipeline_path(@project.namespace, @project, pipeline.id) do - = 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") - = 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 - - if pipeline.triggered? - %span.label.label-primary triggered - - if pipeline.yaml_errors.present? - %span.label.label-danger.has-tooltip{ title: "#{pipeline.yaml_errors}" } yaml invalid - - if pipeline.builds.any?(&:stuck?) - %span.label.label-warning stuck - - %p.commit-title - - if commit = pipeline.commit - = author_avatar(commit, size: 20) - = link_to_gfm truncate(commit.title, length: 60), namespace_project_commit_path(@project.namespace, @project, commit.id), class: "commit-row-message" - - else - Cant find HEAD commit for this branch - - -# - stages_status = pipeline.statuses.latest.stages_status - -# - stages.each do |stage| - -# %td.stage-cell - -# - status = stages_status[stage] - -# - tooltip = "#{stage.titleize}: #{status || 'not found'}" - -# - if status - -# = link_to namespace_project_pipeline_path(@project.namespace, @project, pipeline.id, anchor: stage), class: "has-tooltip ci-status-icon-#{status}", title: tooltip do - -# = ci_icon_for_status(status) - -# - else - -# .light.has-tooltip{ title: tooltip } - -# \- + - 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: stages diff --git a/app/views/projects/merge_requests/show/_pipelines.html.haml b/app/views/projects/merge_requests/show/_pipelines.html.haml index bcaec137371..afe3f3430c6 100644 --- a/app/views/projects/merge_requests/show/_pipelines.html.haml +++ b/app/views/projects/merge_requests/show/_pipelines.html.haml @@ -1 +1 @@ -= render "projects/commit/pipelines_list", pipeline: @pipeline, link_to_commit: true += render "projects/commit/pipelines_list", pipelines: @pipelines, link_to_commit: true From 0d6d7f6e30ee24a82dfede92d0ebbda86d93edb5 Mon Sep 17 00:00:00 2001 From: Annabel Dunstone Date: Mon, 25 Jul 2016 19:30:52 -0500 Subject: [PATCH 04/12] Display all pipelines for Merge Request --- app/controllers/projects/merge_requests_controller.rb | 2 ++ app/views/projects/commit/_pipelines_list.haml | 2 +- app/views/projects/merge_requests/_show.html.haml | 2 +- 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index 5c6396fba9f..acdb5ba5c4f 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -137,6 +137,8 @@ class Projects::MergeRequestsController < Projects::ApplicationController end def pipelines + @pipelines = Ci::Pipeline.where(ref: @merge_request.source_branch) + respond_to do |format| format.html do define_discussion_vars diff --git a/app/views/projects/commit/_pipelines_list.haml b/app/views/projects/commit/_pipelines_list.haml index 0d6de6dfa2e..03db765180a 100644 --- a/app/views/projects/commit/_pipelines_list.haml +++ b/app/views/projects/commit/_pipelines_list.haml @@ -14,4 +14,4 @@ = stage.titleize %th %th - = render pipelines, commit_sha: true, stage: true, allow_retry: true, stages: stages + = render pipelines, commit_sha: true, stage: true, allow_retry: true, stages: pipelines.stages diff --git a/app/views/projects/merge_requests/_show.html.haml b/app/views/projects/merge_requests/_show.html.haml index a78407f26ea..9ae2b52bace 100644 --- a/app/views/projects/merge_requests/_show.html.haml +++ b/app/views/projects/merge_requests/_show.html.haml @@ -56,7 +56,7 @@ %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= @statuses.size + %span.badge %li.builds-tab = link_to builds_namespace_project_merge_request_path(@project.namespace, @project, @merge_request), data: {target: '#builds', action: 'builds', toggle: 'tab'} do Builds From cae0fa7cba98cb39f70deb87fe30bdc9bfa487fc Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Fri, 12 Aug 2016 16:22:48 +0200 Subject: [PATCH 05/12] Properly select a list of Pipelines for a Merge Requests --- .../projects/merge_requests_controller.rb | 2 +- app/models/merge_request.rb | 11 +++++++ spec/models/merge_request_spec.rb | 32 +++++++++++++++++++ 3 files changed, 44 insertions(+), 1 deletion(-) diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index acdb5ba5c4f..d6128b3dfe9 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -137,7 +137,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController end def pipelines - @pipelines = Ci::Pipeline.where(ref: @merge_request.source_branch) + @pipelines = @merge_request.all_pipelines respond_to do |format| format.html do diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 471e32f3b60..dc758a45bcf 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -642,10 +642,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/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index c8ad7ab3e7f..a7957fe3b84 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -419,6 +419,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).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 @@ -443,6 +457,24 @@ describe MergeRequest, models: true do end end + describe '#all_pipelines' do + let(:commit0) { double('commit0', sha: 'sha1') } + let(:commit1) { double('commit1', sha: 'sha2') } + let(:commit2) { double('commit2', sha: 'sha3') } + let!(:pipeline) { create(:ci_empty_pipeline, project: subject.source_project, sha: 'sha1', ref: subject.source_branch) } + let!(:pipeline2) { create(:ci_empty_pipeline, project: subject.source_project, sha: 'sha1', ref: subject.source_branch) } + let!(:pipeline3) { create(:ci_empty_pipeline, project: subject.source_project, sha: 'sha2', ref: subject.source_branch) } + let!(:pipeline4) { create(:ci_empty_pipeline, project: subject.target_project, sha: 'sha1', ref: subject.target_branch) } + + before do + allow(subject).to receive(:commits).and_return([commit0, commit1, commit2]) + end + + it 'returns a pipelines from source projects' do + expect(subject.all_pipelines).to eq([pipeline3, pipeline2, pipeline]) + end + end + describe '#participants' do let(:project) { create(:project, :public) } From aa8cb00b58f5f37e90e27b443f2fea4842ab47d3 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Fri, 12 Aug 2016 16:23:00 +0200 Subject: [PATCH 06/12] Add a feature tests to check if a view can be rendered properly --- .../features/merge_requests/pipelines_spec.rb | 48 +++++++++++++++++++ 1 file changed, 48 insertions(+) create mode 100644 spec/features/merge_requests/pipelines_spec.rb diff --git a/spec/features/merge_requests/pipelines_spec.rb b/spec/features/merge_requests/pipelines_spec.rb new file mode 100644 index 00000000000..2785ab6d05d --- /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 'does click a pipeline tab and sees a list of pipelines' 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 'does not find a pipeline link' do + page.within('.merge-request-tabs') do + expect(page).not_to have_link('Pipelines') + end + end + end +end From db3c5a042d2cd03fb64d7f85b130d065dce3eb42 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Mon, 15 Aug 2016 22:53:09 +0200 Subject: [PATCH 07/12] Use merge_request_diff instead of doubles for testing pipelines for Merge Requests --- .../projects/merge_requests/_show.html.haml | 10 ++++----- .../features/merge_requests/pipelines_spec.rb | 4 ++-- spec/models/merge_request_spec.rb | 21 +++++++------------ 3 files changed, 15 insertions(+), 20 deletions(-) diff --git a/app/views/projects/merge_requests/_show.html.haml b/app/views/projects/merge_requests/_show.html.haml index 9ae2b52bace..13cacf45fb5 100644 --- a/app/views/projects/merge_requests/_show.html.haml +++ b/app/views/projects/merge_requests/_show.html.haml @@ -45,24 +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 + = link_to pipelines_namespace_project_merge_request_path(@project.namespace, @project, @merge_request), data: { target: '#pipelines', action: 'pipelines', toggle: 'tab' } do Pipelines %span.badge %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 diff --git a/spec/features/merge_requests/pipelines_spec.rb b/spec/features/merge_requests/pipelines_spec.rb index 2785ab6d05d..eeb898f083e 100644 --- a/spec/features/merge_requests/pipelines_spec.rb +++ b/spec/features/merge_requests/pipelines_spec.rb @@ -24,7 +24,7 @@ feature 'Pipelines for Merge Requests', feature: true, js: true do visit namespace_project_merge_request_path(project.namespace, project, merge_request) end - scenario 'does click a pipeline tab and sees a list of pipelines' do + scenario 'user visits merge request pipelines tab' do page.within('.merge-request-tabs') do click_link('Pipelines') end @@ -39,7 +39,7 @@ feature 'Pipelines for Merge Requests', feature: true, js: true do visit namespace_project_merge_request_path(project.namespace, project, merge_request) end - scenario 'does not find a pipeline link' do + scenario 'user visits merge request page' do page.within('.merge-request-tabs') do expect(page).not_to have_link('Pipelines') end diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index a7957fe3b84..43a87e07435 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -425,7 +425,7 @@ describe MergeRequest, models: true do let(:commit2) { double('commit2', sha: 'sha3') } before do - allow(subject).to receive(:commits).and_return([commit0, commit1, commit2]) + allow(subject.merge_request_diff).to receive(:commits).and_return([commit0, commit1, commit2]) end it 'returns sha of commits' do @@ -458,20 +458,15 @@ describe MergeRequest, models: true do end describe '#all_pipelines' do - let(:commit0) { double('commit0', sha: 'sha1') } - let(:commit1) { double('commit1', sha: 'sha2') } - let(:commit2) { double('commit2', sha: 'sha3') } - let!(:pipeline) { create(:ci_empty_pipeline, project: subject.source_project, sha: 'sha1', ref: subject.source_branch) } - let!(:pipeline2) { create(:ci_empty_pipeline, project: subject.source_project, sha: 'sha1', ref: subject.source_branch) } - let!(:pipeline3) { create(:ci_empty_pipeline, project: subject.source_project, sha: 'sha2', ref: subject.source_branch) } - let!(:pipeline4) { create(:ci_empty_pipeline, project: subject.target_project, sha: 'sha1', ref: subject.target_branch) } - - before do - allow(subject).to receive(:commits).and_return([commit0, commit1, commit2]) + 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' do - expect(subject.all_pipelines).to eq([pipeline3, pipeline2, pipeline]) + 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 From bcb3937da8fbba5b6a83ba092079444d993a5696 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Tue, 16 Aug 2016 16:18:44 +0200 Subject: [PATCH 08/12] Update fixtures to make development testing easier --- db/fixtures/development/14_builds.rb | 32 ++++++++++++++++++++++------ 1 file changed, 26 insertions(+), 6 deletions(-) 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) From f0d6b1ff83bd51124411c58692c20781733bb51a Mon Sep 17 00:00:00 2001 From: Annabel Dunstone Date: Tue, 16 Aug 2016 13:00:28 -0500 Subject: [PATCH 09/12] Hide branch name and status text on mr pipelines; don't use shorter timeago --- app/assets/stylesheets/pages/pipelines.scss | 16 +++++++++++++++ .../projects/ci/pipelines/_pipeline.html.haml | 20 ++++++++++--------- .../projects/commit/_pipelines_list.haml | 2 +- 3 files changed, 28 insertions(+), 10 deletions(-) diff --git a/app/assets/stylesheets/pages/pipelines.scss b/app/assets/stylesheets/pages/pipelines.scss index 21919fe4d73..08651974892 100644 --- a/app/assets/stylesheets/pages/pipelines.scss +++ b/app/assets/stylesheets/pages/pipelines.scss @@ -229,3 +229,19 @@ box-shadow: none; } } + +.pipelines.tab-pane { + + .content-list.pipelines { + width: auto; + } + + .table.builds { + min-width: 900px; + } + + .stage { + max-width: 60px; + width: 60px; + } +} 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 index 03db765180a..29f4ef8f49e 100644 --- a/app/views/projects/commit/_pipelines_list.haml +++ b/app/views/projects/commit/_pipelines_list.haml @@ -14,4 +14,4 @@ = stage.titleize %th %th - = render pipelines, commit_sha: true, stage: true, allow_retry: true, stages: pipelines.stages + = render pipelines, commit_sha: true, stage: true, allow_retry: true, stages: pipelines.stages, status_icon_only: true, hide_branch: true From dc42d52a17bef5bdaa91f5d0e4f213664d23af49 Mon Sep 17 00:00:00 2001 From: Annabel Dunstone Date: Tue, 16 Aug 2016 15:23:19 -0500 Subject: [PATCH 10/12] Add pipelines badge to MR tab --- app/assets/stylesheets/pages/pipelines.scss | 6 +----- app/views/projects/merge_requests/_show.html.haml | 2 +- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/app/assets/stylesheets/pages/pipelines.scss b/app/assets/stylesheets/pages/pipelines.scss index 08651974892..50ac4d8449b 100644 --- a/app/assets/stylesheets/pages/pipelines.scss +++ b/app/assets/stylesheets/pages/pipelines.scss @@ -233,11 +233,7 @@ .pipelines.tab-pane { .content-list.pipelines { - width: auto; - } - - .table.builds { - min-width: 900px; + overflow: scroll; } .stage { diff --git a/app/views/projects/merge_requests/_show.html.haml b/app/views/projects/merge_requests/_show.html.haml index d97c36cf63b..a1313064725 100644 --- a/app/views/projects/merge_requests/_show.html.haml +++ b/app/views/projects/merge_requests/_show.html.haml @@ -56,7 +56,7 @@ %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 + %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 Builds From e5f9fdf90c1d59a07f76919e9d94599b494ac50f Mon Sep 17 00:00:00 2001 From: Annabel Dunstone Date: Tue, 16 Aug 2016 20:34:49 -0500 Subject: [PATCH 11/12] Refactor merge_request-tabs --- app/assets/javascripts/merge_request_tabs.js | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/app/assets/javascripts/merge_request_tabs.js b/app/assets/javascripts/merge_request_tabs.js index a21cdbc8a10..1bba69a255a 100644 --- a/app/assets/javascripts/merge_request_tabs.js +++ b/app/assets/javascripts/merge_request_tabs.js @@ -188,14 +188,12 @@ } return this._get({ url: source + ".json", - success: (function(_this) { - return function(data) { - document.querySelector("div#pipelines").innerHTML = data.html; - gl.utils.localTimeAgo($('.js-timeago', 'div#pipelines')); - _this.pipelinesLoaded = true; - return _this.scrollToElement("#pipelines"); - }; - })(this) + success: function(data) { + $('#pipelines').html(data.html); + gl.utils.localTimeAgo($('.js-timeago', '#pipelines')); + this.pipelinesLoaded = true; + return this.scrollToElement("#pipelines"); + }.bind(this) }); }; From 62e2989bb77ea91f3e0acc81d8720eeaebb6bfd4 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Wed, 17 Aug 2016 14:49:16 +0100 Subject: [PATCH 12/12] Use have_no_link to test presence of button --- spec/features/merge_requests/pipelines_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/features/merge_requests/pipelines_spec.rb b/spec/features/merge_requests/pipelines_spec.rb index eeb898f083e..9c4c0525267 100644 --- a/spec/features/merge_requests/pipelines_spec.rb +++ b/spec/features/merge_requests/pipelines_spec.rb @@ -41,7 +41,7 @@ feature 'Pipelines for Merge Requests', feature: true, js: true do scenario 'user visits merge request page' do page.within('.merge-request-tabs') do - expect(page).not_to have_link('Pipelines') + expect(page).to have_no_link('Pipelines') end end end