Add branch existence before trying to build path to branch

4 files changed, 70 insertions, 21 deletions

diff --git a/app/presenters/merge_request_presenter.rb b/app/presenters/merge_request_presenter.rb
index fdcc3ad802f..6deafd310a7 100644
--- a/app/presenters/merge_request_presenter.rb
+++ b/app/presenters/merge_request_presenter.rb
@@ -29,7 +29,7 @@ class MergeRequestPresenter < Gitlab::View::Presenter::Delegated
 
   def create_issue_to_resolve_discussions_path
     if can?(current_user, :create_issue, merge_request.project) &&
-      merge_request.project.issues_enabled?
+        merge_request.project.issues_enabled?
 
       new_namespace_project_issue_path(merge_request.project.namespace,
                                        merge_request.project,
@@ -94,6 +94,22 @@ class MergeRequestPresenter < Gitlab::View::Presenter::Delegated
     end
   end
 
+  def target_branch_path
+    if merge_request.target_branch_exists?
+      namespace_project_branch_path(merge_request.project.namespace,
+                                    merge_request.project,
+                                    merge_request.target_branch)
+    end
+  end
+
+  def source_branch_path
+    if merge_request.source_branch_exists?
+      namespace_project_branch_path(merge_request.source_project.namespace,
+                                    merge_request.source_project,
+                                    merge_request.source_branch)
+    end
+  end
+
   def closing_issues_links
     closes_issues = merge_request.closes_issues(current_user)
 
diff --git a/app/serializers/merge_request_entity.rb b/app/serializers/merge_request_entity.rb
index 01c5364c834..ad0a3584f5b 100644
--- a/app/serializers/merge_request_entity.rb
+++ b/app/serializers/merge_request_entity.rb
@@ -67,8 +67,8 @@ class MergeRequestEntity < IssuableEntity
 
   expose :current_user do
     expose :can_remove_source_branch do |merge_request|
-      merge_request.source_project &&
-        merge_request.can_remove_source_branch?(current_user)
+       merge_request.can_remove_source_branch?(current_user) &&
+         merge_request.source_branch_exists?
     end
 
     expose :can_revert_on_current_merge_request do |merge_request|
@@ -83,15 +83,11 @@ class MergeRequestEntity < IssuableEntity
   # Paths
   #
   expose :target_branch_path do |merge_request|
-    namespace_project_branch_path(merge_request.project.namespace,
-                                  merge_request.project,
-                                  merge_request.target_branch)
+    presenter(merge_request).target_branch_path
   end
 
   expose :source_branch_path do |merge_request|
-    namespace_project_branch_path(merge_request.source_project.namespace,
-                                  merge_request.source_project,
-                                  merge_request.source_branch)
+    presenter(merge_request).source_branch_path
   end
 
   expose :conflict_resolution_path do |merge_request|
diff --git a/spec/presenters/merge_request_presenter_spec.rb b/spec/presenters/merge_request_presenter_spec.rb
index 04934cbe12d..1525550c861 100644
--- a/spec/presenters/merge_request_presenter_spec.rb
+++ b/spec/presenters/merge_request_presenter_spec.rb
@@ -243,4 +243,50 @@ describe MergeRequestPresenter do
       is_expected.to be_nil
     end
   end
+
+  describe '#target_branch_path' do
+    subject do
+      described_class.new(resource, current_user: user).target_branch_path
+    end
+
+    context 'when target branch exists' do
+      it 'returns path' do
+        allow(resource).to receive(:target_branch_exists?) { true }
+
+        is_expected
+          .to eq("/#{resource.target_project.full_path}/branches/#{resource.target_branch}")
+      end
+    end
+
+    context 'when target branch does not exists' do
+      it 'returns nil' do
+        allow(resource).to receive(:target_branch_exists?) { false }
+
+        is_expected.to be_nil
+      end
+    end
+  end
+
+  describe '#source_branch_path' do
+    subject do
+      described_class.new(resource, current_user: user).source_branch_path
+    end
+
+    context 'when target branch exists' do
+      it 'returns path' do
+        allow(resource).to receive(:source_branch_exists?) { true }
+
+        is_expected
+          .to eq("/#{resource.source_project.full_path}/branches/#{resource.source_branch}")
+      end
+    end
+
+    context 'when target branch does not exists' do
+      it 'returns nil' do
+        allow(resource).to receive(:source_branch_exists?) { false }
+
+        is_expected.to be_nil
+      end
+    end
+  end
 end
diff --git a/spec/serializers/merge_request_entity_spec.rb b/spec/serializers/merge_request_entity_spec.rb
index c3016732991..190e1520609 100644
--- a/spec/serializers/merge_request_entity_spec.rb
+++ b/spec/serializers/merge_request_entity_spec.rb
@@ -41,10 +41,11 @@ describe MergeRequestEntity do
 
   it 'has important MergeRequest attributes' do
     expect(subject).to include(:diff_head_sha, :merge_commit_message,
-                               :has_conflicts, :has_ci, :merge_path, 
+                               :has_conflicts, :has_ci, :merge_path,
                                :conflict_resolution_path,
                                :cancel_merge_when_pipeline_succeeds_path,
-                               :create_issue_to_resolve_discussions_path)
+                               :create_issue_to_resolve_discussions_path,
+                               :source_branch_path, :target_branch_path)
   end
 
   it 'has email_patches_path' do
@@ -57,16 +58,6 @@ describe MergeRequestEntity do
       .to eq("/#{resource.project.full_path}/merge_requests/#{resource.iid}.diff")
   end
 
-  it 'has target_branch_path' do
-    expect(subject[:target_branch_path])
-      .to eq("/#{resource.target_project.full_path}/branches/#{resource.target_branch}")
-  end
-
-  it 'has source_branch_path' do
-    expect(subject[:source_branch_path])
-      .to eq("/#{resource.source_project.full_path}/branches/#{resource.source_branch}")
-  end
-
   it 'has merge_commit_message_with_description' do
     expect(subject[:merge_commit_message_with_description])
       .to eq(resource.merge_commit_message(include_description: true))