Hide "abraham landed Dxyz irresponsibly" stories from feed

Summary:
Ref T13099. Ref T12787. See PHI417. Differential has new "irresponsible" warnings in the timeline somewhat recently, but these publish feed stories that don't link to the revision or have other relevant details, so they're confusing on the balance.

These have a high strength so they render on top, but we actually just want to hide them from the feed and let "abraham closed Dxyz by committing rXzzz." be the primary story.

Modularize things more so that we can get this behavior. Also, respect `shouldHideForFeed()` at display time, not just publishing time.

Test Plan: Used `bin/differential attach-commit` on a non-accepted revision to "irresponsibly land" a revision. Verified that feed story now shows "closed by commit" instead of "closed irresponsibly".

Maniphest Tasks: T13099, T12787

Differential Revision: https://secure.phabricator.com/D19179

src/applications/differential/xaction/DifferentialRevisionWrongStateTransaction.php

@@ -35,7 +35,8 @@ final class DifferentialRevisionWrongStateTransaction
       $this->renderValue($status->getDisplayName()));
   }
 
-  public function getTitleForFeed() {
-    return null;
+  public function shouldHideForFeed() {
+    return true;
   }
+
 }

src/applications/maniphest/storage/ManiphestTransaction.php

@@ -40,22 +40,6 @@ final class ManiphestTransaction
     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();
 

src/applications/maniphest/xaction/ManiphestTaskUnblockTransaction.php

@@ -112,5 +112,15 @@ final class ManiphestTaskUnblockTransaction
     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();
+  }
 
 }

src/applications/phriction/storage/PhrictionTransaction.php

@@ -54,18 +54,6 @@ final class PhrictionTransaction
     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()) {

src/applications/phriction/xaction/PhrictionDocumentMoveAwayTransaction.php

@@ -59,4 +59,8 @@ final class PhrictionDocumentMoveAwayTransaction
     return 'fa-arrows';
   }
 
+  public function shouldHideForFeed() {
+    return true;
+  }
+
 }

src/applications/phriction/xaction/PhrictionDocumentMoveToTransaction.php

@@ -102,4 +102,8 @@ final class PhrictionDocumentMoveToTransaction
     return 'fa-arrows';
   }
 
+  public function shouldHideForFeed() {
+    return true;
+  }
+
 }

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 @@ class PhabricatorApplicationTransactionFeedStory
   }
 
   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() {

src/applications/transactions/storage/PhabricatorModularTransaction.php

@@ -92,6 +92,14 @@ abstract class PhabricatorModularTransaction
     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) {

src/applications/transactions/storage/PhabricatorModularTransactionType.php

@@ -47,6 +47,10 @@ abstract class PhabricatorModularTransactionType
     return false;
   }
 
+  public function shouldHideForFeed() {
+    return false;
+  }
+
   public function getIcon() {
     return null;
   }