From 3b801fa5678adfbc39ac8900e74423059a362a3f Mon Sep 17 00:00:00 2001 From: Anh Nhan Nguyen Date: Tue, 19 Mar 2013 14:22:26 -0700 Subject: [PATCH] Adding a proper story feed for moving a Phriction Document Summary: Display a proper feed title when moving Phriction Documents. Test Plan: {F36112, size=full} Descriptions for the feeds you see in the image. # New and cool story feed # Fallback for the boring old ones # Normal story feed, unchanged Reviewers: epriestley Reviewed By: epriestley CC: aran, Korvin Maniphest Tasks: T2686 Differential Revision: https://secure.phabricator.com/D5352 --- .../story/PhabricatorFeedStoryPhriction.php | 52 ++++++++++++++++--- .../constants/PhrictionActionConstants.php | 4 +- .../controller/PhrictionMoveController.php | 2 +- .../editor/PhrictionDocumentEditor.php | 23 +++++--- 4 files changed, 65 insertions(+), 16 deletions(-) diff --git a/src/applications/feed/story/PhabricatorFeedStoryPhriction.php b/src/applications/feed/story/PhabricatorFeedStoryPhriction.php index 4791214b75..08f8d37dda 100644 --- a/src/applications/feed/story/PhabricatorFeedStoryPhriction.php +++ b/src/applications/feed/story/PhabricatorFeedStoryPhriction.php @@ -6,6 +6,15 @@ final class PhabricatorFeedStoryPhriction extends PhabricatorFeedStory { return $this->getValue('phid'); } + public function getRequiredHandlePHIDs() { + $required_phids = parent::getRequiredHandlePHIDs(); + $from_phid = $this->getStoryData()->getValue('movedFromPHID'); + if ($from_phid) { + $required_phids[] = $from_phid; + } + return $required_phids; + } + public function renderView() { $data = $this->getStoryData(); @@ -17,14 +26,45 @@ final class PhabricatorFeedStoryPhriction extends PhabricatorFeedStory { $action = $data->getValue('action'); $verb = PhrictionActionConstants::getActionPastTenseVerb($action); - $view->setTitle(hsprintf( - '%s %s the document %s.', - $this->linkTo($author_phid), - $verb, - $this->linkTo($document_phid))); + switch ($action) { + case PhrictionActionConstants::ACTION_MOVE_HERE: + $from_phid = $data->getValue('movedFromPHID'); + + // Older feed stories may not have 'moved_from_phid', in that case + // we fall back to the default behaviour (hence the fallthrough) + if ($from_phid) { + $document_handle = $this->getHandle($document_phid); + $from_handle = $this->getHandle($from_phid); + $view->setTitle(pht( + '%s moved the document %s from %s to %s.', + $this->linkTo($author_phid), + $document_handle->renderLink(), + phutil_tag( + 'a', + array( + 'href' => $from_handle->getURI(), + ), + $from_handle->getURI()), + phutil_tag( + 'a', + array( + 'href' => $document_handle->getURI(), + ), + $document_handle->getURI()))); + break; + } + /* Fallthrough */ + default: + $view->setTitle(pht( + '%s %s the document %s.', + $this->linkTo($author_phid), + $verb, + $this->linkTo($document_phid))); + break; + } + $view->setEpoch($data->getEpoch()); - $action = $data->getValue('action'); switch ($action) { case PhrictionActionConstants::ACTION_CREATE: $full_size = true; diff --git a/src/applications/phriction/constants/PhrictionActionConstants.php b/src/applications/phriction/constants/PhrictionActionConstants.php index 298214adb3..adfd00d7f0 100644 --- a/src/applications/phriction/constants/PhrictionActionConstants.php +++ b/src/applications/phriction/constants/PhrictionActionConstants.php @@ -16,8 +16,8 @@ final class PhrictionActionConstants extends PhrictionConstants { self::ACTION_CREATE => 'created', self::ACTION_EDIT => 'edited', self::ACTION_DELETE => 'deleted', - self::ACTION_MOVE_AWAY => 'moved a document to', - self::ACTION_MOVE_HERE => 'moved a document from', + self::ACTION_MOVE_AWAY => 'moved', + self::ACTION_MOVE_HERE => 'moved', ); return idx($map, $action, "brazenly {$action}'d"); diff --git a/src/applications/phriction/controller/PhrictionMoveController.php b/src/applications/phriction/controller/PhrictionMoveController.php index 3eaac68b4e..613eaccee3 100644 --- a/src/applications/phriction/controller/PhrictionMoveController.php +++ b/src/applications/phriction/controller/PhrictionMoveController.php @@ -102,7 +102,7 @@ final class PhrictionMoveController ->setDescription($content->getDescription()); // Move it! - $target_editor->moveHere($document->getID()); + $target_editor->moveHere($document->getID(), $document->getPHID()); // Retrieve the target doc directly from the editor // No need to load it per Sql again diff --git a/src/applications/phriction/editor/PhrictionDocumentEditor.php b/src/applications/phriction/editor/PhrictionDocumentEditor.php index 89d065c616..c32166c13b 100644 --- a/src/applications/phriction/editor/PhrictionDocumentEditor.php +++ b/src/applications/phriction/editor/PhrictionDocumentEditor.php @@ -14,6 +14,9 @@ final class PhrictionDocumentEditor extends PhabricatorEditor { private $newContent; private $description; + // For the Feed Story when moving documents + private $fromDocumentPHID; + private function __construct() { // } @@ -71,7 +74,8 @@ final class PhrictionDocumentEditor extends PhabricatorEditor { PhrictionChangeType::CHANGE_MOVE_AWAY, true, $new_doc_id); } - public function moveHere($old_doc_id) { + public function moveHere($old_doc_id, $old_doc_phid) { + $this->fromDocumentPHID = $old_doc_phid; return $this->execute( PhrictionChangeType::CHANGE_MOVE_HERE, false, $old_doc_id); } @@ -181,11 +185,11 @@ final class PhrictionDocumentEditor extends PhabricatorEditor { break; case PhrictionChangeType::CHANGE_MOVE_AWAY: $doc_status = PhrictionDocumentStatus::STATUS_MOVED; - $feed_action = PhrictionActionConstants::ACTION_MOVE_AWAY; + $feed_action = null; break; case PhrictionChangeType::CHANGE_MOVE_HERE: $doc_status = PhrictionDocumentStatus::STATUS_EXISTS; - $feed_action = null; + $feed_action = PhrictionActionConstants::ACTION_MOVE_HERE; break; default: throw new Exception( @@ -253,6 +257,10 @@ final class PhrictionDocumentEditor extends PhabricatorEditor { $related_phids[] = $project_phid; } + if ($this->fromDocumentPHID) { + $related_phids[] = $this->fromDocumentPHID; + } + if ($feed_action) { id(new PhabricatorFeedStoryPublisher()) ->setRelatedPHIDs($related_phids) @@ -261,10 +269,11 @@ final class PhrictionDocumentEditor extends PhabricatorEditor { ->setStoryType(PhabricatorFeedStoryTypeConstants::STORY_PHRICTION) ->setStoryData( array( - 'phid' => $document->getPHID(), - 'action' => $feed_action, - 'content' => phutil_utf8_shorten($new_content->getContent(), 140), - 'project' => $project_phid, + 'phid' => $document->getPHID(), + 'action' => $feed_action, + 'content' => phutil_utf8_shorten($new_content->getContent(), 140), + 'project' => $project_phid, + 'movedFromPHID' => $this->fromDocumentPHID, )) ->publish(); }