Differential - return a better response for validation error cases

Summary: Fixes T6989. Basically return a nice dialogue like we do for "NoEffect" transactions. This is a little prettier than the other dialogue was. Also, stop adding TYPE_EDGE as a transaction type as we end up having it 2x, which then makes the error get validated 2x.

Test Plan: tried to add myself as a reviewer and got a nice error message.

Reviewers: chad, epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T6989

Differential Revision: https://secure.phabricator.com/D11448

src/__phutil_library_map__.php

@@ -1290,6 +1290,7 @@ phutil_register_library_map(array(
     'PhabricatorApplicationTransactionTransactionPHIDType' => 'applications/transactions/phid/PhabricatorApplicationTransactionTransactionPHIDType.php',
     'PhabricatorApplicationTransactionValidationError' => 'applications/transactions/error/PhabricatorApplicationTransactionValidationError.php',
     'PhabricatorApplicationTransactionValidationException' => 'applications/transactions/exception/PhabricatorApplicationTransactionValidationException.php',
+    'PhabricatorApplicationTransactionValidationResponse' => 'applications/transactions/response/PhabricatorApplicationTransactionValidationResponse.php',
     'PhabricatorApplicationTransactionValueController' => 'applications/transactions/controller/PhabricatorApplicationTransactionValueController.php',
     'PhabricatorApplicationTransactionView' => 'applications/transactions/view/PhabricatorApplicationTransactionView.php',
     'PhabricatorApplicationUninstallController' => 'applications/meta/controller/PhabricatorApplicationUninstallController.php',
@@ -4476,6 +4477,7 @@ phutil_register_library_map(array(
     'PhabricatorApplicationTransactionTransactionPHIDType' => 'PhabricatorPHIDType',
     'PhabricatorApplicationTransactionValidationError' => 'Phobject',
     'PhabricatorApplicationTransactionValidationException' => 'Exception',
+    'PhabricatorApplicationTransactionValidationResponse' => 'AphrontProxyResponse',
     'PhabricatorApplicationTransactionValueController' => 'PhabricatorApplicationTransactionController',
     'PhabricatorApplicationTransactionView' => 'AphrontView',
     'PhabricatorApplicationUninstallController' => 'PhabricatorApplicationsController',

src/applications/differential/controller/DifferentialCommentSaveController.php

@@ -127,8 +127,9 @@ final class DifferentialCommentSaveController
         ->setCancelURI($revision_uri)
         ->setException($ex);
     } catch (PhabricatorApplicationTransactionValidationException $ex) {
-      // TODO: Provide a nice Response for rendering these in a clean way.
-      throw $ex;
+      return id(new PhabricatorApplicationTransactionValidationResponse())
+        ->setCancelURI($revision_uri)
+        ->setException($ex);
     }
 
     $user = $request->getUser();

src/applications/differential/editor/DifferentialTransactionEditor.php

@@ -55,7 +55,6 @@ final class DifferentialTransactionEditor
     $types = parent::getTransactionTypes();
 
     $types[] = PhabricatorTransactions::TYPE_COMMENT;
-    $types[] = PhabricatorTransactions::TYPE_EDGE;
     $types[] = PhabricatorTransactions::TYPE_VIEW_POLICY;
     $types[] = PhabricatorTransactions::TYPE_EDIT_POLICY;
 

src/applications/transactions/response/PhabricatorApplicationTransactionValidationResponse.php

@@ -0,0 +1,49 @@
+<?php
+
+final class PhabricatorApplicationTransactionValidationResponse
+  extends AphrontProxyResponse {
+
+  private $viewer;
+  private $exception;
+  private $cancelURI;
+
+  public function setCancelURI($cancel_uri) {
+    $this->cancelURI = $cancel_uri;
+    return $this;
+  }
+
+  public function setException(
+    PhabricatorApplicationTransactionValidationException $exception) {
+    $this->exception = $exception;
+    return $this;
+  }
+
+  protected function buildProxy() {
+    return new AphrontDialogResponse();
+  }
+
+  public function reduceProxyResponse() {
+    $request = $this->getRequest();
+
+    $ex = $this->exception;
+    $title = pht('Validation Errors');
+
+    $dialog = id(new AphrontDialogView())
+      ->setUser($request->getUser())
+      ->setTitle($title);
+
+    $list = array();
+    foreach ($ex->getErrors() as $error) {
+      $list[] = phutil_tag(
+        'li',
+        array(),
+        $error->getMessage());
+    }
+
+    $dialog->appendChild(phutil_tag('ul', array(), $list));
+    $dialog->addCancelButton($this->cancelURI);
+
+    return $this->getProxy()->setDialog($dialog);
+  }
+
+}