Allow PR sessions to close on non-PR branch

The CurrentSession will be set to null when user moves to non-PR branch.

Author: jcansdale

src/GitHub.InlineReviews/Services/PullRequestSessionManager.cs

@@ -198,22 +198,31 @@ async Task RepoChanged(ILocalRepositoryModel localRepositoryModel)
                     repository = localRepositoryModel;
                     CurrentSession = null;
                     sessions.Clear();
-                }
 
-                if (string.IsNullOrWhiteSpace(localRepositoryModel?.CloneUrl)) return;
+                    if (localRepositoryModel == null)
+                    {
+                        return;
+                    }
+
+                    if (string.IsNullOrWhiteSpace(localRepositoryModel.CloneUrl))
+                    {
+                        return;
+                    }
+                }
 
-                var modelService = await connectionManager.GetModelService(repository, modelServiceFactory);
                 var session = CurrentSession;
 
-                if (modelService != null)
+                var pr = await service.GetPullRequestForCurrentBranch(localRepositoryModel).FirstOrDefaultAsync();
+                if (pr != null)
                 {
-                    var pr = await service.GetPullRequestForCurrentBranch(localRepositoryModel).FirstOrDefaultAsync();
+                    var changePR =
+                        pr.Item1 != (session?.PullRequest.Base.RepositoryCloneUrl.Owner) ||
+                        pr.Item2 != (session?.PullRequest.Number);
 
-                    if (pr?.Item1 != (CurrentSession?.PullRequest.Base.RepositoryCloneUrl.Owner) ||
-                        pr?.Item2 != (CurrentSession?.PullRequest.Number))
+                    if (changePR)
                     {
-                        var pullRequest = await GetPullRequestForTip(modelService, localRepositoryModel);
-
+                        var modelService = await connectionManager.GetModelService(repository, modelServiceFactory);
+                        var pullRequest = await modelService?.GetPullRequest(pr.Item1, localRepositoryModel.Name, pr.Item2);
                         if (pullRequest != null)
                         {
                             var newSession = await GetSessionInternal(pullRequest);
@@ -235,17 +244,6 @@ async Task RepoChanged(ILocalRepositoryModel localRepositoryModel)
             }
         }
 
-        async Task<IPullRequestModel> GetPullRequestForTip(IModelService modelService, ILocalRepositoryModel localRepositoryModel)
-        {
-            if (modelService != null)
-            {
-                var pr = await service.GetPullRequestForCurrentBranch(localRepositoryModel);
-                if (pr != null) return await modelService.GetPullRequest(pr.Item1, localRepositoryModel.Name, pr.Item2).ToTask();
-            }
-
-            return null;
-        }
-
         async Task<PullRequestSession> GetSessionInternal(IPullRequestModel pullRequest)
         {
             PullRequestSession session = null;

test/GitHub.InlineReviews.UnitTests/Services/PullRequestSessionManagerTests.cs

@@ -127,6 +127,60 @@ public void CurrentSessionChangesWhenBranchChanges()
                 Assert.That(session, Is.Not.SameAs(target.CurrentSession));
             }
 
+            [Test]
+            public void LocalRepositoryModelNull()
+            {
+                var repositoryModel = null as LocalRepositoryModel;
+                var target = new PullRequestSessionManager(
+                    CreatePullRequestService(),
+                    Substitute.For<IPullRequestSessionService>(),
+                    CreateConnectionManager(),
+                    CreateModelServiceFactory(),
+                    CreateTeamExplorerContext(null));
+
+                Assert.That(target.CurrentSession, Is.Null);
+            }
+
+            [Test]
+            public void CurrentSessionChangesToNullIfNoPullRequestForCurrentBranch()
+            {
+                var service = CreatePullRequestService();
+                var teamExplorerContext = CreateTeamExplorerContext(CreateRepositoryModel());
+                var target = new PullRequestSessionManager(
+                    service,
+                    Substitute.For<IPullRequestSessionService>(),
+                    CreateConnectionManager(),
+                    CreateModelServiceFactory(),
+                    teamExplorerContext);
+                Assert.That(target.CurrentSession, Is.Not.Null);
+
+                Tuple<string, int> newPullRequest = null;
+                service.GetPullRequestForCurrentBranch(null).ReturnsForAnyArgs(Observable.Return(newPullRequest));
+                teamExplorerContext.StatusChanged += Raise.Event();
+
+                var session = target.CurrentSession;
+
+                Assert.That(session, Is.Null);
+            }
+
+            [Test]
+            public void CurrentSessionChangesToNullWhenRepoChangedToNull()
+            {
+                var teamExplorerContext = CreateTeamExplorerContext(CreateRepositoryModel());
+                var target = new PullRequestSessionManager(
+                    CreatePullRequestService(),
+                    Substitute.For<IPullRequestSessionService>(),
+                    CreateConnectionManager(),
+                    CreateModelServiceFactory(),
+                    teamExplorerContext);
+                Assert.That(target.CurrentSession, Is.Not.Null);
+
+                SetActiveRepository(teamExplorerContext, null);
+                var session = target.CurrentSession;
+
+                Assert.That(session, Is.Null);
+            }
+
             [Test]
             public void CurrentSessionChangesWhenRepoChanged()
             {