Merge branch '23638-remove-builds-tab' into 'master'

Resolve "Remove Builds tab from Merge Requests and Commits"

Closes #23638

See merge request !7763

app/assets/javascripts/dispatcher.js.es6

@@ -138,7 +138,6 @@
           new MergedButtons();
           break;
         case 'projects:merge_requests:commits':
-        case 'projects:merge_requests:builds':
           new MergedButtons();
           break;
         case 'projects:merge_requests:pipelines':
@@ -168,9 +167,6 @@
             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,16 +59,13 @@
 
   class MergeRequestTabs {
 
-    constructor({ action, setUrl, buildsLoaded, stubLocation } = {}) {
+    constructor({ action, setUrl, 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);
@@ -119,10 +116,6 @@
         $.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();
@@ -180,8 +173,8 @@
     setCurrentAction(action) {
       this.currentAction = action === 'show' ? 'notes' : action;
 
-      // Remove a trailing '/commits' '/diffs' '/builds' '/pipelines' '/new' '/new/diffs'
-      let newState = location.pathname.replace(/\/(commits|diffs|builds|pipelines|new|new\/diffs)(\.html)?\/?$/, '');
+      // Remove a trailing '/commits' '/diffs' '/pipelines' '/new' '/new/diffs'
+      let newState = location.pathname.replace(/\/(commits|diffs|pipelines|new|new\/diffs)(\.html)?\/?$/, '');
 
       // Append the new action if we're on a tab other than 'notes'
       if (this.currentAction !== 'notes') {
@@ -255,22 +248,6 @@
       });
     }
 
-    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', 'builds', 'pipelines', 'changes'];
+      allowedPages = ['show', 'commits', 'pipelines', 'changes'];
       $(document).on('page:change.merge_request', (function(_this) {
         return function() {
           var page;
@@ -173,7 +173,6 @@
               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,13 +8,11 @@ class Projects::CommitController < Projects::ApplicationController
 
   # Authorize
   before_action :require_non_empty_project
-  before_action :authorize_download_code!, except: [:cancel_builds, :retry_builds]
-  before_action :authorize_update_build!, only: [:cancel_builds, :retry_builds]
+  before_action :authorize_download_code!
   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, :builds, :pipelines]
-  before_action :define_status_vars, only: [:show, :builds, :pipelines]
+  before_action :define_commit_vars, only: [:show, :diff_for_path, :pipelines]
+  before_action :define_status_vars, only: [:show, :pipelines]
   before_action :define_note_vars, only: [:show, :diff_for_path]
   before_action :authorize_edit_tree!, only: [:revert, :cherry_pick]
 
@@ -35,25 +33,6 @@ 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)
@@ -98,10 +77,6 @@ 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
 
@@ -133,8 +108,6 @@ 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, :builds, :pipelines, :merge, :merge_check,
+    :edit, :update, :show, :diffs, :commits, :conflicts, :conflict_for_path, :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, :builds, :pipelines]
+  before_action :validates_merge_request, only: [:show, :diffs, :commits, :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]
@@ -201,17 +201,6 @@ 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
-    builds_namespace_project_commit_path(project.namespace, project, pipeline.sha)
+    namespace_project_pipeline_path(project.namespace, project, pipeline)
   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 builds_namespace_project_commit_path(commit.project.namespace, commit.project, commit), class: "ci-status ci-#{status}" do
+  = link_to pipelines_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 ci_status_path(@build.pipeline) do
+    = link_to namespace_project_commit_path(@project.namespace, @project, @build.pipeline.sha) 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,7 +8,3 @@
       = 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,11 +34,6 @@
           = 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
@@ -49,9 +44,6 @@
         = 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"
@@ -66,6 +58,5 @@
   });
 :javascript
   var merge_request = new MergeRequest({
-    action: "#{(@show_changes_tab ? 'new/diffs' : 'new')}",
-    buildsLoaded: "#{@pipeline.present? ? 'true' : 'false'}"
+    action: "#{(@show_changes_tab ? 'new/diffs' : 'new')}"
   });

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

@@ -65,11 +65,6 @@
                 = 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
@@ -98,8 +93,6 @@
 
         #commits.commits.tab-pane
           - # 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

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,12 +24,10 @@
       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();
   }

changelogs/unreleased/23638-remove-builds-tab.yml

+---
+title: Resolve "Remove Builds tab from Merge Requests and Commits"
+merge_request: 7763
+author:

config/routes/project.rb

@@ -32,10 +32,7 @@ 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
@@ -94,7 +91,6 @@ 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,8 +47,6 @@ 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,15 +166,6 @@ 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

@@ -649,10 +649,6 @@ 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 'Builds' }
+      page.within('.merge-request-tabs') { click_link 'Pipelines' }
 
       page.within('table.ci-table') do
-        expect(page).to have_content 'rspec'
-        expect(page).to have_content 'spinach'
+        expect(page).to have_content pipeline.status
+        expect(page).to have_content pipeline.id
       end
 
-      expect(find_link('Cancel running')[:href])
+      expect(page.find('a.btn-remove')[:href])
         .to include fork_project.path_with_namespace
     end
   end

spec/features/projects/commit/builds_spec.rb

 require 'spec_helper'
 
-feature 'project commit builds' do
+feature 'project commit pipelines' do
   given(:project) { create(:project) }
 
   background do
@@ -16,11 +16,13 @@ feature 'project commit builds' do
                            ref: 'master')
     end
 
-    scenario 'user views commit builds page' do
-      visit builds_namespace_project_commit_path(project.namespace,
-                                                 project, project.commit.sha)
+    scenario 'user views commit pipelines page' do
+      visit pipelines_namespace_project_commit_path(project.namespace, project, project.commit.sha)
 
-      expect(page).to have_content('Builds')
+      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
     end
   end
 end