Use Adapters to render Herald transcripts

Summary: Ref T2769. Use Adapters to build all the strings for transcripts, then get rid of the old maps.

Test Plan: Viewed revision and commit transcripts.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T2769

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

src/applications/herald/adapter/HeraldAdapter.php

@@ -190,6 +190,7 @@ abstract class HeraldAdapter {
           self::ACTION_REMOVE_CC  => pht('Remove emails from CC'),
           self::ACTION_EMAIL      => pht('Send an email to'),
           self::ACTION_AUDIT      => pht('Trigger an Audit by'),
+          self::ACTION_FLAG       => pht('Mark with flag'),
         );
       case HeraldRuleTypeConfig::RULE_TYPE_PERSONAL:
         return array(

src/applications/herald/config/HeraldActionConfig.php

@@ -9,70 +9,6 @@ final class HeraldActionConfig {
   const ACTION_AUDIT        = 'audit';
   const ACTION_FLAG         = 'flag';
 
-  // TODO: Remove; still used by transcripts.
-  public static function getActionMessageMapForRuleType($rule_type) {
-    $generic_mappings = array(
-      self::ACTION_NOTHING      => pht('Do nothing'),
-      self::ACTION_ADD_CC       => pht('Add emails to CC'),
-      self::ACTION_REMOVE_CC    => pht('Remove emails from CC'),
-      self::ACTION_EMAIL        => pht('Send an email to'),
-      self::ACTION_AUDIT        => pht('Trigger an Audit'),
-      self::ACTION_FLAG         => pht('Mark with flag'),
-    );
-
-    switch ($rule_type) {
-      case HeraldRuleTypeConfig::RULE_TYPE_GLOBAL:
-        $specific_mappings = array(
-          self::ACTION_AUDIT        => pht('Trigger an Audit for project'),
-        );
-        break;
-      case HeraldRuleTypeConfig::RULE_TYPE_PERSONAL:
-        $specific_mappings = array(
-          self::ACTION_ADD_CC       => pht('CC me'),
-          self::ACTION_REMOVE_CC    => pht('Remove me from CC'),
-          self::ACTION_EMAIL        => pht('Email me'),
-          self::ACTION_AUDIT        => pht('Trigger an Audit by me'),
-        );
-        break;
-      case null:
-        $specific_mappings = array();
-        // Use generic mappings, used on transcript.
-        break;
-      default:
-        throw new Exception("Unknown rule type '${rule_type}'");
-    }
-    return $specific_mappings + $generic_mappings;
-  }
-
-  // TODO: Remove; still used by transcripts.
-  public static function getActionMessageMap($content_type,
-                                             $rule_type) {
-    $map = self::getActionMessageMapForRuleType($rule_type);
-    switch ($content_type) {
-      case HeraldContentTypeConfig::CONTENT_TYPE_DIFFERENTIAL:
-        return array_select_keys(
-          $map,
-          array(
-            self::ACTION_ADD_CC,
-            self::ACTION_REMOVE_CC,
-            self::ACTION_EMAIL,
-            self::ACTION_FLAG,
-            self::ACTION_NOTHING,
-          ));
-      case HeraldContentTypeConfig::CONTENT_TYPE_COMMIT:
-        return array_select_keys(
-          $map,
-          array(
-            self::ACTION_EMAIL,
-            self::ACTION_AUDIT,
-            self::ACTION_FLAG,
-            self::ACTION_NOTHING,
-          ));
-      default:
-        throw new Exception("Unknown content type '{$content_type}'.");
-    }
-  }
-
   /**
    * Create a HeraldAction to save from data.
    *

src/applications/herald/config/HeraldConditionConfig.php

@@ -21,29 +21,4 @@ final class HeraldConditionConfig {
   const CONDITION_NOT_EXISTS      = '!exists';
   const CONDITION_REGEXP_PAIR     = 'regexp-pair';
 
-  // TODO: Remove; still used by Transcripts.
-  public static function getConditionMap() {
-    $map = array(
-      self::CONDITION_CONTAINS        => pht('contains'),
-      self::CONDITION_NOT_CONTAINS    => pht('does not contain'),
-      self::CONDITION_IS              => pht('is'),
-      self::CONDITION_IS_NOT          => pht('is not'),
-      self::CONDITION_IS_ANY          => pht('is any of'),
-      self::CONDITION_IS_NOT_ANY      => pht('is not any of'),
-      self::CONDITION_INCLUDE_ALL     => pht('include all of'),
-      self::CONDITION_INCLUDE_ANY     => pht('include any of'),
-      self::CONDITION_INCLUDE_NONE    => pht('include none of'),
-      self::CONDITION_IS_ME           => pht('is myself'),
-      self::CONDITION_IS_NOT_ME       => pht('is not myself'),
-      self::CONDITION_REGEXP          => pht('matches regexp'),
-      self::CONDITION_RULE            => pht('matches:'),
-      self::CONDITION_NOT_RULE        => pht('does not match:'),
-      self::CONDITION_EXISTS          => pht('exists'),
-      self::CONDITION_NOT_EXISTS      => pht('does not exist'),
-      self::CONDITION_REGEXP_PAIR     => pht('matches regexp pair'),
-    );
-
-    return $map;
-  }
-
 }

src/applications/herald/config/HeraldFieldConfig.php

@@ -21,31 +21,4 @@ final class HeraldFieldConfig {
   const FIELD_DIFFERENTIAL_REVIEWERS = 'differential-reviewers';
   const FIELD_DIFFERENTIAL_CCS       = 'differential-ccs';
 
-  // TODO: Remove; still required by transcripts.
-  public static function getFieldMap() {
-    $map = array(
-      self::FIELD_TITLE                  => pht('Title'),
-      self::FIELD_BODY                   => pht('Body'),
-      self::FIELD_AUTHOR                 => pht('Author'),
-      self::FIELD_REVIEWER               => pht('Reviewer'),
-      self::FIELD_REVIEWERS              => pht('Reviewers'),
-      self::FIELD_CC                     => pht('CCs'),
-      self::FIELD_TAGS                   => pht('Tags'),
-      self::FIELD_DIFF_FILE              => pht('Any changed filename'),
-      self::FIELD_DIFF_CONTENT           => pht('Any changed file content'),
-      self::FIELD_REPOSITORY             => pht('Repository'),
-      self::FIELD_RULE                   => pht('Another Herald rule'),
-      self::FIELD_AFFECTED_PACKAGE       => pht('Any affected package'),
-      self::FIELD_AFFECTED_PACKAGE_OWNER =>
-                                      pht("Any affected package's owner"),
-      self::FIELD_NEED_AUDIT_FOR_PACKAGE =>
-                                      pht('Affected packages that need audit'),
-      self::FIELD_DIFFERENTIAL_REVISION  => pht('Differential revision'),
-      self::FIELD_DIFFERENTIAL_REVIEWERS => pht('Differential reviewers'),
-      self::FIELD_DIFFERENTIAL_CCS       => pht('Differential CCs'),
-    );
-
-    return $map;
-  }
-
 }

src/applications/herald/controller/HeraldTranscriptController.php

@@ -9,6 +9,7 @@ final class HeraldTranscriptController extends HeraldController {
   private $id;
   private $filter;
   private $handles;
+  private $adapter;
 
   public function willProcessRequest(array $data) {
     $this->id = $data['id'];
@@ -19,6 +20,10 @@ final class HeraldTranscriptController extends HeraldController {
     }
   }
 
+  private function getAdapter() {
+    return $this->adapter;
+  }
+
   public function processRequest() {
 
     $xscript = id(new HeraldTranscript())->load($this->id);
@@ -41,6 +46,10 @@ final class HeraldTranscriptController extends HeraldController {
           pht('Details of this transcript have been garbage collected.')));
       $nav->appendChild($notice);
     } else {
+
+      $this->adapter = HeraldAdapter::getAdapterForContentType(
+        $object_xscript->getType());
+
       $filter = $this->getFilterPHIDs();
       $this->filterTranscript($xscript, $filter);
       $phids = array_merge($filter, $this->getTranscriptPHIDs($xscript));
@@ -138,22 +147,6 @@ final class HeraldTranscriptController extends HeraldController {
 
   protected function getFilterPHIDs() {
     return array($this->getRequest()->getUser()->getPHID());
-
-/* TODO
-    $viewer_id = $this->getRequest()->getUser()->getPHID();
-
-    $fbids = array();
-    if ($this->filter == self::FILTER_AFFECTED) {
-      $fbids[] = $viewer_id;
-      require_module_lazy('intern/subscriptions');
-      $datastore = new SubscriberDatabaseStore();
-      $lists = $datastore->getUserMailmanLists($viewer_id);
-      foreach ($lists as $list) {
-        $fbids[] = $list;
-      }
-    }
-    return $fbids;
-*/
   }
 
   protected function getTranscriptPHIDs($xscript) {
@@ -270,8 +263,10 @@ final class HeraldTranscriptController extends HeraldController {
 
   private function buildApplyTranscriptPanel($xscript) {
     $handles = $this->handles;
+    $adapter = $this->getAdapter();
 
-    $action_names = HeraldActionConfig::getActionMessageMapForRuleType(null);
+    $rule_type_global = HeraldRuleTypeConfig::RULE_TYPE_GLOBAL;
+    $action_names = $adapter->getActionNameMap($rule_type_global);
 
     $rows = array();
     foreach ($xscript->getApplyTranscripts() as $apply_xscript) {
@@ -307,7 +302,7 @@ final class HeraldTranscriptController extends HeraldController {
       }
 
       $rows[] = array(
-        $action_names[$apply_xscript->getAction()],
+        idx($action_names, $apply_xscript->getAction(), pht('Unknown')),
         $target,
         hsprintf(
           '<strong>Taken because:</strong> %s<br />'.
@@ -344,8 +339,11 @@ final class HeraldTranscriptController extends HeraldController {
   private function buildActionTranscriptPanel($xscript) {
     $action_xscript = mgroup($xscript->getApplyTranscripts(), 'getRuleID');
 
-    $field_names = HeraldFieldConfig::getFieldMap();
+    $adapter = $this->getAdapter();
-    $condition_names = HeraldConditionConfig::getConditionMap();
+
+
+    $field_names = $adapter->getFieldNameMap();
+    $condition_names = $adapter->getConditionNameMap();
 
     $handles = $this->handles;
 
@@ -379,8 +377,8 @@ final class HeraldTranscriptController extends HeraldController {
           pht(
             '%s Condition: %s %s %s%s',
             $result,
-            $field_names[$cond->getFieldName()],
+            idx($field_names, $cond->getFieldName(), pht('Unknown')),
-            $condition_names[$cond->getCondition()],
+            idx($condition_names, $cond->getCondition(), pht('Unknown')),
             $this->renderConditionTestValue($cond, $handles),
             $note));
       }
@@ -398,37 +396,9 @@ final class HeraldTranscriptController extends HeraldController {
       }
 
       $cond_markup[] = hsprintf('<li>%s %s</li>', $result, $rule->getReason());
-
-/*
-      if ($rule->getResult()) {
-        $actions = idx($action_xscript, $rule_id, array());
-        if ($actions) {
-          $cond_markup[] = <li><div class="action-header">Actions</div></li>;
-          foreach ($actions as $action) {
-
-            $target = $action->getTarget();
-            if ($target) {
-              foreach ((array)$target as $k => $phid) {
-                $target[$k] = $handles[$phid]->getName();
-              }
-              $target = <strong>: {implode(', ', $target)}</strong>;
-            }
-
-            $cond_markup[] =
-              <li>
-                {$action_names[$action->getAction()]}
-                {$target}
-              </li>;
-          }
-        }
-      }
-*/
       $user_phid = $this->getRequest()->getUser()->getPHID();
 
       $name = $rule->getRuleName();
-      if ($rule->getRuleOwner() == $user_phid) {
-//        $name = <a href={"/herald/rule/".$rule->getRuleID()."/"}>{$name}</a>;
-      }
 
       $rule_markup[] =
         phutil_tag(
@@ -458,7 +428,8 @@ final class HeraldTranscriptController extends HeraldController {
 
   private function buildObjectTranscriptPanel($xscript) {
 
-    $field_names = HeraldFieldConfig::getFieldMap();
+    $adapter = $this->getAdapter();
+    $field_names = $adapter->getFieldNameMap();
 
     $object_xscript = $xscript->getObjectTranscript();