Merge branch 'master' into fixes/1348-dont-open-deleted-file

Author: StanleyGoldman

src/GitHub.App/Services/PullRequestService.cs

@@ -392,7 +392,7 @@ public IObservable<Unit> Checkout(ILocalRepositoryModel repository, PullRequestD
                     {
                         await gitClient.Checkout(repo, localBranchName);
                     }
-                    else if (repository.CloneUrl.Owner == pullRequest.HeadRepositoryOwner)
+                    else if (string.Equals(repository.CloneUrl.Owner, pullRequest.HeadRepositoryOwner, StringComparison.OrdinalIgnoreCase))
                     {
                         var remote = await gitClient.GetHttpRemote(repo, "origin");
                         await gitClient.Fetch(repo, remote.Name);
@@ -524,7 +524,7 @@ public IObservable<bool> EnsureLocalBranchesAreMarkedAsPullRequests(ILocalReposi
 
         public bool IsPullRequestFromRepository(ILocalRepositoryModel repository, PullRequestDetailModel pullRequest)
         {
-            return pullRequest.HeadRepositoryOwner == repository.CloneUrl.Owner;
+            return string.Equals(repository.CloneUrl?.Owner, pullRequest.HeadRepositoryOwner, StringComparison.OrdinalIgnoreCase);
         }
 
         public IObservable<Unit> SwitchToBranch(ILocalRepositoryModel repository, PullRequestDetailModel pullRequest)

src/GitHub.App/ViewModels/GitHubPane/IssueListViewModelBase.cs

@@ -7,6 +7,7 @@
 using System.Reactive.Disposables;
 using System.Reactive.Linq;
 using System.Threading.Tasks;
+using System.Windows.Threading;
 using GitHub.Collections;
 using GitHub.Extensions;
 using GitHub.Extensions.Reactive;
@@ -209,6 +210,7 @@ public override Task Refresh()
                     this.WhenAnyValue(x => x.SelectedState),
                     this.WhenAnyValue(x => x.AuthorFilter.Selected),
                     (loading, count, _, __, ___) => Tuple.Create(loading, count))
+                .ObserveOn(RxApp.MainThreadScheduler)
                 .Subscribe(x => UpdateState(x.Item1, x.Item2)));
             dispose.Add(
                 Observable.FromEventPattern<ErrorEventArgs>(

src/GitHub.App/ViewModels/GitHubPane/PullRequestReviewSummaryViewModel.cs

@@ -41,7 +41,7 @@ public static IEnumerable<PullRequestReviewSummaryViewModel> BuildByUser(
         {
             var existing = new Dictionary<string, PullRequestReviewSummaryViewModel>();
 
-            foreach (var review in pullRequest.Reviews.OrderBy(x => x.Id))
+            foreach (var review in pullRequest.Reviews.OrderBy(x => x.SubmittedAt))
             {
                 if (review.State == PullRequestReviewState.Pending && review.Author.Login != currentUser.Login)
                     continue;
@@ -90,6 +90,7 @@ static int ToPriority(PullRequestReviewState state)
             {
                 case PullRequestReviewState.Approved:
                 case PullRequestReviewState.ChangesRequested:
+                case PullRequestReviewState.Dismissed:
                     return 1;
                 case PullRequestReviewState.Pending:
                     return 2;

src/GitHub.InlineReviews/Services/PullRequestSessionManager.cs

@@ -225,7 +225,7 @@ async Task<PullRequestSession> GetSessionInternal(string owner, string name, int
         {
             PullRequestSession session = null;
             WeakReference<PullRequestSession> weakSession;
-            var key = Tuple.Create(owner, number);
+            var key = Tuple.Create(owner.ToLowerInvariant(), number);
 
             if (sessions.TryGetValue(key, out weakSession))
             {

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

@@ -777,6 +777,35 @@ public async Task ShouldCheckoutLocalBranchAsync()
             Assert.That(4, Is.EqualTo(gitClient.ReceivedCalls().Count()));
         }
 
+        [Test]
+        public async Task ShouldCheckoutLocalBranchOwnerCaseMismatchAsync()
+        {
+            var gitClient = MockGitClient();
+            var service = CreateTarget(gitClient, MockGitService());
+
+            var localRepo = Substitute.For<ILocalRepositoryModel>();
+            localRepo.CloneUrl.Returns(new UriString("https://foo.bar/Owner/repo"));
+
+            var pr = new PullRequestDetailModel
+            {
+                Number = 5,
+                BaseRefName = "master",
+                BaseRefSha = "123",
+                BaseRepositoryOwner = "owner",
+                HeadRefName = "prbranch",
+                HeadRefSha = "123",
+                HeadRepositoryOwner = "owner",
+            };
+
+            await service.Checkout(localRepo, pr, "prbranch");
+
+            gitClient.Received().Fetch(Arg.Any<IRepository>(), "origin").Forget();
+            gitClient.Received().Checkout(Arg.Any<IRepository>(), "prbranch").Forget();
+            gitClient.Received().SetConfig(Arg.Any<IRepository>(), "branch.prbranch.ghfvs-pr-owner-number", "owner#5").Forget();
+
+            Assert.That(4, Is.EqualTo(gitClient.ReceivedCalls().Count()));
+        }
+
         [Test]
         public async Task ShouldCheckoutBranchFromForkAsync()
         {
@@ -899,6 +928,19 @@ public async Task ShouldReturnPullRequestBranchForPullRequestFromSameRepositoryA
             Assert.That("source", Is.EqualTo(result.Name));
         }
 
+        [Test]
+        public async Task ShouldReturnPullRequestBranchForPullRequestFromSameRepositoryOwnerCaseMismatchAsync()
+        {
+            var service = CreateTarget(MockGitClient(), MockGitService());
+
+            var localRepo = Substitute.For<ILocalRepositoryModel>();
+            localRepo.CloneUrl.Returns(new UriString("https://github.com/Foo/bar"));
+
+            var result = await service.GetLocalBranches(localRepo, CreatePullRequest(fromFork: false));
+
+            Assert.That("source", Is.EqualTo(result.Name));
+        }
+
         [Test]
         public async Task ShouldReturnMarkedBranchForPullRequestFromForkAsync()
         {

test/GitHub.App.UnitTests/ViewModels/GitHubPane/PullRequestDetailViewModelTests.cs

@@ -56,12 +56,13 @@ public class TheReviewsProperty : TestBaseClass
             [Test]
             public async Task ShouldShowLatestAcceptedOrChangesRequestedReviewAsync()
             {
+                var dateTimeOffset = DateTimeOffset.Now;
                 var target = CreateTarget();
                 var model = CreatePullRequestModel(
-                    CreatePullRequestReviewModel("1", "grokys", PullRequestReviewState.ChangesRequested),
-                    CreatePullRequestReviewModel("2", "shana", PullRequestReviewState.ChangesRequested),
-                    CreatePullRequestReviewModel("3", "grokys", PullRequestReviewState.Approved),
-                    CreatePullRequestReviewModel("4", "grokys", PullRequestReviewState.Commented));
+                    CreatePullRequestReviewModel("1", "grokys", PullRequestReviewState.ChangesRequested, dateTimeOffset.AddMinutes(1)),
+                    CreatePullRequestReviewModel("2", "shana", PullRequestReviewState.ChangesRequested, dateTimeOffset.AddMinutes(2)),
+                    CreatePullRequestReviewModel("3", "grokys", PullRequestReviewState.Approved, dateTimeOffset.AddMinutes(3)),
+                    CreatePullRequestReviewModel("4", "grokys", PullRequestReviewState.Commented, dateTimeOffset.AddMinutes(4)));
 
                 await target.Load(model);
 
@@ -77,10 +78,11 @@ public async Task ShouldShowLatestAcceptedOrChangesRequestedReviewAsync()
             [Test]
             public async Task ShouldShowLatestCommentedReviewIfNothingElsePresentAsync()
             {
+                var dateTimeOffset = DateTimeOffset.Now;
                 var target = CreateTarget();
                 var model = CreatePullRequestModel(
-                    CreatePullRequestReviewModel("1", "shana", PullRequestReviewState.Commented),
-                    CreatePullRequestReviewModel("2", "shana", PullRequestReviewState.Commented));
+                    CreatePullRequestReviewModel("1", "shana", PullRequestReviewState.Commented, dateTimeOffset.AddMinutes(1)),
+                    CreatePullRequestReviewModel("2", "shana", PullRequestReviewState.Commented, dateTimeOffset.AddMinutes(2)));
 
                 await target.Load(model);
 
@@ -107,9 +109,11 @@ public async Task ShouldNotShowStartNewReviewWhenHasPendingReviewAsync()
             [Test]
             public async Task ShouldShowPendingReviewOverApprovedAsync()
             {
+                var dateTimeOffset = DateTimeOffset.Now;
+
                 var target = CreateTarget();
                 var model = CreatePullRequestModel(
-                    CreatePullRequestReviewModel("1", "grokys", PullRequestReviewState.Approved),
+                    CreatePullRequestReviewModel("1", "grokys", PullRequestReviewState.Approved, dateTimeOffset.AddMinutes(1)),
                     CreatePullRequestReviewModel("2", "grokys", PullRequestReviewState.Pending));
 
                 await target.Load(model);
@@ -133,16 +137,34 @@ public async Task ShouldNotShowPendingReviewForOtherUserAsync()
                 Assert.That(target.Reviews[0].Id, Is.Null);
             }
 
+            [Test]
+            public async Task ShouldNotShowChangesRequestedAfterDismissed()
+            {
+                var dateTimeOffset = DateTimeOffset.Now;
+
+                var target = CreateTarget();
+                var model = CreatePullRequestModel(
+                    CreatePullRequestReviewModel("1", "shana", PullRequestReviewState.ChangesRequested, dateTimeOffset.AddMinutes(1)),
+                    CreatePullRequestReviewModel("2", "shana", PullRequestReviewState.Dismissed, dateTimeOffset.AddMinutes(2)));
+
+                await target.Load(model);
+
+                Assert.That(target.Reviews, Has.Count.EqualTo(2));
+                Assert.That(target.Reviews[0].User.Login, Is.EqualTo("shana"));
+                Assert.That(target.Reviews[0].State, Is.EqualTo(PullRequestReviewState.Dismissed));
+                Assert.That(target.Reviews[1].User.Login, Is.EqualTo("grokys"));
+            }
+
             static PullRequestDetailModel CreatePullRequestModel(
                 params PullRequestReviewModel[] reviews)
             {
                 return PullRequestDetailViewModelTests.CreatePullRequestModel(reviews: reviews);
             }
 
-            static PullRequestReviewModel CreatePullRequestReviewModel(
-                string id,
+            static PullRequestReviewModel CreatePullRequestReviewModel(string id,
                 string login,
-                PullRequestReviewState state)
+                PullRequestReviewState state,
+                DateTimeOffset? submittedAt = null)
             {
                 var account = new ActorModel
                 {
@@ -154,6 +176,7 @@ static PullRequestReviewModel CreatePullRequestReviewModel(
                     Id = id,
                     Author = account,
                     State = state,
+                    SubmittedAt = submittedAt
                 };
             }
         }

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

@@ -770,6 +770,19 @@ public async Task GetSessionReturnsSameSessionForSamePullRequest()
                 Assert.That(result1, Is.Not.SameAs(result3));
             }
 
+            [Test]
+            public async Task GetSessionReturnsSameSessionForSamePullRequestOwnerCaseMismatch()
+            {
+                var target = CreateTarget();
+                var newModel = CreatePullRequestModel(NotCurrentBranchPullRequestNumber);
+                var result1 = await target.GetSession("owner", "repo", 5);
+                var result2 = await target.GetSession("Owner", "repo", 5);
+                var result3 = await target.GetSession("owner", "repo", 6);
+
+                Assert.That(result1, Is.SameAs(result2));
+                Assert.That(result1, Is.Not.SameAs(result3));
+            }
+
             [Test]
             public async Task SessionCanBeCollected()
             {