Merge branch '38389-allow-merge-without-success' into 'master'

Allow merge when no pipeline success

Closes #38389

See merge request gitlab-org/gitlab-ce!14633

app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_ready_to_merge.js

@@ -67,7 +67,7 @@ export default {
       return defaultClass;
     },
     iconClass() {
-      if (this.status === 'failed' || !this.commitMessage.length || !this.isMergeAllowed() || this.mr.preventMerge) {
+      if (this.status === 'failed' || !this.commitMessage.length || !this.mr.isMergeAllowed || this.mr.preventMerge) {
         return 'failed';
       }
       return 'success';
@@ -100,13 +100,8 @@ export default {
     },
   },
   methods: {
-    isMergeAllowed() {
-      return !this.mr.onlyAllowMergeIfPipelineSucceeds ||
-        this.mr.isPipelinePassing ||
-        this.mr.isPipelineSkipped;
-    },
     shouldShowMergeControls() {
-      return this.isMergeAllowed() || this.shouldShowMergeWhenPipelineSucceedsText;
+      return this.mr.isMergeAllowed || this.shouldShowMergeWhenPipelineSucceedsText;
     },
     updateCommitMessage() {
       const cmwd = this.mr.commitMessageWithDescription;

app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js

@@ -73,6 +73,7 @@ export default class MergeRequestStore {
     this.canCancelAutomaticMerge = !!data.cancel_merge_when_pipeline_succeeds_path;
     this.hasSHAChanged = this.sha !== data.diff_head_sha;
     this.canBeMerged = data.can_be_merged || false;
+    this.isMergeAllowed = data.mergeable || false;
     this.mergeOngoing = data.merge_ongoing;
 
     // Cherry-pick and Revert actions related

app/serializers/merge_request_entity.rb

@@ -42,6 +42,7 @@ class MergeRequestEntity < IssuableEntity
   expose :commits_count
   expose :cannot_be_merged?, as: :has_conflicts
   expose :can_be_merged?, as: :can_be_merged
+  expose :mergeable?, as: :mergeable
   expose :remove_source_branch?, as: :remove_source_branch
 
   expose :project_archived do |merge_request|

changelogs/unreleased/38389-allow-merge-without-success.yml

+---
+title: Allow merge in MR widget with no pipeline but using "Only allow merge requests
+  to be merged if the pipeline succeeds"
+merge_request: 14633
+author:
+type: fixed

spec/features/merge_requests/widget_spec.rb

@@ -3,10 +3,13 @@ require 'rails_helper'
 describe 'Merge request', :js do
   let(:user) { create(:user) }
   let(:project) { create(:project, :repository) }
+  let(:project_only_mwps) { create(:project, :repository, only_allow_merge_if_pipeline_succeeds: true) }
   let(:merge_request) { create(:merge_request, source_project: project) }
+  let(:merge_request_in_only_mwps_project) { create(:merge_request, source_project: project_only_mwps) }
 
   before do
-    project.team << [user, :master]
+    project.add_master(user)
+    project_only_mwps.add_master(user)
     sign_in(user)
   end
 
@@ -160,6 +163,20 @@ describe 'Merge request', :js do
     end
   end
 
+  context 'view merge request in project with only-mwps setting enabled but no CI is setup' do
+    before do
+      visit project_merge_request_path(project_only_mwps, merge_request_in_only_mwps_project)
+    end
+
+    it 'should be allowed to merge' do
+      # Wait for the `ci_status` and `merge_check` requests
+      wait_for_requests
+
+      expect(page).to have_selector('.accept-merge-request')
+      expect(find('.accept-merge-request')['disabled']).not_to be(true)
+    end
+  end
+
   context 'view merge request with MWPS enabled but automatically merge fails' do
     before do
       merge_request.update(

spec/fixtures/api/schemas/entities/merge_request.json

@@ -46,6 +46,7 @@
     "branch_missing": { "type": "boolean" },
     "has_conflicts": { "type": "boolean" },
     "can_be_merged": { "type": "boolean" },
+    "mergeable": { "type": "boolean" },
     "project_archived": { "type": "boolean" },
     "only_allow_merge_if_pipeline_succeeds": { "type": "boolean" },
     "has_ci": { "type": "boolean" },

spec/javascripts/vue_mr_widget/components/states/mr_widget_ready_to_merge_spec.js

@@ -12,6 +12,7 @@ const createComponent = (customConfig = {}) => {
     pipeline: null,
     isPipelineFailed: false,
     isPipelinePassing: false,
+    isMergeAllowed: true,
     onlyAllowMergeIfPipelineSucceeds: false,
     hasCI: false,
     ciStatus: null,
@@ -212,21 +213,24 @@ describe('MRWidgetReadyToMerge', () => {
 
     describe('isMergeButtonDisabled', () => {
       it('should return false with initial data', () => {
+        vm.mr.isMergeAllowed = true;
         expect(vm.isMergeButtonDisabled).toBeFalsy();
       });
 
       it('should return true when there is no commit message', () => {
+        vm.mr.isMergeAllowed = true;
         vm.commitMessage = '';
         expect(vm.isMergeButtonDisabled).toBeTruthy();
       });
 
       it('should return true if merge is not allowed', () => {
+        vm.mr.isMergeAllowed = false;
         vm.mr.onlyAllowMergeIfPipelineSucceeds = true;
-        vm.mr.isPipelineFailed = true;
         expect(vm.isMergeButtonDisabled).toBeTruthy();
       });
 
       it('should return true when the vm instance is making request', () => {
+        vm.mr.isMergeAllowed = true;
         vm.isMakingRequest = true;
         expect(vm.isMergeButtonDisabled).toBeTruthy();
       });
@@ -234,53 +238,27 @@ describe('MRWidgetReadyToMerge', () => {
   });
 
   describe('methods', () => {
-    describe('isMergeAllowed', () => {
-      it('should return true when no pipeline and not required to succeed', () => {
-        vm.mr.onlyAllowMergeIfPipelineSucceeds = false;
-        vm.mr.isPipelinePassing = false;
-        expect(vm.isMergeAllowed()).toBeTruthy();
-      });
-
-      it('should return true when pipeline failed and not required to succeed', () => {
-        vm.mr.onlyAllowMergeIfPipelineSucceeds = false;
-        vm.mr.isPipelinePassing = false;
-        expect(vm.isMergeAllowed()).toBeTruthy();
-      });
-
-      it('should return false when pipeline failed and required to succeed', () => {
-        vm.mr.onlyAllowMergeIfPipelineSucceeds = true;
-        vm.mr.isPipelinePassing = false;
-        expect(vm.isMergeAllowed()).toBeFalsy();
-      });
-
-      it('should return true when pipeline succeeded and required to succeed', () => {
-        vm.mr.onlyAllowMergeIfPipelineSucceeds = true;
-        vm.mr.isPipelinePassing = true;
-        expect(vm.isMergeAllowed()).toBeTruthy();
-      });
-    });
-
     describe('shouldShowMergeControls', () => {
       it('should return false when an external pipeline is running and required to succeed', () => {
-        spyOn(vm, 'isMergeAllowed').and.returnValue(false);
+        vm.mr.isMergeAllowed = false;
         vm.mr.isPipelineActive = false;
         expect(vm.shouldShowMergeControls()).toBeFalsy();
       });
 
       it('should return true when the build succeeded or build not required to succeed', () => {
-        spyOn(vm, 'isMergeAllowed').and.returnValue(true);
+        vm.mr.isMergeAllowed = true;
         vm.mr.isPipelineActive = false;
         expect(vm.shouldShowMergeControls()).toBeTruthy();
       });
 
       it('should return true when showing the MWPS button and a pipeline is running that needs to be successful', () => {
-        spyOn(vm, 'isMergeAllowed').and.returnValue(false);
+        vm.mr.isMergeAllowed = false;
         vm.mr.isPipelineActive = true;
         expect(vm.shouldShowMergeControls()).toBeTruthy();
       });
 
       it('should return true when showing the MWPS button but not required for the pipeline to succeed', () => {
-        spyOn(vm, 'isMergeAllowed').and.returnValue(true);
+        vm.mr.isMergeAllowed = true;
         vm.mr.isPipelineActive = true;
         expect(vm.shouldShowMergeControls()).toBeTruthy();
       });