diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index b52c480a487..4b1b3c365a5 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -807,8 +807,7 @@ class MergeRequest < ApplicationRecord end def mergeable_to_ref? - return false if merged? - return false if broken? + return false unless mergeable_state?(skip_ci_check: true, skip_discussions_check: true) # Given the `merge_ref_path` will have the same # state the `target_branch` would have. Ideally diff --git a/app/services/merge_requests/merge_to_ref_service.rb b/app/services/merge_requests/merge_to_ref_service.rb index 69cc441f1bb..87147d90c32 100644 --- a/app/services/merge_requests/merge_to_ref_service.rb +++ b/app/services/merge_requests/merge_to_ref_service.rb @@ -41,9 +41,7 @@ module MergeRequests super error = - if Feature.disabled?(:merge_to_tmp_merge_ref_path, project) - 'Feature is not enabled' - elsif !hooks_validation_pass?(merge_request) + if !hooks_validation_pass?(merge_request) hooks_validation_error(merge_request) elsif !@merge_request.mergeable_to_ref? "Merge request is not mergeable to #{target_ref}" diff --git a/changelogs/unreleased/check-mergeability-in-merge-to-ref-service.yml b/changelogs/unreleased/check-mergeability-in-merge-to-ref-service.yml new file mode 100644 index 00000000000..9f615bbb54a --- /dev/null +++ b/changelogs/unreleased/check-mergeability-in-merge-to-ref-service.yml @@ -0,0 +1,5 @@ +--- +title: Check mergeability in MergeToRefService +merge_request: 26757 +author: +type: changed diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 5adeb616e84..138036ab3c2 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -3081,6 +3081,38 @@ describe MergeRequest do end end + describe '#mergeable_to_ref?' do + it 'returns true when merge request is mergeable' do + subject = create(:merge_request) + + expect(subject.mergeable_to_ref?).to be(true) + end + + it 'returns false when merge request is already merged' do + subject = create(:merge_request, :merged) + + expect(subject.mergeable_to_ref?).to be(false) + end + + it 'returns false when merge request is closed' do + subject = create(:merge_request, :closed) + + expect(subject.mergeable_to_ref?).to be(false) + end + + it 'returns false when merge request is work in progress' do + subject = create(:merge_request, title: 'WIP: The feature') + + expect(subject.mergeable_to_ref?).to be(false) + end + + it 'returns false when merge request has no commits' do + subject = create(:merge_request, source_branch: 'empty-branch', target_branch: 'master') + + expect(subject.mergeable_to_ref?).to be(false) + end + end + describe '#merge_participants' do it 'contains author' do expect(subject.merge_participants).to eq([subject.author]) diff --git a/spec/services/merge_requests/merge_to_ref_service_spec.rb b/spec/services/merge_requests/merge_to_ref_service_spec.rb index fabca8f6b4a..a3b48abae26 100644 --- a/spec/services/merge_requests/merge_to_ref_service_spec.rb +++ b/spec/services/merge_requests/merge_to_ref_service_spec.rb @@ -40,15 +40,6 @@ describe MergeRequests::MergeToRefService do end shared_examples_for 'successfully evaluates pre-condition checks' do - it 'returns error when feature is disabled' do - stub_feature_flags(merge_to_tmp_merge_ref_path: false) - - result = service.execute(merge_request) - - expect(result[:status]).to eq(:error) - expect(result[:message]).to eq('Feature is not enabled') - end - it 'returns an error when the failing to process the merge' do allow(project.repository).to receive(:merge_to_ref).and_return(nil) @@ -180,6 +171,17 @@ describe MergeRequests::MergeToRefService do it { expect(todo).not_to be_done } end + context 'when merge request is WIP state' do + it 'fails to merge' do + merge_request = create(:merge_request, title: 'WIP: The feature') + + result = service.execute(merge_request) + + expect(result[:status]).to eq(:error) + expect(result[:message]).to eq("Merge request is not mergeable to #{merge_request.merge_ref_path}") + end + end + it 'returns error when user has no authorization to admin the merge request' do unauthorized_user = create(:user) project.add_reporter(unauthorized_user)