diff --git a/src/applications/feed/story/PhabricatorFeedStory.php b/src/applications/feed/story/PhabricatorFeedStory.php index 69fdf7c02c..51118cc834 100644 --- a/src/applications/feed/story/PhabricatorFeedStory.php +++ b/src/applications/feed/story/PhabricatorFeedStory.php @@ -14,6 +14,7 @@ abstract class PhabricatorFeedStory implements PhabricatorPolicyInterface { private $hasViewed; private $framed; private $hovercard = false; + private $renderingTarget = PhabricatorApplicationTransaction::TARGET_HTML; private $handles = array(); private $objects = array(); @@ -124,6 +125,27 @@ abstract class PhabricatorFeedStory implements PhabricatorPolicyInterface { return $this; } + public function setRenderingTarget($target) { + $this->validateRenderingTarget($target); + $this->renderingTarget = $target; + return $this; + } + + public function getRenderingTarget() { + return $this->renderingTarget; + } + + private function validateRenderingTarget($target) { + switch ($target) { + case PhabricatorApplicationTransaction::TARGET_HTML: + case PhabricatorApplicationTransaction::TARGET_TEXT: + break; + default: + throw new Exception('Unknown rendering target: '.$target); + break; + } + } + public function setObjects(array $objects) { $this->objects = $objects; return $this; @@ -227,16 +249,30 @@ abstract class PhabricatorFeedStory implements PhabricatorPolicyInterface { } final protected function renderHandleList(array $phids) { - $list = array(); + $items = array(); foreach ($phids as $phid) { - $list[] = $this->linkTo($phid); + $items[] = $this->linkTo($phid); } - return phutil_implode_html(', ', $list); + $list = null; + switch ($this->getRenderingTarget()) { + case PhabricatorApplicationTransaction::TARGET_TEXT: + $list = implode(', ', $items); + break; + case PhabricatorApplicationTransaction::TARGET_HTML: + $list = phutil_implode_html(', ', $items); + break; + } + return $list; } final protected function linkTo($phid) { $handle = $this->getHandle($phid); + switch ($this->getRenderingTarget()) { + case PhabricatorApplicationTransaction::TARGET_TEXT: + return $handle->getLinkName(); + } + // NOTE: We render our own link here to customize the styling and add // the '_top' target for framed feeds. @@ -258,14 +294,23 @@ abstract class PhabricatorFeedStory implements PhabricatorPolicyInterface { } final protected function renderString($str) { - return phutil_tag('strong', array(), $str); + switch ($this->getRenderingTarget()) { + case PhabricatorApplicationTransaction::TARGET_TEXT: + return $str; + case PhabricatorApplicationTransaction::TARGET_HTML: + return phutil_tag('strong', array(), $str); + } } final protected function renderSummary($text, $len = 128) { if ($len) { $text = phutil_utf8_shorten($text, $len); } - $text = phutil_escape_html_newlines($text); + switch ($this->getRenderingTarget()) { + case PhabricatorApplicationTransaction::TARGET_HTML: + $text = phutil_escape_html_newlines($text); + break; + } return $text; } diff --git a/src/applications/maniphest/controller/ManiphestTransactionSaveController.php b/src/applications/maniphest/controller/ManiphestTransactionSaveController.php index 181f1df0c5..2ca12c1846 100644 --- a/src/applications/maniphest/controller/ManiphestTransactionSaveController.php +++ b/src/applications/maniphest/controller/ManiphestTransactionSaveController.php @@ -152,10 +152,10 @@ final class ManiphestTransactionSaveController extends ManiphestController { if ($action == ManiphestTransaction::TYPE_OWNER) { if ($task->getOwnerPHID() == $transaction->getNewValue()) { // If this is actually no-op, don't generate the side effect. - break; + } else { + // Otherwise, when a task is reassigned, move the previous owner to CC. + $added_ccs[] = $task->getOwnerPHID(); } - // Otherwise, when a task is reassigned, move the previous owner to CC. - $added_ccs[] = $task->getOwnerPHID(); } if ($did_scuttle || ($action == ManiphestTransaction::TYPE_STATUS)) { diff --git a/src/applications/tokens/feed/PhabricatorTokenGivenFeedStory.php b/src/applications/tokens/feed/PhabricatorTokenGivenFeedStory.php index f8a89d968b..831f2cd507 100644 --- a/src/applications/tokens/feed/PhabricatorTokenGivenFeedStory.php +++ b/src/applications/tokens/feed/PhabricatorTokenGivenFeedStory.php @@ -28,27 +28,29 @@ final class PhabricatorTokenGivenFeedStory $href = $this->getHandle($this->getPrimaryObjectPHID())->getURI(); $view->setHref($href); - $token = $this->getObject($this->getValue('tokenPHID')); + $view->setTitle($this->renderTitle()); + $view->setImage($this->getHandle($author_phid)->getImageURI()); + return $view; + } + + private function renderTitle() { + $token = $this->getObject($this->getValue('tokenPHID')); $title = pht( '%s awarded %s a %s token.', $this->linkTo($this->getValue('authorPHID')), $this->linkTo($this->getValue('objectPHID')), $token->getName()); - $view->setTitle($title); - $view->setImage($this->getHandle($author_phid)->getImageURI()); - - return $view; + return $title; } public function renderText() { - // TODO: This is grotesque; the feed notification handler relies on it. - return htmlspecialchars_decode( - strip_tags( - hsprintf( - '%s', - $this->renderView()->getTitle()))); + $old_target = $this->getRenderingTarget(); + $this->setRenderingTarget(PhabricatorApplicationTransaction::TARGET_TEXT); + $title = $this->renderTitle(); + $this->setRenderingTarget($old_target); + return $title; } } diff --git a/src/applications/transactions/feed/PhabricatorApplicationTransactionFeedStory.php b/src/applications/transactions/feed/PhabricatorApplicationTransactionFeedStory.php index c6916ce38b..34945abdc6 100644 --- a/src/applications/transactions/feed/PhabricatorApplicationTransactionFeedStory.php +++ b/src/applications/transactions/feed/PhabricatorApplicationTransactionFeedStory.php @@ -65,12 +65,13 @@ class PhabricatorApplicationTransactionFeedStory } public function renderText() { - // TODO: This is grotesque; the feed notification handler relies on it. - return htmlspecialchars_decode( - strip_tags( - hsprintf( - '%s', - $this->renderView()->getTitle()))); + $xaction = $this->getPrimaryTransaction(); + $old_target = $xaction->getRenderingTarget(); + $new_target = PhabricatorApplicationTransaction::TARGET_TEXT; + $xaction->setRenderingTarget($new_target); + $text = $xaction->getTitleForFeed($this); + $xaction->setRenderingTarget($old_target); + return $text; } }