Merge branch 'fix/revert-removing-build-tab-again' into '8-16-stable'

app/assets/javascripts/dispatcher.js.es6

@@ -152,6 +152,7 @@
           new MergedButtons();
           break;
         case 'projects:merge_requests:commits':
+        case 'projects:merge_requests:builds':
           new MergedButtons();
           break;
         case "projects:merge_requests:diffs":
@@ -176,6 +177,9 @@
             container: '.js-pipeline-table',
           });
           break;
+        case 'projects:commit:builds':
+          new gl.Pipelines();
+          break;
         case 'projects:commits:show':
         case 'projects:activity':
           shortcut_handler = new ShortcutsNavigation();

app/assets/javascripts/merge_request_tabs.js.es6

@@ -59,13 +59,16 @@
 
   class MergeRequestTabs {
 
-    constructor({ action, setUrl, stubLocation } = {}) {
+    constructor({ action, setUrl, buildsLoaded, stubLocation } = {}) {
       this.diffsLoaded = false;
+      this.buildsLoaded = false;
       this.pipelinesLoaded = false;
       this.commitsLoaded = false;
       this.fixedLayoutPref = null;
 
       this.setUrl = setUrl !== undefined ? setUrl : true;
+      this.buildsLoaded = buildsLoaded || false;
+
       this.setCurrentAction = this.setCurrentAction.bind(this);
       this.tabShown = this.tabShown.bind(this);
       this.showTab = this.showTab.bind(this);
@@ -116,6 +119,10 @@
         $.scrollTo('.merge-request-details .merge-request-tabs', {
           offset: -navBarHeight,
         });
+      } else if (action === 'builds') {
+        this.loadBuilds($target.attr('href'));
+        this.expandView();
+        this.resetViewContainer();
       } else if (action === 'pipelines') {
         this.loadPipelines($target.attr('href'));
         this.expandView();
@@ -173,8 +180,8 @@
     setCurrentAction(action) {
       this.currentAction = action === 'show' ? 'notes' : action;
 
-      // Remove a trailing '/commits' '/diffs' '/pipelines' '/new' '/new/diffs'
-      let newState = location.pathname.replace(/\/(commits|diffs|pipelines|new|new\/diffs)(\.html)?\/?$/, '');
+      // Remove a trailing '/commits' '/diffs' '/builds' '/pipelines' '/new' '/new/diffs'
+      let newState = location.pathname.replace(/\/(commits|diffs|builds|pipelines|new|new\/diffs)(\.html)?\/?$/, '');
 
       // Append the new action if we're on a tab other than 'notes'
       if (this.currentAction !== 'notes') {
@@ -243,6 +250,22 @@
       });
     }
 
+    loadBuilds(source) {
+      if (this.buildsLoaded) {
+        return;
+      }
+      this.ajaxGet({
+        url: `${source}.json`,
+        success: (data) => {
+          document.querySelector('div#builds').innerHTML = data.html;
+          gl.utils.localTimeAgo($('.js-timeago', 'div#builds'));
+          this.buildsLoaded = true;
+          new gl.Pipelines();
+          this.scrollToElement('#builds');
+        },
+      });
+    }
+
     loadPipelines(source) {
       if (this.pipelinesLoaded) {
         return;

app/assets/javascripts/merge_request_widget.js.es6

@@ -74,7 +74,7 @@
 
     MergeRequestWidget.prototype.addEventListeners = function() {
       var allowedPages;
-      allowedPages = ['show', 'commits', 'pipelines', 'changes'];
+      allowedPages = ['show', 'commits', 'builds', 'pipelines', 'changes'];
       $(document).on('page:change.merge_request', (function(_this) {
         return function() {
           var page;
@@ -173,6 +173,7 @@
               message = message.replace('{{title}}', data.title);
               notify(title, message, _this.opts.gitlab_icon, function() {
                 this.close();
+                return Turbolinks.visit(_this.opts.builds_path);
               });
             }
           }

app/controllers/projects/commit_controller.rb

@@ -8,11 +8,13 @@ class Projects::CommitController < Projects::ApplicationController
 
   # Authorize
   before_action :require_non_empty_project
-  before_action :authorize_download_code!
+  before_action :authorize_download_code!, except: [:cancel_builds, :retry_builds]
+  before_action :authorize_update_build!, only: [:cancel_builds, :retry_builds]
   before_action :authorize_read_pipeline!, only: [:pipelines]
+  before_action :authorize_read_commit_status!, only: [:builds]
   before_action :commit
-  before_action :define_commit_vars, only: [:show, :diff_for_path, :pipelines]
-  before_action :define_status_vars, only: [:show, :pipelines]
+  before_action :define_commit_vars, only: [:show, :diff_for_path, :builds, :pipelines]
+  before_action :define_status_vars, only: [:show, :builds, :pipelines]
   before_action :define_note_vars, only: [:show, :diff_for_path]
   before_action :authorize_edit_tree!, only: [:revert, :cherry_pick]
 
@@ -33,6 +35,25 @@ class Projects::CommitController < Projects::ApplicationController
   def pipelines
   end
 
+  def builds
+  end
+
+  def cancel_builds
+    ci_builds.running_or_pending.each(&:cancel)
+
+    redirect_back_or_default default: builds_namespace_project_commit_path(project.namespace, project, commit.sha)
+  end
+
+  def retry_builds
+    ci_builds.latest.failed.each do |build|
+      if build.retryable?
+        Ci::Build.retry(build, current_user)
+      end
+    end
+
+    redirect_back_or_default default: builds_namespace_project_commit_path(project.namespace, project, commit.sha)
+  end
+
   def branches
     @branches = @project.repository.branch_names_contains(commit.id)
     @tags = @project.repository.tag_names_contains(commit.id)
@@ -77,6 +98,10 @@ class Projects::CommitController < Projects::ApplicationController
     @noteable = @commit ||= @project.commit(params[:id])
   end
 
+  def ci_builds
+    @ci_builds ||= Ci::Build.where(pipeline: pipelines)
+  end
+
   def define_commit_vars
     return git_not_found! unless commit
 
@@ -108,6 +133,8 @@ class Projects::CommitController < Projects::ApplicationController
 
   def define_status_vars
     @ci_pipelines = project.pipelines.where(sha: commit.sha)
+    @statuses = CommitStatus.where(pipeline: @ci_pipelines).relevant
+    @builds = Ci::Build.where(pipeline: @ci_pipelines).relevant
   end
 
   def assign_change_commit_vars(mr_source_branch)

app/controllers/projects/merge_requests_controller.rb

@@ -9,10 +9,10 @@ class Projects::MergeRequestsController < Projects::ApplicationController
 
   before_action :module_enabled
   before_action :merge_request, only: [
-    :edit, :update, :show, :diffs, :commits, :conflicts, :conflict_for_path, :pipelines, :merge, :merge_check,
+    :edit, :update, :show, :diffs, :commits, :conflicts, :conflict_for_path, :builds, :pipelines, :merge, :merge_check,
     :ci_status, :ci_environments_status, :toggle_subscription, :cancel_merge_when_build_succeeds, :remove_wip, :resolve_conflicts, :assign_related_issues
   ]
-  before_action :validates_merge_request, only: [:show, :diffs, :commits, :pipelines]
+  before_action :validates_merge_request, only: [:show, :diffs, :commits, :builds, :pipelines]
   before_action :define_show_vars, only: [:show, :diffs, :commits, :conflicts, :conflict_for_path, :builds, :pipelines]
   before_action :define_widget_vars, only: [:merge, :cancel_merge_when_build_succeeds, :merge_check]
   before_action :define_commit_vars, only: [:diffs]
@@ -205,6 +205,17 @@ class Projects::MergeRequestsController < Projects::ApplicationController
     end
   end
 
+  def builds
+    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/_builds') } }
+    end
+  end
+
   def pipelines
     @pipelines = @merge_request.all_pipelines
 

app/helpers/ci_status_helper.rb

 module CiStatusHelper
   def ci_status_path(pipeline)
     project = pipeline.project
-    namespace_project_pipeline_path(project.namespace, project, pipeline)
+    builds_namespace_project_commit_path(project.namespace, project, pipeline.sha)
   end
 
   # Is used by Commit and Merge Request Widget

app/views/projects/_last_commit.html.haml

-
 - ref = local_assigns.fetch(:ref)
 - status = commit.status(ref)
 - if status
-  = link_to pipelines_namespace_project_commit_path(commit.project.namespace, commit.project, commit), class: "ci-status ci-#{status}" do
+  = link_to builds_namespace_project_commit_path(commit.project.namespace, commit.project, commit), class: "ci-status ci-#{status}" do
     = ci_icon_for_status(status)
     = ci_label_for_status(status)
 

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

@@ -7,7 +7,7 @@
     = link_to pipeline_path(@build.pipeline) do
       %strong ##{@build.pipeline.id}
     for commit
-    = link_to namespace_project_commit_path(@project.namespace, @project, @build.pipeline.sha) do
+    = link_to ci_status_path(@build.pipeline) do
       %strong= @build.pipeline.short_sha
     from
     = link_to namespace_project_commits_path(@project.namespace, @project, @build.ref) do

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

+- @ci_pipelines.each do |pipeline|
+  = render "pipeline", pipeline: pipeline, pipeline_details: true

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

@@ -8,3 +8,7 @@
       = link_to pipelines_namespace_project_commit_path(@project.namespace, @project, @commit.id) do
         Pipelines
         %span.badge= @ci_pipelines.count
+  = nav_link(path: 'commit#builds') do
+    = link_to builds_namespace_project_commit_path(@project.namespace, @project, @commit.id) do
+      Builds
+      %span.badge= @statuses.count

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

+- @no_container = true
+- page_title "Builds", "#{@commit.title} (#{@commit.short_id})", "Commits"
+= render "projects/commits/head"
+
+%div{ class: container_class }
+  = render "commit_box"
+
+  = render "ci_menu"
+  = render "builds"

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

@@ -34,6 +34,11 @@
           = link_to url_for(params), data: {target: 'div#pipelines', action: 'pipelines', toggle: 'tab'} do
             Pipelines
             %span.badge= @pipelines.size
+      - if @pipeline.present?
+        %li.builds-tab
+          = link_to url_for(params), data: {target: 'div#builds', action: 'builds', toggle: 'tab'} do
+            Builds
+            %span.badge= @statuses_count
       %li.diffs-tab
         = link_to url_for(params.merge(action: 'new_diffs')), data: {target: 'div#diffs', action: 'new/diffs', toggle: 'tab'} do
           Changes
@@ -44,6 +49,9 @@
         = render "projects/merge_requests/show/commits"
       #diffs.diffs.tab-pane
         - # This tab is always loaded via AJAX
+      - if @pipeline.present?
+        #builds.builds.tab-pane
+          = render "projects/merge_requests/show/builds"
       - if @pipelines.any?
         #pipelines.pipelines.tab-pane
           = render "projects/merge_requests/show/pipelines"
@@ -58,5 +66,6 @@
   });
 :javascript
   var merge_request = new MergeRequest({
-    action: "#{(@show_changes_tab ? 'new/diffs' : 'new')}"
+    action: "#{(@show_changes_tab ? 'new/diffs' : 'new')}",
+    buildsLoaded: "#{@pipeline.present? ? 'true' : 'false'}"
   });

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

@@ -68,6 +68,11 @@
               = link_to pipelines_namespace_project_merge_request_path(@project.namespace, @project, @merge_request), data: { target: '#pipelines', action: 'pipelines', toggle: 'tab' } do
                 Pipelines
                 %span.badge= @pipelines.size
+          - if @pipeline.present?
+            %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_count
           %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
               Changes
@@ -95,6 +100,8 @@
         - # This tab is always loaded via AJAX
       #pipelines.pipelines.tab-pane
         - # This tab is always loaded via AJAX
+      #builds.builds.tab-pane
+        - # This tab is always loaded via AJAX
       #diffs.diffs.tab-pane
         - # This tab is always loaded via AJAX
 

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

+= render "projects/commit/pipeline", pipeline: @pipeline, link_to_commit: true

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

@@ -24,10 +24,12 @@
       preparing: "{{status}} build",
       normal: "Build {{status}}"
     },
+    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') {
+    clearInterval(merge_request_widget.fetchBuildStatusInterval);
     merge_request_widget.cancelPolling();
     merge_request_widget.clearEventListeners();
   }

config/routes/project.rb

@@ -32,7 +32,10 @@ constraints(ProjectUrlConstrainer.new) do
       resources :commit, only: [:show], constraints: { id: /\h{7,40}/ } do
         member do
           get :branches
+          get :builds
           get :pipelines
+          post :cancel_builds
+          post :retry_builds
           post :revert
           post :cherry_pick
           get :diff_for_path
@@ -91,6 +94,7 @@ constraints(ProjectUrlConstrainer.new) do
           get :diffs
           get :conflicts
           get :conflict_for_path
+          get :builds
           get :pipelines
           get :merge_check
           post :merge

features/project/commits/commits.feature

@@ -47,6 +47,8 @@ Feature: Project Commits
     And repository contains ".gitlab-ci.yml" file
     When I click on commit link
     Then I see commit ci info
+    And I click status link
+    Then I see builds list
 
   Scenario: I browse commit with side-by-side diff view
     Given I click on commit link

features/steps/project/commits/commits.rb

@@ -166,6 +166,15 @@ class Spinach::Features::ProjectCommits < Spinach::FeatureSteps
     expect(page).to have_content "Pipeline #1 for 570e7b2a pending"
   end
 
+  step 'I click status link' do
+    find('.commit-ci-menu').click_link "Builds"
+  end
+
+  step 'I see builds list' do
+    expect(page).to have_content "Pipeline #1 for 570e7b2a pending"
+    expect(page).to have_content "1 build"
+  end
+
   step 'I search "submodules" commits' do
     fill_in 'commits-search', with: 'submodules'
   end

spec/controllers/projects/merge_requests_controller_spec.rb

@@ -667,6 +667,10 @@ describe Projects::MergeRequestsController do
     end
   end
 
+  describe 'GET builds' do
+    it_behaves_like "loads labels", :builds
+  end
+
   describe 'GET pipelines' do
     it_behaves_like "loads labels", :pipelines
   end

spec/features/merge_requests/created_from_fork_spec.rb

@@ -54,14 +54,14 @@ feature 'Merge request created from fork' do
 
     scenario 'user visits a pipelines page', js: true do
       visit_merge_request(merge_request)
-      page.within('.merge-request-tabs') { click_link 'Pipelines' }
+      page.within('.merge-request-tabs') { click_link 'Builds' }
 
       page.within('table.ci-table') do
-        expect(page).to have_content pipeline.status
-        expect(page).to have_content pipeline.id
+        expect(page).to have_content 'rspec'
+        expect(page).to have_content 'spinach'
       end
 
-      expect(page.find('a.btn-remove')[:href])
+      expect(find_link('Cancel running')[:href])
         .to include fork_project.path_with_namespace
     end
   end

spec/features/projects/commit/builds_spec.rb

 require 'spec_helper'
 
-feature 'project commit pipelines' do
+feature 'project commit builds' do
   given(:project) { create(:project) }
 
   background do
@@ -16,13 +16,11 @@ feature 'project commit pipelines' do
                            ref: 'master')
     end
 
-    scenario 'user views commit pipelines page' do
-      visit pipelines_namespace_project_commit_path(project.namespace, project, project.commit.sha)
+    scenario 'user views commit builds page' do
+      visit builds_namespace_project_commit_path(project.namespace,
+                                                 project, project.commit.sha)
 
-      page.within('.table-holder') do
-        expect(page).to have_content project.pipelines[0].status # pipeline status
-        expect(page).to have_content project.pipelines[0].id     # pipeline ids
-      end
+      expect(page).to have_content('Builds')
     end
   end
 end