Merge pull request #1822 from github/donokuda/not-delete-nooooooo

Show a confirmation when attempting to delete a comment

Author: StanleyGoldman

src/GitHub.InlineReviews/GitHub.InlineReviews.csproj

@@ -84,6 +84,8 @@
     <Compile Include="Margins\InlineCommentMargin.cs" />
     <Compile Include="Margins\InlineCommentMarginVisible.cs" />
     <Compile Include="Margins\InlineCommentMarginEnabled.cs" />
+    <Compile Include="Services\CommentService.cs" />
+    <Compile Include="Services\ICommentService.cs" />
     <Compile Include="PullRequestStatusBarPackage.cs" />
     <Compile Include="InlineReviewsPackage.cs" />
     <Compile Include="Models\InlineCommentThreadModel.cs" />

src/GitHub.InlineReviews/Peek/InlineCommentPeekableItemSource.cs

@@ -16,17 +16,19 @@ class InlineCommentPeekableItemSource : IPeekableItemSource
         readonly IPullRequestSessionManager sessionManager;
         readonly INextInlineCommentCommand nextCommentCommand;
         readonly IPreviousInlineCommentCommand previousCommentCommand;
+        readonly ICommentService commentService;
 
-        public InlineCommentPeekableItemSource(
-            IInlineCommentPeekService peekService,
+        public InlineCommentPeekableItemSource(IInlineCommentPeekService peekService,
             IPullRequestSessionManager sessionManager,
             INextInlineCommentCommand nextCommentCommand,
-            IPreviousInlineCommentCommand previousCommentCommand)
+            IPreviousInlineCommentCommand previousCommentCommand,
+            ICommentService commentService)
         {
             this.peekService = peekService;
             this.sessionManager = sessionManager;
             this.nextCommentCommand = nextCommentCommand;
             this.previousCommentCommand = previousCommentCommand;
+            this.commentService = commentService;
         }
 
         public void AugmentPeekSession(IPeekSession session, IList<IPeekableItem> peekableItems)
@@ -38,7 +40,8 @@ public void AugmentPeekSession(IPeekSession session, IList<IPeekableItem> peekab
                     session,
                     sessionManager,
                     nextCommentCommand,
-                    previousCommentCommand);
+                    previousCommentCommand,
+                    commentService);
                 viewModel.Initialize().Forget();
                 peekableItems.Add(new InlineCommentPeekableItem(viewModel));
             }

src/GitHub.InlineReviews/Peek/InlineCommentPeekableItemSourceProvider.cs

@@ -20,18 +20,21 @@ class InlineCommentPeekableItemSourceProvider : IPeekableItemSourceProvider
         readonly IPullRequestSessionManager sessionManager;
         readonly INextInlineCommentCommand nextCommentCommand;
         readonly IPreviousInlineCommentCommand previousCommentCommand;
+        readonly ICommentService commentService;
 
         [ImportingConstructor]
         public InlineCommentPeekableItemSourceProvider(
             IInlineCommentPeekService peekService,
             IPullRequestSessionManager sessionManager,
             INextInlineCommentCommand nextCommentCommand,
-            IPreviousInlineCommentCommand previousCommentCommand)
+            IPreviousInlineCommentCommand previousCommentCommand,
+            ICommentService commentService)
         {
             this.peekService = peekService;
             this.sessionManager = sessionManager;
             this.nextCommentCommand = nextCommentCommand;
             this.previousCommentCommand = previousCommentCommand;
+            this.commentService = commentService;
         }
 
         public IPeekableItemSource TryCreatePeekableItemSource(ITextBuffer textBuffer)
@@ -40,7 +43,8 @@ public IPeekableItemSource TryCreatePeekableItemSource(ITextBuffer textBuffer)
                 peekService,
                 sessionManager,
                 nextCommentCommand,
-                previousCommentCommand);
+                previousCommentCommand,
+                commentService);
         }
     }
 }

src/GitHub.InlineReviews/Services/CommentService.cs

@@ -0,0 +1,19 @@
+using System.ComponentModel.Composition;
+using System.Windows.Forms;
+
+namespace GitHub.InlineReviews.Services
+{
+    [Export(typeof(ICommentService))]
+    [PartCreationPolicy(CreationPolicy.NonShared)]
+    public class CommentService:ICommentService
+    {
+        public bool ConfirmCommentDelete()
+        {
+            return MessageBox.Show(
+                VisualStudio.UI.Resources.DeleteCommentConfirmation,
+                VisualStudio.UI.Resources.DeleteCommentConfirmationCaption,
+                MessageBoxButtons.YesNo,
+                MessageBoxIcon.Question) == DialogResult.Yes;
+        }
+    }
+}

src/GitHub.InlineReviews/Services/ICommentService.cs

@@ -0,0 +1,14 @@
+namespace GitHub.InlineReviews.Services
+{
+    /// <summary>
+    /// This service allows for functionality to be injected into the chain of different peek Comment ViewModel types.
+    /// </summary>
+    public interface ICommentService
+    {
+        /// <summary>
+        /// This function uses MessageBox.Show to display a confirmation if a comment should be deleted.
+        /// </summary>
+        /// <returns></returns>
+        bool ConfirmCommentDelete();
+    }
+}

src/GitHub.InlineReviews/ViewModels/CommentViewModel.cs

@@ -3,7 +3,9 @@
 using System.Reactive;
 using System.Reactive.Linq;
 using System.Threading.Tasks;
+using System.Windows;
 using GitHub.Extensions;
+using GitHub.InlineReviews.Services;
 using GitHub.Logging;
 using GitHub.Models;
 using GitHub.ViewModels;
@@ -18,6 +20,7 @@ namespace GitHub.InlineReviews.ViewModels
     public class CommentViewModel : ReactiveObject, ICommentViewModel
     {
         static readonly ILogger log = LogManager.ForContext<CommentViewModel>();
+        ICommentService commentService;
         string body;
         string errorMessage;
         bool isReadOnly;
@@ -30,6 +33,7 @@ public class CommentViewModel : ReactiveObject, ICommentViewModel
         /// <summary>
         /// Initializes a new instance of the <see cref="CommentViewModel"/> class.
         /// </summary>
+        /// <param name="commentService">The comment service</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>
@@ -41,6 +45,7 @@ public class CommentViewModel : ReactiveObject, ICommentViewModel
         /// <param name="updatedAt">The modified date of the comment.</param>
         /// <param name="webUrl"></param>
         protected CommentViewModel(
+            ICommentService commentService,
             ICommentThreadViewModel thread,
             IActorViewModel currentUser,
             int pullRequestId,
@@ -52,6 +57,7 @@ protected CommentViewModel(
             DateTimeOffset updatedAt,
             Uri webUrl)
         {
+            this.commentService = commentService;
             Guard.ArgumentNotNull(thread, nameof(thread));
             Guard.ArgumentNotNull(currentUser, nameof(currentUser));
             Guard.ArgumentNotNull(author, nameof(author));
@@ -102,14 +108,17 @@ protected CommentViewModel(
         /// <summary>
         /// Initializes a new instance of the <see cref="CommentViewModel"/> class.
         /// </summary>
+        /// <param name="commentService">Comment Service</param>
         /// <param name="thread">The thread that the comment is a part of.</param>
         /// <param name="currentUser">The current user.</param>
         /// <param name="model">The comment model.</param>
         protected CommentViewModel(
+            ICommentService commentService,
             ICommentThreadViewModel thread,
             ActorModel currentUser,
             CommentModel model)
             : this(
+                  commentService,
                   thread, 
                   new ActorViewModel(currentUser),
                   model.PullRequestId, 
@@ -130,22 +139,25 @@ protected void AddErrorHandler<T>(ReactiveCommand<T> command)
 
         async Task DoDelete(object unused)
         {
-            try
+            if (commentService.ConfirmCommentDelete())
             {
-                ErrorMessage = null;
-                IsSubmitting = true;
+                try
+                {
+                    ErrorMessage = null;
+                    IsSubmitting = true;
 
-                await Thread.DeleteComment.ExecuteAsyncTask(new Tuple<int, int>(PullRequestId, DatabaseId));
-            }
-            catch (Exception e)
-            {
-                var message = e.Message;
-                ErrorMessage = message;
-                log.Error(e, "Error Deleting comment");
-            }
-            finally
-            {
-                IsSubmitting = false;
+                    await Thread.DeleteComment.ExecuteAsyncTask(new Tuple<int, int>(PullRequestId, DatabaseId));
+                }
+                catch (Exception e)
+                {
+                    var message = e.Message;
+                    ErrorMessage = message;
+                    log.Error(e, "Error Deleting comment");
+                }
+                finally
+                {
+                    IsSubmitting = false;
+                }
             }
         }
 

src/GitHub.InlineReviews/ViewModels/InlineCommentPeekViewModel.cs

@@ -10,6 +10,7 @@
 using GitHub.Extensions.Reactive;
 using GitHub.Factories;
 using GitHub.InlineReviews.Commands;
+using GitHub.InlineReviews.Peek;
 using GitHub.InlineReviews.Services;
 using GitHub.Logging;
 using GitHub.Models;
@@ -31,6 +32,7 @@ public sealed class InlineCommentPeekViewModel : ReactiveObject, IDisposable
         readonly IInlineCommentPeekService peekService;
         readonly IPeekSession peekSession;
         readonly IPullRequestSessionManager sessionManager;
+        readonly ICommentService commentService;
         IPullRequestSession session;
         IPullRequestSessionFile file;
         ICommentThreadViewModel thread;
@@ -44,12 +46,12 @@ public sealed class InlineCommentPeekViewModel : ReactiveObject, IDisposable
         /// <summary>
         /// Initializes a new instance of the <see cref="InlineCommentPeekViewModel"/> class.
         /// </summary>
-        public InlineCommentPeekViewModel(
-            IInlineCommentPeekService peekService,
+        public InlineCommentPeekViewModel(IInlineCommentPeekService peekService,
             IPeekSession peekSession,
             IPullRequestSessionManager sessionManager,
             INextInlineCommentCommand nextCommentCommand,
-            IPreviousInlineCommentCommand previousCommentCommand)
+            IPreviousInlineCommentCommand previousCommentCommand,
+            ICommentService commentService)
         {
             Guard.ArgumentNotNull(peekService, nameof(peekService));
             Guard.ArgumentNotNull(peekSession, nameof(peekSession));
@@ -60,6 +62,7 @@ public InlineCommentPeekViewModel(
             this.peekService = peekService;
             this.peekSession = peekSession;
             this.sessionManager = sessionManager;
+            this.commentService = commentService;
             triggerPoint = peekSession.GetTriggerPoint(peekSession.TextView.TextBuffer);
 
             peekSession.Dismissed += (s, e) => Dispose();
@@ -180,11 +183,11 @@ async Task UpdateThread()
 
             if (thread != null)
             {
-                Thread = new InlineCommentThreadViewModel(session, thread.Comments);
+                Thread = new InlineCommentThreadViewModel(commentService, session, thread.Comments);
             }
             else
             {
-                Thread = new NewInlineCommentThreadViewModel(session, file, lineNumber, leftBuffer);
+                Thread = new NewInlineCommentThreadViewModel(commentService, session, file, lineNumber, leftBuffer);
             }
 
             if (!string.IsNullOrWhiteSpace(placeholderBody))

src/GitHub.InlineReviews/ViewModels/InlineCommentThreadViewModel.cs

@@ -4,6 +4,7 @@
 using System.Reactive.Linq;
 using System.Threading.Tasks;
 using GitHub.Extensions;
+using GitHub.InlineReviews.Services;
 using GitHub.Models;
 using GitHub.Services;
 using ReactiveUI;
@@ -18,10 +19,10 @@ public class InlineCommentThreadViewModel : CommentThreadViewModel
         /// <summary>
         /// Initializes a new instance of the <see cref="InlineCommentThreadViewModel"/> class.
         /// </summary>
+        /// <param name="commentService">The comment service</param>
         /// <param name="session">The current PR review session.</param>
         /// <param name="comments">The comments to display in this inline review.</param>
-        public InlineCommentThreadViewModel(
-            IPullRequestSession session,
+        public InlineCommentThreadViewModel(ICommentService commentService, IPullRequestSession session,
             IEnumerable<InlineCommentModel> comments)
             : base(session.User)
         {
@@ -45,13 +46,14 @@ public InlineCommentThreadViewModel(
             {
                 Comments.Add(new PullRequestReviewCommentViewModel(
                     session,
+                    commentService,
                     this,
                     CurrentUser,
                     comment.Review,
                     comment.Comment));
             }
 
-            Comments.Add(PullRequestReviewCommentViewModel.CreatePlaceholder(session, this, CurrentUser));
+            Comments.Add(PullRequestReviewCommentViewModel.CreatePlaceholder(session, commentService, this, CurrentUser));
         }
 
         /// <summary>

src/GitHub.InlineReviews/ViewModels/NewInlineCommentThreadViewModel.cs

@@ -4,6 +4,7 @@
 using System.Reactive.Linq;
 using System.Threading.Tasks;
 using GitHub.Extensions;
+using GitHub.InlineReviews.Services;
 using GitHub.Models;
 using GitHub.Services;
 using ReactiveUI;
@@ -20,13 +21,14 @@ public class NewInlineCommentThreadViewModel : CommentThreadViewModel
         /// <summary>
         /// Initializes a new instance of the <see cref="InlineCommentThreadViewModel"/> class.
         /// </summary>
+        /// <param name="commentService">The comment service</param>
         /// <param name="session">The current PR review session.</param>
         /// <param name="file">The file being commented on.</param>
         /// <param name="lineNumber">The 0-based line number in the file.</param>
         /// <param name="leftComparisonBuffer">
-        /// True if the comment is being left on the left-hand-side of a diff; otherwise false.
+        ///     True if the comment is being left on the left-hand-side of a diff; otherwise false.
         /// </param>
-        public NewInlineCommentThreadViewModel(
+        public NewInlineCommentThreadViewModel(ICommentService commentService,
             IPullRequestSession session,
             IPullRequestSessionFile file,
             int lineNumber,
@@ -53,7 +55,7 @@ public NewInlineCommentThreadViewModel(
                 Observable.Return(false),
                 o => null);
 
-            var placeholder = PullRequestReviewCommentViewModel.CreatePlaceholder(session, this, CurrentUser);
+            var placeholder = PullRequestReviewCommentViewModel.CreatePlaceholder(session, commentService, this, CurrentUser);
             placeholder.BeginEdit.Execute(null);
             this.WhenAnyValue(x => x.NeedsPush).Subscribe(x => placeholder.IsReadOnly = x);
             Comments.Add(placeholder);