diff options
author | Eric Eastwood <contact@ericeastwood.com> | 2017-10-02 11:11:18 -0500 |
---|---|---|
committer | Eric Eastwood <contact@ericeastwood.com> | 2017-10-05 01:46:16 -0500 |
commit | 6a5b2fe8943190cff40e6e2c5dbc629fa4540151 (patch) | |
tree | 220edd683a2837209e64cfc38c8c2f1d5225f2d4 | |
parent | 8921af39e74976e37e92c786bd957883110f6522 (diff) | |
download | gitlab-ce-6a5b2fe8943190cff40e6e2c5dbc629fa4540151.tar.gz |
Allow merge when no pipeline success38389-allow-merge-without-success
Fix https://gitlab.com/gitlab-org/gitlab-ce/issues/38389
7 files changed, 38 insertions, 39 deletions
diff --git a/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_ready_to_merge.js b/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_ready_to_merge.js index edc1d191bf2..61734163b6e 100644 --- a/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_ready_to_merge.js +++ b/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; diff --git a/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js b/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js index e554082149b..c1f7e64f580 100644 --- a/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js +++ b/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 diff --git a/app/serializers/merge_request_entity.rb b/app/serializers/merge_request_entity.rb index 36537c5bd02..297a459e394 100644 --- a/app/serializers/merge_request_entity.rb +++ b/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| diff --git a/changelogs/unreleased/38389-allow-merge-without-success.yml b/changelogs/unreleased/38389-allow-merge-without-success.yml new file mode 100644 index 00000000000..6a37bcc55fc --- /dev/null +++ b/changelogs/unreleased/38389-allow-merge-without-success.yml @@ -0,0 +1,6 @@ +--- +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 diff --git a/spec/features/merge_requests/widget_spec.rb b/spec/features/merge_requests/widget_spec.rb index 791cfa308c3..ab1353e3369 100644 --- a/spec/features/merge_requests/widget_spec.rb +++ b/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( diff --git a/spec/fixtures/api/schemas/entities/merge_request.json b/spec/fixtures/api/schemas/entities/merge_request.json index 30b4e56bc98..ba094ba1657 100644 --- a/spec/fixtures/api/schemas/entities/merge_request.json +++ b/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" }, diff --git a/spec/javascripts/vue_mr_widget/components/states/mr_widget_ready_to_merge_spec.js b/spec/javascripts/vue_mr_widget/components/states/mr_widget_ready_to_merge_spec.js index c83b947579b..d7019ea408b 100644 --- a/spec/javascripts/vue_mr_widget/components/states/mr_widget_ready_to_merge_spec.js +++ b/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(); }); |