From bc92de8f038012b284ea1cbfbb2f0950943ebd88 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?R=C3=A9my=20Coutable?= <remy@rymai.me>
Date: Fri, 18 Mar 2016 18:16:04 +0100
Subject: [PATCH] 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.
---
app/models/merge_request.rb | 5 ++++-
.../projects/merge_requests/_show.html.haml | 2 +-
spec/models/merge_request_spec.rb | 22 +++++++++++++++++++
3 files changed, 27 insertions(+), 2 deletions(-)
diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb
index 30a7bd47be7..a6140b5b04c 100644
--- a/app/models/merge_request.rb
+++ b/app/models/merge_request.rb
@@ -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
diff --git a/app/views/projects/merge_requests/_show.html.haml b/app/views/projects/merge_requests/_show.html.haml
index b9695157f68..ee5b9fd95a8 100644
--- a/app/views/projects/merge_requests/_show.html.haml
+++ b/app/views/projects/merge_requests/_show.html.haml
@@ -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"
diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb
index 654c71b6825..2165cfb7a32 100644
--- a/spec/models/merge_request_spec.rb
+++ b/spec/models/merge_request_spec.rb
@@ -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) }
--
GitLab