Check mergeability in merge to ref service

and add spec

Add changelog

ok

ok
This commit is contained in:
Shinya Maeda 2019-03-29 14:56:40 +07:00
parent 093629fedc
commit a7d4824ded
5 changed files with 50 additions and 14 deletions

View File

@ -807,8 +807,7 @@ class MergeRequest < ApplicationRecord
end end
def mergeable_to_ref? def mergeable_to_ref?
return false if merged? return false unless mergeable_state?(skip_ci_check: true, skip_discussions_check: true)
return false if broken?
# Given the `merge_ref_path` will have the same # Given the `merge_ref_path` will have the same
# state the `target_branch` would have. Ideally # state the `target_branch` would have. Ideally

View File

@ -41,9 +41,7 @@ module MergeRequests
super super
error = error =
if Feature.disabled?(:merge_to_tmp_merge_ref_path, project) if !hooks_validation_pass?(merge_request)
'Feature is not enabled'
elsif !hooks_validation_pass?(merge_request)
hooks_validation_error(merge_request) hooks_validation_error(merge_request)
elsif !@merge_request.mergeable_to_ref? elsif !@merge_request.mergeable_to_ref?
"Merge request is not mergeable to #{target_ref}" "Merge request is not mergeable to #{target_ref}"

View File

@ -0,0 +1,5 @@
---
title: Check mergeability in MergeToRefService
merge_request: 26757
author:
type: changed

View File

@ -3081,6 +3081,38 @@ describe MergeRequest do
end end
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 describe '#merge_participants' do
it 'contains author' do it 'contains author' do
expect(subject.merge_participants).to eq([subject.author]) expect(subject.merge_participants).to eq([subject.author])

View File

@ -40,15 +40,6 @@ describe MergeRequests::MergeToRefService do
end end
shared_examples_for 'successfully evaluates pre-condition checks' do 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 it 'returns an error when the failing to process the merge' do
allow(project.repository).to receive(:merge_to_ref).and_return(nil) 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 } it { expect(todo).not_to be_done }
end 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 it 'returns error when user has no authorization to admin the merge request' do
unauthorized_user = create(:user) unauthorized_user = create(:user)
project.add_reporter(unauthorized_user) project.add_reporter(unauthorized_user)