Removes builds tab from merge request

Fix specs

app/assets/javascripts/merge_request_tabs.js

+/* eslint-disable max-len, func-names, space-before-function-paren, no-var, space-before-blocks, prefer-rest-params, wrap-iife, no-use-before-define, no-underscore-dangle, no-undef, one-var, one-var-declaration-per-line, quotes, comma-dangle, consistent-return, prefer-template, no-param-reassign, camelcase, vars-on-top, space-in-parens, curly, prefer-arrow-callback, no-unused-vars, no-return-assign, semi, object-shorthand, operator-assignment, padded-blocks, max-len */
+// MergeRequestTabs
+//
+// Handles persisting and restoring the current tab selection and lazily-loading
+// content on the MergeRequests#show page.
+//
+/*= require js.cookie */
+
+//
+// ### Example Markup
+//
+//   <ul class="nav-links merge-request-tabs">
+//     <li class="notes-tab active">
+//       <a data-action="notes" data-target="#notes" data-toggle="tab" href="/foo/bar/merge_requests/1">
+//         Discussion
+//       </a>
+//     </li>
+//     <li class="commits-tab">
+//       <a data-action="commits" data-target="#commits" data-toggle="tab" href="/foo/bar/merge_requests/1/commits">
+//         Commits
+//       </a>
+//     </li>
+//     <li class="diffs-tab">
+//       <a data-action="diffs" data-target="#diffs" data-toggle="tab" href="/foo/bar/merge_requests/1/diffs">
+//         Diffs
+//       </a>
+//     </li>
+//   </ul>
+//
+//   <div class="tab-content">
+//     <div class="notes tab-pane active" id="notes">
+//       Notes Content
+//     </div>
+//     <div class="commits tab-pane" id="commits">
+//       Commits Content
+//     </div>
+//     <div class="diffs tab-pane" id="diffs">
+//       Diffs Content
+//     </div>
+//   </div>
+//
+//   <div class="mr-loading-status">
+//     <div class="loading">
+//       Loading Animation
+//     </div>
+//   </div>
+//
+(function() {
+  var bind = function(fn, me){ return function(){ return fn.apply(me, arguments); }; };
+
+  this.MergeRequestTabs = (function() {
+    MergeRequestTabs.prototype.diffsLoaded = false;
+
+    MergeRequestTabs.prototype.pipelinesLoaded = false;
+
+    MergeRequestTabs.prototype.commitsLoaded = false;
+
+    MergeRequestTabs.prototype.fixedLayoutPref = null;
+
+    function MergeRequestTabs(opts) {
+      this.opts = opts != null ? opts : {};
+      this.opts.setUrl = this.opts.setUrl !== undefined ? this.opts.setUrl : true;
+
+      this.setCurrentAction = bind(this.setCurrentAction, this);
+      this.tabShown = bind(this.tabShown, this);
+      this.showTab = bind(this.showTab, this);
+      // Store the `location` object, allowing for easier stubbing in tests
+      this._location = location;
+      this.bindEvents();
+      this.activateTab(this.opts.action);
+      this.initAffix();
+    }
+
+    MergeRequestTabs.prototype.bindEvents = function() {
+      $(document).on('shown.bs.tab', '.merge-request-tabs a[data-toggle="tab"]', this.tabShown);
+      $(document).on('click', '.js-show-tab', this.showTab);
+    };
+
+    MergeRequestTabs.prototype.unbindEvents = function() {
+      $(document).off('shown.bs.tab', '.merge-request-tabs a[data-toggle="tab"]', this.tabShown);
+      $(document).off('click', '.js-show-tab', this.showTab);
+    };
+
+    MergeRequestTabs.prototype.showTab = function(event) {
+      event.preventDefault();
+      return this.activateTab($(event.target).data('action'));
+    };
+
+    MergeRequestTabs.prototype.tabShown = function(event) {
+      var $target, action, navBarHeight;
+      $target = $(event.target);
+      action = $target.data('action');
+      if (action === 'commits') {
+        this.loadCommits($target.attr('href'));
+        this.expandView();
+        this.resetViewContainer();
+      } else if (this.isDiffAction(action)) {
+        this.loadDiff($target.attr('href'));
+        if ((typeof bp !== "undefined" && bp !== null) && bp.getBreakpointSize() !== 'lg') {
+          this.shrinkView();
+        }
+        if (this.diffViewType() === 'parallel') {
+          this.expandViewContainer();
+        }
+        navBarHeight = $('.navbar-gitlab').outerHeight();
+        $.scrollTo(".merge-request-details .merge-request-tabs", {
+          offset: -navBarHeight
+        });
+      } else if (action === 'pipelines') {
+        this.loadPipelines($target.attr('href'));
+        this.expandView();
+        this.resetViewContainer();
+      } else {
+        this.expandView();
+        this.resetViewContainer();
+      }
+      if (this.opts.setUrl) {
+        this.setCurrentAction(action);
+      }
+    };
+
+    MergeRequestTabs.prototype.scrollToElement = function(container) {
+      var $el, navBarHeight;
+      if (window.location.hash) {
+        navBarHeight = $('.navbar-gitlab').outerHeight() + $('.layout-nav').outerHeight() + document.querySelector('.js-tabs-affix').offsetHeight;
+        $el = $(container + " " + window.location.hash + ":not(.match)");
+        if ($el.length) {
+          return $.scrollTo(container + " " + window.location.hash + ":not(.match)", {
+            offset: -navBarHeight
+          });
+        }
+      }
+    };
+
+    // Activate a tab based on the current action
+    MergeRequestTabs.prototype.activateTab = function(action) {
+      if (action === 'show') {
+        action = 'notes';
+      }
+      // important note: the .tab('show') method triggers 'shown.bs.tab' event itself
+      $(".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
+    MergeRequestTabs.prototype.setCurrentAction = function(action) {
+      var new_state;
+      // Normalize action, just to be safe
+      if (action === 'show') {
+        action = 'notes';
+      }
+      this.currentAction = action;
+      // Remove a trailing '/commits' '/diffs' '/pipelines' '/new' '/new/diffs'
+      new_state = this._location.pathname.replace(/\/(commits|diffs|pipelines|new|new\/diffs)(\.html)?\/?$/, '');
+
+      // Append the new action if we're on a tab other than 'notes'
+      if (action !== 'notes') {
+        new_state += "/" + action;
+      }
+      // Ensure parameters and hash come along for the ride
+      new_state += this._location.search + this._location.hash;
+      history.replaceState({
+        turbolinks: true,
+        url: new_state
+      // Replace the current history state with the new one without breaking
+      // Turbolinks' history.
+      //
+      // See https://github.com/rails/turbolinks/issues/363
+      }, document.title, new_state);
+      return new_state;
+    };
+
+    MergeRequestTabs.prototype.loadCommits = function(source) {
+      if (this.commitsLoaded) {
+        return;
+      }
+      return this._get({
+        url: source + ".json",
+        success: (function(_this) {
+          return function(data) {
+            document.querySelector("div#commits").innerHTML = data.html;
+            gl.utils.localTimeAgo($('.js-timeago', 'div#commits'));
+            _this.commitsLoaded = true;
+            return _this.scrollToElement("#commits");
+          };
+        })(this)
+      });
+    };
+
+    MergeRequestTabs.prototype.loadDiff = function(source) {
+      if (this.diffsLoaded) {
+        return;
+      }
+
+      // We extract pathname for the current Changes tab anchor href
+      // some pages like MergeRequestsController#new has query parameters on that anchor
+      var url = gl.utils.parseUrl(source);
+
+      return this._get({
+        url: (url.pathname + ".json") + this._location.search,
+        success: (function(_this) {
+          return function(data) {
+            $('#diffs').html(data.html);
+
+            if (typeof gl.diffNotesCompileComponents !== 'undefined') {
+              gl.diffNotesCompileComponents();
+            }
+
+            gl.utils.localTimeAgo($('.js-timeago', 'div#diffs'));
+            $('#diffs .js-syntax-highlight').syntaxHighlight();
+            $('#diffs .diff-file').singleFileDiff();
+            if (_this.diffViewType() === 'parallel' && (_this.isDiffAction(_this.currentAction)) ) {
+              _this.expandViewContainer();
+            }
+            _this.diffsLoaded = true;
+            var anchoredDiff = gl.utils.getLocationHash();
+            if (anchoredDiff) _this.openAnchoredDiff(anchoredDiff, function() {
+              _this.scrollToElement("#diffs");
+              _this.highlighSelectedLine();
+            });
+            _this.filesCommentButton = $('.files .diff-file').filesCommentButton();
+            return $(document).off('click', '.diff-line-num a').on('click', '.diff-line-num a', function(e) {
+              e.preventDefault();
+              window.location.hash = $(e.currentTarget).attr('href');
+              _this.highlighSelectedLine();
+              return _this.scrollToElement("#diffs");
+            });
+          };
+        })(this)
+      });
+    };
+
+    MergeRequestTabs.prototype.openAnchoredDiff = function(anchoredDiff, cb) {
+      var diffTitle = $('#file-path-' + anchoredDiff);
+      var diffFile = diffTitle.closest('.diff-file');
+      var nothingHereBlock = $('.nothing-here-block:visible', diffFile);
+      if (nothingHereBlock.length) {
+        diffFile.singleFileDiff(true, cb);
+      } else {
+        cb();
+      }
+    };
+
+    MergeRequestTabs.prototype.highlighSelectedLine = function() {
+      var $diffLine, diffLineTop, hashClassString, locationHash, navBarHeight;
+      $('.hll').removeClass('hll');
+      locationHash = window.location.hash;
+      if (locationHash !== '') {
+        dataLineString = '[data-line-code="' + locationHash.replace('#', '') + '"]';
+        $diffLine = $(locationHash + ":not(.match)", $('#diffs'));
+        if (!$diffLine.is('tr')) {
+          $diffLine = $('#diffs').find("td" + locationHash + ", td" + dataLineString);
+        } else {
+          $diffLine = $diffLine.find('td');
+        }
+        if ($diffLine.length) {
+          $diffLine.addClass('hll');
+          diffLineTop = $diffLine.offset().top;
+          return navBarHeight = $('.navbar-gitlab').outerHeight();
+        }
+      }
+    };
+
+    MergeRequestTabs.prototype.loadPipelines = function(source) {
+      if (this.pipelinesLoaded) {
+        return;
+      }
+      return this._get({
+        url: source + ".json",
+        success: function(data) {
+          $('#pipelines').html(data.html);
+          gl.utils.localTimeAgo($('.js-timeago', '#pipelines'));
+          this.pipelinesLoaded = true;
+          return this.scrollToElement("#pipelines");
+        }.bind(this)
+      });
+    };
+
+    // Show or hide the loading spinner
+    //
+    // status - Boolean, true to show, false to hide
+    MergeRequestTabs.prototype.toggleLoading = function(status) {
+      return $('.mr-loading-status .loading').toggle(status);
+    };
+
+    MergeRequestTabs.prototype._get = function(options) {
+      var defaults;
+      defaults = {
+        beforeSend: (function(_this) {
+          return function() {
+            return _this.toggleLoading(true);
+          };
+        })(this),
+        complete: (function(_this) {
+          return function() {
+            return _this.toggleLoading(false);
+          };
+        })(this),
+        dataType: 'json',
+        type: 'GET'
+      };
+      options = $.extend({}, defaults, options);
+      return $.ajax(options);
+    };
+
+    MergeRequestTabs.prototype.diffViewType = function() {
+      return $('.inline-parallel-buttons a.active').data('view-type');
+    };
+
+    MergeRequestTabs.prototype.isDiffAction = function(action) {
+      return action === 'diffs' || action === 'new/diffs'
+    };
+
+    MergeRequestTabs.prototype.expandViewContainer = function() {
+      var $wrapper = $('.content-wrapper .container-fluid');
+      if (this.fixedLayoutPref === null) {
+        this.fixedLayoutPref = $wrapper.hasClass('container-limited');
+      }
+      $wrapper.removeClass('container-limited');
+    };
+
+    MergeRequestTabs.prototype.resetViewContainer = function() {
+      if (this.fixedLayoutPref !== null) {
+        $('.content-wrapper .container-fluid')
+          .toggleClass('container-limited', this.fixedLayoutPref);
+      }
+    };
+
+    MergeRequestTabs.prototype.shrinkView = function() {
+      var $gutterIcon;
+      $gutterIcon = $('.js-sidebar-toggle i:visible');
+      return setTimeout(function() {
+        if ($gutterIcon.is('.fa-angle-double-right')) {
+          return $gutterIcon.closest('a').trigger('click', [true]);
+        }
+      // Wait until listeners are set
+      // Only when sidebar is expanded
+      }, 0);
+    };
+
+    MergeRequestTabs.prototype.expandView = function() {
+      var $gutterIcon;
+      if (Cookies.get('collapsed_gutter') === 'true') {
+        return;
+      }
+      $gutterIcon = $('.js-sidebar-toggle i:visible');
+      return setTimeout(function() {
+        if ($gutterIcon.is('.fa-angle-double-left')) {
+          return $gutterIcon.closest('a').trigger('click', [true]);
+        }
+      }, 0);
+    // Expand the issuable sidebar unless the user explicitly collapsed it
+    // Wait until listeners are set
+    // Only when sidebar is collapsed
+    };
+
+    MergeRequestTabs.prototype.initAffix = function () {
+      var $tabs = $('.js-tabs-affix');
+
+      // Screen space on small screens is usually very sparse
+      // So we dont affix the tabs on these
+      if (Breakpoints.get().getBreakpointSize() === 'xs' || !$tabs.length) return;
+
+      var $diffTabs = $('#diff-notes-app'),
+        $fixedNav = $('.navbar-fixed-top'),
+        $layoutNav = $('.layout-nav');
+
+      $tabs.off('affix.bs.affix affix-top.bs.affix')
+        .affix({
+          offset: {
+            top: function () {
+              var tabsTop = $diffTabs.offset().top - $tabs.height();
+              tabsTop = tabsTop - ($fixedNav.height() + $layoutNav.height());
+
+              return tabsTop;
+            }
+          }
+        }).on('affix.bs.affix', function () {
+          $diffTabs.css({
+            marginTop: $tabs.height()
+          });
+        }).on('affix-top.bs.affix', function () {
+          $diffTabs.css({
+            marginTop: ''
+          });
+        });
+
+      // Fix bug when reloading the page already scrolling
+      if ($tabs.hasClass('affix')) {
+        $tabs.trigger('affix.bs.affix');
+      }
+    };
+
+    return MergeRequestTabs;
+
+  })();
+
+}).call(this);

app/assets/javascripts/merge_request_widget.js.es6

@@ -45,6 +45,7 @@
       $('#modal_merge_info').modal({
         show: false
       });
+
       this.clearEventListeners();
       this.addEventListeners();
       this.getCIStatus(false);
@@ -74,7 +75,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 +174,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/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]

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/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();
   }

config/routes/project.rb

@@ -92,7 +92,6 @@ constraints(ProjectUrlConstrainer.new) do
           get :diffs
           get :conflicts
           get :conflict_for_path
-          get :builds
           get :pipelines
           get :merge_check
           post :merge

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