diff --git a/src/applications/maniphest/controller/ManiphestTransactionPreviewController.php b/src/applications/maniphest/controller/ManiphestTransactionPreviewController.php index aeb6f80392..2f20c70c84 100644 --- a/src/applications/maniphest/controller/ManiphestTransactionPreviewController.php +++ b/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(); diff --git a/src/applications/maniphest/controller/ManiphestTransactionSaveController.php b/src/applications/maniphest/controller/ManiphestTransactionSaveController.php index 4bb0405a99..613ed3ef58 100644 --- a/src/applications/maniphest/controller/ManiphestTransactionSaveController.php +++ b/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', diff --git a/src/applications/maniphest/editor/ManiphestTransactionEditorPro.php b/src/applications/maniphest/editor/ManiphestTransactionEditorPro.php index fe992aa1a0..81978fb380 100644 --- a/src/applications/maniphest/editor/ManiphestTransactionEditorPro.php +++ b/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: diff --git a/src/applications/maniphest/storage/ManiphestTransactionPro.php b/src/applications/maniphest/storage/ManiphestTransactionPro.php index ba3b279ce5..2c2b1ded30 100644 --- a/src/applications/maniphest/storage/ManiphestTransactionPro.php +++ b/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: