From 57909a705ca832001e58d6bef4af41f6c019bacd Mon Sep 17 00:00:00 2001 From: epriestley Date: Mon, 21 Dec 2015 16:26:34 -0800 Subject: [PATCH] Improve strings for creating blocking subtasks Summary: Ref T6884. Ref T10004. For various reasons we previously didn't publish these transactions, but now do. This is probably a better behavior overall, but we didn't have reasonable strings for them. Parent tasks now show "alice created blocking task Txxx.". Feed now shows nothing, since "alice created task Txxx." is right next to any story we would show and showing them both seems silly. Test Plan: - Created subtasks. - Viewed parent tasks. - Viewed feed. - Saw pretty reasonable strings/stories. Reviewers: chad Reviewed By: chad Maniphest Tasks: T6884, T10004 Differential Revision: https://secure.phabricator.com/D14849 --- .../editor/ManiphestTransactionEditor.php | 10 +++++---- .../storage/ManiphestTransaction.php | 22 ++++++++++++++++++- 2 files changed, 27 insertions(+), 5 deletions(-) diff --git a/src/applications/maniphest/editor/ManiphestTransactionEditor.php b/src/applications/maniphest/editor/ManiphestTransactionEditor.php index 037286244a..42170d4c53 100644 --- a/src/applications/maniphest/editor/ManiphestTransactionEditor.php +++ b/src/applications/maniphest/editor/ManiphestTransactionEditor.php @@ -371,20 +371,22 @@ final class ManiphestTransactionEditor $new = $unblock_xaction->getNewValue(); foreach ($blocked_tasks as $blocked_task) { - $unblock_xactions = array(); - - $unblock_xactions[] = id(new ManiphestTransaction()) + $parent_xaction = id(new ManiphestTransaction()) ->setTransactionType(ManiphestTransaction::TYPE_UNBLOCK) ->setOldValue(array($object->getPHID() => $old)) ->setNewValue(array($object->getPHID() => $new)); + if ($this->getIsNewObject()) { + $parent_xaction->setMetadataValue('blocker.new', true); + } + id(new ManiphestTransactionEditor()) ->setActor($this->getActor()) ->setActingAsPHID($this->getActingAsPHID()) ->setContentSource($this->getContentSource()) ->setContinueOnNoEffect(true) ->setContinueOnMissingFields(true) - ->applyTransactions($blocked_task, $unblock_xactions); + ->applyTransactions($blocked_task, array($parent_xaction)); } } } diff --git a/src/applications/maniphest/storage/ManiphestTransaction.php b/src/applications/maniphest/storage/ManiphestTransaction.php index dc1030265b..6c2c8c306f 100644 --- a/src/applications/maniphest/storage/ManiphestTransaction.php +++ b/src/applications/maniphest/storage/ManiphestTransaction.php @@ -167,6 +167,21 @@ final class ManiphestTransaction return parent::shouldHide(); } + public function shouldHideForFeed() { + switch ($this->getTransactionType()) { + case self::TYPE_UNBLOCK: + // Hide "alice created X, a task blocking Y." from feed because it + // will almost always appear adjacent to "alice created Y". + $is_new = $this->getMetadataValue('blocker.new'); + if ($is_new) { + return true; + } + break; + } + + return parent::shouldHideForFeed(); + } + public function getActionStrength() { switch ($this->getTransactionType()) { case self::TYPE_TITLE: @@ -479,7 +494,12 @@ final class ManiphestTransaction $old_name = ManiphestTaskStatus::getTaskStatusName($old_status); $new_name = ManiphestTaskStatus::getTaskStatusName($new_status); - if ($old_closed && !$new_closed) { + if ($this->getMetadataValue('blocker.new')) { + return pht( + '%s created blocking task %s.', + $this->renderHandleLink($author_phid), + $this->renderHandleLink($blocker_phid)); + } else if ($old_closed && !$new_closed) { return pht( '%s reopened blocking task %s as "%s".', $this->renderHandleLink($author_phid),