Commit 84dd3149 authored by Oswaldo Ferreira's avatar Oswaldo Ferreira

[CE port] Bypass push rules for merge to ref service

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.
parent ededb334
...@@ -28,6 +28,17 @@ module MergeRequests ...@@ -28,6 +28,17 @@ module MergeRequests
private 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. # Overridden in EE.
def error_check! def error_check!
# No-op # No-op
......
...@@ -48,13 +48,13 @@ module MergeRequests ...@@ -48,13 +48,13 @@ module MergeRequests
def error_check! def error_check!
super super
check_source
error = error =
if @merge_request.should_be_rebased? if @merge_request.should_be_rebased?
'Only fast-forward merge is allowed for your project. Please update your source branch' 'Only fast-forward merge is allowed for your project. Please update your source branch'
elsif !@merge_request.mergeable? elsif !@merge_request.mergeable?
'Merge request is not mergeable' 'Merge request is not mergeable'
elsif !source
'No source for merge'
end end
raise_error(error) if error raise_error(error) if error
......
...@@ -16,7 +16,7 @@ module MergeRequests ...@@ -16,7 +16,7 @@ module MergeRequests
def execute(merge_request) def execute(merge_request)
@merge_request = merge_request @merge_request = merge_request
validate! error_check!
commit_id = commit commit_id = commit
...@@ -39,21 +39,9 @@ module MergeRequests ...@@ -39,21 +39,9 @@ module MergeRequests
merge_request.diff_head_sha merge_request.diff_head_sha
end end
def validate! override :error_check!
error_check!
end
def error_check! def error_check!
super check_source
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
end end
## ##
......
...@@ -214,6 +214,19 @@ describe MergeRequests::MergeService do ...@@ -214,6 +214,19 @@ describe MergeRequests::MergeService do
allow(Rails.logger).to receive(:error) allow(Rails.logger).to receive(:error)
end 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 it 'logs and saves error if there is an exception' do
error_message = 'error message' error_message = 'error message'
......
...@@ -191,6 +191,19 @@ describe MergeRequests::MergeToRefService do ...@@ -191,6 +191,19 @@ describe MergeRequests::MergeToRefService do
it { expect(todo).not_to be_done } it { expect(todo).not_to be_done }
end 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 context 'when target ref is passed as a parameter' do
let(:params) { { commit_message: 'merge train', target_ref: target_ref } } let(:params) { { commit_message: 'merge train', target_ref: target_ref } }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment