diff options
author | Mark Chao <mchao@gitlab.com> | 2018-06-30 12:31:36 +0800 |
---|---|---|
committer | Mark Chao <mchao@gitlab.com> | 2018-07-02 19:42:23 +0800 |
commit | d907b894bed848e5c60f101a3cd3172df35dd27a (patch) | |
tree | 666c0c04be2d09fe9e081686c5d07f0a3b41146b | |
parent | 3c0ff4ddb9a910a2da0472cfc6ced745a421a623 (diff) | |
download | gitlab-ce-d907b894bed848e5c60f101a3cd3172df35dd27a.tar.gz |
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.
-rw-r--r-- | app/models/merge_request.rb | 20 | ||||
-rw-r--r-- | changelogs/unreleased/48653-mr-target-branch-missing.yml | 5 | ||||
-rw-r--r-- | spec/models/merge_request_spec.rb | 16 |
3 files changed, 32 insertions, 9 deletions
diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 6c96c8ca391..b4090fd8baf 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -128,14 +128,9 @@ class MergeRequest < ActiveRecord::Base end after_transition unchecked: :cannot_be_merged do |merge_request, transition| - begin - if merge_request.notify_conflict? - NotificationService.new.merge_request_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. + if merge_request.notify_conflict? + NotificationService.new.merge_request_unmergeable(merge_request) + TodoService.new.merge_request_became_unmergeable(merge_request) end end @@ -707,7 +702,14 @@ class MergeRequest < ActiveRecord::Base end 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 def related_notes diff --git a/changelogs/unreleased/48653-mr-target-branch-missing.yml b/changelogs/unreleased/48653-mr-target-branch-missing.yml new file mode 100644 index 00000000000..c2b342b87d2 --- /dev/null +++ b/changelogs/unreleased/48653-mr-target-branch-missing.yml @@ -0,0 +1,5 @@ +--- +title: Fix merge request page rendering error when its target/source branch is missing +merge_request: 20280 +author: +type: fixed diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index ec72fefd137..8c6b411ec9a 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -2190,6 +2190,22 @@ describe MergeRequest do 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 describe 'check_state?' do |