Route Maniphest comments/detail edits through new code

Summary: Ref T2217. When you add comments (or use that interface to make updates), ship it through the new code.

Test Plan: Added comments, made other changes to Maniphest tasks.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T2217

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

src/applications/maniphest/controller/ManiphestTransactionPreviewController.php

@@ -78,17 +78,19 @@ final class ManiphestTransactionPreviewController extends ManiphestController {
       case ManiphestTransactionType::TYPE_PROJECTS:
         if ($value) {
           $value = json_decode($value);
-          $phids = $value;
-          foreach ($task->getProjectPHIDs() as $project_phid) {
-            $phids[] = $project_phid;
-            $value[] = $project_phid;
-          }
-          $transaction->setNewValue($value);
-        } else {
-          $phids = array();
-          $transaction->setNewValue(array());
         }
+        if (!$value) {
+          $value = array();
+        }
+
+        $phids = $value;
+        foreach ($task->getProjectPHIDs() as $project_phid) {
+          $phids[] = $project_phid;
+          $value[] = $project_phid;
+        }
+
         $transaction->setOldValue($task->getProjectPHIDs());
+        $transaction->setNewValue($value);
         break;
       default:
         $phids = array();

src/applications/maniphest/controller/ManiphestTransactionSaveController.php

@@ -59,9 +59,8 @@ final class ManiphestTransactionSaveController extends ManiphestController {
         }
         $new[PhabricatorFilePHIDTypeFile::TYPECONST][$phid] = array();
       }
-      $transaction = new ManiphestTransaction();
+      $transaction = new ManiphestTransactionPro();
       $transaction
-        ->setAuthorPHID($user->getPHID())
         ->setTransactionType(ManiphestTransactionType::TYPE_ATTACH);
       $transaction->setNewValue($new);
       $transactions[] = $transaction;
@@ -77,15 +76,13 @@ final class ManiphestTransactionSaveController extends ManiphestController {
         $request->getStr('comments'),
       ));
 
-    $cc_transaction = new ManiphestTransaction();
+    $cc_transaction = new ManiphestTransactionPro();
     $cc_transaction
-      ->setAuthorPHID($user->getPHID())
       ->setTransactionType(ManiphestTransactionType::TYPE_CCS);
     $force_cc_transaction = false;
 
-    $transaction = new ManiphestTransaction();
+    $transaction = new ManiphestTransactionPro();
     $transaction
-      ->setAuthorPHID($user->getPHID())
       ->setTransactionType($action);
 
     switch ($action) {
@@ -139,10 +136,11 @@ final class ManiphestTransactionSaveController extends ManiphestController {
     }
 
     if ($request->getStr('comments')) {
-      $transactions[] = id(new ManiphestTransaction())
-        ->setAuthorPHID($user->getPHID())
+      $transactions[] = id(new ManiphestTransactionPro())
         ->setTransactionType(PhabricatorTransactions::TYPE_COMMENT)
-        ->setComments($request->getStr('comments'));
+        ->attachComment(
+          id(new ManiphestTransactionComment())
+            ->setContent($request->getStr('comments')));
     }
 
     // When you interact with a task, we add you to the CC list so you get
@@ -168,7 +166,6 @@ final class ManiphestTransactionSaveController extends ManiphestController {
           // Closing an unassigned task. Assign the user as the owner of
           // this task.
           $assign = new ManiphestTransaction();
-          $assign->setAuthorPHID($user->getPHID());
           $assign->setTransactionType(ManiphestTransactionType::TYPE_OWNER);
           $assign->setNewValue($user->getPHID());
           $transactions[] = $assign;
@@ -194,8 +191,10 @@ final class ManiphestTransactionSaveController extends ManiphestController {
 
     if (!$user_owns_task) {
       // If we aren't making the user the new task owner and they aren't the
-      // existing task owner, add them to CC.
-      $added_ccs[] = $user->getPHID();
+      // existing task owner, add them to CC unless they're aleady CC'd.
+      if (!in_array($user->getPHID(), $task->getCCPHIDs())) {
+        $added_ccs[] = $user->getPHID();
+      }
     }
 
     if ($added_ccs || $force_cc_transaction) {
@@ -207,16 +206,6 @@ final class ManiphestTransactionSaveController extends ManiphestController {
       $transactions[] = $cc_transaction;
     }
 
-    $content_source = PhabricatorContentSource::newForSource(
-      PhabricatorContentSource::SOURCE_WEB,
-      array(
-        'ip' => $request->getRemoteAddr(),
-      ));
-
-    foreach ($transactions as $transaction) {
-      $transaction->setContentSource($content_source);
-    }
-
     $event = new PhabricatorEvent(
       PhabricatorEventType::TYPE_MANIPHEST_WILLEDITTASK,
       array(
@@ -231,9 +220,11 @@ final class ManiphestTransactionSaveController extends ManiphestController {
     $task = $event->getValue('task');
     $transactions = $event->getValue('transactions');
 
-    $editor = new ManiphestTransactionEditor();
-    $editor->setActor($user);
-    $editor->applyTransactions($task, $transactions);
+    $editor = id(new ManiphestTransactionEditorPro())
+      ->setActor($user)
+      ->setContentSourceFromRequest($request)
+      ->setContinueOnMissingFields(true)
+      ->applyTransactions($task, $transactions);
 
     $draft = id(new PhabricatorDraft())->loadOneWhere(
       'authorPHID = %s AND draftKey = %s',

src/applications/maniphest/editor/ManiphestTransactionEditorPro.php

@@ -36,7 +36,7 @@ final class ManiphestTransactionEditorPro
       case ManiphestTransactionPro::TYPE_OWNER:
         return $object->getOwnerPHID();
       case ManiphestTransactionPro::TYPE_CCS:
-        return $object->getCCPHIDs();
+        return array_values(array_unique($object->getCCPHIDs()));
       case ManiphestTransactionPro::TYPE_PROJECTS:
         return $object->getProjectPHIDs();
       case ManiphestTransactionPro::TYPE_ATTACH:
@@ -56,10 +56,11 @@ final class ManiphestTransactionEditorPro
       case ManiphestTransactionPro::TYPE_PRIORITY:
       case ManiphestTransactionPro::TYPE_STATUS:
         return (int)$xaction->getNewValue();
+      case ManiphestTransactionPro::TYPE_CCS:
+        return array_values(array_unique($xaction->getNewValue()));
       case ManiphestTransactionPro::TYPE_TITLE:
       case ManiphestTransactionPro::TYPE_DESCRIPTION:
       case ManiphestTransactionPro::TYPE_OWNER:
-      case ManiphestTransactionPro::TYPE_CCS:
       case ManiphestTransactionPro::TYPE_PROJECTS:
       case ManiphestTransactionPro::TYPE_ATTACH:
       case ManiphestTransactionPro::TYPE_EDGE:

src/applications/maniphest/storage/ManiphestTransactionPro.php

@@ -306,7 +306,7 @@ final class ManiphestTransactionPro
             $this->renderHandleLink($author_phid),
             count($removed),
             $this->renderHandleList($removed));
-        } else {
+        } else if ($removed && $added) {
           return pht(
             '%s changed projects, added %d: %s; removed %d: %s',
             $this->renderHandleLink($author_phid),
@@ -314,6 +314,11 @@ final class ManiphestTransactionPro
             $this->renderHandleList($added),
             count($removed),
             $this->renderHandleList($removed));
+        } else {
+          // This is hit when rendering previews.
+          return pht(
+            '%s changed projects...',
+            $this->renderHandleLink($author_phid));
         }
 
       case self::TYPE_PRIORITY: