From eeeca7c59c540275d6f047ed4043e6585f132849 Mon Sep 17 00:00:00 2001
From: Stanley Goldman <Stanley.Goldman@gmail.com>
Date: Fri, 20 Jul 2018 09:20:02 -0400
Subject: [PATCH 1/3] Adding functionality to delete a pull request comment

---
 .../Services/IPullRequestSession.cs           |  5 ++--
 src/GitHub.Exports/Models/CommentModel.cs     | 10 +++++++
 .../SampleData/CommentViewModelDesigner.cs    |  2 ++
 .../Services/IPullRequestSessionService.cs    | 15 +++++-----
 .../Services/PullRequestSession.cs            | 10 ++++---
 .../Services/PullRequestSessionService.cs     |  9 ++++--
 .../ViewModels/CommentViewModel.cs            | 19 ++++++++++--
 .../ViewModels/ICommentViewModel.cs           | 10 +++++++
 .../InlineCommentThreadViewModel.cs           |  4 +--
 .../PullRequestReviewCommentViewModel.cs      | 30 ++++++++++++-------
 10 files changed, 82 insertions(+), 32 deletions(-)

diff --git a/src/GitHub.Exports.Reactive/Services/IPullRequestSession.cs b/src/GitHub.Exports.Reactive/Services/IPullRequestSession.cs
index 7f1f22e103..7c7b9ef045 100644
--- a/src/GitHub.Exports.Reactive/Services/IPullRequestSession.cs
+++ b/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.
diff --git a/src/GitHub.Exports/Models/CommentModel.cs b/src/GitHub.Exports/Models/CommentModel.cs
index eb2520b807..cb647dc056 100644
--- a/src/GitHub.Exports/Models/CommentModel.cs
+++ b/src/GitHub.Exports/Models/CommentModel.cs
@@ -11,6 +11,16 @@ 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; }
 
         /// <summary>
         /// Gets the author of the comment.
diff --git a/src/GitHub.InlineReviews/SampleData/CommentViewModelDesigner.cs b/src/GitHub.InlineReviews/SampleData/CommentViewModelDesigner.cs
index dca3732639..ddd907015c 100644
--- a/src/GitHub.InlineReviews/SampleData/CommentViewModelDesigner.cs
+++ b/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; }
diff --git a/src/GitHub.InlineReviews/Services/IPullRequestSessionService.cs b/src/GitHub.InlineReviews/Services/IPullRequestSessionService.cs
index d816271980..a867a4393a 100644
--- a/src/GitHub.InlineReviews/Services/IPullRequestSessionService.cs
+++ b/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>
diff --git a/src/GitHub.InlineReviews/Services/PullRequestSession.cs b/src/GitHub.InlineReviews/Services/PullRequestSession.cs
index 5076ab45e6..236063334e 100644
--- a/src/GitHub.InlineReviews/Services/PullRequestSession.cs
+++ b/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/>
diff --git a/src/GitHub.InlineReviews/Services/PullRequestSessionService.cs b/src/GitHub.InlineReviews/Services/PullRequestSessionService.cs
index 163acfb70c..a23ae8b38f 100644
--- a/src/GitHub.InlineReviews/Services/PullRequestSessionService.cs
+++ b/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,7 +652,8 @@ 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);
@@ -658,10 +661,10 @@ public virtual async Task<string> GetPullRequestMergeBase(ILocalRepositoryModel
             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/>
diff --git a/src/GitHub.InlineReviews/ViewModels/CommentViewModel.cs b/src/GitHub.InlineReviews/ViewModels/CommentViewModel.cs
index 89276e7693..70ac6a42f8 100644
--- a/src/GitHub.InlineReviews/ViewModels/CommentViewModel.cs
+++ b/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
         {
diff --git a/src/GitHub.InlineReviews/ViewModels/ICommentViewModel.cs b/src/GitHub.InlineReviews/ViewModels/ICommentViewModel.cs
index 8f5e2d292c..9bb64b0bfb 100644
--- a/src/GitHub.InlineReviews/ViewModels/ICommentViewModel.cs
+++ b/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>
diff --git a/src/GitHub.InlineReviews/ViewModels/InlineCommentThreadViewModel.cs b/src/GitHub.InlineReviews/ViewModels/InlineCommentThreadViewModel.cs
index 7ec86f0768..1e729eb52c 100644
--- a/src/GitHub.InlineReviews/ViewModels/InlineCommentThreadViewModel.cs
+++ b/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);
         }
     }
 }
diff --git a/src/GitHub.InlineReviews/ViewModels/PullRequestReviewCommentViewModel.cs b/src/GitHub.InlineReviews/ViewModels/PullRequestReviewCommentViewModel.cs
index a311582bbb..947cb0d282 100644
--- a/src/GitHub.InlineReviews/ViewModels/PullRequestReviewCommentViewModel.cs
+++ b/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,

From 7c3869fbec8b815db75134feff75f0a3c0d7c4d5 Mon Sep 17 00:00:00 2001
From: Stanley Goldman <Stanley.Goldman@gmail.com>
Date: Fri, 20 Jul 2018 12:39:52 -0400
Subject: [PATCH 2/3] Bug found in #1789

- Added functionality to InlineCommentPeekViewModel to listen to the CancelEdit command of every assigned NewInlineCommentThreadViewModel
- Added functionality to the InlineCommentPeekService to dismiss the session when that event is fired

Co-authored-by: Steven Kirk <grokys@users.noreply.github.com>
---
 src/GitHub.InlineReviews/GitHub.InlineReviews.csproj  |  4 ++++
 .../Services/InlineCommentPeekService.cs              | 11 ++++-------
 .../ViewModels/InlineCommentPeekViewModel.cs          |  8 ++++++++
 3 files changed, 16 insertions(+), 7 deletions(-)

diff --git a/src/GitHub.InlineReviews/GitHub.InlineReviews.csproj b/src/GitHub.InlineReviews/GitHub.InlineReviews.csproj
index e560eb9651..10c1103cfc 100644
--- a/src/GitHub.InlineReviews/GitHub.InlineReviews.csproj
+++ b/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>
diff --git a/src/GitHub.InlineReviews/Services/InlineCommentPeekService.cs b/src/GitHub.InlineReviews/Services/InlineCommentPeekService.cs
index 57a62abf9d..892a7c47b1 100644
--- a/src/GitHub.InlineReviews/Services/InlineCommentPeekService.cs
+++ b/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;
         }
diff --git a/src/GitHub.InlineReviews/ViewModels/InlineCommentPeekViewModel.cs b/src/GitHub.InlineReviews/ViewModels/InlineCommentPeekViewModel.cs
index 2ada559882..d2058be474 100644
--- a/src/GitHub.InlineReviews/ViewModels/InlineCommentPeekViewModel.cs
+++ b/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();

From 4718774028b4c8a2a682d2aa5d1224a5cb7d38a2 Mon Sep 17 00:00:00 2001
From: Stanley Goldman <Stanley.Goldman@gmail.com>
Date: Fri, 20 Jul 2018 12:46:16 -0400
Subject: [PATCH 3/3] Adding comments explaining PullRequestId

---
 src/GitHub.Exports/Models/CommentModel.cs | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/src/GitHub.Exports/Models/CommentModel.cs b/src/GitHub.Exports/Models/CommentModel.cs
index cb647dc056..2b601714dd 100644
--- a/src/GitHub.Exports/Models/CommentModel.cs
+++ b/src/GitHub.Exports/Models/CommentModel.cs
@@ -18,9 +18,12 @@ public class CommentModel
         public int DatabaseId { get; set; }
 
         /// <summary>
-        /// Gets the PullRequestId of the comment
+        /// 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.