Merge branch 'master' into fixes/1828-pr-list-deadlock

StanleyGoldman · web-flow · commit 1c94bccf8eef · 2018-08-07T08:27:04.000-04:00
diff --git a/src/GitHub.App/ViewModels/GitHubPane/PullRequestReviewSummaryViewModel.cs b/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;
diff --git a/src/GitHub.InlineReviews/Margins/InlineCommentMarginProvider.cs b/src/GitHub.InlineReviews/Margins/InlineCommentMarginProvider.cs
@@ -1,11 +1,13 @@
 ﻿using System;
 using System.ComponentModel.Composition;
+using Microsoft.VisualStudio.Shell;
 using Microsoft.VisualStudio.Utilities;
 using Microsoft.VisualStudio.Text.Editor;
 using Microsoft.VisualStudio.Text.Tagging;
 using Microsoft.VisualStudio.Text.Classification;
-using GitHub.InlineReviews.Services;
 using GitHub.Services;
+using GitHub.VisualStudio;
+using GitHub.InlineReviews.Services;
 
 namespace GitHub.InlineReviews.Margins
 {
@@ -17,28 +19,41 @@ namespace GitHub.InlineReviews.Margins
     [TextViewRole(PredefinedTextViewRoles.Interactive)]
     internal sealed class InlineCommentMarginProvider : IWpfTextViewMarginProvider
     {
-        readonly IEditorFormatMapService editorFormatMapService;
-        readonly IViewTagAggregatorFactoryService tagAggregatorFactory;
-        readonly IInlineCommentPeekService peekService;
+        readonly Lazy<IEditorFormatMapService> editorFormatMapService;
+        readonly Lazy<IViewTagAggregatorFactoryService> tagAggregatorFactory;
+        readonly Lazy<IInlineCommentPeekService> peekService;
         readonly Lazy<IPullRequestSessionManager> sessionManager;
+        readonly UIContext uiContext;
 
         [ImportingConstructor]
         public InlineCommentMarginProvider(
-            IGitHubServiceProvider serviceProvider,
-            IEditorFormatMapService editorFormatMapService,
-            IViewTagAggregatorFactoryService tagAggregatorFactory,
-            IInlineCommentPeekService peekService)
+            Lazy<IPullRequestSessionManager> sessionManager,
+            Lazy<IEditorFormatMapService> editorFormatMapService,
+            Lazy<IViewTagAggregatorFactoryService> tagAggregatorFactory,
+            Lazy<IInlineCommentPeekService> peekService)
         {
+            this.sessionManager = sessionManager;
             this.editorFormatMapService = editorFormatMapService;
             this.tagAggregatorFactory = tagAggregatorFactory;
             this.peekService = peekService;
-            sessionManager = new Lazy<IPullRequestSessionManager>(() => serviceProvider.GetService<IPullRequestSessionManager>());
+
+            uiContext = UIContext.FromUIContextGuid(new Guid(Guids.UIContext_Git));
         }
 
         public IWpfTextViewMargin CreateMargin(IWpfTextViewHost wpfTextViewHost, IWpfTextViewMargin parent)
         {
+            if (!uiContext.IsActive)
+            {
+                // Only create margin when in the context of a Git repository
+                return null;
+            }
+
             return new InlineCommentMargin(
-                wpfTextViewHost, peekService, editorFormatMapService, tagAggregatorFactory, sessionManager);
+                wpfTextViewHost,
+                peekService.Value,
+                editorFormatMapService.Value,
+                tagAggregatorFactory.Value,
+                sessionManager);
         }
     }
 }
diff --git a/src/GitHub.InlineReviews/Margins/PullRequestFileMarginProvider.cs b/src/GitHub.InlineReviews/Margins/PullRequestFileMarginProvider.cs
@@ -3,6 +3,8 @@
 using GitHub.Commands;
 using GitHub.Services;
 using GitHub.Settings;
+using GitHub.VisualStudio;
+using Microsoft.VisualStudio.Shell;
 using Microsoft.VisualStudio.Utilities;
 using Microsoft.VisualStudio.Text.Editor;
 
@@ -19,25 +21,28 @@ namespace GitHub.InlineReviews.Margins
     [TextViewRole(PredefinedTextViewRoles.Editable)]
     internal sealed class PullRequestFileMarginProvider : IWpfTextViewMarginProvider
     {
-        readonly IPullRequestSessionManager sessionManager;
-        readonly IToggleInlineCommentMarginCommand enableInlineCommentsCommand;
-        readonly IGoToSolutionOrPullRequestFileCommand goToSolutionOrPullRequestFileCommand;
-        readonly IPackageSettings packageSettings;
+        readonly Lazy<IPullRequestSessionManager> sessionManager;
+        readonly Lazy<IToggleInlineCommentMarginCommand> enableInlineCommentsCommand;
+        readonly Lazy<IGoToSolutionOrPullRequestFileCommand> goToSolutionOrPullRequestFileCommand;
+        readonly Lazy<IPackageSettings> packageSettings;
         readonly Lazy<IUsageTracker> usageTracker;
+        readonly UIContext uiContext;
 
         [ImportingConstructor]
         public PullRequestFileMarginProvider(
-            IToggleInlineCommentMarginCommand enableInlineCommentsCommand,
-            IGoToSolutionOrPullRequestFileCommand goToSolutionOrPullRequestFileCommand,
-            IPullRequestSessionManager sessionManager,
-            IPackageSettings packageSettings,
+            Lazy<IToggleInlineCommentMarginCommand> enableInlineCommentsCommand,
+            Lazy<IGoToSolutionOrPullRequestFileCommand> goToSolutionOrPullRequestFileCommand,
+            Lazy<IPullRequestSessionManager> sessionManager,
+            Lazy<IPackageSettings> packageSettings,
             Lazy<IUsageTracker> usageTracker)
         {
             this.enableInlineCommentsCommand = enableInlineCommentsCommand;
             this.goToSolutionOrPullRequestFileCommand = goToSolutionOrPullRequestFileCommand;
             this.sessionManager = sessionManager;
             this.packageSettings = packageSettings;
             this.usageTracker = usageTracker;
+
+            uiContext = UIContext.FromUIContextGuid(new Guid(Guids.UIContext_Git));
         }
 
         /// <summary>
@@ -50,8 +55,14 @@ public PullRequestFileMarginProvider(
         /// </returns>
         public IWpfTextViewMargin CreateMargin(IWpfTextViewHost wpfTextViewHost, IWpfTextViewMargin marginContainer)
         {
+            if (!uiContext.IsActive)
+            {
+                // Only create margin when in the context of a Git repository
+                return null;
+            }
+
             // Comments in the editor feature flag
-            if (!packageSettings.EditorComments)
+            if (!packageSettings.Value.EditorComments)
             {
                 return null;
             }
@@ -63,7 +74,11 @@ public IWpfTextViewMargin CreateMargin(IWpfTextViewHost wpfTextViewHost, IWpfTex
             }
 
             return new PullRequestFileMargin(
-                wpfTextViewHost.TextView, enableInlineCommentsCommand, goToSolutionOrPullRequestFileCommand, sessionManager, usageTracker);
+                wpfTextViewHost.TextView,
+                enableInlineCommentsCommand.Value,
+                goToSolutionOrPullRequestFileCommand.Value,
+                sessionManager.Value,
+                usageTracker);
         }
 
         bool IsDiffView(ITextView textView) => textView.Roles.Contains("DIFF");
diff --git a/src/GitHub.TeamFoundation.14/Services/VSGitExt.cs b/src/GitHub.TeamFoundation.14/Services/VSGitExt.cs
@@ -12,6 +12,7 @@
 using Microsoft.VisualStudio.Threading;
 using Microsoft.VisualStudio.TeamFoundation.Git.Extensibility;
 using Task = System.Threading.Tasks.Task;
+using static Microsoft.VisualStudio.VSConstants;
 
 namespace GitHub.VisualStudio.Base
 {
@@ -51,9 +52,8 @@ public VSGitExt(IAsyncServiceProvider asyncServiceProvider, IVSUIContextFactory
             // Start with empty array until we have a chance to initialize.
             ActiveRepositories = Array.Empty<ILocalRepositoryModel>();
 
-            // The IGitExt service isn't available when a TFS based solution is opened directly.
-            // It will become available when moving to a Git based solution (and cause a UIContext event to fire).
-            var context = factory.GetUIContext(new Guid(Guids.GitSccProviderId));
+            // Initialize when we enter the context of a Git repository
+            var context = factory.GetUIContext(UICONTEXT.RepositoryOpen_guid);
             context.WhenActivated(() => JoinableTaskFactory.RunAsync(InitializeAsync).Task.Forget(log));
         }
 
diff --git a/src/GitHub.VisualStudio/GitContextPackage.cs b/src/GitHub.VisualStudio/GitContextPackage.cs
@@ -2,31 +2,28 @@
 using System.Threading;
 using System.Runtime.InteropServices;
 using GitHub.Exports;
-using GitHub.Logging;
 using GitHub.Services;
 using Microsoft.VisualStudio.Shell;
-using Serilog;
 using Task = System.Threading.Tasks.Task;
+using static Microsoft.VisualStudio.VSConstants;
 
 namespace GitHub.VisualStudio
 {
     /// <summary>
     /// This package creates a custom UIContext <see cref="Guids.UIContext_Git"/> that is activated when a
-    /// repository is active in <see cref="IVSGitExt"/>.
+    /// repository is active in <see cref="IVSGitExt"/> and the current process is Visual Studio (not Blend).
     /// </summary>
     [PackageRegistration(UseManagedResourcesOnly = true, AllowsBackgroundLoading = true)]
     [Guid(Guids.UIContext_Git)]
-    // this is the Git service GUID, so we load whenever it loads
-    [ProvideAutoLoad(Guids.GitSccProviderId, PackageAutoLoadFlags.BackgroundLoad)]
+    // Initialize when we enter the context of a Git repository
+    [ProvideAutoLoad(UICONTEXT.RepositoryOpen_string, PackageAutoLoadFlags.BackgroundLoad)]
     public class GitContextPackage : AsyncPackage
     {
-        static readonly ILogger log = LogManager.ForContext<GitContextPackage>();
-
         protected async override Task InitializeAsync(CancellationToken cancellationToken, IProgress<ServiceProgressData> progress)
         {
             if (!ExportForVisualStudioProcessAttribute.IsVisualStudioProcess())
             {
-                log.Warning("Don't activate 'UIContext_Git' for non-Visual Studio process");
+                // Don't activate 'UIContext_Git' for non-Visual Studio process
                 return;
             }
 
diff --git a/src/GitHub.VisualStudio/Resources/icons/mark_github.xaml b/src/GitHub.VisualStudio/Resources/icons/mark_github.xaml
@@ -1,17 +1,19 @@
-﻿<Viewbox xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
-         Width="1024" Height="1024">
+﻿<Viewbox
+        xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
+        xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
+        xmlns:PresentationOptions="http://schemas.microsoft.com/winfx/2006/xaml/presentation/options"
+        Width="1024" Height="1024">
+
+    <Viewbox.Resources>
+        <ResourceDictionary>
+            <Color x:Key="GitHubContextMenuIconColor">#424242</Color>
+            <SolidColorBrush x:Key="GitHubContextMenuIconBrush" Color="{StaticResource GitHubContextMenuIconColor}" PresentationOptions:Freeze="true" />
+        </ResourceDictionary>
+    </Viewbox.Resources>
+
     <Border BorderBrush="Transparent" BorderThickness="1">
-        <ghfvs:OcticonImage xmlns:ghfvs="https://github.com/github/VisualStudio"
-                     Foreground="{DynamicResource GitHubContextMenuIconBrush}"
-                     Icon="mark_github">
-            <ghfvs:OcticonImage.Resources>
-                <ResourceDictionary>
-                    <ResourceDictionary.MergedDictionaries>
-                        <ghfvs:SharedDictionaryManager Source="pack://application:,,,/GitHub.VisualStudio.UI;component/SharedDictionary.xaml" />
-                        <ghfvs:SharedDictionaryManager Source="pack://application:,,,/GitHub.UI;component/SharedDictionary.xaml" />
-                    </ResourceDictionary.MergedDictionaries>
-                </ResourceDictionary>
-            </ghfvs:OcticonImage.Resources>
-        </ghfvs:OcticonImage>
+        <Canvas xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation" Width="16" Height="16">
+            <Path xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml" Fill="{StaticResource GitHubContextMenuIconBrush}" Data="M8 0C3.58 0 0 3.58 0 8c0 3.54 2.29 6.53 5.47 7.59.4.07.55-.17.55-.38 0-.19-.01-.82-.01-1.49-2.01.37-2.53-.49-2.69-.94-.09-.23-.48-.94-.82-1.13-.28-.15-.68-.52-.01-.53.63-.01 1.08.58 1.23.82.72 1.21 1.87.87 2.33.66.07-.52.28-.87.51-1.07-1.78-.2-3.64-.89-3.64-3.95 0-.87.31-1.59.82-2.15-.08-.2-.36-1.02.08-2.12 0 0 .67-.21 2.2.82.64-.18 1.32-.27 2-.27.68 0 1.36.09 2 .27 1.53-1.04 2.2-.82 2.2-.82.44 1.1.16 1.92.08 2.12.51.56.82 1.27.82 2.15 0 3.07-1.87 3.75-3.65 3.95.29.25.54.73.54 1.48 0 1.07-.01 1.93-.01 2.2 0 .21.15.46.55.38A8.013 8.013 0 0 0 16 8c0-4.42-3.58-8-8-8z"/>
+        </Canvas>
     </Border>
 </Viewbox>
diff --git a/test/GitHub.App.UnitTests/ViewModels/GitHubPane/PullRequestDetailViewModelTests.cs b/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);
@@ -139,10 +143,10 @@ static PullRequestDetailModel CreatePullRequestModel(
                 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 +158,7 @@ static PullRequestReviewModel CreatePullRequestReviewModel(
                     Id = id,
                     Author = account,
                     State = state,
+                    SubmittedAt = submittedAt
                 };
             }
         }
diff --git a/test/GitHub.TeamFoundation.UnitTests/VSGitExtTests.cs b/test/GitHub.TeamFoundation.UnitTests/VSGitExtTests.cs
@@ -13,14 +13,15 @@
 using Microsoft.VisualStudio.Shell;
 using Microsoft.VisualStudio.Threading;
 using Task = System.Threading.Tasks.Task;
+using static Microsoft.VisualStudio.VSConstants;
 
 public class VSGitExtTests
 {
     public class TheConstructor : TestBaseClass
     {
         [TestCase(true, 1)]
         [TestCase(false, 0)]
-        public void GetServiceIGitExt_WhenSccProviderContextIsActive(bool isActive, int expectCalls)
+        public void GetServiceIGitExt_WhenRepositoryOpenIsActive(bool isActive, int expectCalls)
         {
             var context = CreateVSUIContext(isActive);
             var sp = Substitute.For<IAsyncServiceProvider>();
@@ -135,7 +136,7 @@ public void WhenUIContextChanged_FiredUsingThreadPoolThread()
     public class TheActiveRepositoriesProperty : TestBaseClass
     {
         [Test]
-        public void SccProviderContextNotActive_IsEmpty()
+        public void RepositoryOpenContextNotActive_IsEmpty()
         {
             var context = CreateVSUIContext(false);
             var target = CreateVSGitExt(context);
@@ -144,7 +145,7 @@ public void SccProviderContextNotActive_IsEmpty()
         }
 
         [Test]
-        public void SccProviderContextIsActive_InitializeWithActiveRepositories()
+        public void RepositoryOpenIsActive_InitializeWithActiveRepositories()
         {
             var repoPath = "repoPath";
             var repoFactory = Substitute.For<ILocalRepositoryModelFactory>();
@@ -217,8 +218,7 @@ static VSGitExt CreateVSGitExt(IVSUIContext context = null, IGitExt gitExt = nul
         repoFactory = repoFactory ?? Substitute.For<ILocalRepositoryModelFactory>();
         joinableTaskContext = joinableTaskContext ?? new JoinableTaskContext();
         var factory = Substitute.For<IVSUIContextFactory>();
-        var contextGuid = new Guid(Guids.GitSccProviderId);
-        factory.GetUIContext(contextGuid).Returns(context);
+        factory.GetUIContext(UICONTEXT.RepositoryOpen_guid).Returns(context);
         sp.GetServiceAsync(typeof(IGitExt)).Returns(gitExt);
         var vsGitExt = new VSGitExt(sp, factory, repoFactory, joinableTaskContext);
         vsGitExt.JoinTillEmpty();

Original file line number	Diff line number	Diff line change
`@@ -41,7 +41,7 @@ public static IEnumerable<PullRequestReviewSummaryViewModel> BuildByUser(`
`41`	`41`	`{`
`42`	`42`	`var existing = new Dictionary<string, PullRequestReviewSummaryViewModel>();`
`43`	`43`
`44`		`- foreach (var review in pullRequest.Reviews.OrderBy(x => x.Id))`
	`44`	`+ foreach (var review in pullRequest.Reviews.OrderBy(x => x.SubmittedAt))`
`45`	`45`	`{`
`46`	`46`	`if (review.State == PullRequestReviewState.Pending && review.Author.Login != currentUser.Login)`
`47`	`47`	`continue;`