Load and attach objects when loading application transactions

Summary:
Ref T3886. Fixes the removed TODO. This also implements the generally reasonable policy "you have to be able to see an object in order to see its transactions". That was implicit before (we never load transactions without loading an object first) but is now explicit.

This fixes bad (nonspecialized) rendering of custom field transactions in Projects, and shortly in Differential, where stories would read "alincoln edited this object." instead of a more specific string.

Test Plan: Viewed a project edit, saw a more specific string. Browed ApplicationTransaction applications.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T3886

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

src/applications/transactions/query/PhabricatorApplicationTransactionQuery.php

@@ -120,6 +120,27 @@ abstract class PhabricatorApplicationTransactionQuery
     return $xactions;
   }
 
+  protected function willFilterPage(array $xactions) {
+    $object_phids = array_keys(mpull($xactions, null, 'getObjectPHID'));
+
+    $objects = id(new PhabricatorObjectQuery())
+      ->setViewer($this->getViewer())
+      ->setParentQuery($this)
+      ->withPHIDs($object_phids)
+      ->execute();
+
+    foreach ($xactions as $key => $xaction) {
+      $object_phid = $xaction->getObjectPHID();
+      if (empty($objects[$object_phid])) {
+        unset($xactions[$key]);
+        continue;
+      }
+      $xaction->attachObject($objects[$object_phid]);
+    }
+
+    return $xactions;
+  }
+
   private function buildWhereClause(AphrontDatabaseConnection $conn_r) {
     $where = array();
 

src/applications/transactions/storage/PhabricatorApplicationTransaction.php

@@ -29,6 +29,7 @@ abstract class PhabricatorApplicationTransaction
   private $renderingTarget = self::TARGET_HTML;
   private $transactionGroup = array();
   private $viewer = self::ATTACHABLE;
+  private $object = self::ATTACHABLE;
 
   abstract public function getApplicationTransactionType();
 
@@ -110,6 +111,15 @@ abstract class PhabricatorApplicationTransaction
     return $this;
   }
 
+  public function attachObject($object) {
+    $this->object = $object;
+    return $this;
+  }
+
+  public function getObject() {
+    return $this->assertAttached($this->object);
+  }
+
 /* -(  Rendering  )---------------------------------------------------------- */
 
   public function setRenderingTarget($rendering_target) {
@@ -388,10 +398,7 @@ abstract class PhabricatorApplicationTransaction
       case PhabricatorTransactions::TYPE_CUSTOMFIELD:
         $key = $this->getMetadataValue('customfield:key');
         $field = PhabricatorCustomField::getObjectField(
-          // TODO: This is a giant hack, but we currently don't have a way to
-          // get the contextual object and this pathway is only hit by
-          // Maniphest. We should provide a way to get the actual object here.
-          new ManiphestTask(),
+          $this->getObject(),
           PhabricatorCustomField::ROLE_APPLICATIONTRANSACTIONS,
           $key);
         if ($field) {
@@ -455,10 +462,7 @@ abstract class PhabricatorApplicationTransaction
       case PhabricatorTransactions::TYPE_CUSTOMFIELD:
         $key = $this->getMetadataValue('customfield:key');
         $field = PhabricatorCustomField::getObjectField(
-          // TODO: This is a giant hack, but we currently don't have a way to
-          // get the contextual object and this pathway is only hit by
-          // Maniphest. We should provide a way to get the actual object here.
-          new ManiphestTask(),
+          $this->getObject(),
           PhabricatorCustomField::ROLE_APPLICATIONTRANSACTIONS,
           $key);
         if ($field) {