Merge pull request #1545 from github/feature/pr-user-reviews

Display Pull Request Reviews for a User.

Author: grokys

src/GitHub.App/Api/ApiClient.cs

@@ -88,6 +88,11 @@ public IObservable<User> GetUser()
             return gitHubClient.User.Current();
         }
 
+        public IObservable<User> GetUser(string login)
+        {
+            return gitHubClient.User.Get(login);
+        }
+
         public IObservable<Organization> GetOrganizations()
         {
             // Organization.GetAllForCurrent doesn't return all of the information we need (we 

src/GitHub.App/GitHub.App.csproj

@@ -212,6 +212,9 @@
     <Compile Include="Models\PullRequestDetailArgument.cs" />
     <Compile Include="Models\PullRequestReviewModel.cs" />
     <Compile Include="SampleData\PullRequestFilesViewModelDesigner.cs" />
+    <Compile Include="SampleData\PullRequestReviewFileCommentViewModelDesigner.cs" />
+    <Compile Include="SampleData\PullRequestReviewViewModelDesigner.cs" />
+    <Compile Include="SampleData\PullRequestUserReviewsViewModelDesigner.cs" />
     <Compile Include="Services\EnterpriseCapabilitiesService.cs" />
     <Compile Include="Services\GlobalConnection.cs" />
     <Compile Include="Services\PullRequestEditorService.cs" />
@@ -240,7 +243,10 @@
     <Compile Include="ViewModels\GitHubPane\PullRequestCreationViewModel.cs" />
     <Compile Include="ViewModels\GitHubPane\NotAGitHubRepositoryViewModel.cs" />
     <Compile Include="ViewModels\GitHubPane\NotAGitRepositoryViewModel.cs" />
+    <Compile Include="ViewModels\GitHubPane\PullRequestReviewFileCommentViewModel.cs" />
     <Compile Include="ViewModels\GitHubPane\PullRequestReviewSummaryViewModel.cs" />
+    <Compile Include="ViewModels\GitHubPane\PullRequestReviewViewModel.cs" />
+    <Compile Include="ViewModels\GitHubPane\PullRequestUserReviewsViewModel.cs" />
     <Compile Include="ViewModels\RepositoryFormViewModel.cs" />
     <Compile Include="ViewModels\TeamExplorer\RepositoryPublishViewModel.cs" />
     <Compile Include="Caches\CacheIndex.cs" />

src/GitHub.App/Models/PullRequestReviewModel.cs

@@ -10,5 +10,6 @@ public class PullRequestReviewModel : IPullRequestReviewModel
         public string Body { get; set; }
         public PullRequestReviewState State { get; set; }
         public string CommitId { get; set; }
+        public DateTimeOffset? SubmittedAt { get; set; }
     }
 }

src/GitHub.App/SampleData/PullRequestReviewFileCommentViewModelDesigner.cs

@@ -0,0 +1,13 @@
+using System.Reactive;
+using GitHub.ViewModels.GitHubPane;
+using ReactiveUI;
+
+namespace GitHub.SampleData
+{
+    public class PullRequestReviewFileCommentViewModelDesigner : IPullRequestReviewFileCommentViewModel
+    {
+        public string Body { get; set; }
+        public string RelativePath { get; set; }
+        public ReactiveCommand<Unit> Open { get; }
+    }
+}

src/GitHub.App/SampleData/PullRequestReviewViewModelDesigner.cs

@@ -0,0 +1,73 @@
+using System;
+using System.Collections.Generic;
+using System.Diagnostics.CodeAnalysis;
+using System.Threading.Tasks;
+using GitHub.Models;
+using GitHub.ViewModels.GitHubPane;
+using ReactiveUI;
+
+namespace GitHub.SampleData
+{
+    [ExcludeFromCodeCoverage]
+    public class PullRequestReviewViewModelDesigner : PanePageViewModelBase, IPullRequestReviewViewModel
+    {
+        public PullRequestReviewViewModelDesigner()
+        {
+            PullRequestModel = new PullRequestModel(
+                419,
+                "Fix a ton of potential crashers, odd code and redundant calls in ModelService",
+                new AccountDesigner { Login = "Haacked", IsUser = true },
+                DateTimeOffset.Now - TimeSpan.FromDays(2));
+
+            Model = new PullRequestReviewModel
+            {
+                
+                SubmittedAt = DateTimeOffset.Now - TimeSpan.FromDays(1),
+                User = new AccountDesigner { Login = "Haacked", IsUser = true },
+            };
+
+            Body = @"Just a few comments. I don't feel too strongly about them though.
+
+Otherwise, very nice work here! ✨";
+
+            StateDisplay = "approved";
+
+            FileComments = new[]
+            {
+                new PullRequestReviewFileCommentViewModelDesigner
+                {
+                    Body = @"These should probably be properties. Most likely they should be readonly properties. I know that makes creating instances of these not look as nice as using property initializers when constructing an instance, but if these properties should never be mutated after construction, then it guides future consumers to the right behavior.
+
+However, if you're two-way binding these properties to a UI, then ignore the readonly part and make them properties. But in that case they should probably be reactive properties (or implement INPC).",
+                    RelativePath = "src/GitHub.Exports.Reactive/ViewModels/IPullRequestListViewModel.cs",
+                },
+                new PullRequestReviewFileCommentViewModelDesigner
+                {
+                    Body = "While I have no problems with naming a variable ass I think we should probably avoid swear words in case Microsoft runs their Policheck tool against this code.",
+                    RelativePath = "src/GitHub.App/ViewModels/PullRequestListViewModel.cs",
+                },
+            };
+
+            OutdatedFileComments = new[]
+            {
+                new PullRequestReviewFileCommentViewModelDesigner
+                {
+                    Body = @"So this is just casting a mutable list to an IReadOnlyList which can be cast back to List. I know we probably won't do that, but I'm thinking of the next person to come along. The safe thing to do is to wrap List with a ReadOnlyList. We have an extension method ToReadOnlyList for observables. Wouldn't be hard to write one for IEnumerable.",
+                    RelativePath = "src/GitHub.Exports.Reactive/ViewModels/IPullRequestListViewModel.cs",
+                },
+            };
+        }
+
+        public string Body { get; }
+        public IReadOnlyList<IPullRequestReviewFileCommentViewModel> FileComments { get; set; }
+        public bool IsExpanded { get; set; }
+        public bool HasDetails { get; set; }
+        public ILocalRepositoryModel LocalRepository { get; set; }
+        public IPullRequestReviewModel Model { get; set; }
+        public ReactiveCommand<object> NavigateToPullRequest { get; }
+        public IReadOnlyList<IPullRequestReviewFileCommentViewModel> OutdatedFileComments { get; set; }
+        public IPullRequestModel PullRequestModel { get; set; }
+        public string RemoteRepositoryOwner { get; set; }
+        public string StateDisplay { get; set; }
+    }
+}

src/GitHub.App/SampleData/PullRequestUserReviewsViewModelDesigner.cs

@@ -0,0 +1,74 @@
+using System;
+using System.Collections.Generic;
+using System.Diagnostics.CodeAnalysis;
+using System.Threading.Tasks;
+using GitHub.Models;
+using GitHub.ViewModels.GitHubPane;
+using ReactiveUI;
+
+namespace GitHub.SampleData
+{
+    [ExcludeFromCodeCoverage]
+    public class PullRequestUserReviewsViewModelDesigner : PanePageViewModelBase, IPullRequestUserReviewsViewModel
+    {
+        public PullRequestUserReviewsViewModelDesigner()
+        {
+            User = new AccountDesigner { Login = "Haacked", IsUser = true };
+            PullRequestNumber = 123;
+            PullRequestTitle = "Error handling/bubbling from viewmodels to views to viewhosts";
+            Reviews = new[]
+            {
+                new PullRequestReviewViewModelDesigner()
+                {
+                    IsExpanded = true,
+                    HasDetails = true,
+                    FileComments = new PullRequestReviewFileCommentViewModel[0],
+                    StateDisplay = "approved",
+                    Model = new PullRequestReviewModel
+                    {
+                        State = PullRequestReviewState.Approved,
+                        SubmittedAt = DateTimeOffset.Now - TimeSpan.FromDays(1),
+                        User = User,
+                    },
+                },
+                new PullRequestReviewViewModelDesigner()
+                {
+                    IsExpanded = true,
+                    HasDetails = true,
+                    StateDisplay = "requested changes",
+                    Model = new PullRequestReviewModel
+                    {
+                        State = PullRequestReviewState.ChangesRequested,
+                        SubmittedAt = DateTimeOffset.Now - TimeSpan.FromDays(2),
+                        User = User,
+                    },
+                },
+                new PullRequestReviewViewModelDesigner()
+                {
+                    IsExpanded = false,
+                    HasDetails = false,
+                    StateDisplay = "commented",
+                    Model = new PullRequestReviewModel
+                    {
+                        State = PullRequestReviewState.Commented,
+                        SubmittedAt = DateTimeOffset.Now - TimeSpan.FromDays(2),
+                        User = User,
+                    },
+                }
+            };
+        }
+
+        public ILocalRepositoryModel LocalRepository { get; set; }
+        public string RemoteRepositoryOwner { get; set; }
+        public int PullRequestNumber { get; set; }
+        public IAccount User { get; set; }
+        public IReadOnlyList<IPullRequestReviewViewModel> Reviews { get; set; }
+        public string PullRequestTitle { get; set; }
+        public ReactiveCommand<object> NavigateToPullRequest { get; }
+
+        public Task InitializeAsync(ILocalRepositoryModel localRepository, IConnection connection, string owner, string repo, int pullRequestNumber, string login)
+        {
+            return Task.CompletedTask;
+        }
+    }
+}

src/GitHub.App/Services/ModelService.cs

@@ -57,6 +57,13 @@ public IObservable<IAccount> GetCurrentUser()
             return GetUserFromCache().Select(Create);
         }
 
+        public IObservable<IAccount> GetUser(string login)
+        {
+            return hostCache.GetAndRefreshObject("user|" + login,
+                () => ApiClient.GetUser(login).Select(AccountCacheItem.Create), TimeSpan.FromMinutes(5), TimeSpan.FromDays(7))
+                .Select(Create);
+        }
+
         public IObservable<GitIgnoreItem> GetGitIgnoreTemplates()
         {
             return Observable.Defer(() =>
@@ -398,6 +405,7 @@ async Task<IList<IPullRequestReviewModel>> GetPullRequestReviews(string owner, s
                             Body = y.Body,
                             CommitId = y.Commit.Oid,
                             State = FromGraphQL(y.State),
+                            SubmittedAt = y.SubmittedAt,
                             User = Create(y.Author.Login, y.Author.AvatarUrl(null))
                         }).ToList()
                     });
@@ -623,6 +631,7 @@ IPullRequestModel Create(PullRequestCacheItem prCacheItem)
                         Body = x.Body,
                         State = x.State,
                         CommitId = x.CommitId,
+                        SubmittedAt = x.SubmittedAt,
                     }).ToList(),
                 ReviewComments = prCacheItem.ReviewComments.Select(x =>
                     (IPullRequestReviewCommentModel)new PullRequestReviewCommentModel
@@ -892,6 +901,7 @@ public PullRequestReviewCacheItem(IPullRequestReviewModel review)
                 };
                 Body = review.Body;
                 State = review.State;
+                SubmittedAt = review.SubmittedAt;
             }
 
             public long Id { get; set; }
@@ -900,6 +910,7 @@ public PullRequestReviewCacheItem(IPullRequestReviewModel review)
             public string Body { get; set; }
             public GitHub.Models.PullRequestReviewState State { get; set; }
             public string CommitId { get; set; }
+            public DateTimeOffset? SubmittedAt { get; set; }
         }
 
         public class PullRequestReviewCommentCacheItem

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

@@ -33,6 +33,7 @@ public sealed class GitHubPaneViewModel : ViewModelBase, IGitHubPaneViewModel, I
     {
         static readonly ILogger log = LogManager.ForContext<GitHubPaneViewModel>();
         static readonly Regex pullUri = CreateRoute("/:owner/:repo/pull/:number");
+        static readonly Regex pullUserReviewsUri = CreateRoute("/:owner/:repo/pull/:number/reviews/:login");
 
         readonly IViewViewModelFactory viewModelFactory;
         readonly ISimpleApiClientFactory apiClientFactory;
@@ -243,6 +244,14 @@ public async Task NavigateTo(Uri uri)
                 var number = int.Parse(match.Groups["number"].Value);
                 await ShowPullRequest(owner, repo, number);
             }
+            else if ((match = pullUserReviewsUri.Match(uri.AbsolutePath))?.Success == true)
+            {
+                var owner = match.Groups["owner"].Value;
+                var repo = match.Groups["repo"].Value;
+                var number = int.Parse(match.Groups["number"].Value);
+                var login = match.Groups["login"].Value;
+                await ShowPullRequestReviews(owner, repo, number, login);
+            }
             else
             {
                 throw new NotSupportedException("Unrecognised GitHub pane URL: " + uri.AbsolutePath);
@@ -282,6 +291,20 @@ public Task ShowPullRequest(string owner, string repo, int number)
                 x => x.RemoteRepositoryOwner == owner && x.LocalRepository.Name == repo && x.Number == number);
         }
 
+        /// <inheritdoc/>
+        public Task ShowPullRequestReviews(string owner, string repo, int number, string login)
+        {
+            Guard.ArgumentNotNull(owner, nameof(owner));
+            Guard.ArgumentNotNull(repo, nameof(repo));
+
+            return NavigateTo<IPullRequestUserReviewsViewModel>(
+                x => x.InitializeAsync(LocalRepository, Connection, owner, repo, number, login),
+                x => x.RemoteRepositoryOwner == owner &&
+                     x.LocalRepository.Name == repo &&
+                     x.PullRequestNumber == number &&
+                     x.User.Login == login);
+        }
+
         async Task CreateInitializeTask(IServiceProvider paneServiceProvider)
         {
             await UpdateContent(teamExplorerContext.ActiveRepository);
@@ -408,7 +431,7 @@ static async Task<bool> IsValidRepository(ISimpleApiClient client)
         static Regex CreateRoute(string route)
         {
             // Build RegEx from route (:foo to named group (?<foo>[\w_.-]+)).
-            var routeFormat = new Regex("(:([a-z]+))\\b").Replace(route, @"(?<$2>[\w_.-]+)");
+            var routeFormat = "^" + new Regex("(:([a-z]+))\\b").Replace(route, @"(?<$2>[\w_.-]+)") + "$";
             return new Regex(routeFormat, RegexOptions.ExplicitCapture | RegexOptions.IgnoreCase);
         }
     }

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

@@ -18,6 +18,7 @@
 using LibGit2Sharp;
 using ReactiveUI;
 using Serilog;
+using static System.FormattableString;
 
 namespace GitHub.ViewModels.GitHubPane
 {
@@ -636,7 +637,16 @@ async Task DoSyncSubmodules(object unused)
 
         void DoShowReview(object item)
         {
-            // TODO
+            var review = (PullRequestReviewSummaryViewModel)item;
+
+            if (review.State == PullRequestReviewState.Pending)
+            {
+                throw new NotImplementedException();
+            }
+            else
+            {
+                NavigateTo(Invariant($"{RemoteRepositoryOwner}/{LocalRepository.Name}/pull/{Number}/reviews/{review.User.Login}"));
+            }
         }
 
         class CheckoutCommandState : IPullRequestCheckoutState

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

@@ -0,0 +1,45 @@
+using System;
+using System.Diagnostics.CodeAnalysis;
+using System.Reactive;
+using GitHub.Extensions;
+using GitHub.Models;
+using GitHub.Services;
+using ReactiveUI;
+
+namespace GitHub.ViewModels.GitHubPane
+{
+    /// <summary>
+    /// A view model for a file comment in a <see cref="PullRequestReviewViewModel"/>.
+    /// </summary>
+    public class PullRequestReviewFileCommentViewModel : IPullRequestReviewFileCommentViewModel
+    {
+        [SuppressMessage("Microsoft.Performance", "CA1823:AvoidUnusedPrivateFields", Justification = "This will be used in a later PR")]
+        readonly IPullRequestEditorService editorService;
+        [SuppressMessage("Microsoft.Performance", "CA1823:AvoidUnusedPrivateFields", Justification = "This will be used in a later PR")]
+        readonly IPullRequestSession session;
+        readonly IPullRequestReviewCommentModel model;
+
+        public PullRequestReviewFileCommentViewModel(
+            IPullRequestEditorService editorService,
+            IPullRequestSession session,
+            IPullRequestReviewCommentModel model)
+        {
+            Guard.ArgumentNotNull(editorService, nameof(editorService));
+            Guard.ArgumentNotNull(session, nameof(session));
+            Guard.ArgumentNotNull(model, nameof(model));
+
+            this.editorService = editorService;
+            this.session = session;
+            this.model = model;
+        }
+
+        /// <inheritdoc/>
+        public string Body => model.Body;
+
+        /// <inheritdoc/>
+        public string RelativePath => model.Path;
+
+        /// <inheritdoc/>
+        public ReactiveCommand<Unit> Open { get; }
+    }
+}