Lift handling of Herald "email" effect to Adapter

Summary:
Ref T7731. Every adapter subclass currently implements this effect in an essentially identical way.

Some day far from now the effects will be modular and this mess will vanish completely, but reduce its sprawl for now.

Test Plan: I'll test this thoroughly at the end of the change sequence since writing rules is a pain.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7731

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

src/applications/differential/editor/DifferentialTransactionEditor.php

@@ -1731,8 +1731,7 @@ final class DifferentialTransactionEditor
     }
 
     // Save extra email PHIDs for later.
-    $email_phids = $adapter->getEmailPHIDsAddedByHerald();
+    $this->heraldEmailPHIDs = $adapter->getEmailPHIDs();
-    $this->heraldEmailPHIDs = array_keys($email_phids);
 
     // Apply build plans.
     HarbormasterBuildable::applyBuildPlans(

src/applications/diffusion/herald/HeraldPreCommitAdapter.php

@@ -4,7 +4,6 @@ abstract class HeraldPreCommitAdapter extends HeraldAdapter {
 
   private $log;
   private $hookEngine;
-  private $emailPHIDs = array();
 
   public function setPushLog(PhabricatorRepositoryPushLog $log) {
     $this->log = $log;
@@ -28,10 +27,6 @@ abstract class HeraldPreCommitAdapter extends HeraldAdapter {
     return $this->log;
   }
 
-  public function getEmailPHIDs() {
-    return array_values($this->emailPHIDs);
-  }
-
   public function supportsRuleType($rule_type) {
     switch ($rule_type) {
       case HeraldRuleTypeConfig::RULE_TYPE_GLOBAL:
@@ -107,13 +102,7 @@ abstract class HeraldPreCommitAdapter extends HeraldAdapter {
             pht('Did nothing.'));
           break;
         case self::ACTION_EMAIL:
-          foreach ($effect->getTarget() as $phid) {
+          $result[] = $this->applyEmailEffect($effect);
-            $this->emailPHIDs[$phid] = $phid;
-          }
-          $result[] = new HeraldApplyTranscript(
-            $effect,
-            true,
-            pht('Added mailable to mail targets.'));
           break;
         case self::ACTION_BLOCK:
           $result[] = new HeraldApplyTranscript(

src/applications/herald/adapter/HeraldAdapter.php

@@ -109,6 +109,11 @@ abstract class HeraldAdapter {
   private $customFields = false;
   private $customActions = null;
   private $queuedTransactions = array();
+  private $emailPHIDs = array();
+
+  public function getEmailPHIDs() {
+    return array_values($this->emailPHIDs);
+  }
 
   public function getCustomActions() {
     if ($this->customActions === null) {
@@ -1032,6 +1037,18 @@ abstract class HeraldAdapter {
       pht('Added flag.'));
   }
 
+  protected function applyEmailEffect(HeraldEffect $effect) {
+
+    foreach ($effect->getTarget() as $phid) {
+      $this->emailPHIDs[$phid] = $phid;
+    }
+
+    return new HeraldApplyTranscript(
+      $effect,
+      true,
+      pht('Added mailable to mail targets.'));
+  }
+
   public static function getAllAdapters() {
     static $adapters;
     if (!$adapters) {

src/applications/herald/adapter/HeraldCommitAdapter.php

@@ -13,7 +13,6 @@ final class HeraldCommitAdapter extends HeraldAdapter {
   protected $commitData;
   private $commitDiff;
 
-  protected $emailPHIDs = array();
   protected $addCCPHIDs = array();
   protected $auditMap = array();
   protected $buildPlans = array();
@@ -217,10 +216,6 @@ final class HeraldCommitAdapter extends HeraldAdapter {
     return $this->commit->getPHID();
   }
 
-  public function getEmailPHIDs() {
-    return array_keys($this->emailPHIDs);
-  }
-
   public function getAddCCMap() {
     return $this->addCCPHIDs;
   }
@@ -499,13 +494,7 @@ final class HeraldCommitAdapter extends HeraldAdapter {
             pht('Great success at doing nothing.'));
           break;
         case self::ACTION_EMAIL:
-          foreach ($effect->getTarget() as $phid) {
+          $result[] = $this->applyEmailEffect($effect);
-            $this->emailPHIDs[$phid] = true;
-          }
-          $result[] = new HeraldApplyTranscript(
-            $effect,
-            true,
-            pht('Added address to email targets.'));
           break;
         case self::ACTION_ADD_CC:
           foreach ($effect->getTarget() as $phid) {

src/applications/herald/adapter/HeraldDifferentialRevisionAdapter.php

@@ -11,7 +11,6 @@ final class HeraldDifferentialRevisionAdapter
 
   protected $newCCs = array();
   protected $remCCs = array();
-  protected $emailPHIDs = array();
   protected $addReviewerPHIDs = array();
   protected $blockingReviewerPHIDs = array();
   protected $buildPlans = array();
@@ -131,10 +130,6 @@ final class HeraldDifferentialRevisionAdapter
     return $this->remCCs;
   }
 
-  public function getEmailPHIDsAddedByHerald() {
-    return $this->emailPHIDs;
-  }
-
   public function getReviewersAddedByHerald() {
     return $this->addReviewerPHIDs;
   }
@@ -333,8 +328,9 @@ final class HeraldDifferentialRevisionAdapter
             $this->revision->getPHID());
           break;
         case self::ACTION_EMAIL:
+          $result[] = $this->applyEmailEffect($effect);
+          break;
         case self::ACTION_ADD_CC:
-          $op = ($action == self::ACTION_EMAIL) ? 'email' : 'CC';
           $base_target = $effect->getTarget();
           $forbidden = array();
           foreach ($base_target as $key => $fbid) {
@@ -342,11 +338,7 @@ final class HeraldDifferentialRevisionAdapter
               $forbidden[] = $fbid;
               unset($base_target[$key]);
             } else {
-              if ($action == self::ACTION_EMAIL) {
+              $this->newCCs[$fbid] = true;
-                $this->emailPHIDs[$fbid] = true;
-              } else {
-                $this->newCCs[$fbid] = true;
-              }
             }
           }
 
@@ -358,18 +350,18 @@ final class HeraldDifferentialRevisionAdapter
               $result[] = new HeraldApplyTranscript(
                 $effect,
                 true,
-                pht('Added these addresses to %s list. '.
+                pht('Added these addresses to CC list. '.
-                'Others could not be added.', $op));
+                'Others could not be added.'));
             }
             $result[] = new HeraldApplyTranscript(
               $failed,
               false,
-              pht('%s forbidden, these addresses have unsubscribed.', $op));
+              pht('CC forbidden, these addresses have unsubscribed.'));
           } else {
             $result[] = new HeraldApplyTranscript(
               $effect,
               true,
-              pht('Added addresses to %s list.', $op));
+              pht('Added addresses to CC list.'));
           }
           break;
         case self::ACTION_REMOVE_CC:

src/applications/herald/adapter/HeraldManiphestTaskAdapter.php

@@ -6,11 +6,6 @@ final class HeraldManiphestTaskAdapter extends HeraldAdapter {
   private $ccPHIDs = array();
   private $assignPHID;
   private $projectPHIDs = array();
-  private $emailPHIDs = array();
-
-  public function getEmailPHIDs() {
-    return $this->emailPHIDs;
-  }
 
   public function getAdapterApplicationClass() {
     return 'PhabricatorManiphestApplication';
@@ -178,13 +173,7 @@ final class HeraldManiphestTaskAdapter extends HeraldAdapter {
             pht('Added addresses to cc list.'));
           break;
         case self::ACTION_EMAIL:
-          foreach ($effect->getTarget() as $phid) {
+          $result[] = $this->applyEmailEffect($effect);
-            $this->emailPHIDs[] = $phid;
-          }
-          $result[] = new HeraldApplyTranscript(
-            $effect,
-            true,
-            pht('Added addresses to email list.'));
           break;
         case self::ACTION_FLAG:
           $result[] = parent::applyFlagEffect(

src/applications/phriction/herald/PhrictionDocumentHeraldAdapter.php

@@ -4,7 +4,6 @@ final class PhrictionDocumentHeraldAdapter extends HeraldAdapter {
 
   private $document;
   private $ccPHIDs = array();
-  private $emailPHIDs = array();
 
   public function getAdapterApplicationClass() {
     return 'PhabricatorPhrictionApplication';
@@ -30,15 +29,11 @@ final class PhrictionDocumentHeraldAdapter extends HeraldAdapter {
     $this->ccPHIDs = $cc_phids;
     return $this;
   }
+
   public function getCcPHIDs() {
     return $this->ccPHIDs;
   }
 
-  public function getEmailPHIDs() {
-    return $this->emailPHIDs;
-  }
-
-
   public function getAdapterContentName() {
     return pht('Phriction Documents');
   }
@@ -143,13 +138,7 @@ final class PhrictionDocumentHeraldAdapter extends HeraldAdapter {
             $this->getDocument()->getPHID());
           break;
         case self::ACTION_EMAIL:
-          foreach ($effect->getTarget() as $phid) {
+          $result[] = $this->applyEmailEffect($effect);
-            $this->emailPHIDs[] = $phid;
-          }
-          $result[] = new HeraldApplyTranscript(
-            $effect,
-            true,
-            pht('Added addresses to email list.'));
           break;
         default:
           $custom_result = parent::handleCustomHeraldEffect($effect);