Revert "Merge branch '28010-mr-merge-button-default-to-danger' into 'master'"

This reverts commit 071bf3b73424c2b2f8298bb5054c2ea2b135d0f7.

9 files changed, 38 insertions, 125 deletions

diff --git a/app/assets/javascripts/merge_request_widget.js.es6 b/app/assets/javascripts/merge_request_widget.js.es6
index 38c51b5ec88..4ab33420e59 100644
--- a/app/assets/javascripts/merge_request_widget.js.es6
+++ b/app/assets/javascripts/merge_request_widget.js.es6
@@ -129,9 +129,8 @@ require('./smart_interval');
     };
 
     MergeRequestWidget.prototype.getMergeStatus = function() {
-      return $.get(this.opts.merge_check_url, (data) => {
+      return $.get(this.opts.merge_check_url, function(data) {
         var $html = $(data);
-        this.updateMergeButton(this.status, this.hasCi, $html);
         $('.mr-widget-body').replaceWith($html.find('.mr-widget-body'));
         $('.mr-widget-footer').replaceWith($html.find('.mr-widget-footer'));
       });
@@ -155,9 +154,9 @@ require('./smart_interval');
       return $.getJSON(this.opts.ci_status_url, (function(_this) {
         return function(data) {
           var message, status, title;
-          _this.status = data.status;
-          _this.hasCi = data.has_ci;
-          _this.updateMergeButton(_this.status, _this.hasCi);
+          if (!data.status) {
+            return;
+          }
           if (data.environments && data.environments.length) _this.renderEnvironments(data.environments);
           if (data.status !== _this.opts.ci_status ||
               data.sha !== _this.opts.ci_sha ||
@@ -228,51 +227,42 @@ require('./smart_interval');
     };
 
     MergeRequestWidget.prototype.showCIStatus = function(state) {
-      // TODO: Can this variable be removed? - Felipe Artur
       var allowed_states;
       if (state == null) {
         return;
       }
       $('.ci_widget').hide();
-      $('.ci_widget.ci-' + state).show();
-
-      this.initMiniPipelineGraph();
-    };
-
-    MergeRequestWidget.prototype.showCICoverage = function(coverage) {
-      var text = `Coverage ${coverage}%`;
-      return $('.ci_widget:visible .ci-coverage').text(text);
-    };
-
-    MergeRequestWidget.prototype.updateMergeButton = function(state, hasCi, $html) {
-      const allowed_states = ["failed", "canceled", "running", "pending", "success", "success_with_warnings", "skipped", "not_found"];
-      let stateClass = 'btn-danger';
-      if (!hasCi) {
-        stateClass = 'btn-create';
-      } else if (indexOf.call(allowed_states, state) !== -1) {
+      allowed_states = ["failed", "canceled", "running", "pending", "success", "success_with_warnings", "skipped", "not_found"];
+      if (indexOf.call(allowed_states, state) >= 0) {
+        $('.ci_widget.ci-' + state).show();
         switch (state) {
           case "failed":
           case "canceled":
           case "not_found":
-            stateClass = 'btn-danger';
+            this.setMergeButtonClass('btn-danger');
             break;
           case "running":
-            stateClass = 'btn-info';
+            this.setMergeButtonClass('btn-info');
             break;
           case "success":
           case "success_with_warnings":
-            stateClass = 'btn-create';
+            this.setMergeButtonClass('btn-create');
         }
       } else {
         $('.ci_widget.ci-error').show();
-        stateClass = 'btn-danger';
+        this.setMergeButtonClass('btn-danger');
       }
+      this.initMiniPipelineGraph();
+    };
 
-      this.setMergeButtonClass(stateClass, $html);
+    MergeRequestWidget.prototype.showCICoverage = function(coverage) {
+      var text;
+      text = 'Coverage ' + coverage + '%';
+      return $('.ci_widget:visible .ci-coverage').text(text);
     };
 
-    MergeRequestWidget.prototype.setMergeButtonClass = function(css_class, $html = $('.mr-state-widget')) {
-      return $html.find('.js-merge-button').removeClass('btn-danger btn-info btn-create').addClass(css_class);
+    MergeRequestWidget.prototype.setMergeButtonClass = function(css_class) {
+      return $('.js-merge-button,.accept-action .dropdown-toggle').removeClass('btn-danger btn-info btn-create').addClass(css_class);
     };
 
     MergeRequestWidget.prototype.updatePipelineUrls = function(id) {
diff --git a/app/assets/javascripts/merge_request_widget/ci_bundle.js.es6 b/app/assets/javascripts/merge_request_widget/ci_bundle.js.es6
index 21d7c3e168e..5840916846b 100644
--- a/app/assets/javascripts/merge_request_widget/ci_bundle.js.es6
+++ b/app/assets/javascripts/merge_request_widget/ci_bundle.js.es6
@@ -15,15 +15,15 @@
     });
 
     $(document)
-    .off('click', '.accept-merge-request')
-    .on('click', '.accept-merge-request', () => {
-      $('.js-merge-button, .js-merge-when-pipeline-succeeds-button').html('<i class="fa fa-spinner fa-spin"></i> Merge in progress');
+    .off('click', '.accept_merge_request')
+    .on('click', '.accept_merge_request', () => {
+      $('.js-merge-button').html('<i class="fa fa-spinner fa-spin"></i> Merge in progress');
     });
 
     $(document)
-    .off('click', '.merge-when-pipeline-succeeds')
-    .on('click', '.merge-when-pipeline-succeeds', () => {
-      $('#merge_when_pipeline_succeeds').val('1');
+    .off('click', '.merge_when_build_succeeds')
+    .on('click', '.merge_when_build_succeeds', () => {
+      $('#merge_when_build_succeeds').val('1');
     });
 
     $(document)
diff --git a/app/assets/stylesheets/pages/merge_requests.scss b/app/assets/stylesheets/pages/merge_requests.scss
index 19e289e9dff..230af1a21f4 100644
--- a/app/assets/stylesheets/pages/merge_requests.scss
+++ b/app/assets/stylesheets/pages/merge_requests.scss
@@ -29,7 +29,7 @@
         background-color: $gl-success;
       }
 
-      .accept-merge-request {
+      .accept_merge_request {
         &.ci-pending,
         &.ci-running {
           @include btn-blue;
@@ -42,12 +42,6 @@
           @include btn-red;
         }
       }
-
-      .dropdown-toggle {
-        .fa {
-          color: inherit;
-        }
-      }
     }
 
     .accept-control {
diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb
index ea0ab108872..fbad66c5c40 100644
--- a/app/controllers/projects/merge_requests_controller.rb
+++ b/app/controllers/projects/merge_requests_controller.rb
@@ -308,7 +308,6 @@ class Projects::MergeRequestsController < Projects::ApplicationController
 
   def merge_check
     @merge_request.check_if_can_be_merged
-    @pipelines = @merge_request.all_pipelines
 
     render partial: "projects/merge_requests/widget/show.html.haml", layout: false
   end
@@ -427,7 +426,6 @@ class Projects::MergeRequestsController < Projects::ApplicationController
 
   def ci_status
     pipeline = @merge_request.head_pipeline
-    @pipelines = @merge_request.all_pipelines
 
     if pipeline
       status = pipeline.status
@@ -446,8 +444,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController
       sha: (merge_request.diff_head_commit.short_id if merge_request.diff_head_sha),
       status: status,
       coverage: coverage,
-      pipeline: pipeline.try(:id),
-      has_ci: @merge_request.has_ci?
+      pipeline: pipeline.try(:id)
     }
 
     render json: response
diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb
index 1716f2d113a..3c9bfb6302a 100644
--- a/app/models/merge_request.rb
+++ b/app/models/merge_request.rb
@@ -692,10 +692,7 @@ class MergeRequest < ActiveRecord::Base
   end
 
   def has_ci?
-    has_ci_integration = source_project.try(:ci_service)
-    uses_gitlab_ci = all_pipelines.any?
-
-    (has_ci_integration || uses_gitlab_ci) && commits.any?
+    source_project.try(:ci_service) && commits.any?
   end
 
   def branch_missing?
diff --git a/app/views/projects/merge_requests/widget/open/_accept.html.haml b/app/views/projects/merge_requests/widget/open/_accept.html.haml
index 20a7316abe8..b730ced4214 100644
--- a/app/views/projects/merge_requests/widget/open/_accept.html.haml
+++ b/app/views/projects/merge_requests/widget/open/_accept.html.haml
@@ -1,6 +1,8 @@
 - content_for :page_specific_javascripts do
   = page_specific_javascript_bundle_tag('merge_request_widget')
 
+- status_class = @pipeline ? " ci-#{@pipeline.status}" : nil
+
 = form_for [:merge, @project.namespace.becomes(Namespace), @project, @merge_request], remote: true, method: :post, html: { class: 'accept-mr-form js-quick-submit js-requires-input' } do |f|
   = hidden_field_tag :authenticity_token, form_authenticity_token
   = hidden_field_tag :sha, @merge_request.diff_head_sha
@@ -9,10 +11,10 @@
       .accept-action
         - if @pipeline && @pipeline.active?
           %span.btn-group
-            = button_tag class: "btn btn-info js-merge-when-pipeline-succeeds-button merge-when-pipeline-succeeds" do
+            = button_tag class: "btn btn-create js-merge-button merge_when_build_succeeds" do
               Merge When Pipeline Succeeds
-            - unless @project.only_allow_merge_if_pipeline_succeeds?
-              = button_tag class: "btn btn-info dropdown-toggle", 'data-toggle' => 'dropdown' do
+            - unless @project.only_allow_merge_if_build_succeeds?
+              = button_tag class: "btn btn-success dropdown-toggle", 'data-toggle' => 'dropdown' do
                 = icon('caret-down')
                 %span.sr-only
                   Select Merge Moment
@@ -22,11 +24,11 @@
                     = icon('check fw')
                     Merge When Pipeline Succeeds
                 %li
-                  = link_to "#", class: "accept-merge-request" do
+                  = link_to "#", class: "accept_merge_request" do
                     = icon('warning fw')
                     Merge Immediately
         - else
-          = f.button class: "btn btn-grouped js-merge-button accept-merge-request" do
+          = f.button class: "btn btn-create btn-grouped js-merge-button accept_merge_request #{status_class}" do
             Accept Merge Request
       - if @merge_request.force_remove_source_branch?
         .accept-control
diff --git a/changelogs/unreleased/28010-mr-merge-button-default-to-danger.yml b/changelogs/unreleased/28010-mr-merge-button-default-to-danger.yml
deleted file mode 100644
index 06bb669ceac..00000000000
--- a/changelogs/unreleased/28010-mr-merge-button-default-to-danger.yml
+++ /dev/null
@@ -1,4 +0,0 @@
----
-title: Default to subtle MR mege button until CI status is available
-merge_request:
-author:
diff --git a/spec/features/merge_requests/merge_immediately_with_pipeline_spec.rb b/spec/features/merge_requests/merge_immediately_with_pipeline_spec.rb
index 0ceaf7bc830..f2f8f11ab28 100644
--- a/spec/features/merge_requests/merge_immediately_with_pipeline_spec.rb
+++ b/spec/features/merge_requests/merge_immediately_with_pipeline_spec.rb
@@ -34,7 +34,7 @@ feature 'Merge immediately', :feature, :js do
 
         click_link 'Merge Immediately'
 
-        expect(find('.js-merge-when-pipeline-succeeds-button')).to have_content('Merge in progress')
+        expect(find('.js-merge-button')).to have_content('Merge in progress')
       end
     end
   end
diff --git a/spec/features/merge_requests/widget_spec.rb b/spec/features/merge_requests/widget_spec.rb
index b575aeff0d8..4ad944366c8 100644
--- a/spec/features/merge_requests/widget_spec.rb
+++ b/spec/features/merge_requests/widget_spec.rb
@@ -3,8 +3,8 @@ require 'rails_helper'
 describe 'Merge request', :feature, :js do
   include WaitForAjax
 
-  let(:user) { create(:user) }
   let(:project) { create(:project) }
+  let(:user) { create(:user) }
   let(:merge_request) { create(:merge_request, source_project: project) }
 
   before do
@@ -31,7 +31,7 @@ describe 'Merge request', :feature, :js do
 
       wait_for_ajax
 
-      expect(page).to have_selector('.accept-merge-request')
+      expect(page).to have_selector('.accept_merge_request')
     end
   end
 
@@ -51,69 +51,6 @@ describe 'Merge request', :feature, :js do
         expect(find('.js-environment-link')[:href]).to include(environment.formatted_external_url)
       end
     end
-
-    it 'shows green accept merge request button' do
-      # Wait for the `ci_status` and `merge_check` requests
-      wait_for_ajax
-      expect(page).to have_selector('.accept-merge-request.btn-create')
-    end
-  end
-
-  context 'view merge request with external CI service' do
-    before do
-      create(:service, project: project,
-                       active: true,
-                       type: 'CiService',
-                       category: 'ci')
-
-      visit namespace_project_merge_request_path(project.namespace, project, merge_request)
-    end
-
-    it 'has danger button while waiting for external CI status' do
-      # Wait for the `ci_status` and `merge_check` requests
-      wait_for_ajax
-      expect(page).to have_selector('.accept-merge-request.btn-danger')
-    end
-  end
-
-  context 'view merge request with failed GitLab CI pipelines' do
-    before do
-      commit_status = create(:commit_status, project: project, status: 'failed')
-      pipeline = create(:ci_pipeline, project: project,
-                                      sha: merge_request.diff_head_sha,
-                                      ref: merge_request.source_branch,
-                                      status: 'failed',
-                                      statuses: [commit_status])
-      create(:ci_build, :pending, pipeline: pipeline)
-
-      visit namespace_project_merge_request_path(project.namespace, project, merge_request)
-    end
-
-    it 'has danger button when not succeeded' do
-      # Wait for the `ci_status` and `merge_check` requests
-      wait_for_ajax
-      expect(page).to have_selector('.accept-merge-request.btn-danger')
-    end
-  end
-
-  context 'view merge request with MWBS button' do
-    before do
-      commit_status = create(:commit_status, project: project, status: 'pending')
-      pipeline = create(:ci_pipeline, project: project,
-                                      sha: merge_request.diff_head_sha,
-                                      ref: merge_request.source_branch,
-                                      status: 'pending',
-                                      statuses: [commit_status])
-      create(:ci_build, :pending, pipeline: pipeline)
-
-      visit namespace_project_merge_request_path(project.namespace, project, merge_request)
-    end
-
-    it 'has info button when MWBS button' do
-      # Wait for the `ci_status` and `merge_check` requests
-      wait_for_ajax
-      expect(page).to have_selector('.merge-when-pipeline-succeeds.btn-info')
-    end
   end
 
   context 'merge error' do