summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorOswaldo Ferreira <oswaldo@gitlab.com>2019-07-05 16:31:10 -0300
committerOswaldo Ferreira <oswaldo@gitlab.com>2019-07-09 14:36:29 -0300
commit84dd31493ec1a4d1463e13a3dc672f5cf5398aa3 (patch)
tree8db439885c6d85c1bf9e6389746c9930e877d48e
parentededb334c66fa25cb8518258d85b1f7da1dd8f26 (diff)
downloadgitlab-ce-osw-bypass-push-rules-for-merge-to-ref.tar.gz
[CE port] Bypass push rules for merge to ref serviceosw-bypass-push-rules-for-merge-to-ref
Turns out push rules to validate commit message does not apply in the context of automatic merge to the refs/merge-requests/:iid/merge. Mainly because if it fails to merge to it, we currently can't give enough preemptive feedback to the user and it'll turn the merge request unmergeable (given we automatically mark it as unmergeable if we can't merge to the ref). In general, it's a systematic operation, which already bypasses user authorization and git hooks. Therefore, this commit makes it bypass the push rules at EE as well.
-rw-r--r--app/services/merge_requests/merge_base_service.rb11
-rw-r--r--app/services/merge_requests/merge_service.rb4
-rw-r--r--app/services/merge_requests/merge_to_ref_service.rb18
-rw-r--r--spec/services/merge_requests/merge_service_spec.rb13
-rw-r--r--spec/services/merge_requests/merge_to_ref_service_spec.rb13
5 files changed, 42 insertions, 17 deletions
diff --git a/app/services/merge_requests/merge_base_service.rb b/app/services/merge_requests/merge_base_service.rb
index 095bdca5472..1ed396cee1e 100644
--- a/app/services/merge_requests/merge_base_service.rb
+++ b/app/services/merge_requests/merge_base_service.rb
@@ -28,6 +28,17 @@ module MergeRequests
private
+ def check_source
+ unless source
+ raise_error('No source for merge')
+ end
+ end
+
+ # Overridden in EE.
+ def check_size_limit
+ # No-op
+ end
+
# Overridden in EE.
def error_check!
# No-op
diff --git a/app/services/merge_requests/merge_service.rb b/app/services/merge_requests/merge_service.rb
index d8a78001b79..3e0f5aa181c 100644
--- a/app/services/merge_requests/merge_service.rb
+++ b/app/services/merge_requests/merge_service.rb
@@ -48,13 +48,13 @@ module MergeRequests
def error_check!
super
+ check_source
+
error =
if @merge_request.should_be_rebased?
'Only fast-forward merge is allowed for your project. Please update your source branch'
elsif !@merge_request.mergeable?
'Merge request is not mergeable'
- elsif !source
- 'No source for merge'
end
raise_error(error) if error
diff --git a/app/services/merge_requests/merge_to_ref_service.rb b/app/services/merge_requests/merge_to_ref_service.rb
index 0ea50a5dbf5..37b5805ae7e 100644
--- a/app/services/merge_requests/merge_to_ref_service.rb
+++ b/app/services/merge_requests/merge_to_ref_service.rb
@@ -16,7 +16,7 @@ module MergeRequests
def execute(merge_request)
@merge_request = merge_request
- validate!
+ error_check!
commit_id = commit
@@ -39,21 +39,9 @@ module MergeRequests
merge_request.diff_head_sha
end
- def validate!
- error_check!
- end
-
+ override :error_check!
def error_check!
- super
-
- error =
- if !hooks_validation_pass?(merge_request)
- hooks_validation_error(merge_request)
- elsif source.blank?
- 'No source for merge'
- end
-
- raise_error(error) if error
+ check_source
end
##
diff --git a/spec/services/merge_requests/merge_service_spec.rb b/spec/services/merge_requests/merge_service_spec.rb
index aa759ac9edc..22578436c18 100644
--- a/spec/services/merge_requests/merge_service_spec.rb
+++ b/spec/services/merge_requests/merge_service_spec.rb
@@ -214,6 +214,19 @@ describe MergeRequests::MergeService do
allow(Rails.logger).to receive(:error)
end
+ context 'when source is missing' do
+ it 'logs and saves error' do
+ allow(merge_request).to receive(:diff_head_sha) { nil }
+
+ error_message = 'No source for merge'
+
+ service.execute(merge_request)
+
+ expect(merge_request.merge_error).to eq(error_message)
+ expect(Rails.logger).to have_received(:error).with(a_string_matching(error_message))
+ end
+ end
+
it 'logs and saves error if there is an exception' do
error_message = 'error message'
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 14012b4ea2d..758679edc45 100644
--- a/spec/services/merge_requests/merge_to_ref_service_spec.rb
+++ b/spec/services/merge_requests/merge_to_ref_service_spec.rb
@@ -191,6 +191,19 @@ describe MergeRequests::MergeToRefService do
it { expect(todo).not_to be_done }
end
+ context 'when source is missing' do
+ it 'returns error' do
+ allow(merge_request).to receive(:diff_head_sha) { nil }
+
+ error_message = 'No source for merge'
+
+ result = service.execute(merge_request)
+
+ expect(result[:status]).to eq(:error)
+ expect(result[:message]).to eq(error_message)
+ end
+ end
+
context 'when target ref is passed as a parameter' do
let(:params) { { commit_message: 'merge train', target_ref: target_ref } }