Avoid naming clashes in local PR branch names.

Author: grokys

src/GitHub.App/Services/PullRequestService.cs

@@ -88,9 +88,19 @@ public IObservable<Unit> FetchAndCheckout(ILocalRepositoryModel repository, int
             return DoFetchAndCheckout(repository, pullRequestNumber, localBranchName).ToObservable();
         }
 
-        public string GetDefaultLocalBranchName(int pullRequestNumber, string pullRequestTitle)
+        public string GetDefaultLocalBranchName(ILocalRepositoryModel repository, int pullRequestNumber, string pullRequestTitle)
         {
-            return "pr/" + pullRequestNumber + "-" + GetSafeBranchName(pullRequestTitle);
+            var initial = "pr/" + pullRequestNumber + "-" + GetSafeBranchName(pullRequestTitle);
+            var current = initial;
+            var repo = gitService.GetRepository(repository.LocalPath);
+            var index = 2;
+
+            while (repo.Branches[current] != null)
+            {
+                current = initial + '-' + index++;
+            }
+
+            return current;
         }
 
         public IObservable<HistoryDivergence> CalculateHistoryDivergence(ILocalRepositoryModel repository, int pullRequestNumber)

src/GitHub.App/ViewModels/PullRequestDetailViewModel.cs

@@ -465,7 +465,7 @@ IObservable<Unit> SwitchToBranch()
 
         IObservable<Unit> FetchAndCheckout()
         {
-            var branchName = pullRequestsService.GetDefaultLocalBranchName(Number, Title);
+            var branchName = pullRequestsService.GetDefaultLocalBranchName(repository, Number, Title);
 
             return pullRequestsService.FetchAndCheckout(repository, Number, branchName)
                 .Catch<Unit, Exception>(ex =>

src/GitHub.Exports.Reactive/Services/IPullRequestService.cs

@@ -22,13 +22,13 @@ IObservable<IPullRequestModel> CreatePullRequest(IRepositoryHost host,
         IObservable<Unit> FetchAndCheckout(ILocalRepositoryModel repository, int pullRequestNumber, string localBranchName);
 
         /// <summary>
-        /// Gets the name of the local branch that a call to <see cref="Checkout(ILocalRepositoryModel, IPullRequestModel)"/>
-        /// will check out to.
+        /// Calculates the name of a local branch for a pull request avoiding clashes with existing branches.
         /// </summary>
+        /// <param name="repository">The repository.</param>
         /// <param name="pullRequestNumber">The pull request number.</param>
         /// <param name="pullRequestTitle">The pull request title.</param>
         /// <returns></returns>
-        string GetDefaultLocalBranchName(int pullRequestNumber, string pullRequestTitle);
+        string GetDefaultLocalBranchName(ILocalRepositoryModel repository, int pullRequestNumber, string pullRequestTitle);
 
         /// <summary>
         /// Gets the local branches that exist for the specified pull request.

src/UnitTests/GitHub.App/Services/PullRequestServiceTests.cs

@@ -6,6 +6,9 @@
 using GitHub.Models;
 using System;
 using GitHub.Services;
+using Rothko;
+using LibGit2Sharp;
+using System.Collections.Generic;
 
 public class PullRequestServiceTests : TestBaseClass
 {
@@ -49,4 +52,59 @@ public void CreatePullRequestAllArgsMandatory()
         Assert.NotNull(pr);
     }
 
+    public class TheGetDefaultLocalBranchNameMethod
+    {
+        [Fact]
+        public void ShouldReturnCorrectDefaultLocalBranchName()
+        {
+            var service = new PullRequestService(
+                Substitute.For<IGitClient>(),
+                MockGitService(),
+                Substitute.For<IOperatingSystem>(),
+                Substitute.For<IUsageTracker>());
+
+            var localRepo = Substitute.For<ILocalRepositoryModel>();
+            var result = service.GetDefaultLocalBranchName(localRepo, 123, "Pull requests can be \"named\" all sorts of thing's (sic)");
+            Assert.Equal("pr/123-pull-requests-can-be-named-all-sorts-of-thing-s-sic-", result);
+        }
+
+        [Fact]
+        public void DefaultLocalBranchNameShouldNotClashWithExistingBranchNames()
+        {
+            var service = new PullRequestService(
+                Substitute.For<IGitClient>(),
+                MockGitService(),
+                Substitute.For<IOperatingSystem>(),
+                Substitute.For<IUsageTracker>());
+
+            var localRepo = Substitute.For<ILocalRepositoryModel>();
+            var result = service.GetDefaultLocalBranchName(localRepo, 123, "foo1");
+            Assert.Equal("pr/123-foo1-3", result);
+        }
+
+        private static IGitService MockGitService()
+        {
+            var repository = Substitute.For<IRepository>();
+            var branches = MockBranches("pr/123-foo1", "pr/123-foo1-2");
+            repository.Branches.Returns(branches);
+
+            var result = Substitute.For<IGitService>();
+            result.GetRepository(Arg.Any<string>()).Returns(repository);
+            return result;
+        }
+    }
+
+    private static BranchCollection MockBranches(params string[] names)
+    {
+        var result = Substitute.For<BranchCollection>();
+
+        foreach (var name in names)
+        {
+            var branch = Substitute.For<Branch>();
+            branch.CanonicalName.Returns("refs/heads/" + name);
+            result[name].Returns(branch);
+        }
+
+        return result;
+    }
 }