Commit 6a5b2fe8 authored by Eric Eastwood's avatar Eric Eastwood

Allow merge when no pipeline success

Fix https://gitlab.com/gitlab-org/gitlab-ce/issues/38389
parent 8921af39
...@@ -67,7 +67,7 @@ export default { ...@@ -67,7 +67,7 @@ export default {
return defaultClass; return defaultClass;
}, },
iconClass() { 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 'failed';
} }
return 'success'; return 'success';
...@@ -100,13 +100,8 @@ export default { ...@@ -100,13 +100,8 @@ export default {
}, },
}, },
methods: { methods: {
isMergeAllowed() {
return !this.mr.onlyAllowMergeIfPipelineSucceeds ||
this.mr.isPipelinePassing ||
this.mr.isPipelineSkipped;
},
shouldShowMergeControls() { shouldShowMergeControls() {
return this.isMergeAllowed() || this.shouldShowMergeWhenPipelineSucceedsText; return this.mr.isMergeAllowed || this.shouldShowMergeWhenPipelineSucceedsText;
}, },
updateCommitMessage() { updateCommitMessage() {
const cmwd = this.mr.commitMessageWithDescription; const cmwd = this.mr.commitMessageWithDescription;
......
...@@ -73,6 +73,7 @@ export default class MergeRequestStore { ...@@ -73,6 +73,7 @@ export default class MergeRequestStore {
this.canCancelAutomaticMerge = !!data.cancel_merge_when_pipeline_succeeds_path; this.canCancelAutomaticMerge = !!data.cancel_merge_when_pipeline_succeeds_path;
this.hasSHAChanged = this.sha !== data.diff_head_sha; this.hasSHAChanged = this.sha !== data.diff_head_sha;
this.canBeMerged = data.can_be_merged || false; this.canBeMerged = data.can_be_merged || false;
this.isMergeAllowed = data.mergeable || false;
this.mergeOngoing = data.merge_ongoing; this.mergeOngoing = data.merge_ongoing;
// Cherry-pick and Revert actions related // Cherry-pick and Revert actions related
......
...@@ -42,6 +42,7 @@ class MergeRequestEntity < IssuableEntity ...@@ -42,6 +42,7 @@ class MergeRequestEntity < IssuableEntity
expose :commits_count expose :commits_count
expose :cannot_be_merged?, as: :has_conflicts expose :cannot_be_merged?, as: :has_conflicts
expose :can_be_merged?, as: :can_be_merged expose :can_be_merged?, as: :can_be_merged
expose :mergeable?, as: :mergeable
expose :remove_source_branch?, as: :remove_source_branch expose :remove_source_branch?, as: :remove_source_branch
expose :project_archived do |merge_request| expose :project_archived do |merge_request|
......
---
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
...@@ -3,10 +3,13 @@ require 'rails_helper' ...@@ -3,10 +3,13 @@ require 'rails_helper'
describe 'Merge request', :js do describe 'Merge request', :js do
let(:user) { create(:user) } let(:user) { create(:user) }
let(:project) { create(:project, :repository) } 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) { create(:merge_request, source_project: project) }
let(:merge_request_in_only_mwps_project) { create(:merge_request, source_project: project_only_mwps) }
before do before do
project.team << [user, :master] project.add_master(user)
project_only_mwps.add_master(user)
sign_in(user) sign_in(user)
end end
...@@ -160,6 +163,20 @@ describe 'Merge request', :js do ...@@ -160,6 +163,20 @@ describe 'Merge request', :js do
end end
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 context 'view merge request with MWPS enabled but automatically merge fails' do
before do before do
merge_request.update( merge_request.update(
......
...@@ -46,6 +46,7 @@ ...@@ -46,6 +46,7 @@
"branch_missing": { "type": "boolean" }, "branch_missing": { "type": "boolean" },
"has_conflicts": { "type": "boolean" }, "has_conflicts": { "type": "boolean" },
"can_be_merged": { "type": "boolean" }, "can_be_merged": { "type": "boolean" },
"mergeable": { "type": "boolean" },
"project_archived": { "type": "boolean" }, "project_archived": { "type": "boolean" },
"only_allow_merge_if_pipeline_succeeds": { "type": "boolean" }, "only_allow_merge_if_pipeline_succeeds": { "type": "boolean" },
"has_ci": { "type": "boolean" }, "has_ci": { "type": "boolean" },
......
...@@ -12,6 +12,7 @@ const createComponent = (customConfig = {}) => { ...@@ -12,6 +12,7 @@ const createComponent = (customConfig = {}) => {
pipeline: null, pipeline: null,
isPipelineFailed: false, isPipelineFailed: false,
isPipelinePassing: false, isPipelinePassing: false,
isMergeAllowed: true,
onlyAllowMergeIfPipelineSucceeds: false, onlyAllowMergeIfPipelineSucceeds: false,
hasCI: false, hasCI: false,
ciStatus: null, ciStatus: null,
...@@ -212,21 +213,24 @@ describe('MRWidgetReadyToMerge', () => { ...@@ -212,21 +213,24 @@ describe('MRWidgetReadyToMerge', () => {
describe('isMergeButtonDisabled', () => { describe('isMergeButtonDisabled', () => {
it('should return false with initial data', () => { it('should return false with initial data', () => {
vm.mr.isMergeAllowed = true;
expect(vm.isMergeButtonDisabled).toBeFalsy(); expect(vm.isMergeButtonDisabled).toBeFalsy();
}); });
it('should return true when there is no commit message', () => { it('should return true when there is no commit message', () => {
vm.mr.isMergeAllowed = true;
vm.commitMessage = ''; vm.commitMessage = '';
expect(vm.isMergeButtonDisabled).toBeTruthy(); expect(vm.isMergeButtonDisabled).toBeTruthy();
}); });
it('should return true if merge is not allowed', () => { it('should return true if merge is not allowed', () => {
vm.mr.isMergeAllowed = false;
vm.mr.onlyAllowMergeIfPipelineSucceeds = true; vm.mr.onlyAllowMergeIfPipelineSucceeds = true;
vm.mr.isPipelineFailed = true;
expect(vm.isMergeButtonDisabled).toBeTruthy(); expect(vm.isMergeButtonDisabled).toBeTruthy();
}); });
it('should return true when the vm instance is making request', () => { it('should return true when the vm instance is making request', () => {
vm.mr.isMergeAllowed = true;
vm.isMakingRequest = true; vm.isMakingRequest = true;
expect(vm.isMergeButtonDisabled).toBeTruthy(); expect(vm.isMergeButtonDisabled).toBeTruthy();
}); });
...@@ -234,53 +238,27 @@ describe('MRWidgetReadyToMerge', () => { ...@@ -234,53 +238,27 @@ describe('MRWidgetReadyToMerge', () => {
}); });
describe('methods', () => { 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', () => { describe('shouldShowMergeControls', () => {
it('should return false when an external pipeline is running and required to succeed', () => { 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; vm.mr.isPipelineActive = false;
expect(vm.shouldShowMergeControls()).toBeFalsy(); expect(vm.shouldShowMergeControls()).toBeFalsy();
}); });
it('should return true when the build succeeded or build not required to succeed', () => { 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; vm.mr.isPipelineActive = false;
expect(vm.shouldShowMergeControls()).toBeTruthy(); expect(vm.shouldShowMergeControls()).toBeTruthy();
}); });
it('should return true when showing the MWPS button and a pipeline is running that needs to be successful', () => { 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; vm.mr.isPipelineActive = true;
expect(vm.shouldShowMergeControls()).toBeTruthy(); expect(vm.shouldShowMergeControls()).toBeTruthy();
}); });
it('should return true when showing the MWPS button but not required for the pipeline to succeed', () => { 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; vm.mr.isPipelineActive = true;
expect(vm.shouldShowMergeControls()).toBeTruthy(); expect(vm.shouldShowMergeControls()).toBeTruthy();
}); });
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment