Make Herald action modularization more aggressive

Summary:
Ref T8726. Herald actions are technically sort-of modular already, but make them more aggressively modular similar to `HeraldField`.

I plan to obsolete and replace `HeraldCustomAction`.

Test Plan: Saw actions in nice groups; created and ran a "Do Nothing" action. Transcripts are a bit rough for now.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: joshuaspence, epriestley

Maniphest Tasks: T8726

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

resources/celerity/map.php

@@ -382,7 +382,7 @@ return array(
     'rsrc/js/application/doorkeeper/behavior-doorkeeper-tag.js' => 'e5822781',
     'rsrc/js/application/files/behavior-icon-composer.js' => '8ef9ab58',
     'rsrc/js/application/files/behavior-launch-icon-composer.js' => '48086888',
-    'rsrc/js/application/herald/HeraldRuleEditor.js' => '52684226',
+    'rsrc/js/application/herald/HeraldRuleEditor.js' => '91a6031b',
     'rsrc/js/application/herald/PathTypeahead.js' => 'f7fc67ec',
     'rsrc/js/application/herald/herald-rule-editor.js' => '7ebaeed3',
     'rsrc/js/application/maniphest/behavior-batch-editor.js' => '782ab6e7',
@@ -538,7 +538,7 @@ return array(
     'global-drag-and-drop-css' => '697324ad',
     'harbormaster-css' => '49d64eb4',
     'herald-css' => '826075fa',
-    'herald-rule-editor' => '52684226',
+    'herald-rule-editor' => '91a6031b',
     'herald-test-css' => '778b008e',
     'inline-comment-summary-css' => '51efda3a',
     'javelin-aphlict' => '5359e785',
@@ -1172,15 +1172,6 @@ return array(
       'javelin-dom',
       'javelin-reactor-dom',
     ),
-    52684226 => array(
-      'multirow-row-manager',
-      'javelin-install',
-      'javelin-util',
-      'javelin-dom',
-      'javelin-stratcom',
-      'javelin-json',
-      'phabricator-prefab',
-    ),
     '5359e785' => array(
       'javelin-install',
       'javelin-util',
@@ -1539,6 +1530,15 @@ return array(
       'javelin-dom',
       'javelin-stratcom',
     ),
+    '91a6031b' => array(
+      'multirow-row-manager',
+      'javelin-install',
+      'javelin-util',
+      'javelin-dom',
+      'javelin-stratcom',
+      'javelin-json',
+      'phabricator-prefab',
+    ),
     '93d0c9e3' => array(
       'javelin-behavior',
       'javelin-stratcom',

src/__phutil_library_map__.php

@@ -1007,6 +1007,8 @@ phutil_register_library_map(array(
     'HarbormasterUploadArtifactBuildStepImplementation' => 'applications/harbormaster/step/HarbormasterUploadArtifactBuildStepImplementation.php',
     'HarbormasterWaitForPreviousBuildStepImplementation' => 'applications/harbormaster/step/HarbormasterWaitForPreviousBuildStepImplementation.php',
     'HarbormasterWorker' => 'applications/harbormaster/worker/HarbormasterWorker.php',
+    'HeraldAction' => 'applications/herald/action/HeraldAction.php',
+    'HeraldActionGroup' => 'applications/herald/action/HeraldActionGroup.php',
     'HeraldActionRecord' => 'applications/herald/storage/HeraldActionRecord.php',
     'HeraldAdapter' => 'applications/herald/adapter/HeraldAdapter.php',
     'HeraldAlwaysField' => 'applications/herald/field/HeraldAlwaysField.php',
@@ -1024,6 +1026,7 @@ phutil_register_library_map(array(
     'HeraldDifferentialDiffAdapter' => 'applications/differential/herald/HeraldDifferentialDiffAdapter.php',
     'HeraldDifferentialRevisionAdapter' => 'applications/differential/herald/HeraldDifferentialRevisionAdapter.php',
     'HeraldDisableController' => 'applications/herald/controller/HeraldDisableController.php',
+    'HeraldDoNothingAction' => 'applications/herald/action/HeraldDoNothingAction.php',
     'HeraldEditFieldGroup' => 'applications/herald/field/HeraldEditFieldGroup.php',
     'HeraldEffect' => 'applications/herald/engine/HeraldEffect.php',
     'HeraldEmptyFieldValue' => 'applications/herald/value/HeraldEmptyFieldValue.php',
@@ -1032,17 +1035,20 @@ phutil_register_library_map(array(
     'HeraldFieldGroup' => 'applications/herald/field/HeraldFieldGroup.php',
     'HeraldFieldTestCase' => 'applications/herald/field/__tests__/HeraldFieldTestCase.php',
     'HeraldFieldValue' => 'applications/herald/value/HeraldFieldValue.php',
+    'HeraldGroup' => 'applications/herald/group/HeraldGroup.php',
     'HeraldInvalidActionException' => 'applications/herald/engine/exception/HeraldInvalidActionException.php',
     'HeraldInvalidConditionException' => 'applications/herald/engine/exception/HeraldInvalidConditionException.php',
     'HeraldManageGlobalRulesCapability' => 'applications/herald/capability/HeraldManageGlobalRulesCapability.php',
     'HeraldManiphestTaskAdapter' => 'applications/maniphest/herald/HeraldManiphestTaskAdapter.php',
     'HeraldNewController' => 'applications/herald/controller/HeraldNewController.php',
     'HeraldNewObjectField' => 'applications/herald/field/HeraldNewObjectField.php',
+    'HeraldNotifyActionGroup' => 'applications/herald/action/HeraldNotifyActionGroup.php',
     'HeraldObjectTranscript' => 'applications/herald/storage/transcript/HeraldObjectTranscript.php',
     'HeraldPholioMockAdapter' => 'applications/pholio/herald/HeraldPholioMockAdapter.php',
     'HeraldPreCommitAdapter' => 'applications/diffusion/herald/HeraldPreCommitAdapter.php',
     'HeraldPreCommitContentAdapter' => 'applications/diffusion/herald/HeraldPreCommitContentAdapter.php',
     'HeraldPreCommitRefAdapter' => 'applications/diffusion/herald/HeraldPreCommitRefAdapter.php',
+    'HeraldPreventActionGroup' => 'applications/herald/action/HeraldPreventActionGroup.php',
     'HeraldProjectsField' => 'applications/project/herald/HeraldProjectsField.php',
     'HeraldRecursiveConditionsException' => 'applications/herald/engine/exception/HeraldRecursiveConditionsException.php',
     'HeraldRelatedFieldGroup' => 'applications/herald/field/HeraldRelatedFieldGroup.php',
@@ -1065,6 +1071,7 @@ phutil_register_library_map(array(
     'HeraldSelectFieldValue' => 'applications/herald/value/HeraldSelectFieldValue.php',
     'HeraldSpaceField' => 'applications/spaces/herald/HeraldSpaceField.php',
     'HeraldSubscribersField' => 'applications/subscriptions/herald/HeraldSubscribersField.php',
+    'HeraldSupportActionGroup' => 'applications/herald/action/HeraldSupportActionGroup.php',
     'HeraldSupportFieldGroup' => 'applications/herald/field/HeraldSupportFieldGroup.php',
     'HeraldTestConsoleController' => 'applications/herald/controller/HeraldTestConsoleController.php',
     'HeraldTextFieldValue' => 'applications/herald/value/HeraldTextFieldValue.php',
@@ -1077,6 +1084,7 @@ phutil_register_library_map(array(
     'HeraldTranscriptQuery' => 'applications/herald/query/HeraldTranscriptQuery.php',
     'HeraldTranscriptSearchEngine' => 'applications/herald/query/HeraldTranscriptSearchEngine.php',
     'HeraldTranscriptTestCase' => 'applications/herald/storage/__tests__/HeraldTranscriptTestCase.php',
+    'HeraldUtilityActionGroup' => 'applications/herald/action/HeraldUtilityActionGroup.php',
     'Javelin' => 'infrastructure/javelin/Javelin.php',
     'JavelinReactorUIExample' => 'applications/uiexample/examples/JavelinReactorUIExample.php',
     'JavelinUIExample' => 'applications/uiexample/examples/JavelinUIExample.php',
@@ -4684,6 +4692,8 @@ phutil_register_library_map(array(
     'HarbormasterUploadArtifactBuildStepImplementation' => 'HarbormasterBuildStepImplementation',
     'HarbormasterWaitForPreviousBuildStepImplementation' => 'HarbormasterBuildStepImplementation',
     'HarbormasterWorker' => 'PhabricatorWorker',
+    'HeraldAction' => 'Phobject',
+    'HeraldActionGroup' => 'HeraldGroup',
     'HeraldActionRecord' => 'HeraldDAO',
     'HeraldAdapter' => 'Phobject',
     'HeraldAlwaysField' => 'HeraldField',
@@ -4701,25 +4711,29 @@ phutil_register_library_map(array(
     'HeraldDifferentialDiffAdapter' => 'HeraldDifferentialAdapter',
     'HeraldDifferentialRevisionAdapter' => 'HeraldDifferentialAdapter',
     'HeraldDisableController' => 'HeraldController',
+    'HeraldDoNothingAction' => 'HeraldAction',
     'HeraldEditFieldGroup' => 'HeraldFieldGroup',
     'HeraldEffect' => 'Phobject',
     'HeraldEmptyFieldValue' => 'HeraldFieldValue',
     'HeraldEngine' => 'Phobject',
     'HeraldField' => 'Phobject',
-    'HeraldFieldGroup' => 'Phobject',
+    'HeraldFieldGroup' => 'HeraldGroup',
     'HeraldFieldTestCase' => 'PhutilTestCase',
     'HeraldFieldValue' => 'Phobject',
+    'HeraldGroup' => 'Phobject',
     'HeraldInvalidActionException' => 'Exception',
     'HeraldInvalidConditionException' => 'Exception',
     'HeraldManageGlobalRulesCapability' => 'PhabricatorPolicyCapability',
     'HeraldManiphestTaskAdapter' => 'HeraldAdapter',
     'HeraldNewController' => 'HeraldController',
     'HeraldNewObjectField' => 'HeraldField',
+    'HeraldNotifyActionGroup' => 'HeraldActionGroup',
     'HeraldObjectTranscript' => 'Phobject',
     'HeraldPholioMockAdapter' => 'HeraldAdapter',
     'HeraldPreCommitAdapter' => 'HeraldAdapter',
     'HeraldPreCommitContentAdapter' => 'HeraldPreCommitAdapter',
     'HeraldPreCommitRefAdapter' => 'HeraldPreCommitAdapter',
+    'HeraldPreventActionGroup' => 'HeraldActionGroup',
     'HeraldProjectsField' => 'HeraldField',
     'HeraldRecursiveConditionsException' => 'Exception',
     'HeraldRelatedFieldGroup' => 'HeraldFieldGroup',
@@ -4748,6 +4762,7 @@ phutil_register_library_map(array(
     'HeraldSelectFieldValue' => 'HeraldFieldValue',
     'HeraldSpaceField' => 'HeraldField',
     'HeraldSubscribersField' => 'HeraldField',
+    'HeraldSupportActionGroup' => 'HeraldActionGroup',
     'HeraldSupportFieldGroup' => 'HeraldFieldGroup',
     'HeraldTestConsoleController' => 'HeraldController',
     'HeraldTextFieldValue' => 'HeraldFieldValue',
@@ -4764,6 +4779,7 @@ phutil_register_library_map(array(
     'HeraldTranscriptQuery' => 'PhabricatorCursorPagedPolicyAwareQuery',
     'HeraldTranscriptSearchEngine' => 'PhabricatorApplicationSearchEngine',
     'HeraldTranscriptTestCase' => 'PhabricatorTestCase',
+    'HeraldUtilityActionGroup' => 'HeraldActionGroup',
     'Javelin' => 'Phobject',
     'JavelinReactorUIExample' => 'PhabricatorUIExample',
     'JavelinUIExample' => 'PhabricatorUIExample',

src/applications/differential/herald/HeraldDifferentialDiffAdapter.php

@@ -75,7 +75,6 @@ final class HeraldDifferentialDiffAdapter extends HeraldDifferentialAdapter {
         return array_merge(
           array(
             self::ACTION_BLOCK,
-            self::ACTION_NOTHING,
           ),
           parent::getActions($rule_type));
     }

src/applications/differential/herald/HeraldDifferentialRevisionAdapter.php

@@ -169,7 +169,6 @@ final class HeraldDifferentialRevisionAdapter
             self::ACTION_ADD_BLOCKING_REVIEWERS,
             self::ACTION_APPLY_BUILD_PLANS,
             self::ACTION_REQUIRE_SIGNATURE,
-            self::ACTION_NOTHING,
           ),
           parent::getActions($rule_type));
       case HeraldRuleTypeConfig::RULE_TYPE_PERSONAL:
@@ -181,7 +180,6 @@ final class HeraldDifferentialRevisionAdapter
             self::ACTION_FLAG,
             self::ACTION_ADD_REVIEWERS,
             self::ACTION_ADD_BLOCKING_REVIEWERS,
-            self::ACTION_NOTHING,
           ),
           parent::getActions($rule_type));
     }

src/applications/diffusion/herald/HeraldCommitAdapter.php

@@ -94,7 +94,6 @@ final class HeraldCommitAdapter extends HeraldAdapter {
             self::ACTION_EMAIL,
             self::ACTION_AUDIT,
             self::ACTION_APPLY_BUILD_PLANS,
-            self::ACTION_NOTHING,
           ),
           parent::getActions($rule_type));
       case HeraldRuleTypeConfig::RULE_TYPE_PERSONAL:
@@ -105,7 +104,6 @@ final class HeraldCommitAdapter extends HeraldAdapter {
             self::ACTION_EMAIL,
             self::ACTION_FLAG,
             self::ACTION_AUDIT,
-            self::ACTION_NOTHING,
           ),
           parent::getActions($rule_type));
     }

src/applications/diffusion/herald/HeraldPreCommitAdapter.php

@@ -81,14 +81,12 @@ abstract class HeraldPreCommitAdapter extends HeraldAdapter {
           array(
             self::ACTION_BLOCK,
             self::ACTION_EMAIL,
-            self::ACTION_NOTHING,
           ),
           parent::getActions($rule_type));
       case HeraldRuleTypeConfig::RULE_TYPE_PERSONAL:
         return array_merge(
           array(
             self::ACTION_EMAIL,
-            self::ACTION_NOTHING,
           ),
           parent::getActions($rule_type));
     }

src/applications/herald/action/HeraldAction.php

@@ -0,0 +1,118 @@
+<?php
+
+abstract class HeraldAction extends Phobject {
+
+  private $adapter;
+  private $applyLog = array();
+
+  const STANDARD_NONE = 'standard.none';
+  const STANDARD_PHID_LIST = 'standard.phid.list';
+
+  abstract public function getHeraldActionName();
+  abstract public function supportsObject($object);
+  abstract public function supportsRuleType($rule_type);
+  abstract public function applyEffect($object, HeraldEffect $effect);
+
+  public function getActionGroupKey() {
+    return null;
+  }
+
+  public function getActionsForObject($object) {
+    return array($this->getActionConstant() => $this);
+  }
+
+  protected function getDatasource() {
+    throw new PhutilMethodNotImplementedException();
+  }
+
+  protected function getDatasourceValueMap() {
+    return null;
+  }
+
+  public function getHeraldActionStandardType() {
+    throw new PhutilMethodNotImplementedException();
+  }
+
+  public function getHeraldActionValueType() {
+    switch ($this->getHeraldActionStandardType()) {
+      case self::STANDARD_NONE:
+        return new HeraldEmptyFieldValue();
+      case self::STANDARD_PHID_LIST:
+        $tokenizer = id(new HeraldTokenizerFieldValue())
+          ->setKey($this->getHeraldFieldName())
+          ->setDatasource($this->getDatasource());
+
+        $value_map = $this->getDatasourceValueMap();
+        if ($value_map !== null) {
+          $tokenizer->setValueMap($value_map);
+        }
+
+        return $tokenizer;
+    }
+
+    throw new PhutilMethodNotImplementedException();
+  }
+
+  public function willSaveActionValue($value) {
+    return $value;
+  }
+
+  final public function setAdapter(HeraldAdapter $adapter) {
+    $this->adapter = $adapter;
+    return $this;
+  }
+
+  final public function getAdapter() {
+    return $this->adapter;
+  }
+
+  final public function getActionConstant() {
+    $class = new ReflectionClass($this);
+
+    $const = $class->getConstant('ACTIONCONST');
+    if ($const === false) {
+      throw new Exception(
+        pht(
+          '"%s" class "%s" must define a "%s" property.',
+          __CLASS__,
+          get_class($this),
+          'ACTIONCONST'));
+    }
+
+    $limit = self::getActionConstantByteLimit();
+    if (!is_string($const) || (strlen($const) > $limit)) {
+      throw new Exception(
+        pht(
+          '"%s" class "%s" has an invalid "%s" property. Action constants '.
+          'must be strings and no more than %s bytes in length.',
+          __CLASS__,
+          get_class($this),
+          'ACTIONCONST',
+          new PhutilNumber($limit)));
+    }
+
+    return $const;
+  }
+
+  final public static function getActionConstantByteLimit() {
+    return 64;
+  }
+
+  final public static function getAllActions() {
+    return id(new PhutilClassMapQuery())
+      ->setAncestorClass(__CLASS__)
+      ->setUniqueMethod('getActionConstant')
+      ->execute();
+  }
+
+  protected function logEffect($type, $data = null) {
+    return;
+  }
+
+  final public function getApplyTranscript(HeraldEffect $effect) {
+    $context = 'v2/'.phutil_json_encode($this->applyLog);
+    $this->applyLog = array();
+    return new HeraldApplyTranscript($effect, true, $context);
+  }
+
+}

src/applications/herald/action/HeraldActionGroup.php

@@ -0,0 +1,28 @@
+<?php
+
+abstract class HeraldActionGroup extends HeraldGroup {
+
+  final public function getGroupKey() {
+    $class = new ReflectionClass($this);
+
+    $const = $class->getConstant('ACTIONGROUPKEY');
+    if ($const === false) {
+      throw new Exception(
+        pht(
+          '"%s" class "%s" must define a "%s" property.',
+          __CLASS__,
+          get_class($this),
+          'ACTIONGROUPKEY'));
+    }
+
+    return $const;
+  }
+
+  final public static function getAllActionGroups() {
+    return id(new PhutilClassMapQuery())
+      ->setAncestorClass(__CLASS__)
+      ->setUniqueMethod('getGroupKey')
+      ->setSortMethod('getSortKey')
+      ->execute();
+  }
+}

src/applications/herald/action/HeraldDoNothingAction.php

@@ -0,0 +1,32 @@
+<?php
+
+final class HeraldDoNothingAction extends HeraldAction {
+
+  const ACTIONCONST = 'nothing';
+  const DO_NOTHING = 'do.nothing';
+
+  public function getHeraldActionName() {
+    return pht('Do nothing');
+  }
+
+  public function getActionGroupKey() {
+    return HeraldUtilityActionGroup::ACTIONGROUPKEY;
+  }
+
+  public function supportsObject($object) {
+    return true;
+  }
+
+  public function supportsRuleType($rule_type) {
+    return true;
+  }
+
+  public function applyEffect($object, HeraldEffect $effect) {
+    $this->logEffect($effect, self::DO_NOTHING);
+  }
+
+  public function getHeraldActionStandardType() {
+    return self::STANDARD_NONE;
+  }
+
+}

src/applications/herald/action/HeraldNotifyActionGroup.php

@@ -0,0 +1,15 @@
+<?php
+
+final class HeraldNotifyActionGroup extends HeraldActionGroup {
+
+  const ACTIONGROUPKEY = 'notify';
+
+  public function getGroupLabel() {
+    return pht('Notify');
+  }
+
+  protected function getGroupOrder() {
+    return 2000;
+  }
+
+}

src/applications/herald/action/HeraldPreventActionGroup.php

@@ -0,0 +1,15 @@
+<?php
+
+final class HeraldPreventActionGroup extends HeraldActionGroup {
+
+  const ACTIONGROUPKEY = 'prevent';
+
+  public function getGroupLabel() {
+    return pht('Prevent');
+  }
+
+  protected function getGroupOrder() {
+    return 3000;
+  }
+
+}

src/applications/herald/action/HeraldSupportActionGroup.php

@@ -0,0 +1,15 @@
+<?php
+
+final class HeraldSupportActionGroup extends HeraldActionGroup {
+
+  const ACTIONGROUPKEY = 'support';
+
+  public function getGroupLabel() {
+    return pht('Supporting Applications');
+  }
+
+  protected function getGroupOrder() {
+    return 4000;
+  }
+
+}

src/applications/herald/action/HeraldUtilityActionGroup.php

@@ -0,0 +1,15 @@
+<?php
+
+final class HeraldUtilityActionGroup extends HeraldActionGroup {
+
+  const ACTIONGROUPKEY = 'utility';
+
+  public function getGroupLabel() {
+    return pht('Utility');
+  }
+
+  protected function getGroupOrder() {
+    return 10000;
+  }
+
+}

src/applications/herald/adapter/HeraldAdapter.php

@@ -29,7 +29,6 @@ abstract class HeraldAdapter extends Phobject {
   const ACTION_ADD_CC       = 'addcc';
   const ACTION_REMOVE_CC    = 'remcc';
   const ACTION_EMAIL        = 'email';
-  const ACTION_NOTHING      = 'nothing';
   const ACTION_AUDIT        = 'audit';
   const ACTION_FLAG         = 'flag';
   const ACTION_ASSIGN_TASK  = 'assigntask';
@@ -50,6 +49,7 @@ abstract class HeraldAdapter extends Phobject {
   private $forcedEmailPHIDs = array();
   private $unsubscribedPHIDs;
   private $fieldMap;
+  private $actionMap;
 
   public function getEmailPHIDs() {
     return array_values($this->emailPHIDs);
@@ -615,6 +615,78 @@ abstract class HeraldAdapter extends Phobject {
 
 /* -(  Actions  )------------------------------------------------------------ */
 
+  private function getActionImplementationMap() {
+    if ($this->actionMap === null) {
+      // We can't use PhutilClassMapQuery here because action expansion
+      // depends on the adapter and object.
+
+      $object = $this->getObject();
+
+      $map = array();
+      $all = HeraldAction::getAllActions();
+      foreach ($all as $key => $action) {
+        $action = id(clone $action)->setAdapter($this);
+
+        if (!$action->supportsObject($object)) {
+          continue;
+        }
+
+        $subactions = $action->getActionsForObject($object);
+        foreach ($subactions as $subkey => $subaction) {
+          if (isset($map[$subkey])) {
+            throw new Exception(
+              pht(
+                'Two HeraldActions (of classes "%s" and "%s") have the same '.
+                'action key ("%s") after expansion for an object of class '.
+                '"%s" inside adapter "%s". Each action must have a unique '.
+                'action key.',
+                get_class($subaction),
+                get_class($map[$subkey]),
+                $subkey,
+                get_class($object),
+                get_class($this)));
+          }
+
+          $subaction = id(clone $subaction)->setAdapter($this);
+
+          $map[$subkey] = $subaction;
+        }
+      }
+      $this->actionMap = $map;
+    }
+
+    return $this->actionMap;
+  }
+
+  private function getActionsForRuleType($rule_type) {
+    $actions = $this->getActionImplementationMap();
+
+    foreach ($actions as $key => $action) {
+      if (!$action->supportsRuleType($rule_type)) {
+        unset($actions[$key]);
+      }
+    }
+
+    return $actions;
+  }
+
+  private function getActionImplementation($key) {
+    return idx($this->getActionImplementationMap(), $key);
+  }
+
+  public function getActionKeys() {
+    return array_keys($this->getActionImplementationMap());
+  }
+
+  public function getActionGroupKey($action_key) {
+    $action = $this->getActionImplementation($action_key);
+    if (!$action) {
+      return null;
+    }
+
+    return $action->getActionGroupKey();
+  }
+
   public function getCustomActionsForRuleType($rule_type) {
     $results = array();
     foreach ($this->getCustomActions() as $custom_action) {
@@ -640,6 +712,10 @@ abstract class HeraldAdapter extends Phobject {
       }
     }
 
+    foreach ($this->getActionsForRuleType($rule_type) as $key => $action) {
+      $actions[] = $key;
+    }
+
     return $actions;
   }
 
@@ -648,7 +724,6 @@ abstract class HeraldAdapter extends Phobject {
       case HeraldRuleTypeConfig::RULE_TYPE_GLOBAL:
       case HeraldRuleTypeConfig::RULE_TYPE_OBJECT:
         $standard = array(
-          self::ACTION_NOTHING      => pht('Do nothing'),
           self::ACTION_ADD_CC       => pht('Add Subscribers'),
           self::ACTION_REMOVE_CC    => pht('Remove Subscribers'),
           self::ACTION_EMAIL        => pht('Send an email to'),
@@ -666,7 +741,6 @@ abstract class HeraldAdapter extends Phobject {
         break;
       case HeraldRuleTypeConfig::RULE_TYPE_PERSONAL:
         $standard = array(
-          self::ACTION_NOTHING      => pht('Do nothing'),
           self::ACTION_ADD_CC       => pht('Add me as a subscriber'),
           self::ACTION_REMOVE_CC    => pht('Remove me as a subscriber'),
           self::ACTION_EMAIL        => pht('Send me an email'),
@@ -685,6 +759,10 @@ abstract class HeraldAdapter extends Phobject {
     $custom_actions = $this->getCustomActionsForRuleType($rule_type);
     $standard += mpull($custom_actions, 'getActionName', 'getActionKey');
 
+    foreach ($this->getActionsForRuleType($rule_type) as $key => $action) {
+      $standard[$key] = $action->getHeraldActionName();
+    }
+
     return $standard;
   }
 
@@ -692,6 +770,14 @@ abstract class HeraldAdapter extends Phobject {
     HeraldRule $rule,
     HeraldActionRecord $action) {
 
+    $impl = $this->getActionImplementation($action->getAction());
+    if ($impl) {
+      $target = $action->getTarget();
+      $target = $impl->willSaveActionValue($target);
+      $action->setTarget($target);
+      return;
+    }
+
     $target = $action->getTarget();
     if (is_array($target)) {
       $target = array_keys($target);
@@ -720,7 +806,6 @@ abstract class HeraldAdapter extends Phobject {
           }
           break;
         case self::ACTION_BLOCK:
-        case self::ACTION_NOTHING:
           break;
         default:
           throw new HeraldInvalidActionException(
@@ -744,6 +829,11 @@ abstract class HeraldAdapter extends Phobject {
   }
 
   public function getValueTypeForAction($action, $rule_type) {
+    $impl = $this->getActionImplementation($action);
+    if ($impl) {
+      return $impl->getHeraldActionValueType();
+    }
+
     $is_personal = ($rule_type == HeraldRuleTypeConfig::RULE_TYPE_PERSONAL);
 
     if ($is_personal) {
@@ -751,7 +841,6 @@ abstract class HeraldAdapter extends Phobject {
         case self::ACTION_ADD_CC:
         case self::ACTION_REMOVE_CC:
         case self::ACTION_EMAIL:
-        case self::ACTION_NOTHING:
         case self::ACTION_AUDIT:
         case self::ACTION_ASSIGN_TASK:
         case self::ACTION_ADD_REVIEWERS:
@@ -771,8 +860,6 @@ abstract class HeraldAdapter extends Phobject {
         case self::ACTION_EMAIL:
           return $this->buildTokenizerFieldValue(
             new PhabricatorMetaMTAMailableDatasource());
-        case self::ACTION_NOTHING:
-          return new HeraldEmptyFieldValue();
         case self::ACTION_ADD_PROJECTS:
         case self::ACTION_REMOVE_PROJECTS:
           return $this->buildTokenizerFieldValue(
@@ -1108,6 +1195,12 @@ abstract class HeraldAdapter extends Phobject {
   protected function applyStandardEffect(HeraldEffect $effect) {
     $action = $effect->getAction();
 
+    $impl = $this->getActionImplementation($action);
+    if ($impl) {
+      $impl->applyEffect($this->getObject(), $effect);
+      return $impl->getApplyTranscript($effect);
+    }
+
     $rule_type = $effect->getRule()->getRuleType();
     $supported = $this->getActions($rule_type);
     $supported = array_fuse($supported);
@@ -1133,8 +1226,6 @@ abstract class HeraldAdapter extends Phobject {
         return $this->applyFlagEffect($effect);
       case self::ACTION_EMAIL:
         return $this->applyEmailEffect($effect);
-      case self::ACTION_NOTHING:
-        return $this->applyNothingEffect($effect);
       default:
         break;
     }
@@ -1153,13 +1244,6 @@ abstract class HeraldAdapter extends Phobject {
     return $result;
   }
 
-  private function applyNothingEffect(HeraldEffect $effect) {
-    return new HeraldApplyTranscript(
-      $effect,
-      true,
-      pht('Did nothing.'));
-  }
-
   /**
    * @task apply
    */

src/applications/herald/controller/HeraldRuleController.php

@@ -427,40 +427,10 @@ final class HeraldRuleController extends HeraldController {
       }
     }
 
-    $group_map = array();
-    foreach ($field_map as $field_key => $field_name) {
-      $group_key = $adapter->getFieldGroupKey($field_key);
-      $group_map[$group_key][$field_key] = $field_name;
-    }
-
-    $field_groups = HeraldFieldGroup::getAllFieldGroups();
-
-    $groups = array();
-    foreach ($group_map as $group_key => $options) {
-      asort($options);
-
-      $field_group = idx($field_groups, $group_key);
-      if ($field_group) {
-        $group_label = $field_group->getGroupLabel();
-        $group_order = $field_group->getSortKey();
-      } else {
-        $group_label = nonempty($group_key, pht('Other'));
-        $group_order = 'Z';
-      }
-
-      $groups[] = array(
-        'label' => $group_label,
-        'options' => $options,
-        'order' => $group_order,
-      );
-    }
-
-    $groups = array_values(isort($groups, 'order'));
-
     $config_info = array();
-    $config_info['fields'] = $groups;
+    $config_info['fields'] = $this->getFieldGroups($adapter, $field_map);
     $config_info['conditions'] = $all_conditions;
-    $config_info['actions'] = $action_map;
+    $config_info['actions'] = $this->getActionGroups($adapter, $action_map);
     $config_info['valueMap'] = array();
 
     foreach ($field_map as $field => $name) {
@@ -492,7 +462,7 @@ final class HeraldRuleController extends HeraldController {
 
     $config_info['rule_type'] = $rule->getRuleType();
 
-    foreach ($config_info['actions'] as $action => $name) {
+    foreach ($action_map as $action => $name) {
       try {
         $value_key = $adapter->getValueTypeForAction(
           $action,
@@ -659,4 +629,55 @@ final class HeraldRuleController extends HeraldController {
     return $all_rules;
   }
 
+  private function getFieldGroups(HeraldAdapter $adapter, array $field_map) {
+    $group_map = array();
+    foreach ($field_map as $field_key => $field_name) {
+      $group_key = $adapter->getFieldGroupKey($field_key);
+      $group_map[$group_key][$field_key] = $field_name;
+    }
+
+    return $this->getGroups(
+      $group_map,
+      HeraldFieldGroup::getAllFieldGroups());
+  }
+
+  private function getActionGroups(HeraldAdapter $adapter, array $action_map) {
+    $group_map = array();
+    foreach ($action_map as $action_key => $action_name) {
+      $group_key = $adapter->getActionGroupKey($action_key);
+      $group_map[$group_key][$action_key] = $action_name;
+    }
+
+    return $this->getGroups(
+      $group_map,
+      HeraldActionGroup::getAllActionGroups());
+  }
+
+  private function getGroups(array $item_map, array $group_list) {
+    assert_instances_of($group_list, 'HeraldGroup');
+
+    $groups = array();
+    foreach ($item_map as $group_key => $options) {
+      asort($options);
+
+      $group_object = idx($group_list, $group_key);
+      if ($group_object) {
+        $group_label = $group_object->getGroupLabel();
+        $group_order = $group_object->getSortKey();
+      } else {
+        $group_label = nonempty($group_key, pht('Other'));
+        $group_order = 'Z';
+      }
+
+      $groups[] = array(
+        'label' => $group_label,
+        'options' => $options,
+        'order' => $group_order,
+      );
+    }
+
+    return array_values(isort($groups, 'order'));
+  }
+
+
 }

src/applications/herald/controller/HeraldTranscriptController.php

@@ -347,9 +347,6 @@ final class HeraldTranscriptController extends HeraldController {
 
       $target = $apply_xscript->getTarget();
       switch ($apply_xscript->getAction()) {
-        case HeraldAdapter::ACTION_NOTHING:
-          $target = null;
-          break;
         case HeraldAdapter::ACTION_FLAG:
           $target = PhabricatorFlagColor::getColorName($target);
           break;
@@ -358,6 +355,8 @@ final class HeraldTranscriptController extends HeraldController {
           $target = $target;
           break;
         default:
+          // TODO: This should be driven by HeraldActions.
+
           if (is_array($target) && $target) {
             foreach ($target as $k => $phid) {
               if (isset($handles[$phid])) {
@@ -388,6 +387,9 @@ final class HeraldTranscriptController extends HeraldController {
 
       $item->setHeader(pht('%s: %s', $rule, $target));
       $item->addAttribute($apply_xscript->getReason());
+
+      // TODO: This is a bit of a mess while actions convert.
+
       $item->addAttribute(
         pht('Outcome: %s', $apply_xscript->getAppliedReason()));
 

src/applications/herald/field/HeraldFieldGroup.php

@@ -1,12 +1,6 @@
 <?php
 
-abstract class HeraldFieldGroup extends Phobject {
-
-  abstract public function getGroupLabel();
-
-  protected function getGroupOrder() {
-    return 1000;
-  }
+abstract class HeraldFieldGroup extends HeraldGroup {
 
   final public function getGroupKey() {
     $class = new ReflectionClass($this);
@@ -18,16 +12,12 @@ abstract class HeraldFieldGroup extends Phobject {
           '"%s" class "%s" must define a "%s" property.',
           __CLASS__,
           get_class($this),
-          'FIELDCONST'));
+          'FIELDGROUPKEY'));
     }
 
     return $const;
   }
 
-  public function getSortKey() {
-    return sprintf('A%08d%s', $this->getGroupOrder(), $this->getGroupLabel());
-  }
-
   final public static function getAllFieldGroups() {
     return id(new PhutilClassMapQuery())
       ->setAncestorClass(__CLASS__)

src/applications/herald/group/HeraldGroup.php

@@ -0,0 +1,15 @@
+<?php
+
+abstract class HeraldGroup extends Phobject {
+
+  abstract public function getGroupLabel();
+
+  protected function getGroupOrder() {
+    return 1000;
+  }
+
+  public function getSortKey() {
+    return sprintf('A%08d%s', $this->getGroupOrder(), $this->getGroupLabel());
+  }
+
+}

src/applications/maniphest/herald/HeraldManiphestTaskAdapter.php

@@ -76,7 +76,6 @@ final class HeraldManiphestTaskAdapter extends HeraldAdapter {
             self::ACTION_REMOVE_CC,
             self::ACTION_EMAIL,
             self::ACTION_ASSIGN_TASK,
-            self::ACTION_NOTHING,
           ),
           parent::getActions($rule_type));
       case HeraldRuleTypeConfig::RULE_TYPE_PERSONAL:
@@ -87,7 +86,6 @@ final class HeraldManiphestTaskAdapter extends HeraldAdapter {
             self::ACTION_EMAIL,
             self::ACTION_FLAG,
             self::ACTION_ASSIGN_TASK,
-            self::ACTION_NOTHING,
           ),
           parent::getActions($rule_type));
     }

src/applications/pholio/herald/HeraldPholioMockAdapter.php

@@ -54,7 +54,6 @@ final class HeraldPholioMockAdapter extends HeraldAdapter {
           array(
             self::ACTION_ADD_CC,
             self::ACTION_REMOVE_CC,
-            self::ACTION_NOTHING,
           ),
           parent::getActions($rule_type));
       case HeraldRuleTypeConfig::RULE_TYPE_PERSONAL:
@@ -63,7 +62,6 @@ final class HeraldPholioMockAdapter extends HeraldAdapter {
             self::ACTION_ADD_CC,
             self::ACTION_REMOVE_CC,
             self::ACTION_FLAG,
-            self::ACTION_NOTHING,
           ),
           parent::getActions($rule_type));
     }

src/applications/phriction/herald/PhrictionDocumentHeraldAdapter.php

@@ -56,7 +56,6 @@ final class PhrictionDocumentHeraldAdapter extends HeraldAdapter {
             self::ACTION_ADD_CC,
             self::ACTION_REMOVE_CC,
             self::ACTION_EMAIL,
-            self::ACTION_NOTHING,
           ),
           parent::getActions($rule_type));
       case HeraldRuleTypeConfig::RULE_TYPE_PERSONAL:
@@ -66,7 +65,6 @@ final class PhrictionDocumentHeraldAdapter extends HeraldAdapter {
             self::ACTION_REMOVE_CC,
             self::ACTION_EMAIL,
             self::ACTION_FLAG,
-            self::ACTION_NOTHING,
           ),
           parent::getActions($rule_type));
     }

webroot/rsrc/js/application/herald/HeraldRuleEditor.js

@@ -322,21 +322,11 @@ JX.install('HeraldRuleEditor', {
     _renderCondition : function(row_id) {
       var groups = this._config.info.fields;
 
-      var optgroups = [];
-      for (var ii = 0; ii < groups.length; ii++) {
-        var group = groups[ii];
-        var options = [];
-        for (var k in group.options) {
-          options.push(JX.$N('option', {value: k}, group.options[k]));
-        }
-        optgroups.push(JX.$N('optgroup', {label: group.label}, options));
-      }
-
       var attrs = {
         sigil: 'field-select'
       };
 
-      var field_select = JX.$N('select', attrs, optgroups);
+      var field_select = this._renderGroupSelect(groups, attrs);
       field_select.value = this._config.conditions[row_id][0];
 
       var field_cell = JX.$N('td', {sigil: 'field-cell'}, field_select);
@@ -352,6 +342,21 @@ JX.install('HeraldRuleEditor', {
         delete actions[k];
       }
     },
+
+    _renderGroupSelect: function(groups, attrs) {
+      var optgroups = [];
+      for (var ii = 0; ii < groups.length; ii++) {
+        var group = groups[ii];
+        var options = [];
+        for (var k in group.options) {
+          options.push(JX.$N('option', {value: k}, group.options[k]));
+        }
+        optgroups.push(JX.$N('optgroup', {label: group.label}, options));
+      }
+
+      return JX.$N('select', attrs, optgroups);
+    },
+
     _newAction : function(data) {
       data = data || [];
       var temprow = this._actionsRowManager.addRow([]);
@@ -361,11 +366,16 @@ JX.install('HeraldRuleEditor', {
                                                 this._renderAction(data));
       this._onactionchange(r);
     },
+
     _renderAction : function(action) {
-      var action_select = this._renderSelect(
-        this._config.info.actions,
-        action[0],
-        'action-select');
+      var groups = this._config.info.actions;
+      var attrs = {
+        sigil: 'action-select'
+      };
+
+      var action_select = this._renderGroupSelect(groups, attrs);
+      action_select.value = action[0];
+
       var action_cell = JX.$N('td', {sigil: 'action-cell'}, action_select);
 
       var target_cell = JX.$N(