diff --git a/src/applications/differential/xaction/DifferentialRevisionWrongStateTransaction.php b/src/applications/differential/xaction/DifferentialRevisionWrongStateTransaction.php --- a/src/applications/differential/xaction/DifferentialRevisionWrongStateTransaction.php +++ b/src/applications/differential/xaction/DifferentialRevisionWrongStateTransaction.php @@ -35,7 +35,8 @@ $this->renderValue($status->getDisplayName())); } - public function getTitleForFeed() { - return null; + public function shouldHideForFeed() { + return true; } + } diff --git a/src/applications/maniphest/storage/ManiphestTransaction.php b/src/applications/maniphest/storage/ManiphestTransaction.php --- a/src/applications/maniphest/storage/ManiphestTransaction.php +++ b/src/applications/maniphest/storage/ManiphestTransaction.php @@ -40,22 +40,6 @@ return parent::shouldGenerateOldValue(); } - public function shouldHideForFeed() { - // NOTE: Modular transactions don't currently support this, and it has - // very few callsites, and it's publish-time rather than display-time. - // This should probably become a supported, display-time behavior. For - // discussion, see T12787. - - // 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; - } - - return parent::shouldHideForFeed(); - } - public function getRequiredHandlePHIDs() { $phids = parent::getRequiredHandlePHIDs(); diff --git a/src/applications/maniphest/xaction/ManiphestTaskUnblockTransaction.php b/src/applications/maniphest/xaction/ManiphestTaskUnblockTransaction.php --- a/src/applications/maniphest/xaction/ManiphestTaskUnblockTransaction.php +++ b/src/applications/maniphest/xaction/ManiphestTaskUnblockTransaction.php @@ -112,5 +112,15 @@ return 'fa-shield'; } + public function shouldHideForFeed() { + // 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; + } + + return parent::shouldHideForFeed(); + } } diff --git a/src/applications/phriction/storage/PhrictionTransaction.php b/src/applications/phriction/storage/PhrictionTransaction.php --- a/src/applications/phriction/storage/PhrictionTransaction.php +++ b/src/applications/phriction/storage/PhrictionTransaction.php @@ -54,18 +54,6 @@ return parent::shouldHideForMail($xactions); } - public function shouldHideForFeed() { - switch ($this->getTransactionType()) { - case PhrictionDocumentMoveToTransaction::TRANSACTIONTYPE: - case PhrictionDocumentMoveAwayTransaction::TRANSACTIONTYPE: - return true; - case PhrictionDocumentTitleTransaction::TRANSACTIONTYPE: - return $this->getMetadataValue('stub:create:phid', false); - } - return parent::shouldHideForFeed(); - } - - public function getMailTags() { $tags = array(); switch ($this->getTransactionType()) { diff --git a/src/applications/phriction/xaction/PhrictionDocumentMoveAwayTransaction.php b/src/applications/phriction/xaction/PhrictionDocumentMoveAwayTransaction.php --- a/src/applications/phriction/xaction/PhrictionDocumentMoveAwayTransaction.php +++ b/src/applications/phriction/xaction/PhrictionDocumentMoveAwayTransaction.php @@ -59,4 +59,8 @@ return 'fa-arrows'; } + public function shouldHideForFeed() { + return true; + } + } diff --git a/src/applications/phriction/xaction/PhrictionDocumentMoveToTransaction.php b/src/applications/phriction/xaction/PhrictionDocumentMoveToTransaction.php --- a/src/applications/phriction/xaction/PhrictionDocumentMoveToTransaction.php +++ b/src/applications/phriction/xaction/PhrictionDocumentMoveToTransaction.php @@ -102,4 +102,8 @@ return 'fa-arrows'; } + public function shouldHideForFeed() { + return true; + } + } diff --git a/src/applications/transactions/feed/PhabricatorApplicationTransactionFeedStory.php b/src/applications/transactions/feed/PhabricatorApplicationTransactionFeedStory.php --- a/src/applications/transactions/feed/PhabricatorApplicationTransactionFeedStory.php +++ b/src/applications/transactions/feed/PhabricatorApplicationTransactionFeedStory.php @@ -6,6 +6,8 @@ class PhabricatorApplicationTransactionFeedStory extends PhabricatorFeedStory { + private $primaryTransactionPHID; + public function getPrimaryObjectPHID() { return $this->getValue('objectPHID'); } @@ -27,7 +29,36 @@ } protected function getPrimaryTransactionPHID() { - return head($this->getValue('transactionPHIDs')); + if ($this->primaryTransactionPHID === null) { + // Transactions are filtered and sorted before they're stored, but the + // rendering logic can change between the time an edit occurs and when + // we actually render the story. Recalculate the filtering at display + // time because it's cheap and gets us better results when things change + // by letting the changes apply retroactively. + + $xaction_phids = $this->getValue('transactionPHIDs'); + + $xactions = array(); + foreach ($xaction_phids as $xaction_phid) { + $xactions[] = $this->getObject($xaction_phid); + } + + foreach ($xactions as $key => $xaction) { + if ($xaction->shouldHideForFeed()) { + unset($xactions[$key]); + } + } + + if ($xactions) { + $primary_phid = head($xactions)->getPHID(); + } else { + $primary_phid = head($xaction_phids); + } + + $this->primaryTransactionPHID = $primary_phid; + } + + return $this->primaryTransactionPHID; } public function getPrimaryTransaction() { diff --git a/src/applications/transactions/storage/PhabricatorModularTransaction.php b/src/applications/transactions/storage/PhabricatorModularTransaction.php --- a/src/applications/transactions/storage/PhabricatorModularTransaction.php +++ b/src/applications/transactions/storage/PhabricatorModularTransaction.php @@ -92,6 +92,14 @@ return parent::shouldHide(); } + final public function shouldHideForFeed() { + if ($this->getTransactionImplementation()->shouldHideForFeed()) { + return true; + } + + return parent::shouldHideForFeed(); + } + /* final */ public function getIcon() { $icon = $this->getTransactionImplementation()->getIcon(); if ($icon !== null) { diff --git a/src/applications/transactions/storage/PhabricatorModularTransactionType.php b/src/applications/transactions/storage/PhabricatorModularTransactionType.php --- a/src/applications/transactions/storage/PhabricatorModularTransactionType.php +++ b/src/applications/transactions/storage/PhabricatorModularTransactionType.php @@ -47,6 +47,10 @@ return false; } + public function shouldHideForFeed() { + return false; + } + public function getIcon() { return null; }