Add a safeguard in MergeRequest#compute_diverged_commits_count
We have to ensure source_sha and target_sha are not nil before calling Gitlab::Git::Commit.between.
This commit is contained in:
parent
4a8a8282d9
commit
bc92de8f03
|
@ -516,7 +516,7 @@ class MergeRequest < ActiveRecord::Base
|
|||
end
|
||||
|
||||
def target_sha
|
||||
@target_sha ||= target_project.repository.commit(target_branch).sha
|
||||
@target_sha ||= target_project.repository.commit(target_branch).try(:sha)
|
||||
end
|
||||
|
||||
def source_sha
|
||||
|
@ -572,8 +572,11 @@ class MergeRequest < ActiveRecord::Base
|
|||
end
|
||||
|
||||
def compute_diverged_commits_count
|
||||
return 0 unless source_sha && target_sha
|
||||
|
||||
Gitlab::Git::Commit.between(target_project.repository.raw_repository, source_sha, target_sha).size
|
||||
end
|
||||
private :compute_diverged_commits_count
|
||||
|
||||
def diverged_from_target_branch?
|
||||
diverged_commits_count > 0
|
||||
|
|
|
@ -34,7 +34,7 @@
|
|||
%span into
|
||||
= link_to namespace_project_commits_path(@project.namespace, @project, @merge_request.target_branch), class: "label-branch" do
|
||||
= @merge_request.target_branch
|
||||
- if @merge_request.mergeable? && @merge_request.diverged_from_target_branch?
|
||||
- if @merge_request.open? && @merge_request.diverged_from_target_branch?
|
||||
%span (#{pluralize(@merge_request.diverged_commits_count, 'commit')} behind)
|
||||
|
||||
= render "projects/merge_requests/show/how_to_merge"
|
||||
|
|
|
@ -86,6 +86,16 @@ describe MergeRequest, models: true do
|
|||
end
|
||||
end
|
||||
|
||||
describe '#target_sha' do
|
||||
context 'when the target branch does not exist anymore' do
|
||||
subject { create(:merge_request).tap { |mr| mr.update_attribute(:target_branch, 'deleted') } }
|
||||
|
||||
it 'returns nil' do
|
||||
expect(subject.target_sha).to be_nil
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
describe '#source_sha' do
|
||||
let(:last_branch_commit) { subject.source_project.repository.commit(subject.source_branch) }
|
||||
|
||||
|
@ -310,6 +320,18 @@ describe MergeRequest, models: true do
|
|||
let(:project) { create(:project) }
|
||||
let(:fork_project) { create(:project, forked_from_project: project) }
|
||||
|
||||
context 'when the target branch does not exist anymore' do
|
||||
subject { create(:merge_request).tap { |mr| mr.update_attribute(:target_branch, 'deleted') } }
|
||||
|
||||
it 'does not crash' do
|
||||
expect{ subject.diverged_commits_count }.not_to raise_error
|
||||
end
|
||||
|
||||
it 'returns 0' do
|
||||
expect(subject.diverged_commits_count).to eq(0)
|
||||
end
|
||||
end
|
||||
|
||||
context 'diverged on same repository' do
|
||||
subject(:merge_request_with_divergence) { create(:merge_request, :diverged, source_project: project, target_project: project) }
|
||||
|
||||
|
|
Loading…
Reference in New Issue