diff --git a/src/applications/herald/adapter/HeraldAdapter.php b/src/applications/herald/adapter/HeraldAdapter.php --- a/src/applications/herald/adapter/HeraldAdapter.php +++ b/src/applications/herald/adapter/HeraldAdapter.php @@ -110,6 +110,7 @@ private $queuedTransactions = array(); private $emailPHIDs = array(); private $forcedEmailPHIDs = array(); + private $unsubscribedPHIDs; public function getEmailPHIDs() { return array_values($this->emailPHIDs); @@ -1555,6 +1556,9 @@ case self::ACTION_ADD_PROJECTS: case self::ACTION_REMOVE_PROJECTS: return $this->applyProjectsEffect($effect); + case self::ACTION_ADD_CC: + case self::ACTION_REMOVE_CC: + return $this->applySubscribersEffect($effect); case self::ACTION_FLAG: return $this->applyFlagEffect($effect); case self::ACTION_EMAIL: @@ -1615,6 +1619,97 @@ pht('Added projects.')); } + /** + * @task apply + */ + private function applySubscribersEffect(HeraldEffect $effect) { + if ($effect->getAction() == self::ACTION_ADD_CC) { + $kind = '+'; + $is_add = true; + } else { + $kind = '-'; + $is_add = false; + } + + $subscriber_phids = array_fuse($effect->getTarget()); + if (!$subscriber_phids) { + return new HeraldApplyTranscript( + $effect, + false, + pht('This action lists no users or objects to affect.')); + } + + // The "Add Subscribers" rule only adds subscribers who haven't previously + // unsubscribed from the object explicitly. Filter these subscribers out + // before continuing. + $unsubscribed = array(); + if ($is_add) { + if ($this->unsubscribedPHIDs === null) { + $this->unsubscribedPHIDs = PhabricatorEdgeQuery::loadDestinationPHIDs( + $this->getObject()->getPHID(), + PhabricatorObjectHasUnsubscriberEdgeType::EDGECONST); + } + + foreach ($this->unsubscribedPHIDs as $phid) { + if (isset($subscriber_phids[$phid])) { + $unsubscribed[$phid] = $phid; + unset($subscriber_phids[$phid]); + } + } + } + + if (!$subscriber_phids) { + return new HeraldApplyTranscript( + $effect, + false, + pht('All targets have previously unsubscribed explicitly.')); + } + + // Filter out PHIDs which aren't valid subscribers. Lower levels of the + // stack will fail loudly if we try to add subscribers with invalid PHIDs + // or unknown PHID types, so drop them here. + $invalid = array(); + foreach ($subscriber_phids as $phid) { + $type = phid_get_type($phid); + switch ($type) { + case PhabricatorPeopleUserPHIDType::TYPECONST: + case PhabricatorProjectProjectPHIDType::TYPECONST: + break; + default: + $invalid[$phid] = $phid; + unset($subscriber_phids[$phid]); + break; + } + } + + if (!$subscriber_phids) { + return new HeraldApplyTranscript( + $effect, + false, + pht('All targets are invalid as subscribers.')); + } + + $xaction = $this->newTransaction() + ->setTransactionType(PhabricatorTransactions::TYPE_SUBSCRIBERS) + ->setNewValue( + array( + $kind => $subscriber_phids, + )); + + $this->queueTransaction($xaction); + + // TODO: We could be more detailed about this, but doing it meaningfully + // probably requires substantial changes to how transactions are rendered + // first. + if ($is_add) { + $message = pht('Subscribed targets.'); + } else { + $message = pht('Unsubscribed targets.'); + } + + return new HeraldApplyTranscript($effect, true, $message); + } + /** * @task apply diff --git a/src/applications/herald/adapter/HeraldPholioMockAdapter.php b/src/applications/herald/adapter/HeraldPholioMockAdapter.php --- a/src/applications/herald/adapter/HeraldPholioMockAdapter.php +++ b/src/applications/herald/adapter/HeraldPholioMockAdapter.php @@ -3,7 +3,6 @@ final class HeraldPholioMockAdapter extends HeraldAdapter { private $mock; - private $ccPHIDs = array(); public function getAdapterApplicationClass() { return 'PhabricatorPholioApplication'; @@ -29,14 +28,6 @@ return $this->mock; } - private function setCcPHIDs(array $cc_phids) { - $this->ccPHIDs = $cc_phids; - return $this; - } - public function getCcPHIDs() { - return $this->ccPHIDs; - } - public function getAdapterContentName() { return pht('Pholio Mocks'); } @@ -71,6 +62,7 @@ return array_merge( array( self::ACTION_ADD_CC, + self::ACTION_REMOVE_CC, self::ACTION_NOTHING, ), parent::getActions($rule_type)); @@ -78,6 +70,7 @@ return array_merge( array( self::ACTION_ADD_CC, + self::ACTION_REMOVE_CC, self::ACTION_FLAG, self::ACTION_NOTHING, ), @@ -117,15 +110,6 @@ foreach ($effects as $effect) { $action = $effect->getAction(); switch ($action) { - case self::ACTION_ADD_CC: - foreach ($effect->getTarget() as $phid) { - $this->ccPHIDs[] = $phid; - } - $result[] = new HeraldApplyTranscript( - $effect, - true, - pht('Added address to cc list.')); - break; default: $result[] = $this->applyStandardEffect($effect); break; diff --git a/src/applications/pholio/controller/PholioMockEditController.php b/src/applications/pholio/controller/PholioMockEditController.php --- a/src/applications/pholio/controller/PholioMockEditController.php +++ b/src/applications/pholio/controller/PholioMockEditController.php @@ -339,7 +339,7 @@ ->setDatasource(new PhabricatorProjectDatasource())) ->appendControl( id(new AphrontFormTokenizerControl()) - ->setLabel(pht('CC')) + ->setLabel(pht('Subscribers')) ->setName('cc') ->setValue($v_cc) ->setUser($user) diff --git a/src/applications/pholio/editor/PholioMockEditor.php b/src/applications/pholio/editor/PholioMockEditor.php --- a/src/applications/pholio/editor/PholioMockEditor.php +++ b/src/applications/pholio/editor/PholioMockEditor.php @@ -453,24 +453,6 @@ ->setMock($object); } - protected function didApplyHeraldRules( - PhabricatorLiskDAO $object, - HeraldAdapter $adapter, - HeraldTranscript $transcript) { - - $xactions = array(); - - $cc_phids = $adapter->getCcPHIDs(); - if ($cc_phids) { - $value = array_fuse($cc_phids); - $xactions[] = id(new PholioTransaction()) - ->setTransactionType(PhabricatorTransactions::TYPE_SUBSCRIBERS) - ->setNewValue(array('+' => $value)); - } - - return $xactions; - } - protected function sortTransactions(array $xactions) { $head = array(); $tail = array();