CI details cleanup

- Add page titles to CI settings.
- Fix CI admin navigation.
- Remove duplicated scope.
- Use monospace font for commit sha.
- Add page title and header title to build page.
- Proper authorization for cancel/retry builds.
- Use gitlab pagination theme for builds and group members.
- Don't paginate builds widget on build page.
- Add badges to commit page Changes/Builds tabs.
- Add "Builds" to commit Builds tab page title.
- Add and use Ci::Build#retryable? method.
- Add CI::Build#retried? method.
- Allow all failed commit builds to be retried.
- Proper authorization for cancel/retry all builds.
- Remove unused param.
- Use time_ago_with_tooltip where appropriate.
- Tweak builds index text
- Remove duplication between builds/build and commit_statuses/commit_status.
- Use POST rather than GET for canceling and retrying builds.
- Remove redundant URL helpers.
- Add build ID to build page.
- Link branch name on build page.
- Move commit/:sha/ci to commit/:sha/builds.

app/controllers/projects/builds_controller.rb

@@ -30,7 +30,7 @@ class Projects::BuildsController < Projects::ApplicationController
   def show
     @builds = @ci_project.commits.find_by_sha(@build.sha).builds.order('id DESC')
-    @builds = @builds.where("id not in (?)", @build.id).page(params[:page]).per(20)
+    @builds = @builds.where("id not in (?)", @build.id)
     @commit = @build.commit
     respond_to do |format|
@@ -42,17 +42,13 @@ class Projects::BuildsController < Projects::ApplicationController
   end
   def retry
-    if @build.commands.blank?
+    unless @build.retryable?
       return page_404
     end
     build = Ci::Build.retry(@build)
-    if params[:return_to]
-      redirect_to URI.parse(params[:return_to]).path
-    else
-      redirect_to build_path(build)
-    end
+    redirect_to build_path(build)
   end
   def status

app/controllers/projects/commit_controller.rb

@@ -7,14 +7,14 @@ class Projects::CommitController < Projects::ApplicationController
   before_action :authorize_download_code!, except: [:cancel_builds]
   before_action :authorize_manage_builds!, only: [:cancel_builds]
   before_action :commit
+  before_action :authorize_manage_builds!, only: [:cancel_builds, :retry_builds]
+  before_action :define_show_vars, only: [:show, :builds]
   def show
     return git_not_found! unless @commit
     @line_notes = commit.notes.inline
-    @diffs = @commit.diffs
     @note = @project.build_commit_note(commit)
-    @notes_count = commit.notes.count
     @notes = commit.notes.not_inline.fresh
     @noteable = @commit
     @comments_allowed = @reply_allowed = true
@@ -23,8 +23,6 @@ class Projects::CommitController < Projects::ApplicationController
       commit_id: @commit.id
     }
-    @ci_commit = project.ci_commit(commit.sha)
-
     respond_to do |format|
       format.html
       format.diff  { render text: @commit.to_diff }
@@ -32,20 +30,25 @@ class Projects::CommitController < Projects::ApplicationController
     end
   end
-  def ci
-    @ci_commit = @project.ci_commit(@commit.sha)
-    @builds = @ci_commit.builds if @ci_commit
-    @notes_count = @commit.notes.count
+  def builds
     @ci_project = @project.gitlab_ci_project
   end
   def cancel_builds
-    @ci_commit = @project.ci_commit(@commit.sha)
-    @ci_commit.builds.running_or_pending.each(&:cancel)
-    redirect_to ci_namespace_project_commit_path(project.namespace, project, commit.sha)
+    ci_commit.builds.running_or_pending.each(&:cancel)
+    redirect_to builds_namespace_project_commit_path(project.namespace, project, commit.sha)
   end
+  def retry_builds
+    ci_commit.builds.latest.failed.each do |build|
+      if build.retryable?
+        Ci::Build.retry(build)
+      end
+    end
+
+    redirect_to builds_namespace_project_commit_path(project.namespace, project, commit.sha)
+  end
   def branches
     @branches = @project.repository.branch_names_contains(commit.id)
@@ -53,11 +56,22 @@ class Projects::CommitController < Projects::ApplicationController
     render layout: false
   end
+  private
+
   def commit
     @commit ||= @project.commit(params[:id])
   end
-  private
+  def ci_commit
+    @ci_commit ||= project.ci_commit(commit.sha)
+  end
+
+  def define_show_vars
+    @diffs = commit.diffs
+    @notes_count = commit.notes.count
+    
+    @builds = ci_commit.builds if ci_commit
+  end
   def authorize_manage_builds!
     unless can?(current_user, :manage_builds, project)

app/helpers/builds_helper.rb

-module BuildsHelper
-  def build_ref_link build
-    gitlab_ref_link build.project, build.ref
-  end
-
-  def build_commit_link build
-    gitlab_commit_link build.project, build.short_sha
-  end
-
-  def build_url(build)
-    namespace_project_build_path(build.gl_project, build.project, build)
-  end
-end

app/helpers/ci/gitlab_helper.rb

@@ -4,25 +4,6 @@ module Ci
       { :"data-no-turbolink" => "data-no-turbolink" }
     end
-    def gitlab_ref_link project, ref
-      gitlab_url = project.gitlab_url.dup
-      gitlab_url << "/commits/#{ref}"
-      link_to ref, gitlab_url, no_turbolink
-    end
-
-    def gitlab_compare_link project, before, after
-      gitlab_url = project.gitlab_url.dup
-      gitlab_url << "/compare/#{before}...#{after}"
-
-      link_to "#{before}...#{after}", gitlab_url, no_turbolink
-    end
-
-    def gitlab_commit_link project, sha
-      gitlab_url = project.gitlab_url.dup
-      gitlab_url << "/commit/#{sha}"
-      link_to Ci::Commit.truncate_sha(sha), gitlab_url, no_turbolink
-    end
-
     def yaml_web_editor_link(project)
       commits = project.commits

app/helpers/ci_status_helper.rb

 module CiStatusHelper
   def ci_status_path(ci_commit)
     project = ci_commit.gl_project
-    ci_namespace_project_commit_path(project.namespace, project, ci_commit.sha)
+    builds_namespace_project_commit_path(project.namespace, project, ci_commit.sha)
   end
   def ci_status_icon(ci_commit)

app/models/ci/build.rb

@@ -106,6 +106,14 @@ module Ci
       failed? && allow_failure?
     end
+    def retryable?
+      commands.present?
+    end
+
+    def retried?
+      !self.commit.latest_builds_for_ref(self.ref).include?(self)
+    end
+
     def trace_html
       html = Ci::Ansi2html::convert(trace) if trace.present?
       html || ''
@@ -222,7 +230,7 @@ module Ci
     end
     def retry_url
-      if commands.present?
+      if retryable?
         Gitlab::Application.routes.url_helpers.
           retry_namespace_project_build_path(gl_project.namespace, gl_project, self)
       end

app/models/commit_status.rb

@@ -15,8 +15,8 @@ class CommitStatus < ActiveRecord::Base
   scope :pending, -> { where(status: 'pending') }
   scope :success, -> { where(status: 'success') }
   scope :failed, -> { where(status: 'failed')  }
-  scope :running_or_pending, -> { where(status:[:running, :pending]) }
-  scope :finished, -> { where(status:[:success, :failed, :canceled]) }
+  scope :running_or_pending, -> { where(status: [:running, :pending]) }
+  scope :finished, -> { where(status: [:success, :failed, :canceled]) }
   scope :latest, -> { where(id: unscope(:select).select('max(id)').group(:name, :ref)) }
   scope :ordered, -> { order(:ref, :stage_idx, :name) }
   scope :for_ref, ->(ref) { where(ref: ref) }

app/models/project_services/ci/hip_chat_message.rb

@@ -11,7 +11,7 @@ module Ci
     def to_s
       lines = Array.new
       lines.push("<a href=\"#{ci_project_url(project)}\">#{project.name}</a> - ")
-      lines.push("<a href=\"#{ci_namespace_project_commit_url(commit.gl_project.namespace, commit.gl_project, commit.sha)}\">Commit ##{commit.id}</a></br>")
+      lines.push("<a href=\"#{builds_namespace_project_commit_url(commit.gl_project.namespace, commit.gl_project, commit.sha)}\">Commit ##{commit.id}</a></br>")
       lines.push("#{commit.short_sha} #{commit.git_author_name} - #{commit.git_commit_message}</br>")
       lines.push("#{humanized_status(commit_status)} in #{commit.duration} second(s).")
       lines.join('')

app/models/project_services/ci/slack_message.rb

@@ -45,7 +45,7 @@ module Ci
     def attachment_message
       out = "<#{ci_project_url(project)}|#{project_name}>: "
-      out << "Commit <#{ci_namespace_project_commit_url(commit.gl_project.namespace, commit.gl_project, commit.sha)}|\##{commit.id}> "
+      out << "Commit <#{builds_namespace_project_commit_url(commit.gl_project.namespace, commit.gl_project, commit.sha)}|\##{commit.id}> "
       out << "(<#{commit_sha_link}|#{commit.short_sha}>) "
       out << "of <#{commit_ref_link}|#{commit.ref}> "
       out << "by #{commit.git_author_name} " if commit.git_author_name

app/models/project_services/gitlab_ci_service.rb

@@ -71,7 +71,7 @@ class GitlabCiService < CiService
   def build_page(sha, ref)
     if project.gitlab_ci_project.present?
-      ci_namespace_project_commit_url(project.namespace, project, sha)
+      builds_namespace_project_commit_url(project.namespace, project, sha)
     end
   end

app/views/ci/notify/build_fail_email.html.haml

@@ -7,7 +7,7 @@
     = @project.name
 %p
-  Commit link: #{gitlab_commit_link(@project, @build.commit.short_sha)}
+  Commit: #{link_to @build.short_sha, namespace_project_commit_path(@build.gl_project.namespace, @build.gl_project, @build.sha)}
 %p
   Author: #{@build.commit.git_author_name}
 %p
@@ -16,4 +16,4 @@
   Message: #{@build.commit.git_commit_message}
 %p
-  Url: #{link_to @build.short_sha, namespace_project_build_url(@build.gl_project.namespace, @build.gl_project, @build)}
+  Build details: #{link_to "Build #{@build.id}", namespace_project_build_url(@build.gl_project.namespace, @build.gl_project, @build)}

app/views/ci/notify/build_success_email.html.haml

@@ -8,7 +8,7 @@
     = @project.name
 %p
-  Commit link: #{gitlab_commit_link(@project, @build.commit.short_sha)}
+  Commit: #{link_to @build.short_sha, namespace_project_commit_path(@build.gl_project.namespace, @build.gl_project, @build.sha)}
 %p
   Author: #{@build.commit.git_author_name}
 %p
@@ -17,4 +17,4 @@
   Message: #{@build.commit.git_commit_message}
 %p
-  Url: #{link_to @build.short_sha, namespace_project_build_url(@build.gl_project.namespace, @build.gl_project, @build)}
+  Build details: #{link_to "Build #{@build.id}", namespace_project_build_url(@build.gl_project.namespace, @build.gl_project, @build)}

app/views/dashboard/groups/index.html.haml

@@ -16,4 +16,4 @@
     - group = group_member.group
     = render 'shared/groups/group', group: group, group_member: group_member
-= paginate @group_members
+= paginate @group_members, theme: 'gitlab'

app/views/layouts/ci/_nav_admin.html.haml

@@ -9,23 +9,25 @@
   = nav_link path: 'projects#index' do
     = link_to ci_admin_projects_path do
       = icon('list-alt fw')
-      Projects
+      %span
+        Projects
   = nav_link path: 'events#index' do
     = link_to ci_admin_events_path do
       = icon('book fw')
-      Events
+      %span
+        Events
   = nav_link path: ['runners#index', 'runners#show'] do
     = link_to ci_admin_runners_path do
       = icon('cog fw')
-      Runners
-      %small.pull-right
-        = Ci::Runner.count(:all)
+      %span
+        Runners
+        %span.count= Ci::Runner.count(:all)
   = nav_link path: 'builds#index' do
     = link_to ci_admin_builds_path do
       = icon('link fw')
-      Builds
-      %small.pull-right
-        = Ci::Build.count(:all)
+      %span
+        Builds
+        %span.count= Ci::Build.count(:all)
   = nav_link(controller: :application_settings, html_options: { class: 'separate-item'}) do
     = link_to ci_admin_application_settings_path do
       = icon('cogs fw')

app/views/projects/_last_commit.html.haml

@@ -6,7 +6,7 @@
       = ci_commit.status
   = link_to commit.short_id, namespace_project_commit_path(project.namespace, project, commit), class: "commit_short_id"
-  = link_to_gfm commit.title, namespace_project_commit_path(project.namespace, project, commit.id), class: "commit-row-message"
+  = link_to_gfm commit.title, namespace_project_commit_path(project.namespace, project, commit), class: "commit-row-message"
   ·
   #{time_ago_with_tooltip(commit.committed_date, skip_js: true)} by
   = commit_author_link(commit, avatar: true, size: 24)

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

-%tr.build
-  %td.status
-    = ci_status_with_icon(build.status)
-
-  %td.commit_status-link
-    - if build.target_url
-      = link_to build.target_url do
-        %strong Build ##{build.id}
-    - else
-      %strong Build ##{build.id}
-
-    - if build.show_warning?
-      %i.fa.fa-warning.text-warning
-
-  %td
-    = link_to build.short_sha, namespace_project_commit_path(@project.namespace, @project, build.sha)
-
-  %td
-    = link_to build.ref, namespace_project_commits_path(@project.namespace, @project, build.ref)
-
-  %td
-    - if build.runner
-      = runner_link(build.runner)
-    - else
-      .light none
-
-  %td
-    = build.name
-
-    .pull-right
-      - if build.tags.any?
-        - build.tags.each do |tag|
-          %span.label.label-primary
-            = tag
-      - if build.trigger_request
-        %span.label.label-info triggered
-      - if build.allow_failure
-        %span.label.label-danger allowed to fail
-
-  %td.duration
-    - if build.duration
-      #{duration_in_words(build.finished_at, build.started_at)}
-
-  %td.timestamp
-    - if build.finished_at
-      %span #{time_ago_in_words build.finished_at} ago
-
-  %td
-    .pull-right
-      - if current_user && can?(current_user, :manage_builds, @project)
-        - if build.cancel_url
-          = link_to build.cancel_url, title: 'Cancel' do
-            %i.fa.fa-remove.cred

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

+- header_title project_title(@project, "Builds", project_builds_path(@project))

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

 - page_title "Builds"
-- header_title project_title(@project, "Builds", project_builds_path(@project))
+= render "header_title"
 .project-issuable-filter
   .controls
     - if @ci_project && current_user && can?(current_user, :manage_builds, @project)
       .pull-left.hidden-xs
         - if @all_builds.running_or_pending.any?
-          = link_to 'Cancel all', cancel_all_namespace_project_builds_path(@project.namespace, @project), data: { confirm: 'Are you sure?' }, class: 'btn btn-danger'
+          = link_to 'Cancel all', cancel_all_namespace_project_builds_path(@project.namespace, @project), data: { confirm: 'Are you sure?' }, class: 'btn btn-danger', method: :post
   %ul.center-top-menu
     %li{class: ('active' if @scope.nil?)}
@@ -25,7 +25,7 @@
         %span.badge.js-totalbuilds-count= @all_builds.count(:id)
 .gray-content-block
-  List of #{@scope || 'running'} builds from this project
+  #{(@scope || 'running').capitalize} builds from this project
 %ul.content-list
   - if @builds.blank?
@@ -40,14 +40,14 @@
             %th Build ID
             %th Commit
             %th Ref
-            %th Runner
+            %th Stage
             %th Name
             %th Duration
             %th Finished at
             %th
         - @builds.each do |build|
-          = render 'projects/builds/build', build: build
-    = paginate @builds
+          = render 'projects/commit_statuses/commit_status', commit_status: build, commit_sha: true, stage: true, allow_retry: true
+    = paginate @builds, theme: 'gitlab'

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

+- page_title "#{@build.name} (#{@build.id})", "Builds"
+= render "header_title"
+
 .build-page
   .gray-content-block
-    Build for commit
+    Build ##{@build.id} for commit
     %strong.monospace
       = link_to @build.commit.short_sha, ci_status_path(@build.commit)
     from
-    %code #{@build.ref}
+    = link_to @build.ref, namespace_project_commits_path(@project.namespace, @project, @build.ref)
   #up-build-trace
   - if @commit.matrix_for_ref?(@build.ref)
@@ -20,7 +23,7 @@
                 = build.id
-      - unless @commit.latest_builds_for_ref(@build.ref).include?(@build)
+      - if @build.retried?
         %li.active
           %a
             Build ##{@build.id}
@@ -37,7 +40,7 @@
             %i.fa.fa-time
             #{duration_in_words(@build.finished_at, @build.started_at)}
         .pull-right
-          = @build.updated_at.stamp('19:00 Aug 27')
+          #{time_ago_with_tooltip(@build.finished_at) if @build.finished_at}
   - if @build.show_warning?
     - unless @build.any_runners_online?
@@ -87,13 +90,13 @@
       .build-widget
         %h4.title
-          Build
+          Build ##{@build.id}
           - if current_user && can?(current_user, :manage_builds, @project)
             .pull-right
-              - if @build.active?
-                = link_to "Cancel", cancel_namespace_project_build_path(@project.namespace, @project, @build), class: 'btn btn-sm btn-danger'
-              - elsif @build.commands.present?
-                = link_to "Retry", retry_namespace_project_build_path(@project.namespace, @project, @build), class: 'btn btn-sm btn-primary', method: :post
+              - if @build.cancel_url
+                = link_to "Cancel", @build.cancel_url, class: 'btn btn-sm btn-danger', method: :post
+              - elsif @build.retry_url
+                = link_to "Retry", @build.retry_url, class: 'btn btn-sm btn-primary', method: :post
         - if @build.duration
           %p
@@ -101,15 +104,15 @@
             #{duration_in_words(@build.finished_at, @build.started_at)}
         %p
           %span.attr-name Created:
-          #{time_ago_in_words(@build.created_at)} ago
+          #{time_ago_with_tooltip(@build.created_at)}
         - if @build.finished_at
           %p
             %span.attr-name Finished:
-            #{time_ago_in_words(@build.finished_at)} ago
+            #{time_ago_with_tooltip(@build.finished_at)}
         %p
           %span.attr-name Runner:
           - if @build.runner && current_user && current_user.admin
-            \#{link_to "##{@build.runner.id}", ci_admin_runner_path(@build.runner.id)}
+            = link_to "##{@build.runner.id}", ci_admin_runner_path(@build.runner.id)
           - elsif @build.runner
             \##{@build.runner.id}
@@ -134,10 +137,11 @@
         %h4.title
           Commit
           .pull-right
-            %small #{build_commit_link @build}
+            %small 
+              = link_to @build.commit.short_sha, ci_status_path(@build.commit), class: "monospace"
         %p
           %span.attr-name Branch:
-          #{build_ref_link @build}
+          = link_to @build.ref, namespace_project_commits_path(@project.namespace, @project, @build.ref)
         %p
           %span.attr-name Author:
           #{@build.commit.git_author_name}
@@ -155,7 +159,9 @@
       - if @builds.present?
         .build-widget
-          %h4.title #{pluralize(@builds.count(:id), "other build")} for #{@build.short_sha}:
+          %h4.title #{pluralize(@builds.count(:id), "other build")} for 
+          = succeed ":" do
+            = link_to @build.commit.short_sha, ci_status_path(@build.commit), class: "monospace"
           %table.table.builds
             - @builds.each_with_index do |build, i|
               %tr.build
@@ -171,8 +177,5 @@
                 %td.status= build.status
-          = paginate @builds
-
-
   :javascript
     new CiBuild("#{namespace_project_build_url(@project.namespace, @project, @build)}", "#{@build.status}")

app/views/projects/ci_services/edit.html.haml

+- page_title @service.title, "CI Services"
 = render 'form'