Merge pull request #1838 from github/fixes/pull-request-review-cancel

Adding functionality to confirm before cancelling a review

Author: StanleyGoldman

src/GitHub.App/GitHub.App.csproj

@@ -197,6 +197,7 @@
       <HintPath>..\..\packages\System.ValueTuple.4.5.0\lib\net461\System.ValueTuple.dll</HintPath>
     </Reference>
     <Reference Include="System.Web" />
+    <Reference Include="System.Windows.Forms" />
     <Reference Include="System.Xaml" />
     <Reference Include="System.Xml.Linq" />
     <Reference Include="System.Data.DataSetExtensions" />

src/GitHub.App/Resources.Designer.cs

@@ -321,4 +321,10 @@ https://git-scm.com/download/win</value>
   <data name="SwitchOriginTitle" xml:space="preserve">
     <value>Switch Origin</value>
   </data>
+  <data name="CancelPendingReviewConfirmation" xml:space="preserve">
+    <value>Are you sure you want to cancel this review? You will lose all your pending comments.</value>
+  </data>
+  <data name="CancelPendingReviewConfirmationCaption" xml:space="preserve">
+    <value>Cancel Review</value>
+  </data>
 </root>

src/GitHub.App/Resources.resx

@@ -11,6 +11,7 @@
 using System.Text;
 using System.Text.RegularExpressions;
 using System.Threading.Tasks;
+using System.Windows.Forms;
 using GitHub.Api;
 using GitHub.Extensions;
 using GitHub.Logging;
@@ -714,6 +715,16 @@ public IObservable<Unit> RemoveUnusedRemotes(ILocalRepositoryModel repository)
             });
         }
 
+        /// <inheritdoc />
+        public bool ConfirmCancelPendingReview()
+        {
+            return MessageBox.Show(
+                       GitHub.App.Resources.CancelPendingReviewConfirmation,
+                       GitHub.App.Resources.CancelPendingReviewConfirmationCaption,
+                       MessageBoxButtons.YesNo,
+                       MessageBoxIcon.Question) == DialogResult.Yes;
+        }
+
         async Task<string> CreateRemote(IRepository repo, UriString cloneUri)
         {
             foreach (var remote in repo.Network.Remotes)

src/GitHub.App/Services/PullRequestService.cs

@@ -24,6 +24,7 @@ public class PullRequestReviewAuthoringViewModel : PanePageViewModelBase, IPullR
 
         readonly IPullRequestEditorService editorService;
         readonly IPullRequestSessionManager sessionManager;
+        readonly IPullRequestService pullRequestService;
         IPullRequestSession session;
         IDisposable sessionSubscription;
         PullRequestReviewModel model;
@@ -35,6 +36,7 @@ public class PullRequestReviewAuthoringViewModel : PanePageViewModelBase, IPullR
 
         [ImportingConstructor]
         public PullRequestReviewAuthoringViewModel(
+            IPullRequestService pullRequestService,
             IPullRequestEditorService editorService,
             IPullRequestSessionManager sessionManager,
             IPullRequestFilesViewModel files)
@@ -43,6 +45,7 @@ public PullRequestReviewAuthoringViewModel(
             Guard.ArgumentNotNull(sessionManager, nameof(sessionManager));
             Guard.ArgumentNotNull(files, nameof(files));
 
+            this.pullRequestService = pullRequestService;
             this.editorService = editorService;
             this.sessionManager = sessionManager;
 
@@ -271,10 +274,17 @@ async Task DoCancel(object arg)
             {
                 if (Model?.Id != null)
                 {
-                    await session.CancelReview();
+                    if (pullRequestService.ConfirmCancelPendingReview())
+                    {
+                        await session.CancelReview();
+                        Close();
+                    }
+                }
+                else
+                {
+                    Close();
                 }
 
-                Close();
             }
             catch (Exception ex)
             {

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

@@ -224,5 +224,11 @@ IObservable<IReadOnlyList<CommitMessage>> GetMessagesForUniqueCommits(
             string baseBranch,
             string compareBranch,
             int maxCommits);
+
+        /// <summary>
+        /// Displays a confirmation diaglog to ask if the user wants to cancel a pending review.
+        /// </summary>
+        /// <returns></returns>
+        bool ConfirmCancelPendingReview();
     }
 }

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

@@ -368,7 +368,10 @@ public async Task Cancel_Calls_Session_CancelReview_And_Closes_When_Has_Pending_
             var session = CreateSession(model: model);
             var closed = false;
 
-            var target = CreateTarget(model, session);
+            var pullRequestService = Substitute.For<IPullRequestService>();
+            pullRequestService.ConfirmCancelPendingReview().Returns(true);
+
+            var target = CreateTarget(model, session, pullRequestService);
             await InitializeAsync(target);
 
             target.CloseRequested.Subscribe(_ => closed = true);
@@ -397,15 +400,18 @@ public async Task Cancel_Just_Closes_When_Has_No_Pending_Review_Async()
 
         static PullRequestReviewAuthoringViewModel CreateTarget(
             PullRequestDetailModel model,
-            IPullRequestSession session = null)
+            IPullRequestSession session = null,
+            IPullRequestService pullRequestService = null)
         {
             session = session ?? CreateSession(model: model);
 
             return CreateTarget(
+                pullRequestService: pullRequestService,
                 sessionManager: CreateSessionManager(session));
         }
 
         static PullRequestReviewAuthoringViewModel CreateTarget(
+            IPullRequestService pullRequestService = null,
             IPullRequestEditorService editorService = null,
             IPullRequestSessionManager sessionManager = null,
             IPullRequestFilesViewModel files = null)
@@ -415,6 +421,7 @@ static PullRequestReviewAuthoringViewModel CreateTarget(
             files = files ?? Substitute.For<IPullRequestFilesViewModel>();
 
             return new PullRequestReviewAuthoringViewModel(
+                pullRequestService,
                 editorService,
                 sessionManager,
                 files);