Remove legacy pre-loading of handles from Herald rendering

Summary: Ref T13480. When Herald renders rules, it partly uses a very old handle pre-loading mechanism where PHIDs are extracted and loaded upfront. This was obsoleted a long time ago and was pretty shaky even when it worked. Get rid of it to simplify the code a little.

Test Plan: Viewed Herald rules rendered into static text with PHID list actions, saw handles. Grepped for all affected methods.

Maniphest Tasks: T13480

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

src/applications/herald/adapter/HeraldAdapter.php

@@ -942,7 +942,6 @@ abstract class HeraldAdapter extends Phobject {
 
   public function renderRuleAsText(
     HeraldRule $rule,
-    PhabricatorHandleList $handles,
     PhabricatorUser $viewer) {
 
     require_celerity_resource('herald-css');
@@ -973,7 +972,7 @@ abstract class HeraldAdapter extends Phobject {
         ),
         array(
           $icon,
-          $this->renderConditionAsText($condition, $handles, $viewer),
+          $this->renderConditionAsText($condition, $viewer),
         ));
     }
 
@@ -1004,7 +1003,7 @@ abstract class HeraldAdapter extends Phobject {
         ),
         array(
           $icon,
-          $this->renderActionAsText($viewer, $action, $handles),
+          $this->renderActionAsText($viewer, $action),
         ));
     }
 
@@ -1018,7 +1017,6 @@ abstract class HeraldAdapter extends Phobject {
 
   private function renderConditionAsText(
     HeraldCondition $condition,
-    PhabricatorHandleList $handles,
     PhabricatorUser $viewer) {
 
     $field_type = $condition->getFieldName();
@@ -1033,7 +1031,7 @@ abstract class HeraldAdapter extends Phobject {
     $condition_type = $condition->getFieldCondition();
     $condition_name = idx($this->getConditionNameMap(), $condition_type);
 
-    $value = $this->renderConditionValueAsText($condition, $handles, $viewer);
+    $value = $this->renderConditionValueAsText($condition, $viewer);
 
     return array(
       $field_name,
@@ -1046,36 +1044,23 @@ abstract class HeraldAdapter extends Phobject {
 
   private function renderActionAsText(
     PhabricatorUser $viewer,
-    HeraldActionRecord $action,
+    HeraldActionRecord $action_record) {
-    PhabricatorHandleList $handles) {
 
-    $impl = $this->getActionImplementation($action->getAction());
+    $action_type = $action_record->getAction();
-    if ($impl) {
+    $action_value = $action_record->getTarget();
-      $impl->setViewer($viewer);
 
-      $value = $action->getTarget();
+    $action = $this->getActionImplementation($action_type);
-      return $impl->renderActionDescription($value);
+    if (!$action) {
+      return pht('Unknown Action ("%s")', $action_type);
     }
 
-    $rule_global = HeraldRuleTypeConfig::RULE_TYPE_GLOBAL;
+    $action->setViewer($viewer);
 
-    $action_type = $action->getAction();
+    return $action->renderActionDescription($action_value);
-
-    $default = pht('(Unknown Action "%s") equals', $action_type);
-
-    $action_name = idx(
-      $this->getActionNameMap($rule_global),
-      $action_type,
-      $default);
-
-    $target = $this->renderActionTargetAsText($action, $handles);
-
-    return hsprintf('    %s %s', $action_name, $target);
   }
 
   private function renderConditionValueAsText(
     HeraldCondition $condition,
-    PhabricatorHandleList $handles,
     PhabricatorUser $viewer) {
 
     $field = $this->requireFieldImplementation($condition->getFieldName());
@@ -1086,76 +1071,6 @@ abstract class HeraldAdapter extends Phobject {
       $condition->getValue());
   }
 
-  private function renderActionTargetAsText(
-    HeraldActionRecord $action,
-    PhabricatorHandleList $handles) {
-
-    // TODO: This should be driven through HeraldAction.
-
-    $target = $action->getTarget();
-    if (!is_array($target)) {
-      $target = array($target);
-    }
-    foreach ($target as $index => $val) {
-      switch ($action->getAction()) {
-        default:
-          $handle = $handles->getHandleIfExists($val);
-          if ($handle) {
-            $target[$index] = $handle->renderLink();
-          }
-          break;
-      }
-    }
-    $target = phutil_implode_html(', ', $target);
-    return $target;
-  }
-
-  /**
-   * Given a @{class:HeraldRule}, this function extracts all the phids that
-   * we'll want to load as handles later.
-   *
-   * This function performs a somewhat hacky approach to figuring out what
-   * is and is not a phid - try to get the phid type and if the type is
-   * *not* unknown assume its a valid phid.
-   *
-   * Don't try this at home. Use more strongly typed data at home.
-   *
-   * Think of the children.
-   */
-  public static function getHandlePHIDs(HeraldRule $rule) {
-    $phids = array($rule->getAuthorPHID());
-    foreach ($rule->getConditions() as $condition) {
-      $value = $condition->getValue();
-      if (!is_array($value)) {
-        $value = array($value);
-      }
-      foreach ($value as $val) {
-        if (phid_get_type($val) !=
-            PhabricatorPHIDConstants::PHID_TYPE_UNKNOWN) {
-          $phids[] = $val;
-        }
-      }
-    }
-
-    foreach ($rule->getActions() as $action) {
-      $target = $action->getTarget();
-      if (!is_array($target)) {
-        $target = array($target);
-      }
-      foreach ($target as $val) {
-        if (phid_get_type($val) !=
-            PhabricatorPHIDConstants::PHID_TYPE_UNKNOWN) {
-          $phids[] = $val;
-        }
-      }
-    }
-
-    if ($rule->isObjectRule()) {
-      $phids[] = $rule->getTriggerObjectPHID();
-    }
-
-    return $phids;
-  }
 
 /* -(  Applying Effects  )--------------------------------------------------- */
 

src/applications/herald/controller/HeraldRuleViewController.php

@@ -143,12 +143,11 @@ final class HeraldRuleViewController extends HeraldController {
   private function buildDescriptionView(HeraldRule $rule) {
     $viewer = $this->getRequest()->getUser();
     $view = id(new PHUIPropertyListView())
-      ->setUser($viewer);
+      ->setViewer($viewer);
 
     $adapter = HeraldAdapter::getAdapterForContentType($rule->getContentType());
     if ($adapter) {
-      $handles = $viewer->loadHandles(HeraldAdapter::getHandlePHIDs($rule));
+      $rule_text = $adapter->renderRuleAsText($rule, $viewer);
-      $rule_text = $adapter->renderRuleAsText($rule, $handles, $viewer);
       $view->addTextContent($rule_text);
       return $view;
     }