Merge pull request #1789 from github/fixes/delete-comment

Fixing functionality to delete a pull request comment

src/GitHub.Exports.Reactive/Services/IPullRequestSession.cs

@@ -138,9 +138,10 @@ public interface IPullRequestSession
         /// <summary>
         /// Deletes a pull request comment.
         /// </summary>
-        /// <param name="number">The number of the pull request comment to delete</param>
+        /// <param name="pullRequestId">The number of the pull request id of the comment</param>
+        /// <param name="commentDatabaseId">The number of the pull request comment to delete</param>
         /// <returns>A task which completes when the session has completed updating.</returns>
-        Task DeleteComment(int number);
+        Task DeleteComment(int pullRequestId, int commentDatabaseId);
 
         /// <summary>
         /// Edit a PR review comment reply.

src/GitHub.Exports/Models/CommentModel.cs

@@ -11,6 +11,19 @@ public class CommentModel
         /// Gets the ID of the comment.
         /// </summary>
         public string Id { get; set; }
+
+        /// <summary>
+        /// Gets the DatabaseId of the comment.
+        /// </summary>
+        public int DatabaseId { get; set; }
+
+        /// <summary>
+        /// Gets the PullRequestId of the comment.
+        /// </summary>
+        public int PullRequestId { get; set; }
+        // The GraphQL Api does not allow for deleting of pull request comments.
+        // REST Api must be used, and PullRequestId is needed to reload the pull request.
+        // This field should be removed with better GraphQL support.
 
         /// <summary>
         /// Gets the author of the comment.

src/GitHub.InlineReviews/GitHub.InlineReviews.csproj

@@ -191,6 +191,10 @@
       <Project>{9aea02db-02b5-409c-b0ca-115d05331a6b}</Project>
       <Name>GitHub.Exports</Name>
     </ProjectReference>
+    <ProjectReference Include="..\GitHub.Extensions.Reactive\GitHub.Extensions.Reactive.csproj">
+      <Project>{6559E128-8B40-49A5-85A8-05565ED0C7E3}</Project>
+      <Name>GitHub.Extensions.Reactive</Name>
+    </ProjectReference>
     <ProjectReference Include="..\GitHub.Extensions\GitHub.Extensions.csproj">
       <Project>{6AFE2E2D-6DB0-4430-A2EA-F5F5388D2F78}</Project>
       <Name>GitHub.Extensions</Name>

src/GitHub.InlineReviews/SampleData/CommentViewModelDesigner.cs

@@ -16,6 +16,8 @@ public CommentViewModelDesigner()
         }
 
         public string Id { get; set; }
+        public int PullRequestId { get; set; }
+        public int DatabaseId { get; set; }
         public string Body { get; set; }
         public string ErrorMessage { get; set; }
         public CommentEditState EditState { get; set; }

src/GitHub.InlineReviews/Services/IPullRequestSessionService.cs

@@ -322,21 +322,20 @@ public interface IPullRequestSessionService
         /// Delete a PR review comment.
         /// </summary>
         /// <param name="localRepository">The local repository.</param>
-        /// <param name="remoteRepositoryOwner">The owner of the repository fork to delete from.</param>
-        /// <param name="user">The user deleting the comment.</param>
-        /// <param name="number">The pull request comment number.</param>
+        /// <param name="remoteRepositoryOwner">The owner of the repository.</param>
+        /// <param name="pullRequestId">The pull request id of the comment</param>
+        /// <param name="commentDatabaseId">The pull request comment number.</param>
         /// <returns>The updated state of the pull request.</returns>
-        Task<PullRequestDetailModel> DeleteComment(
-            ILocalRepositoryModel localRepository,
+        Task<PullRequestDetailModel> DeleteComment(ILocalRepositoryModel localRepository,
             string remoteRepositoryOwner,
-            int number);
+            int pullRequestId,
+            int commentDatabaseId);
 
         /// <summary>
         /// Edit a PR review comment.
         /// </summary>
         /// <param name="localRepository">The local repository.</param>
-        /// <param name="remoteRepositoryOwner">The owner of the repository fork to delete from.</param>
-        /// <param name="user">The user deleting the comment.</param>
+        /// <param name="remoteRepositoryOwner">The owner of the repository.</param>
         /// <param name="commentNodeId">The pull request comment node id.</param>
         /// <param name="body">The replacement comment body.</param>
         /// <returns>The updated state of the pull request.</returns>

src/GitHub.InlineReviews/Services/InlineCommentPeekService.cs

@@ -107,12 +107,7 @@ public ITrackingPoint Show(ITextView textView, AddInlineCommentTag tag)
 
             var session = peekBroker.TriggerPeekSession(options);
             var item = session.PeekableItems.OfType<InlineCommentPeekableItem>().FirstOrDefault();
-
-            if (item != null)
-            {
-                var placeholder = item.ViewModel.Thread.Comments.Last();
-                placeholder.CancelEdit.Take(1).Subscribe(_ => session.Dismiss());
-            }
+            item?.ViewModel.Close.Take(1).Subscribe(_ => session.Dismiss());
 
             return trackingPoint;
         }
@@ -134,7 +129,9 @@ public ITrackingPoint Show(ITextView textView, ShowInlineCommentTag tag)
 
             ExpandCollapsedRegions(textView, line.Extent);
 
-            peekBroker.TriggerPeekSession(options);
+            var session = peekBroker.TriggerPeekSession(options);
+            var item = session.PeekableItems.OfType<InlineCommentPeekableItem>().FirstOrDefault();
+            item?.ViewModel.Close.Take(1).Subscribe(_ => session.Dismiss());
 
             return trackingPoint;
         }

src/GitHub.InlineReviews/Services/PullRequestSession.cs

@@ -159,13 +159,15 @@ public string GetRelativePath(string path)
         }
 
         /// <inheritdoc/>
-        public async Task DeleteComment(
-            int number)
+        public async Task DeleteComment(int pullRequestId, int commentDatabaseId)
         {
-            await service.DeleteComment(
+            var model = await service.DeleteComment(
                 LocalRepository,
                 RepositoryOwner,
-                number);
+                pullRequestId,
+                commentDatabaseId);
+
+            await Update(model);
         }
 
         /// <inheritdoc/>

src/GitHub.InlineReviews/Services/PullRequestSessionService.cs

@@ -307,6 +307,8 @@ public virtual async Task<PullRequestDetailModel> ReadPullRequestDetail(HostAddr
                             Comments = review.Comments(null, null, null, null).AllPages().Select(comment => new CommentAdapter
                             {
                                 Id = comment.Id.Value,
+                                PullRequestId = comment.PullRequest.Number,
+                                DatabaseId = comment.DatabaseId.Value,
                                 Author = new ActorModel
                                 {
                                     Login = comment.Author.Login,
@@ -650,18 +652,19 @@ public virtual async Task<string> GetPullRequestMergeBase(ILocalRepositoryModel
         public async Task<PullRequestDetailModel> DeleteComment(
             ILocalRepositoryModel localRepository,
             string remoteRepositoryOwner,
-            int number)
+            int pullRequestId,
+            int commentDatabaseId)
         {
             var address = HostAddress.Create(localRepository.CloneUrl.Host);
             var apiClient = await apiClientFactory.Create(address);
 
             await apiClient.DeletePullRequestReviewComment(
                 remoteRepositoryOwner,
                 localRepository.Name,
-                number);
+                commentDatabaseId);
 
             await usageTracker.IncrementCounter(x => x.NumberOfPRReviewDiffViewInlineCommentDelete);
-            return await ReadPullRequestDetail(address, remoteRepositoryOwner, localRepository.Name, number);
+            return await ReadPullRequestDetail(address, remoteRepositoryOwner, localRepository.Name, pullRequestId);
         }
 
         /// <inheritdoc/>

src/GitHub.InlineReviews/ViewModels/CommentViewModel.cs

@@ -32,15 +32,20 @@ public class CommentViewModel : ReactiveObject, ICommentViewModel
         /// </summary>
         /// <param name="thread">The thread that the comment is a part of.</param>
         /// <param name="currentUser">The current user.</param>
+        /// <param name="pullRequestId">The pull request id of the comment.</param>
         /// <param name="commentId">The GraphQL ID of the comment.</param>
+        /// <param name="databaseId">The database id of the comment.</param>
         /// <param name="body">The comment body.</param>
         /// <param name="state">The comment edit state.</param>
         /// <param name="author">The author of the comment.</param>
         /// <param name="updatedAt">The modified date of the comment.</param>
+        /// <param name="webUrl"></param>
         protected CommentViewModel(
             ICommentThreadViewModel thread,
             IActorViewModel currentUser,
+            int pullRequestId,
             string commentId,
+            int databaseId,
             string body,
             CommentEditState state,
             IActorViewModel author,
@@ -54,6 +59,8 @@ public class CommentViewModel : ReactiveObject, ICommentViewModel
             Thread = thread;
             CurrentUser = currentUser;
             Id = commentId;
+            DatabaseId = databaseId;
+            PullRequestId = pullRequestId;
             Body = body;
             EditState = state;
             Author = author;
@@ -104,8 +111,10 @@ public class CommentViewModel : ReactiveObject, ICommentViewModel
             CommentModel model)
             : this(
                   thread, 
-                  new ActorViewModel(currentUser), 
+                  new ActorViewModel(currentUser),
+                  model.PullRequestId, 
                   model.Id, 
+                  model.DatabaseId, 
                   model.Body, 
                   CommentEditState.None, 
                   new ActorViewModel(model.Author), 
@@ -126,7 +135,7 @@ async Task DoDelete(object unused)
                 ErrorMessage = null;
                 IsSubmitting = true;
 
-                await Thread.DeleteComment.ExecuteAsyncTask(Id);
+                await Thread.DeleteComment.ExecuteAsyncTask(new Tuple<int, int>(PullRequestId, DatabaseId));
             }
             catch (Exception e)
             {
@@ -192,6 +201,12 @@ async Task DoCommitEdit(object unused)
         /// <inheritdoc/>
         public string Id { get; private set; }
 
+        /// <inheritdoc/>
+        public int DatabaseId { get; private set; }
+
+        /// <inheritdoc/>
+        public int PullRequestId { get; private set; }
+
         /// <inheritdoc/>
         public string Body
         {

src/GitHub.InlineReviews/ViewModels/ICommentViewModel.cs

@@ -23,6 +23,16 @@ public interface ICommentViewModel : IViewModel
         /// </summary>
         string Id { get; }
 
+        /// <summary>
+        /// Gets the Database ID of the comment.
+        /// </summary>
+        int DatabaseId { get; }
+
+        /// <summary>
+        /// The pull request id of the comment
+        /// </summary>
+        int PullRequestId { get; }
+
         /// <summary>
         /// Gets or sets the body of the comment.
         /// </summary>

src/GitHub.InlineReviews/ViewModels/InlineCommentPeekViewModel.cs

@@ -7,6 +7,7 @@
 using GitHub.Api;
 using GitHub.Commands;
 using GitHub.Extensions;
+using GitHub.Extensions.Reactive;
 using GitHub.Factories;
 using GitHub.InlineReviews.Commands;
 using GitHub.InlineReviews.Services;
@@ -63,6 +64,11 @@ public sealed class InlineCommentPeekViewModel : ReactiveObject, IDisposable
 
             peekSession.Dismissed += (s, e) => Dispose();
 
+            Close = this.WhenAnyValue(x => x.Thread)
+                .SelectMany(x => x is NewInlineCommentThreadViewModel
+                    ? x.Comments.Single().CancelEdit.SelectUnit()
+                    : Observable.Never<Unit>());
+
             NextComment = ReactiveCommand.CreateAsyncTask(
                 Observable.Return(nextCommentCommand.Enabled),
                 _ => nextCommentCommand.Execute(new InlineCommentNavigationParams
@@ -97,6 +103,8 @@ public ICommentThreadViewModel Thread
         /// </summary>
         public ReactiveCommand<Unit> PreviousComment { get; }
 
+        public IObservable<Unit> Close { get; }
+
         public void Dispose()
         {
             threadSubscription?.Dispose();

src/GitHub.InlineReviews/ViewModels/InlineCommentThreadViewModel.cs

@@ -80,8 +80,8 @@ async Task DoDeleteComment(object parameter)
         {
             Guard.ArgumentNotNull(parameter, nameof(parameter));
 
-            var number = (int)parameter;
-            await Session.DeleteComment(number);
+            var item = (Tuple<int, int>)parameter;
+            await Session.DeleteComment(item.Item1, item.Item2);
         }
     }
 }

src/GitHub.InlineReviews/ViewModels/PullRequestReviewCommentViewModel.cs

@@ -30,24 +30,28 @@ public class PullRequestReviewCommentViewModel : CommentViewModel, IPullRequestR
         /// <param name="session">The pull request session.</param>
         /// <param name="thread">The thread that the comment is a part of.</param>
         /// <param name="currentUser">The current user.</param>
+        /// <param name="pullRequestId">The pull request id of the comment.</param>
         /// <param name="commentId">The GraphQL ID of the comment.</param>
+        /// <param name="databaseId">The database id of the comment.</param>
         /// <param name="body">The comment body.</param>
         /// <param name="state">The comment edit state.</param>
         /// <param name="author">The author of the comment.</param>
         /// <param name="updatedAt">The modified date of the comment.</param>
         /// <param name="isPending">Whether this is a pending comment.</param>
-        public PullRequestReviewCommentViewModel(
-            IPullRequestSession session,
+        /// <param name="webUrl"></param>
+        public PullRequestReviewCommentViewModel(IPullRequestSession session,
             ICommentThreadViewModel thread,
             IActorViewModel currentUser,
+            int pullRequestId,
             string commentId,
+            int databaseId,
             string body,
             CommentEditState state,
             IActorViewModel author,
             DateTimeOffset updatedAt,
             bool isPending,
             Uri webUrl)
-            : base(thread, currentUser, commentId, body, state, author, updatedAt, webUrl)
+            : base(thread, currentUser, pullRequestId, commentId, databaseId, body, state, author, updatedAt, webUrl)
         {
             Guard.ArgumentNotNull(session, nameof(session));
 
@@ -87,14 +91,16 @@ public class PullRequestReviewCommentViewModel : CommentViewModel, IPullRequestR
             PullRequestReviewModel review,
             PullRequestReviewCommentModel model)
             : this(
-                  session, 
-                  thread, 
-                  currentUser, 
-                  model.Id, 
-                  model.Body, 
-                  CommentEditState.None, 
-                  new ActorViewModel(model.Author), 
-                  model.CreatedAt, 
+                  session,
+                  thread,
+                  currentUser,
+                  model.PullRequestId,
+                  model.Id,
+                  model.DatabaseId,
+                  model.Body,
+                  CommentEditState.None,
+                  new ActorViewModel(model.Author),
+                  model.CreatedAt,
                   review.State == PullRequestReviewState.Pending,
                   model.Url != null ? new Uri(model.Url) : null)
         {
@@ -116,7 +122,9 @@ public class PullRequestReviewCommentViewModel : CommentViewModel, IPullRequestR
                 session,
                 thread,
                 currentUser,
+                0,
                 null,
+                0,
                 string.Empty,
                 CommentEditState.Placeholder,
                 currentUser,