Merge branch '1595-merge-when-pipeline-succeeds-visible-if-approvals-are-pending' into 'master'

Resolve ""Merge when pipeline succeeds" visible if approvals are pending"

Closes #1595

See merge request !1101

app/assets/javascripts/merge_request_widget.js.es6

@@ -144,8 +144,9 @@
     MergeRequestWidget.prototype.getMergeStatus = function() {
       return $.get(this.opts.merge_check_url, function(data) {
         var $html = $(data);
+
         $('.mr-widget-body').replaceWith($html.find('.mr-widget-body'));
-        $('.mr-widget-footer:not(.mr-approvals-footer)').replaceWith($html.find('.mr-widget-footer:not(.mr-approvals-footer)'));
+        $('.mr-widget-footer').replaceWith($html.find('.mr-widget-footer'));
       });
     };
 

app/assets/javascripts/merge_request_widget/ci_bundle.js.es6

@@ -46,7 +46,10 @@
       .on('click', '.js-rebase-button', () => {
         $('.js-rebase-button').html("<i class='fa fa-spinner fa-spin'></i> Rebase in progress");
       });
-    } else {
+    } else if (!$('.mr-approvals-footer').length) {
+      // getMergeStatus replaces the MR widget with new, updated HTML, which means any persistent
+      // event management gets clobbered. When the MR is approvable, MR status is already managed
+      // and breaks when the DOM is clobbered.
       merge_request_widget.getMergeStatus();
     }
   });

app/assets/javascripts/merge_request_widget/widget_store.js.es6

@@ -23,7 +23,7 @@
 
     initWidgetState() {
       this.assignToData('showApprovals', false);
-      this.assignToData('disableAcceptance', Boolean(this.rootEl.dataset.approvalPending));
+      this.assignToData('disableAcceptance', this.rootEl.dataset.approvalPending === 'true');
     }
 
     initApprovals() {

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

 - content_for :page_specific_javascripts do
   = page_specific_javascript_tag('merge_request_widget/widget_bundle.js')
 
-- approval_pending = @merge_request.requires_approve? && !@merge_request.approved?
-
-#merge-request-widget-app.mr-state-widget{ 'data-endpoint'=> merge_request_path(@merge_request), 'data-approval-pending' => approval_pending }
+#merge-request-widget-app.mr-state-widget{ 'data-endpoint'=> merge_request_path(@merge_request), 'data-approval-pending' => (!@merge_request.approved?).to_s }
   = render 'projects/merge_requests/widget/heading'
   .mr-widget-body
     -# After conflicts are resolved, the user is redirected back to the MR page.

app/views/projects/merge_requests/widget/open/_accept.html.haml

@@ -11,10 +11,10 @@
       .accept-action
         - if @pipeline && @pipeline.active?
           %span.btn-group
-            = button_tag class: "btn btn-create js-merge-button merge_when_build_succeeds" do
+            = button_tag class: "btn btn-create js-merge-button merge_when_build_succeeds", disabled: !@merge_request.approved?, ":disabled" => "disableAcceptance" do
               Merge When Pipeline Succeeds
             - unless @project.only_allow_merge_if_build_succeeds?
-              = button_tag class: "btn btn-success dropdown-toggle", 'data-toggle' => 'dropdown' do
+              = button_tag class: "btn btn-success dropdown-toggle", 'data-toggle' => 'dropdown', disabled: !@merge_request.approved?, ":disabled" => "disableAcceptance" do
                 = icon('caret-down')
                 %span.sr-only
                   Select Merge Moment
@@ -28,7 +28,7 @@
                     = icon('warning fw')
                     Merge Immediately
         - else
-          = f.button class: "btn btn-create btn-grouped js-merge-button accept_merge_request #{status_class}", ':disabled' => 'disableAcceptance' do
+          = f.button class: "btn btn-create btn-grouped js-merge-button accept_merge_request #{status_class}", disabled: !@merge_request.approved?, ":disabled" => "disableAcceptance" do
             Accept Merge Request
       - if @merge_request.force_remove_source_branch?
         .accept-control

app/views/projects/merge_requests/widget/open/_rebase.html.haml

@@ -21,7 +21,9 @@
     = hidden_field_tag :authenticity_token, form_authenticity_token
     .accept-merge-holder.clearfix.js-toggle-container
       .accept-action
-        = f.button class: "btn btn-reopen js-rebase-button" do
+        = f.button class: "btn btn-reopen js-rebase-button", disabled: !@merge_request.approved?, ":disabled" => "disableAcceptance" do
           Rebase onto #{@merge_request.target_branch}
       .accept-control
         Fast-forward merge is not possible. Rebase the source branch onto the target branch or merge target branch into source branch to allow this merge request to be merged.
+        - if @merge_request.requires_approve?
+          %b{ "v-show" => "disableAcceptance" } (Rebasing is disabled until merge request has been approved)

changelogs/unreleased-ee/1595-merge-when-pipeline-succeeds-visible-if-approvals-are-pending.yml

+---
+title: Disable all merge acceptance buttons pending MR approval.
+merge_request: !1101
+author:

spec/features/merge_requests/approvals_spec.rb

@@ -238,6 +238,50 @@ feature 'Merge request approvals', js: true, feature: true do
         expect(page).to have_no_css('.approver-avatar')
       end
     end
+
+    context 'when CI is running but no approval given', js: true do
+      before do
+        create :approver_group, group: group, target: merge_request
+        create(:ci_empty_pipeline, project: project, sha: merge_request.diff_head_sha, ref: merge_request.source_branch)
+        visit namespace_project_merge_request_path(project.namespace, project, merge_request)
+      end
+
+      it 'I am unable to set Merge When Pipeline Succeeds' do
+        # before approval status is loaded
+        expect(page).to have_button('Merge When Pipeline Succeeds', disabled: true)
+
+        wait_for_ajax
+
+        # after approval status is loaded
+        expect(page).to have_button('Merge When Pipeline Succeeds', disabled: true)
+      end
+    end
+
+    context 'when rebase is needed but no approval given', js: true do
+      let(:project) do
+        create(:project,
+          approvals_before_merge: 1,
+          merge_requests_rebase_enabled: true,
+          merge_requests_ff_only_enabled: true )
+      end
+
+      let(:merge_request) { create(:merge_request, source_project: project, source_branch: 'feature', target_branch: 'wip' ) }
+
+      before do
+        create :approver_group, group: group, target: merge_request
+        visit namespace_project_merge_request_path(project.namespace, project, merge_request)
+      end
+
+      it 'I am unable to rebase the merge request' do
+        # before approval status is loaded
+        expect(page).to have_button('Rebase onto wip', disabled: true)
+
+        wait_for_ajax
+
+        # after approval status is loaded
+        expect(page).to have_button('Rebase onto wip', disabled: true)
+      end
+    end
   end
 end