From 74147778564023b5f9e46d544d6ede719952eead Mon Sep 17 00:00:00 2001 From: epriestley Date: Tue, 5 Nov 2013 09:03:59 -0800 Subject: [PATCH] Select all available bodies when rendering a feed story Summary: Fixes T4060. The logic here is: - When you take several actions at once, we show a single feed story for all of them. - We choose the "most interesting" title for the feed story. For example, "close task" is more interesting than "add CC". Currently, the issue with this is: - "Add comment" is the //least interesting// title. I think this is correct: all other actions are more interesting than the fact that you added a comment. - We try to conserve the number of objects we need to load by rendering only the most interesting transaction. To fix this: - Stop being so conservative; load all of the transactions and all of their PHIDs. - Add bodies from any transactions which render bodies. In all cases (I think?) this is a maximum of one comment adding a body. The end result is a story like this: epriestley closed T123: the building is on fire. "Okay guys I put the fire out" Test Plan: See screenshot. Reviewers: chad, btrahan Reviewed By: chad CC: aran, asherkin Maniphest Tasks: T4060 Differential Revision: https://secure.phabricator.com/D7504 --- ...ricatorApplicationTransactionFeedStory.php | 32 ++++++++++++------- 1 file changed, 20 insertions(+), 12 deletions(-) diff --git a/src/applications/transactions/feed/PhabricatorApplicationTransactionFeedStory.php b/src/applications/transactions/feed/PhabricatorApplicationTransactionFeedStory.php index c8faf99a94..3b556b8489 100644 --- a/src/applications/transactions/feed/PhabricatorApplicationTransactionFeedStory.php +++ b/src/applications/transactions/feed/PhabricatorApplicationTransactionFeedStory.php @@ -11,16 +11,19 @@ class PhabricatorApplicationTransactionFeedStory } public function getRequiredObjectPHIDs() { - return array( - $this->getPrimaryTransactionPHID(), - ); + return $this->getValue('transactionPHIDs'); } public function getRequiredHandlePHIDs() { $phids = array(); - $phids[] = array($this->getValue('objectPHID')); - $phids[] = $this->getPrimaryTransaction()->getRequiredHandlePHIDs(); - return array_mergev($phids); + $phids[] = $this->getValue('objectPHID'); + foreach ($this->getValue('transactionPHIDs') as $xaction_phid) { + $xaction = $this->getObject($xaction_phid); + foreach ($xaction->getRequiredHandlePHIDs() as $handle_phid) { + $phids[] = $handle_phid; + } + } + return $phids; } protected function getPrimaryTransactionPHID() { @@ -40,18 +43,23 @@ class PhabricatorApplicationTransactionFeedStory $view->setAppIconFromPHID($handle->getPHID()); $xaction_phids = $this->getValue('transactionPHIDs'); - $xaction = $this->getObject(head($xaction_phids)); + $xaction = $this->getPrimaryTransaction(); $xaction->setHandles($this->getHandles()); $view->setTitle($xaction->getTitleForFeed($this)); - $body = $xaction->getBodyForFeed($this); - if (nonempty($body)) { - $view->appendChild($body); + + foreach ($xaction_phids as $xaction_phid) { + $secondary_xaction = $this->getObject($xaction_phid); + $secondary_xaction->setHandles($this->getHandles()); + + $body = $secondary_xaction->getBodyForFeed($this); + if (nonempty($body)) { + $view->appendChild($body); + } } $view->setImage( - $this->getHandle( - $this->getPrimaryTransaction()->getAuthorPHID())->getImageURI()); + $this->getHandle($xaction->getAuthorPHID())->getImageURI()); return $view; }