Merge redundant Herald effect code

Summary: Ref T7849. Lift more action handling out of adapters. In theory, adapters will some day do no action handling. That day is not today, but it is now a step closer.

Test Plan:
  - Wrote a rule using the email and flag actions.
  - Ran that rule.
  - Got an email and flag.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7849

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

src/applications/diffusion/herald/HeraldPreCommitAdapter.php

@@ -101,9 +101,6 @@ abstract class HeraldPreCommitAdapter extends HeraldAdapter {
             true,
             pht('Did nothing.'));
           break;
-        case self::ACTION_EMAIL:
-          $result[] = $this->applyEmailEffect($effect);
-          break;
         case self::ACTION_BLOCK:
           $result[] = new HeraldApplyTranscript(
             $effect,
@@ -111,14 +108,7 @@ abstract class HeraldPreCommitAdapter extends HeraldAdapter {
             pht('Blocked push.'));
           break;
         default:
-          $custom_result = parent::handleCustomHeraldEffect($effect);
+          $result[] = $this->applyStandardEffect($effect);
-          if ($custom_result === null) {
-            throw new Exception(pht(
-              "No rules to handle action '%s'.",
-              $action));
-          }
-
-          $result[] = $custom_result;
           break;
       }
     }

src/applications/herald/adapter/HeraldAdapter.php

@@ -1000,65 +1000,6 @@ abstract class HeraldAdapter {
     );
   }
 
-
-  public static function applyFlagEffect(HeraldEffect $effect, $phid) {
-    $color = $effect->getTarget();
-
-    $rule = $effect->getRule();
-    $user = $rule->getAuthor();
-
-    $flag = PhabricatorFlagQuery::loadUserFlag($user, $phid);
-    if ($flag) {
-      return new HeraldApplyTranscript(
-        $effect,
-        false,
-        pht('Object already flagged.'));
-    }
-
-    $handle = id(new PhabricatorHandleQuery())
-      ->setViewer($user)
-      ->withPHIDs(array($phid))
-      ->executeOne();
-
-    $flag = new PhabricatorFlag();
-    $flag->setOwnerPHID($user->getPHID());
-    $flag->setType($handle->getType());
-    $flag->setObjectPHID($handle->getPHID());
-
-    // TOOD: Should really be transcript PHID, but it doesn't exist yet.
-    $flag->setReasonPHID($user->getPHID());
-
-    $flag->setColor($color);
-    $flag->setNote(
-      pht('Flagged by Herald Rule "%s".', $rule->getName()));
-    $flag->save();
-
-    return new HeraldApplyTranscript(
-      $effect,
-      true,
-      pht('Added flag.'));
-  }
-
-  protected function applyEmailEffect(HeraldEffect $effect) {
-
-    foreach ($effect->getTarget() as $phid) {
-      $this->emailPHIDs[$phid] = $phid;
-
-      // If this is a personal rule, we'll force delivery of a real email. This
-      // effect is stronger than notification preferences, so you get an actual
-      // email even if your preferences are set to "Notify" or "Ignore".
-      $rule = $effect->getRule();
-      if ($rule->isPersonalRule()) {
-        $this->forcedEmailPHIDs[$phid] = $phid;
-      }
-    }
-
-    return new HeraldApplyTranscript(
-      $effect,
-      true,
-      pht('Added mailable to mail targets.'));
-  }
-
   public static function getAllAdapters() {
     static $adapters;
     if (!$adapters) {
@@ -1532,4 +1473,101 @@ abstract class HeraldAdapter {
   }
 
 
+/* -(  Applying Effects  )--------------------------------------------------- */
+
+
+  /**
+   * @task apply
+   */
+  protected function applyStandardEffect(HeraldEffect $effect) {
+    $action = $effect->getAction();
+
+    switch ($action) {
+      case self::ACTION_FLAG:
+        return $this->applyFlagEffect($effect);
+      case self::ACTION_EMAIL:
+        return $this->applyEmailEffect($effect);
+      default:
+        break;
+    }
+
+    $result = $this->handleCustomHeraldEffect($effect);
+
+    if (!$result) {
+      throw new Exception(
+        pht(
+          'No custom action exists to handle rule action "%s".',
+          $action));
+    }
+
+    return $result;
+  }
+
+
+  /**
+   * @task apply
+   */
+  private function applyFlagEffect(HeraldEffect $effect) {
+    $phid = $this->getPHID();
+    $color = $effect->getTarget();
+
+    $rule = $effect->getRule();
+    $user = $rule->getAuthor();
+
+    $flag = PhabricatorFlagQuery::loadUserFlag($user, $phid);
+    if ($flag) {
+      return new HeraldApplyTranscript(
+        $effect,
+        false,
+        pht('Object already flagged.'));
+    }
+
+    $handle = id(new PhabricatorHandleQuery())
+      ->setViewer($user)
+      ->withPHIDs(array($phid))
+      ->executeOne();
+
+    $flag = new PhabricatorFlag();
+    $flag->setOwnerPHID($user->getPHID());
+    $flag->setType($handle->getType());
+    $flag->setObjectPHID($handle->getPHID());
+
+    // TOOD: Should really be transcript PHID, but it doesn't exist yet.
+    $flag->setReasonPHID($user->getPHID());
+
+    $flag->setColor($color);
+    $flag->setNote(
+      pht('Flagged by Herald Rule "%s".', $rule->getName()));
+    $flag->save();
+
+    return new HeraldApplyTranscript(
+      $effect,
+      true,
+      pht('Added flag.'));
+  }
+
+
+  /**
+   * @task apply
+   */
+  private function applyEmailEffect(HeraldEffect $effect) {
+    foreach ($effect->getTarget() as $phid) {
+      $this->emailPHIDs[$phid] = $phid;
+
+      // If this is a personal rule, we'll force delivery of a real email. This
+      // effect is stronger than notification preferences, so you get an actual
+      // email even if your preferences are set to "Notify" or "Ignore".
+      $rule = $effect->getRule();
+      if ($rule->isPersonalRule()) {
+        $this->forcedEmailPHIDs[$phid] = $phid;
+      }
+    }
+
+    return new HeraldApplyTranscript(
+      $effect,
+      true,
+      pht('Added mailable to mail targets.'));
+  }
+
+
 }

src/applications/herald/adapter/HeraldCommitAdapter.php

@@ -493,9 +493,6 @@ final class HeraldCommitAdapter extends HeraldAdapter {
             true,
             pht('Great success at doing nothing.'));
           break;
-        case self::ACTION_EMAIL:
-          $result[] = $this->applyEmailEffect($effect);
-          break;
         case self::ACTION_ADD_CC:
           foreach ($effect->getTarget() as $phid) {
             if (empty($this->addCCPHIDs[$phid])) {
@@ -529,20 +526,8 @@ final class HeraldCommitAdapter extends HeraldAdapter {
             true,
             pht('Applied build plans.'));
           break;
-        case self::ACTION_FLAG:
-          $result[] = parent::applyFlagEffect(
-            $effect,
-            $this->commit->getPHID());
-          break;
         default:
-          $custom_result = parent::handleCustomHeraldEffect($effect);
+          $result[] = $this->applyStandardEffect($effect);
-          if ($custom_result === null) {
-            throw new Exception(pht(
-              "No rules to handle action '%s'.",
-              $action));
-          }
-
-          $result[] = $custom_result;
           break;
       }
     }

src/applications/herald/adapter/HeraldDifferentialRevisionAdapter.php

@@ -322,14 +322,6 @@ final class HeraldDifferentialRevisionAdapter
             true,
             pht('OK, did nothing.'));
           break;
-        case self::ACTION_FLAG:
-          $result[] = parent::applyFlagEffect(
-            $effect,
-            $this->revision->getPHID());
-          break;
-        case self::ACTION_EMAIL:
-          $result[] = $this->applyEmailEffect($effect);
-          break;
         case self::ACTION_ADD_CC:
           $base_target = $effect->getTarget();
           $forbidden = array();
@@ -412,14 +404,7 @@ final class HeraldDifferentialRevisionAdapter
             pht('Required signatures.'));
           break;
         default:
-          $custom_result = parent::handleCustomHeraldEffect($effect);
+          $result[] = $this->applyStandardEffect($effect);
-          if ($custom_result === null) {
-            throw new Exception(pht(
-              "No rules to handle action '%s'.",
-              $action));
-          }
-
-          $result[] = $custom_result;
           break;
       }
     }

src/applications/herald/adapter/HeraldManiphestTaskAdapter.php

@@ -172,14 +172,6 @@ final class HeraldManiphestTaskAdapter extends HeraldAdapter {
             true,
             pht('Added addresses to cc list.'));
           break;
-        case self::ACTION_EMAIL:
-          $result[] = $this->applyEmailEffect($effect);
-          break;
-        case self::ACTION_FLAG:
-          $result[] = parent::applyFlagEffect(
-            $effect,
-            $this->getTask()->getPHID());
-          break;
         case self::ACTION_ASSIGN_TASK:
           $target_array = $effect->getTarget();
           $assign_phid = reset($target_array);
@@ -199,14 +191,7 @@ final class HeraldManiphestTaskAdapter extends HeraldAdapter {
             pht('Added projects.'));
           break;
         default:
-          $custom_result = parent::handleCustomHeraldEffect($effect);
+          $result[] = $this->applyStandardEffect($effect);
-          if ($custom_result === null) {
-            throw new Exception(pht(
-              "No rules to handle action '%s'.",
-              $action));
-          }
-
-          $result[] = $custom_result;
           break;
       }
     }

src/applications/herald/adapter/HeraldPholioMockAdapter.php

@@ -131,20 +131,8 @@ final class HeraldPholioMockAdapter extends HeraldAdapter {
             true,
             pht('Added address to cc list.'));
           break;
-        case self::ACTION_FLAG:
-          $result[] = parent::applyFlagEffect(
-            $effect,
-            $this->getMock()->getPHID());
-          break;
         default:
-          $custom_result = parent::handleCustomHeraldEffect($effect);
+          $result[] = $this->applyStandardEffect($effect);
-          if ($custom_result === null) {
-            throw new Exception(pht(
-              "No rules to handle action '%s'.",
-              $action));
-          }
-
-          $result[] = $custom_result;
           break;
       }
     }

src/applications/phriction/herald/PhrictionDocumentHeraldAdapter.php

@@ -132,23 +132,8 @@ final class PhrictionDocumentHeraldAdapter extends HeraldAdapter {
             true,
             pht('Added address to cc list.'));
           break;
-        case self::ACTION_FLAG:
-          $result[] = parent::applyFlagEffect(
-            $effect,
-            $this->getDocument()->getPHID());
-          break;
-        case self::ACTION_EMAIL:
-          $result[] = $this->applyEmailEffect($effect);
-          break;
         default:
-          $custom_result = parent::handleCustomHeraldEffect($effect);
+          $result[] = $this->applyStandardEffect($effect);
-          if ($custom_result === null) {
-            throw new Exception(pht(
-              "No rules to handle action '%s'.",
-              $action));
-          }
-
-          $result[] = $custom_result;
           break;
       }
     }