Merge pull request #1562 from github/feature/pr-review-authoring

Start PR Reviews from Inline Comments

Author: jcansdale

src/GitHub.App/Api/ApiClient.cs

@@ -48,6 +48,27 @@ public IObservable<Repository> CreateRepository(NewRepository repository, string
             return (isUser ? client.Create(repository) : client.Create(login, repository));
         }
 
+        public IObservable<PullRequestReview> PostPullRequestReview(
+            string owner,
+            string name,
+            int number,
+            string commitId,
+            string body,
+            PullRequestReviewEvent e)
+        {
+            Guard.ArgumentNotEmptyString(owner, nameof(owner));
+            Guard.ArgumentNotEmptyString(name, nameof(name));
+
+            var review = new PullRequestReviewCreate
+            {
+                Body = body,
+                CommitId = commitId,
+                Event = e,
+            };
+
+            return gitHubClient.PullRequest.Review.Create(owner, name, number, review);
+        }
+
         public IObservable<PullRequestReviewComment> CreatePullRequestReviewComment(
             string owner,
             string name,

src/GitHub.App/GitHub.App.csproj

@@ -212,6 +212,7 @@
     <Compile Include="Models\PullRequestDetailArgument.cs" />
     <Compile Include="Models\PullRequestReviewModel.cs" />
     <Compile Include="SampleData\PullRequestFilesViewModelDesigner.cs" />
+    <Compile Include="SampleData\PullRequestReviewAuthoringViewModelDesigner.cs" />
     <Compile Include="SampleData\PullRequestReviewFileCommentViewModelDesigner.cs" />
     <Compile Include="SampleData\PullRequestReviewViewModelDesigner.cs" />
     <Compile Include="SampleData\PullRequestUserReviewsViewModelDesigner.cs" />
@@ -243,6 +244,7 @@
     <Compile Include="ViewModels\GitHubPane\PullRequestCreationViewModel.cs" />
     <Compile Include="ViewModels\GitHubPane\NotAGitHubRepositoryViewModel.cs" />
     <Compile Include="ViewModels\GitHubPane\NotAGitRepositoryViewModel.cs" />
+    <Compile Include="ViewModels\GitHubPane\PullRequestReviewAuthoringViewModel.cs" />
     <Compile Include="ViewModels\GitHubPane\PullRequestReviewFileCommentViewModel.cs" />
     <Compile Include="ViewModels\GitHubPane\PullRequestReviewSummaryViewModel.cs" />
     <Compile Include="ViewModels\GitHubPane\PullRequestReviewViewModel.cs" />

src/GitHub.App/SampleData/PullRequestReviewAuthoringViewModelDesigner.cs

@@ -0,0 +1,62 @@
+using System;
+using System.Collections.Generic;
+using System.Reactive;
+using System.Threading.Tasks;
+using GitHub.Models;
+using GitHub.ViewModels.GitHubPane;
+using ReactiveUI;
+
+namespace GitHub.SampleData
+{
+    public class PullRequestReviewAuthoringViewModelDesigner : PanePageViewModelBase, IPullRequestReviewAuthoringViewModel
+    {
+        public PullRequestReviewAuthoringViewModelDesigner()
+        {
+            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));
+
+            Files = new PullRequestFilesViewModelDesigner();
+
+            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",
+                },
+            };
+        }
+
+        public string Body { get; set; }
+        public bool CanApproveRequestChanges { get; set; }
+        public IReadOnlyList<IPullRequestReviewFileCommentViewModel> FileComments { get; }
+        public IPullRequestFilesViewModel Files { get; }
+        public ILocalRepositoryModel LocalRepository { get; set; }
+        public IPullRequestReviewModel Model { get; set; }
+        public ReactiveCommand<object> NavigateToPullRequest { get; }
+        public string OperationError { get; set; }
+        public IPullRequestModel PullRequestModel { get; set; }
+        public string RemoteRepositoryOwner { get; set; }
+        public ReactiveCommand<Unit> Submit { get; }
+
+        public Task InitializeAsync(
+            ILocalRepositoryModel localRepository,
+            IConnection connection,
+            string owner,
+            string repo,
+            int pullRequestNumber)
+        {
+            throw new NotImplementedException();
+        }
+    }
+}

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 pullNewReviewUri = CreateRoute("/:owner/:repo/pull/:number/review/new");
         static readonly Regex pullUserReviewsUri = CreateRoute("/:owner/:repo/pull/:number/reviews/:login");
 
         readonly IViewViewModelFactory viewModelFactory;
@@ -244,6 +245,13 @@ public async Task NavigateTo(Uri uri)
                 var number = int.Parse(match.Groups["number"].Value);
                 await ShowPullRequest(owner, repo, number);
             }
+            else if ((match = pullNewReviewUri.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);
+                await ShowPullRequestReviewAuthoring(owner, repo, number);
+            }
             else if ((match = pullUserReviewsUri.Match(uri.AbsolutePath))?.Success == true)
             {
                 var owner = match.Groups["owner"].Value;
@@ -305,6 +313,19 @@ public Task ShowPullRequestReviews(string owner, string repo, int number, string
                      x.User.Login == login);
         }
 
+        /// <inheritdoc/>
+        public Task ShowPullRequestReviewAuthoring(string owner, string repo, int number)
+        {
+            Guard.ArgumentNotNull(owner, nameof(owner));
+            Guard.ArgumentNotNull(repo, nameof(repo));
+
+            return NavigateTo<IPullRequestReviewAuthoringViewModel>(
+                x => x.InitializeAsync(LocalRepository, Connection, owner, repo, number),
+                x => x.RemoteRepositoryOwner == owner &&
+                     x.LocalRepository.Name == repo &&
+                     x.PullRequestModel.Number == number);
+        }
+
         async Task CreateInitializeTask(IServiceProvider paneServiceProvider)
         {
             await UpdateContent(teamExplorerContext.ActiveRepository);

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

@@ -53,6 +53,7 @@ public sealed class PullRequestDetailViewModel : PanePageViewModelBase, IPullReq
         bool active;
         bool refreshOnActivate;
         Uri webUrl;
+        IDisposable sessionSubscription;
 
         /// <summary>
         /// Initializes a new instance of the <see cref="PullRequestDetailViewModel"/> class.
@@ -441,6 +442,11 @@ public async Task Load(IPullRequestModel pullRequest)
                     UpdateState = null;
                 }
 
+                sessionSubscription?.Dispose();
+                sessionSubscription = Session.WhenAnyValue(x => x.HasPendingReview)
+                    .Skip(1)
+                    .Subscribe(x => Reviews = PullRequestReviewSummaryViewModel.BuildByUser(Session.User, Session.PullRequest).ToList());
+
                 if (firstLoad)
                 {
                     usageTracker.IncrementCounter(x => x.NumberOfPullRequestsOpened).Forget();
@@ -620,7 +626,7 @@ void DoShowReview(object item)
 
             if (review.State == PullRequestReviewState.Pending)
             {
-                throw new NotImplementedException();
+                NavigateTo(Invariant($"{RemoteRepositoryOwner}/{LocalRepository.Name}/pull/{Number}/review/new"));
             }
             else
             {