From 8779da45033d82f8f10d1fa6ca51d59d54d0ed2c Mon Sep 17 00:00:00 2001
From: Douwe Maan <douwe@selenight.nl>
Date: Thu, 11 Aug 2016 19:39:18 -0500
Subject: [PATCH] Don't show new MR URL after push when it doesn't make sense

---
 .../merge_requests/get_urls_service.rb        | 13 ++++++-
 .../merge_requests/get_urls_service_spec.rb   | 34 +++++++++++++++++++
 2 files changed, 46 insertions(+), 1 deletion(-)

diff --git a/app/services/merge_requests/get_urls_service.rb b/app/services/merge_requests/get_urls_service.rb
index 501fd135e16..08c1f72d65a 100644
--- a/app/services/merge_requests/get_urls_service.rb
+++ b/app/services/merge_requests/get_urls_service.rb
@@ -30,10 +30,21 @@ module MergeRequests
     end
 
     def get_branches(changes)
+      return [] if project.empty_repo?
+      return [] unless project.merge_requests_enabled
+
       changes_list = Gitlab::ChangesList.new(changes)
       changes_list.map do |change|
         next unless Gitlab::Git.branch_ref?(change[:ref])
-        Gitlab::Git.branch_name(change[:ref])
+
+        # Deleted branch
+        next if Gitlab::Git.blank_ref?(change[:newrev])
+
+        # Default branch
+        branch_name = Gitlab::Git.branch_name(change[:ref])
+        next if branch_name == project.default_branch
+
+        branch_name
       end.compact
     end
 
diff --git a/spec/services/merge_requests/get_urls_service_spec.rb b/spec/services/merge_requests/get_urls_service_spec.rb
index ec26770c3eb..8a4b76367e3 100644
--- a/spec/services/merge_requests/get_urls_service_spec.rb
+++ b/spec/services/merge_requests/get_urls_service_spec.rb
@@ -7,7 +7,9 @@ describe MergeRequests::GetUrlsService do
   let(:new_merge_request_url) { "http://localhost/#{project.namespace.name}/#{project.path}/merge_requests/new?merge_request%5Bsource_branch%5D=#{source_branch}" }
   let(:show_merge_request_url) { "http://localhost/#{project.namespace.name}/#{project.path}/merge_requests/#{merge_request.iid}" }
   let(:new_branch_changes) { "#{Gitlab::Git::BLANK_SHA} 570e7b2abdd848b95f2f578043fc23bd6f6fd24d refs/heads/#{source_branch}" }
+  let(:deleted_branch_changes) { "d14d6c0abdd253381df51a723d58691b2ee1ab08 #{Gitlab::Git::BLANK_SHA} refs/heads/#{source_branch}" }
   let(:existing_branch_changes) { "d14d6c0abdd253381df51a723d58691b2ee1ab08 570e7b2abdd848b95f2f578043fc23bd6f6fd24d refs/heads/#{source_branch}" }
+  let(:default_branch_changes) { "d14d6c0abdd253381df51a723d58691b2ee1ab08 570e7b2abdd848b95f2f578043fc23bd6f6fd24d refs/heads/master" }
 
   describe "#execute" do
     shared_examples 'new_merge_request_link' do
@@ -32,6 +34,28 @@ describe MergeRequests::GetUrlsService do
       end
     end
 
+    shared_examples 'no_merge_request_url' do
+      it 'returns no URL' do
+        result = service.execute(changes)
+        expect(result).to be_empty
+      end
+    end
+
+    context 'pushing to default branch' do
+      let(:changes) { default_branch_changes }
+      it_behaves_like 'no_merge_request_url'
+    end
+
+    context 'pushing to project with MRs disabled' do
+      let(:changes) { new_branch_changes }
+
+      before do
+        project.merge_requests_enabled = false
+      end
+
+      it_behaves_like 'no_merge_request_url'
+    end
+
     context 'pushing one completely new branch' do
       let(:changes) { new_branch_changes }
       it_behaves_like 'new_merge_request_link'
@@ -42,6 +66,11 @@ describe MergeRequests::GetUrlsService do
       it_behaves_like 'new_merge_request_link'
     end
 
+    context 'pushing to deleted branch' do
+      let(:changes) { deleted_branch_changes }
+      it_behaves_like 'no_merge_request_url'
+    end
+
     context 'pushing to existing branch and merge request opened' do
       let!(:merge_request) { create(:merge_request, source_project: project, source_branch: source_branch) }
       let(:changes) { existing_branch_changes }
@@ -61,6 +90,11 @@ describe MergeRequests::GetUrlsService do
       let(:changes) { existing_branch_changes }
       # Source project is now the forked one
       let(:service) { MergeRequests::GetUrlsService.new(forked_project) }
+
+      before do
+        allow(forked_project).to receive(:empty_repo?).and_return(false)
+      end
+
       it_behaves_like 'show_merge_request_url'
     end