Use standard subscribers effects in Herald Adapter for revisions

Summary:
Ref T8455. Use standard effects for revisions, instead of a custom effect.

This fixes the major issue (conduit error) in T8455 because the standard effect now performs PHID type filtering.

This retains other behaviors (in particular: not re-CC'ing explicitly removed CCs).

Test Plan:
  - With a Herald rule that adds a mailing list as a CC, created a revision before the change and hit the error in T8455. After the change, saw correct behavior.
  - Wrote a normal Herald rule to add CCs and created a revision, saw it fire properly.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T8455

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

src/applications/differential/editor/DifferentialTransactionEditor.php

@@ -1554,10 +1554,6 @@ final class DifferentialTransactionEditor
     PhabricatorLiskDAO $object,
     array $xactions) {
 
-    $unsubscribed_phids = PhabricatorEdgeQuery::loadDestinationPHIDs(
-      $object->getPHID(),
-      PhabricatorObjectHasUnsubscriberEdgeType::EDGECONST);
-
     $revision = id(new DifferentialRevisionQuery())
       ->setViewer($this->getActor())
       ->withPHIDs(array($object->getPHID()))
@@ -1577,7 +1573,6 @@ final class DifferentialTransactionEditor
     $reviewer_phids = mpull($reviewers, 'getReviewerPHID');
 
     $adapter->setExplicitReviewers($reviewer_phids);
-    $adapter->setForbiddenCCs($unsubscribed_phids);
 
     return $adapter;
   }
@@ -1589,24 +1584,6 @@ final class DifferentialTransactionEditor
 
     $xactions = array();
 
-    // Build a transaction to adjust CCs.
-    $ccs = array(
-      '+' => array_keys($adapter->getCCsAddedByHerald()),
-      '-' => array_keys($adapter->getCCsRemovedByHerald()),
-    );
-    $value = array();
-    foreach ($ccs as $type => $phids) {
-      foreach ($phids as $phid) {
-        $value[$type][$phid] = $phid;
-      }
-    }
-
-    if ($value) {
-      $xactions[] = id(new DifferentialTransaction())
-        ->setTransactionType(PhabricatorTransactions::TYPE_SUBSCRIBERS)
-        ->setNewValue($value);
-    }
-
     // Build a transaction to adjust reviewers.
     $reviewers = array(
       DifferentialReviewerStatus::STATUS_ADDED =>

src/applications/herald/adapter/HeraldDifferentialRevisionAdapter.php

@@ -6,10 +6,6 @@ final class HeraldDifferentialRevisionAdapter
   protected $revision;
 
   protected $explicitReviewers;
-  protected $forbiddenCCs;
-
-  protected $newCCs = array();
-  protected $remCCs = array();
   protected $addReviewerPHIDs = array();
   protected $blockingReviewerPHIDs = array();
   protected $buildPlans = array();
@@ -114,19 +110,6 @@ final class HeraldDifferentialRevisionAdapter
     return $this;
   }
 
-  public function setForbiddenCCs($forbidden_ccs) {
-    $this->forbiddenCCs = $forbidden_ccs;
-    return $this;
-  }
-
-  public function getCCsAddedByHerald() {
-    return array_diff_key($this->newCCs, $this->remCCs);
-  }
-
-  public function getCCsRemovedByHerald() {
-    return $this->remCCs;
-  }
-
   public function getReviewersAddedByHerald() {
     return $this->addReviewerPHIDs;
   }
@@ -286,57 +269,9 @@ final class HeraldDifferentialRevisionAdapter
 
     $result = array();
 
-    $forbidden_ccs = array_fill_keys(
-      nonempty($this->forbiddenCCs, array()),
-      true);
-
     foreach ($effects as $effect) {
       $action = $effect->getAction();
       switch ($action) {
-        case self::ACTION_ADD_CC:
-          $base_target = $effect->getTarget();
-          $forbidden = array();
-          foreach ($base_target as $key => $fbid) {
-            if (isset($forbidden_ccs[$fbid])) {
-              $forbidden[] = $fbid;
-              unset($base_target[$key]);
-            } else {
-              $this->newCCs[$fbid] = true;
-            }
-          }
-
-          if ($forbidden) {
-            $failed = clone $effect;
-            $failed->setTarget($forbidden);
-            if ($base_target) {
-              $effect->setTarget($base_target);
-              $result[] = new HeraldApplyTranscript(
-                $effect,
-                true,
-                pht(
-                  'Added these addresses to CC list. '.
-                  'Others could not be added.'));
-            }
-            $result[] = new HeraldApplyTranscript(
-              $failed,
-              false,
-              pht('CC forbidden, these addresses have unsubscribed.'));
-          } else {
-            $result[] = new HeraldApplyTranscript(
-              $effect,
-              true,
-              pht('Added addresses to CC list.'));
-          }
-          break;
-        case self::ACTION_REMOVE_CC:
-          foreach ($effect->getTarget() as $fbid) {
-            $this->remCCs[$fbid] = true;
-          }
-          $result[] = new HeraldApplyTranscript(
-            $effect,
-            true,
-            pht('Removed addresses from CC list.'));
-          break;
         case self::ACTION_ADD_REVIEWERS:
           foreach ($effect->getTarget() as $phid) {
             $this->addReviewerPHIDs[$phid] = true;