Commit d907b894 authored by Mark Chao's avatar Mark Chao

Fix notify_conflict? raising exception when branches do not exist

repository.can_be_merged? can raise error if diff_head_sha or
target_branch are absent, therefore check those explicitly.
parent 3c0ff4dd
...@@ -128,14 +128,9 @@ class MergeRequest < ActiveRecord::Base ...@@ -128,14 +128,9 @@ class MergeRequest < ActiveRecord::Base
end end
after_transition unchecked: :cannot_be_merged do |merge_request, transition| after_transition unchecked: :cannot_be_merged do |merge_request, transition|
begin if merge_request.notify_conflict?
if merge_request.notify_conflict? NotificationService.new.merge_request_unmergeable(merge_request)
NotificationService.new.merge_request_unmergeable(merge_request) TodoService.new.merge_request_became_unmergeable(merge_request)
TodoService.new.merge_request_became_unmergeable(merge_request)
end
rescue Gitlab::Git::CommandError
# Checking mergeability can trigger exception, e.g. non-utf8
# We ignore this type of errors.
end end
end end
...@@ -707,7 +702,14 @@ class MergeRequest < ActiveRecord::Base ...@@ -707,7 +702,14 @@ class MergeRequest < ActiveRecord::Base
end end
def notify_conflict? def notify_conflict?
(opened? || locked?) && !project.repository.can_be_merged?(diff_head_sha, target_branch) (opened? || locked?) &&
has_commits? &&
!branch_missing? &&
!project.repository.can_be_merged?(diff_head_sha, target_branch)
rescue Gitlab::Git::CommandError
# Checking mergeability can trigger exception, e.g. non-utf8
# We ignore this type of errors.
false
end end
def related_notes def related_notes
......
---
title: Fix merge request page rendering error when its target/source branch is missing
merge_request: 20280
author:
type: fixed
...@@ -2190,6 +2190,22 @@ describe MergeRequest do ...@@ -2190,6 +2190,22 @@ describe MergeRequest do
end end
end end
end end
context 'source branch is missing' do
subject { create(:merge_request, :invalid, :opened, merge_status: :unchecked, target_branch: 'master') }
before do
allow(subject.project.repository).to receive(:can_be_merged?).and_call_original
end
it 'does not raise error' do
expect(notification_service).not_to receive(:merge_request_unmergeable)
expect(todo_service).not_to receive(:merge_request_became_unmergeable)
expect { subject.mark_as_unmergeable }.not_to raise_error
expect(subject.cannot_be_merged?).to eq(true)
end
end
end end
describe 'check_state?' do describe 'check_state?' do
......
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