diff options
author | Sean McGivern <sean@mcgivern.me.uk> | 2018-07-02 12:50:26 +0000 |
---|---|---|
committer | Sean McGivern <sean@mcgivern.me.uk> | 2018-07-02 12:50:26 +0000 |
commit | e7a5a3151492c0bc6b1a262e5caffd66bd25674c (patch) | |
tree | b33a271d7a8b4ffe0120da68e01feffa6f461d31 | |
parent | 601eb8df30c5d84236fee190caa6fd72b1c37cb3 (diff) | |
parent | d907b894bed848e5c60f101a3cd3172df35dd27a (diff) | |
download | gitlab-ce-e7a5a3151492c0bc6b1a262e5caffd66bd25674c.tar.gz |
Merge branch '48653-mr-target-branch-missing' into 'master'
Fix notify_conflict? raising exception due to absent branches
Closes #48653
See merge request gitlab-org/gitlab-ce!20280
-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 |