summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorShinya Maeda <shinya@gitlab.com>2019-03-29 14:56:40 +0700
committerShinya Maeda <shinya@gitlab.com>2019-04-01 14:02:57 +0700
commita7d4824ded0c5ae6a155d7bdc87a1ba71eb3df5c (patch)
tree855a0cd3c16093fb76f38738876da23da6b2ccc1
parent093629fedc43e8b481c6626765e3fcf0603add17 (diff)
downloadgitlab-ce-check-mergeability-in-merge-to-ref-service.tar.gz
Check mergeability in merge to ref servicecheck-mergeability-in-merge-to-ref-service
and add spec Add changelog ok ok
-rw-r--r--app/models/merge_request.rb3
-rw-r--r--app/services/merge_requests/merge_to_ref_service.rb4
-rw-r--r--changelogs/unreleased/check-mergeability-in-merge-to-ref-service.yml5
-rw-r--r--spec/models/merge_request_spec.rb32
-rw-r--r--spec/services/merge_requests/merge_to_ref_service_spec.rb20
5 files changed, 50 insertions, 14 deletions
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)