Use modern ApplicationTransactions "no effect" stuff in Maniphest

Summary: Fixes T912. This was very nearly working, it just needed a little tweaking on the last mile.

Test Plan:
Made updates with no effect, and updates with an effect. Made a no-effect update and posted just the comment part.

{F129037}

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T912

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

src/applications/maniphest/controller/ManiphestTaskDetailController.php

@@ -224,6 +224,7 @@ final class ManiphestTaskDetailController extends ManiphestController {
     $comment_form = new AphrontFormView();
     $comment_form
       ->setUser($user)
+      ->setWorkflow(true)
       ->setAction('/maniphest/transaction/save/')
       ->setEncType('multipart/form-data')
       ->addHiddenInput('taskID', $task->getID())

src/applications/maniphest/controller/ManiphestTransactionSaveController.php

@@ -1,18 +1,11 @@
 <?php
 
-/**
- * @group maniphest
- */
 final class ManiphestTransactionSaveController extends ManiphestController {
 
   public function processRequest() {
     $request = $this->getRequest();
     $user = $request->getUser();
 
-    // TODO: T603 This doesn't require CAN_EDIT because non-editors can still
-    // leave comments, probably? For now, this just nondisruptive. Smooth this
-    // out once policies are more clear.
-
     $task = id(new ManiphestTaskQuery())
       ->setViewer($user)
       ->withIDs(array($request->getStr('taskID')))
@@ -21,6 +14,8 @@ final class ManiphestTransactionSaveController extends ManiphestController {
       return new Aphront404Response();
     }
 
+    $task_uri = '/'.$task->getMonogram();
+
     $transactions = array();
 
     $action = $request->getStr('action');
@@ -135,14 +130,6 @@ final class ManiphestTransactionSaveController extends ManiphestController {
       $transactions[] = $transaction;
     }
 
-    if ($request->getStr('comments')) {
-      $transactions[] = id(new ManiphestTransaction())
-        ->setTransactionType(PhabricatorTransactions::TYPE_COMMENT)
-        ->attachComment(
-          id(new ManiphestTransactionComment())
-            ->setContent($request->getStr('comments')));
-    }
-
     // When you interact with a task, we add you to the CC list so you get
     // further updates, and possibly assign the task to you if you took an
     // ownership action (closing it) but it's currently unowned. We also move
@@ -208,6 +195,15 @@ final class ManiphestTransactionSaveController extends ManiphestController {
       $transactions[] = $cc_transaction;
     }
 
+    $comments = $request->getStr('comments');
+    if (strlen($comments) || !$transactions) {
+      $transactions[] = id(new ManiphestTransaction())
+        ->setTransactionType(PhabricatorTransactions::TYPE_COMMENT)
+        ->attachComment(
+          id(new ManiphestTransactionComment())
+            ->setContent($comments));
+    }
+
     $event = new PhabricatorEvent(
       PhabricatorEventType::TYPE_MANIPHEST_WILLEDITTASK,
       array(
@@ -226,7 +222,15 @@ final class ManiphestTransactionSaveController extends ManiphestController {
       ->setActor($user)
       ->setContentSourceFromRequest($request)
       ->setContinueOnMissingFields(true)
-      ->applyTransactions($task, $transactions);
+      ->setContinueOnNoEffect($request->isContinueRequest());
+
+    try {
+      $editor->applyTransactions($task, $transactions);
+    } catch (PhabricatorApplicationTransactionNoEffectException $ex) {
+      return id(new PhabricatorApplicationTransactionNoEffectResponse())
+        ->setCancelURI($task_uri)
+        ->setException($ex);
+    }
 
     $draft = id(new PhabricatorDraft())->loadOneWhere(
       'authorPHID = %s AND draftKey = %s',
@@ -247,8 +251,7 @@ final class ManiphestTransactionSaveController extends ManiphestController {
     $event->setAphrontRequest($request);
     PhutilEventEngine::dispatchEvent($event);
 
-    return id(new AphrontRedirectResponse())
-      ->setURI('/T'.$task->getID());
+    return id(new AphrontRedirectResponse())->setURI($task_uri);
   }
 
 }

src/applications/maniphest/storage/ManiphestTask.php

@@ -119,6 +119,10 @@ final class ManiphestTask extends ManiphestDAO
     return $this;
   }
 
+  public function getMonogram() {
+    return 'T'.$this->getID();
+  }
+
   public function attachGroupByProjectPHID($phid) {
     $this->groupByProjectPHID = $phid;
     return $this;

src/applications/maniphest/storage/ManiphestTransaction.php

@@ -709,5 +709,20 @@ final class ManiphestTransaction
     return $tags;
   }
 
+  public function getNoEffectDescription() {
+
+    switch ($this->getTransactionType()) {
+      case self::TYPE_STATUS:
+        return pht('The task already has the selected status.');
+      case self::TYPE_OWNER:
+        return pht('The task already has the selected owner.');
+      case self::TYPE_PROJECTS:
+        return pht('The task is already associated with those projects.');
+      case self::TYPE_PRIORITY:
+        return pht('The task already has the selected priority.');
+    }
+
+    return parent::getNoEffectDescription();
+  }
 
 }