From 3103ce33b82dfd98c37be228ce6bd41e358ba82b Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Tue, 18 Feb 2014 16:32:35 -0800
Subject: [PATCH] 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
---
 ...PhabricatorApplicationTransactionQuery.php | 21 +++++++++++++++++++
 .../PhabricatorApplicationTransaction.php     | 20 +++++++++++-------
 2 files changed, 33 insertions(+), 8 deletions(-)

diff --git a/src/applications/transactions/query/PhabricatorApplicationTransactionQuery.php b/src/applications/transactions/query/PhabricatorApplicationTransactionQuery.php
index 1ef212d7e5..2e0b724302 100644
--- a/src/applications/transactions/query/PhabricatorApplicationTransactionQuery.php
+++ b/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();
 
diff --git a/src/applications/transactions/storage/PhabricatorApplicationTransaction.php b/src/applications/transactions/storage/PhabricatorApplicationTransaction.php
index fbb13222c9..e378860aa5 100644
--- a/src/applications/transactions/storage/PhabricatorApplicationTransaction.php
+++ b/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) {