Merge branch 'mr-builds' into 'master'

Add Builds tab to MR detail page

## Before

![Screen_Shot_2015-12-08_at_13.21.04](/uploads/0d26060bef78f3de076e33439b8a4789/Screen_Shot_2015-12-08_at_13.21.04.png)

## After

![Screen_Shot_2015-12-08_at_13.25.20](/uploads/be7d7fd6d4fc8c5cc366b5b3778b813a/Screen_Shot_2015-12-08_at_13.25.20.png)

Fixes #3907

See merge request !2016

app/assets/javascripts/merge_request_tabs.js.coffee

@@ -43,6 +43,7 @@
 #
 class @MergeRequestTabs
   diffsLoaded: false
+  buildsLoaded: false
   commitsLoaded: false
 
   constructor: (@opts = {}) ->
@@ -54,6 +55,12 @@ class @MergeRequestTabs
 
   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)
@@ -63,6 +70,8 @@ class @MergeRequestTabs
       @loadCommits($target.attr('href'))
     else if action == 'diffs'
       @loadDiff($target.attr('href'))
+    else if action == 'builds'
+      @loadBuilds($target.attr('href'))
 
     @setCurrentAction(action)
 
@@ -101,7 +110,7 @@ class @MergeRequestTabs
     action = 'notes' if action == 'show'
 
     # Remove a trailing '/commits' or '/diffs'
-    new_state = @_location.pathname.replace(/\/(commits|diffs)(\.html)?\/?$/, '')
+    new_state = @_location.pathname.replace(/\/(commits|diffs|builds)(\.html)?\/?$/, '')
 
     # Append the new action if we're on a tab other than 'notes'
     unless action == 'notes'
@@ -139,6 +148,17 @@ class @MergeRequestTabs
         @diffsLoaded = true
         @scrollToElement("#diffs")
 
+  loadBuilds: (source) ->
+    return if @buildsLoaded
+
+    @_get
+      url: "#{source}.json"
+      success: (data) =>
+        document.getElementById('builds').innerHTML = data.html
+        $('.js-timeago').timeago()
+        @buildsLoaded = true
+        @scrollToElement("#builds")
+
   # Show or hide the loading spinner
   #
   # status - Boolean, true to show, false to hide

app/assets/stylesheets/pages/merge_requests.scss

@@ -81,6 +81,10 @@
     &.ci-error {
       color: $gl-danger;
     }
+
+    a.monospace {
+      color: inherit;
+    }
   }
 
   .mr-widget-body,

app/controllers/projects/commit_controller.rb

@@ -37,7 +37,7 @@ class Projects::CommitController < Projects::ApplicationController
   def cancel_builds
     ci_commit.builds.running_or_pending.each(&:cancel)
 
-    redirect_to builds_namespace_project_commit_path(project.namespace, project, commit.sha)
+    redirect_back_or_default default: builds_namespace_project_commit_path(project.namespace, project, commit.sha)
   end
 
   def retry_builds
@@ -47,7 +47,7 @@ class Projects::CommitController < Projects::ApplicationController
       end
     end
 
-    redirect_to builds_namespace_project_commit_path(project.namespace, project, commit.sha)
+    redirect_back_or_default default: builds_namespace_project_commit_path(project.namespace, project, commit.sha)
   end
 
   def branches
@@ -74,8 +74,8 @@ class Projects::CommitController < Projects::ApplicationController
     end
 
     @notes_count = commit.notes.count
-    
-    @builds = ci_commit.builds if ci_commit
+
+    @statuses = ci_commit.statuses if ci_commit
   end
 
   def authorize_manage_builds!

app/controllers/projects/merge_requests_controller.rb

 class Projects::MergeRequestsController < Projects::ApplicationController
   before_action :module_enabled
   before_action :merge_request, only: [
-    :edit, :update, :show, :diffs, :commits, :merge, :merge_check,
+    :edit, :update, :show, :diffs, :commits, :builds, :merge, :merge_check,
     :ci_status, :toggle_subscription
   ]
-  before_action :closes_issues, only: [:edit, :update, :show, :diffs, :commits]
-  before_action :validates_merge_request, only: [:show, :diffs, :commits]
-  before_action :define_show_vars, only: [:show, :diffs, :commits]
-  before_action :ensure_ref_fetched, only: [:show, :commits, :diffs]
+  before_action :closes_issues, only: [:edit, :update, :show, :diffs, :commits, :builds]
+  before_action :validates_merge_request, only: [:show, :diffs, :commits, :builds]
+  before_action :define_show_vars, only: [:show, :diffs, :commits, :builds]
+  before_action :ensure_ref_fetched, only: [:show, :diffs, :commits, :builds]
 
   # Allow read any merge_request
   before_action :authorize_read_merge_request!
@@ -79,6 +79,15 @@ class Projects::MergeRequestsController < Projects::ApplicationController
     end
   end
 
+  def builds
+    @ci_project = @merge_request.source_project.gitlab_ci_project
+
+    respond_to do |format|
+      format.html { render 'show' }
+      format.json { render json: { html: view_to_html_string('projects/merge_requests/show/_builds') } }
+    end
+  end
+
   def new
     params[:merge_request] ||= ActionController::Parameters.new(source_project: @project)
     @merge_request = MergeRequests::BuildService.new(project, current_user, merge_request_params).execute
@@ -91,20 +100,19 @@ class Projects::MergeRequestsController < Projects::ApplicationController
 
     @target_project = merge_request.target_project
     @source_project = merge_request.source_project
-    @commits = @merge_request.compare_commits
+    @commits = @merge_request.compare_commits.reverse
     @commit = @merge_request.last_commit
     @first_commit = @merge_request.first_commit
     @diffs = @merge_request.compare_diffs
+
+    @ci_project = @source_project.gitlab_ci_project
+    @ci_commit = @merge_request.ci_commit
+    @statuses = @ci_commit.statuses if @ci_commit
+
     @note_counts = Note.where(commit_id: @commits.map(&:id)).
       group(:commit_id).count
   end
 
-  def edit
-    @source_project = @merge_request.source_project
-    @target_project = @merge_request.target_project
-    @target_branches = @merge_request.target_project.repository.branch_names
-  end
-
   def create
     @target_branches ||= []
     @merge_request = MergeRequests::CreateService.new(project, current_user, merge_request_params).execute
@@ -118,6 +126,12 @@ class Projects::MergeRequestsController < Projects::ApplicationController
     end
   end
 
+  def edit
+    @source_project = @merge_request.source_project
+    @target_project = @merge_request.target_project
+    @target_branches = @merge_request.target_project.repository.branch_names
+  end
+
   def update
     @merge_request = MergeRequests::UpdateService.new(project, current_user, merge_request_params).execute(@merge_request)
 
@@ -265,6 +279,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController
     @merge_request_diff = @merge_request.merge_request_diff
 
     @ci_commit = @merge_request.ci_commit
+    @statuses = @ci_commit.statuses if @ci_commit
 
     if @merge_request.locked_long_ago?
       @merge_request.unlock_mr

app/views/projects/commit/_builds.html.haml

+.gray-content-block.middle-block
+  .pull-right
+    - if @ci_project && can?(current_user, :manage_builds, @ci_commit.gl_project)
+      - if @ci_commit.builds.latest.failed.any?(&:retryable?)
+        = link_to "Retry failed", retry_builds_namespace_project_commit_path(@ci_commit.gl_project.namespace, @ci_commit.gl_project, @ci_commit.sha), class: 'btn btn-grouped btn-primary', method: :post
+
+      - if @ci_commit.builds.running_or_pending.any?
+        = link_to "Cancel running", cancel_builds_namespace_project_commit_path(@ci_commit.gl_project.namespace, @ci_commit.gl_project, @ci_commit.sha), data: { confirm: 'Are you sure?' }, class: 'btn btn-grouped btn-danger', method: :post
+
+  .oneline
+    = pluralize @statuses.count(:id), "build"
+    - if defined?(link_to_commit) && link_to_commit
+      for commit
+      = link_to @ci_commit.short_sha, namespace_project_commit_path(@ci_commit.gl_project.namespace, @ci_commit.gl_project, @ci_commit.sha), class: "monospace"
+    - if @ci_commit.duration > 0
+      in
+      = time_interval_in_words @ci_commit.duration
+
+- if @ci_commit.yaml_errors.present?
+  .bs-callout.bs-callout-danger
+    %h4 Found errors in your .gitlab-ci.yml:
+    %ul
+      - @ci_commit.yaml_errors.split(",").each do |error|
+        %li= error
+
+- if @ci_commit.gl_project.builds_enabled? && !@ci_commit.ci_yaml_file
+  .bs-callout.bs-callout-warning
+    \.gitlab-ci.yml not found in this commit
+
+.table-holder
+  %table.table.builds
+    %thead
+      %tr
+        %th Status
+        %th Build ID
+        %th Ref
+        %th Stage
+        %th Name
+        %th Duration
+        %th Finished at
+        - if @ci_project && @ci_project.coverage_enabled?
+          %th Coverage
+        %th
+    - @ci_commit.refs.each do |ref|
+      = render partial: "projects/commit_statuses/commit_status", collection: @ci_commit.statuses.for_ref(ref).latest.ordered,
+               locals: { coverage: @ci_project.try(:coverage_enabled?), stage: true, allow_retry: true }
+
+- if @ci_commit.retried.any?
+  .gray-content-block.second-block
+    Retried builds
+
+  .table-holder
+    %table.table.builds
+      %thead
+        %tr
+          %th Status
+          %th Build ID
+          %th Ref
+          %th Stage
+          %th Name
+          %th Duration
+          %th Finished at
+          - if @ci_project && @ci_project.coverage_enabled?
+            %th Coverage
+          %th
+      = render partial: "projects/commit_statuses/commit_status", collection: @ci_commit.retried,
+               locals: { coverage: @ci_project.try(:coverage_enabled?), stage: true }

app/views/projects/commit/_ci_menu.html.haml

@@ -6,4 +6,4 @@
   = nav_link(path: 'commit#builds') do
     = link_to builds_namespace_project_commit_path(@project.namespace, @project, @commit.id) do
       Builds
-      %span.badge= @builds.count(:id)
+      %span.badge= @statuses.count

app/views/projects/commit/builds.html.haml

@@ -3,70 +3,4 @@
 = render "commit_box"
 = render "ci_menu"
 
-
-- if @ci_commit.yaml_errors.present?
-  .bs-callout.bs-callout-danger
-    %h4 Found errors in your .gitlab-ci.yml:
-    %ul
-      - @ci_commit.yaml_errors.split(",").each do |error|
-        %li= error
-
-- unless @ci_commit.ci_yaml_file
-  .bs-callout.bs-callout-warning
-    \.gitlab-ci.yml not found in this commit
-
-.gray-content-block.second-block
-  Latest builds
-
-  .pull-right
-    - if @ci_commit.duration > 0
-      %i.fa.fa-time
-      #{time_interval_in_words @ci_commit.duration}
-
-     
-
-    - if @ci_project && current_user && can?(current_user, :manage_builds, @project)
-      - if @ci_commit.builds.latest.failed.any?(&:retryable?)
-        = link_to "Retry failed", retry_builds_namespace_project_commit_path(@project.namespace, @project, @commit.sha), class: 'btn btn-xs btn-primary', method: :post
-
-      - if @ci_commit.builds.running_or_pending.any?
-        = link_to "Cancel running", cancel_builds_namespace_project_commit_path(@project.namespace, @project, @commit.sha), class: 'btn btn-xs btn-danger', method: :post
-
-.table-holder
-  %table.table.builds
-    %thead
-      %tr
-        %th Status
-        %th Build ID
-        %th Ref
-        %th Stage
-        %th Name
-        %th Duration
-        %th Finished at
-        - if @ci_project && @ci_project.coverage_enabled?
-          %th Coverage
-        %th
-    - @ci_commit.refs.each do |ref|
-      = render partial: "projects/commit_statuses/commit_status", collection: @ci_commit.statuses.for_ref(ref).latest.ordered,
-               locals: { coverage: @ci_project.try(:coverage_enabled?), stage: true, allow_retry: true }
-
-- if @ci_commit.retried.any?
-  .gray-content-block.second-block
-    Retried builds
-
-  .table-holder
-    %table.table.builds
-      %thead
-        %tr
-          %th Status
-          %th Build ID
-          %th Ref
-          %th Stage
-          %th Name
-          %th Duration
-          %th Finished at
-          - if @ci_project && @ci_project.coverage_enabled?
-            %th Coverage
-          %th
-      = render partial: "projects/commit_statuses/commit_status", collection: @ci_commit.retried,
-               locals: { coverage: @ci_project.try(:coverage_enabled?), stage: true }
+= render "builds"

app/views/projects/merge_requests/_new_submit.html.haml

@@ -27,6 +27,11 @@
       = link_to url_for(params), data: {target: 'div#diffs', action: 'diffs', toggle: 'tab'} do
         Changes
         %span.badge= @diffs.size
+    - if @ci_commit
+      %li.builds-tab.active
+        = link_to url_for(params), data: {target: 'div#builds', action: 'builds', toggle: 'tab'} do
+          Builds
+          %span.badge= @statuses.size
 
   .tab-content
     #commits.commits.tab-pane
@@ -42,6 +47,9 @@
         .alert.alert-danger
           %h4 This comparison includes a huge diff.
           %p To preserve performance the line changes are not shown.
+    - if @ci_commit
+      #builds.builds.tab-pane
+        = render "projects/merge_requests/show/builds"
 
 :javascript
   $('.assign-to-me-link').on('click', function(e){

app/views/projects/merge_requests/_show.html.haml

@@ -26,8 +26,7 @@
               %li= link_to "Plain Diff",    merge_request_path(@merge_request, format: :diff)
       .normal
         %span Request to merge
-        %span.label-branch
-          = source_branch_with_namespace(@merge_request)
+        %span.label-branch= source_branch_with_namespace(@merge_request)
         %span into
         = link_to namespace_project_commits_path(@project.namespace, @project, @merge_request.target_branch), class: "label-branch" do
           = @merge_request.target_branch
@@ -55,6 +54,11 @@
         = 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.diffs.size
+      - if @ci_commit
+        %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
+            %span.badge= @statuses.size
 
   .tab-content
     #notes.notes.tab-pane.voting_notes
@@ -63,6 +67,8 @@
       - # This tab is always loaded via AJAX
     #diffs.diffs.tab-pane
       - # This tab is always loaded via AJAX
+    #builds.builds.tab-pane
+      - # This tab is always loaded via AJAX
 
   .mr-loading-status
     = spinner

app/views/projects/merge_requests/show/_builds.html.haml

+= render "projects/commit/builds", link_to_commit: true

app/views/projects/merge_requests/widget/_heading.html.haml

 - if @ci_commit
-  - status = @ci_commit.status
   .mr-widget-heading
-    .ci_widget{class: "ci-#{status}"}
+    .ci_widget{class: "ci-#{@ci_commit.status}"}
       = ci_status_icon(@ci_commit)
-      %span CI build #{status}
-      for #{@merge_request.last_commit_short_sha}.
+      %span
+        Build
+        = ci_status_label(@ci_commit)
+      for
+      = succeed "." do
+        = link_to @ci_commit.short_sha, namespace_project_commit_path(@merge_request.source_project.namespace, @merge_request.source_project, @ci_commit.sha), class: "monospace"
       %span.ci-coverage
-      = link_to "View build details", ci_status_path(@ci_commit)
+      = link_to "View details", builds_namespace_project_merge_request_path(@project.namespace, @project, @merge_request), class: "js-show-tab", data: {action: 'builds'}
 
 - elsif @merge_request.has_ci?
   - # Compatibility with old CI integrations (ex jenkins) when you request status from CI server via AJAX
   - # Remove in later versions when services like Jenkins will set CI status via Commit status API
   .mr-widget-heading
-    - [:success, :skipped, :canceled, :failed, :running, :pending].each do |status|
+    - %w[success skipped canceled failed running pending].each do |status|
       .ci_widget{class: "ci-#{status}", style: "display:none"}
-        - if status == :success
-          - status = "passed"
-          = icon("check-circle")
-        - else
-          = icon("circle")
-        %span CI build #{status}
-        for #{@merge_request.last_commit_short_sha}.
+        = ci_icon_for_status(status)
+        %span
+          CI build
+          = ci_label_for_status(status)
+        for
+        - commit = @merge_request.last_commit
+        = succeed "." do
+          = link_to commit.short_id, namespace_project_commit_path(@merge_request.source_project.namespace, @merge_request.source_project, commit), class: "monospace"
         %span.ci-coverage
-        - if ci_build_details_path(@merge_request)
-          = link_to "View build details", ci_build_details_path(@merge_request), :"data-no-turbolink" => "data-no-turbolink"
+        - if details_path = ci_build_details_path(@merge_request)
+          = link_to "View details", details_path, :"data-no-turbolink" => "data-no-turbolink"
 
     .ci_widget
       = icon("spinner spin")

config/routes.rb

@@ -570,8 +570,9 @@ Rails.application.routes.draw do
 
         resources :merge_requests, constraints: { id: /\d+/ }, except: [:destroy] do
           member do
-            get :diffs
             get :commits
+            get :diffs
+            get :builds
             post :merge
             get :merge_check
             get :ci_status

features/steps/project/commits/commits.rb

@@ -118,6 +118,6 @@ class Spinach::Features::ProjectCommits < Spinach::FeatureSteps
 
   step 'I see builds list' do
     expect(page).to have_content "build: pending"
-    expect(page).to have_content "Latest builds"
+    expect(page).to have_content "1 build"
   end
 end

lib/gitlab/markdown/commit_reference_filter.rb

@@ -47,6 +47,17 @@ module Gitlab
       def object_link_title(commit)
         commit.link_title
       end
+
+      def object_link_text_extras(object, matches)
+        extras = super
+
+        path = matches[:path] if matches.names.include?("path")
+        if path == '/builds'
+          extras.unshift "builds"
+        end
+
+        extras
+      end
     end
   end
 end

lib/gitlab/markdown/merge_request_reference_filter.rb

@@ -24,8 +24,14 @@ module Gitlab
       def object_link_text_extras(object, matches)
         extras = super
 
-        if matches.names.include?("path") && matches[:path] && matches[:path] == '/diffs'
+        path = matches[:path] if matches.names.include?("path")
+        case path
+        when '/diffs'
           extras.unshift "diffs"
+        when '/commits'
+          extras.unshift "commits"
+        when '/builds'
+          extras.unshift "builds"
         end
 
         extras