From 8ae08a3de70ba857f7513d766a8360dece7d6423 Mon Sep 17 00:00:00 2001 From: epriestley Date: Sat, 18 Jul 2015 08:07:31 -0700 Subject: [PATCH] Make "Add Subscribers" and "Remove Subscribers" Herald actions modular Summary: Ref T8726. Converts these actions to be modular. No real surprises in this change. Test Plan: {F658709} - Wrote some rules. - Migrated them forward. - Used a bunch of these rules. Reviewers: btrahan Reviewed By: btrahan Subscribers: joshuaspence, epriestley Maniphest Tasks: T8726 Differential Revision: https://secure.phabricator.com/D13699 --- .../sql/autopatches/20150724.herald.1.sql | 27 ++ src/__phutil_library_map__.php | 10 + .../HeraldDifferentialRevisionAdapter.php | 4 - .../diffusion/herald/HeraldCommitAdapter.php | 4 - .../herald/action/HeraldAction.php | 24 +- .../herald/adapter/HeraldAdapter.php | 144 +++-------- .../herald/HeraldManiphestTaskAdapter.php | 4 - .../pholio/herald/HeraldPholioMockAdapter.php | 19 -- .../herald/PhrictionDocumentHeraldAdapter.php | 4 - ...icatorSubscriptionsAddSelfHeraldAction.php | 37 +++ ...ubscriptionsAddSubscribersHeraldAction.php | 40 +++ .../PhabricatorSubscriptionsHeraldAction.php | 235 ++++++++++++++++++ ...torSubscriptionsRemoveSelfHeraldAction.php | 37 +++ ...criptionsRemoveSubscribersHeraldAction.php | 40 +++ .../PhabricatorUSEnglishTranslation.php | 34 +++ 15 files changed, 513 insertions(+), 150 deletions(-) create mode 100644 resources/sql/autopatches/20150724.herald.1.sql create mode 100644 src/applications/subscriptions/herald/PhabricatorSubscriptionsAddSelfHeraldAction.php create mode 100644 src/applications/subscriptions/herald/PhabricatorSubscriptionsAddSubscribersHeraldAction.php create mode 100644 src/applications/subscriptions/herald/PhabricatorSubscriptionsHeraldAction.php create mode 100644 src/applications/subscriptions/herald/PhabricatorSubscriptionsRemoveSelfHeraldAction.php create mode 100644 src/applications/subscriptions/herald/PhabricatorSubscriptionsRemoveSubscribersHeraldAction.php diff --git a/resources/sql/autopatches/20150724.herald.1.sql b/resources/sql/autopatches/20150724.herald.1.sql new file mode 100644 index 0000000000..aaf9b69196 --- /dev/null +++ b/resources/sql/autopatches/20150724.herald.1.sql @@ -0,0 +1,27 @@ +UPDATE {$NAMESPACE}_herald.herald_actionrecord a + JOIN {$NAMESPACE}_herald.herald_rule r + ON a.ruleID = r.id + SET a.action = 'subscribers.add' + WHERE r.ruleType != 'personal' + AND a.action = 'addcc'; + +UPDATE {$NAMESPACE}_herald.herald_actionrecord a + JOIN {$NAMESPACE}_herald.herald_rule r + ON a.ruleID = r.id + SET a.action = 'subscribers.self.add' + WHERE r.ruleType = 'personal' + AND a.action = 'addcc'; + +UPDATE {$NAMESPACE}_herald.herald_actionrecord a + JOIN {$NAMESPACE}_herald.herald_rule r + ON a.ruleID = r.id + SET a.action = 'subscribers.remove' + WHERE r.ruleType != 'personal' + AND a.action = 'remcc'; + +UPDATE {$NAMESPACE}_herald.herald_actionrecord a + JOIN {$NAMESPACE}_herald.herald_rule r + ON a.ruleID = r.id + SET a.action = 'subscribers.self.remove' + WHERE r.ruleType = 'personal' + AND a.action = 'remcc'; diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 4254da43cc..8904abc6de 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -2880,10 +2880,15 @@ phutil_register_library_map(array( 'PhabricatorSubscribedToObjectEdgeType' => 'applications/transactions/edges/PhabricatorSubscribedToObjectEdgeType.php', 'PhabricatorSubscribersQuery' => 'applications/subscriptions/query/PhabricatorSubscribersQuery.php', 'PhabricatorSubscriptionTriggerClock' => 'infrastructure/daemon/workers/clock/PhabricatorSubscriptionTriggerClock.php', + 'PhabricatorSubscriptionsAddSelfHeraldAction' => 'applications/subscriptions/herald/PhabricatorSubscriptionsAddSelfHeraldAction.php', + 'PhabricatorSubscriptionsAddSubscribersHeraldAction' => 'applications/subscriptions/herald/PhabricatorSubscriptionsAddSubscribersHeraldAction.php', 'PhabricatorSubscriptionsApplication' => 'applications/subscriptions/application/PhabricatorSubscriptionsApplication.php', 'PhabricatorSubscriptionsEditController' => 'applications/subscriptions/controller/PhabricatorSubscriptionsEditController.php', 'PhabricatorSubscriptionsEditor' => 'applications/subscriptions/editor/PhabricatorSubscriptionsEditor.php', + 'PhabricatorSubscriptionsHeraldAction' => 'applications/subscriptions/herald/PhabricatorSubscriptionsHeraldAction.php', 'PhabricatorSubscriptionsListController' => 'applications/subscriptions/controller/PhabricatorSubscriptionsListController.php', + 'PhabricatorSubscriptionsRemoveSelfHeraldAction' => 'applications/subscriptions/herald/PhabricatorSubscriptionsRemoveSelfHeraldAction.php', + 'PhabricatorSubscriptionsRemoveSubscribersHeraldAction' => 'applications/subscriptions/herald/PhabricatorSubscriptionsRemoveSubscribersHeraldAction.php', 'PhabricatorSubscriptionsSubscribeEmailCommand' => 'applications/subscriptions/command/PhabricatorSubscriptionsSubscribeEmailCommand.php', 'PhabricatorSubscriptionsSubscribersPolicyRule' => 'applications/subscriptions/policyrule/PhabricatorSubscriptionsSubscribersPolicyRule.php', 'PhabricatorSubscriptionsTransactionController' => 'applications/subscriptions/controller/PhabricatorSubscriptionsTransactionController.php', @@ -6894,10 +6899,15 @@ phutil_register_library_map(array( 'PhabricatorSubscribedToObjectEdgeType' => 'PhabricatorEdgeType', 'PhabricatorSubscribersQuery' => 'PhabricatorQuery', 'PhabricatorSubscriptionTriggerClock' => 'PhabricatorTriggerClock', + 'PhabricatorSubscriptionsAddSelfHeraldAction' => 'PhabricatorSubscriptionsHeraldAction', + 'PhabricatorSubscriptionsAddSubscribersHeraldAction' => 'PhabricatorSubscriptionsHeraldAction', 'PhabricatorSubscriptionsApplication' => 'PhabricatorApplication', 'PhabricatorSubscriptionsEditController' => 'PhabricatorController', 'PhabricatorSubscriptionsEditor' => 'PhabricatorEditor', + 'PhabricatorSubscriptionsHeraldAction' => 'HeraldAction', 'PhabricatorSubscriptionsListController' => 'PhabricatorController', + 'PhabricatorSubscriptionsRemoveSelfHeraldAction' => 'PhabricatorSubscriptionsHeraldAction', + 'PhabricatorSubscriptionsRemoveSubscribersHeraldAction' => 'PhabricatorSubscriptionsHeraldAction', 'PhabricatorSubscriptionsSubscribeEmailCommand' => 'MetaMTAEmailTransactionCommand', 'PhabricatorSubscriptionsSubscribersPolicyRule' => 'PhabricatorPolicyRule', 'PhabricatorSubscriptionsTransactionController' => 'PhabricatorController', diff --git a/src/applications/differential/herald/HeraldDifferentialRevisionAdapter.php b/src/applications/differential/herald/HeraldDifferentialRevisionAdapter.php index 38a6e8b376..47a132338a 100644 --- a/src/applications/differential/herald/HeraldDifferentialRevisionAdapter.php +++ b/src/applications/differential/herald/HeraldDifferentialRevisionAdapter.php @@ -162,8 +162,6 @@ final class HeraldDifferentialRevisionAdapter case HeraldRuleTypeConfig::RULE_TYPE_GLOBAL: return array_merge( array( - self::ACTION_ADD_CC, - self::ACTION_REMOVE_CC, self::ACTION_EMAIL, self::ACTION_ADD_REVIEWERS, self::ACTION_ADD_BLOCKING_REVIEWERS, @@ -174,8 +172,6 @@ final class HeraldDifferentialRevisionAdapter case HeraldRuleTypeConfig::RULE_TYPE_PERSONAL: return array_merge( array( - self::ACTION_ADD_CC, - self::ACTION_REMOVE_CC, self::ACTION_EMAIL, self::ACTION_ADD_REVIEWERS, self::ACTION_ADD_BLOCKING_REVIEWERS, diff --git a/src/applications/diffusion/herald/HeraldCommitAdapter.php b/src/applications/diffusion/herald/HeraldCommitAdapter.php index 146fba2fc3..302027b539 100644 --- a/src/applications/diffusion/herald/HeraldCommitAdapter.php +++ b/src/applications/diffusion/herald/HeraldCommitAdapter.php @@ -89,8 +89,6 @@ final class HeraldCommitAdapter extends HeraldAdapter { case HeraldRuleTypeConfig::RULE_TYPE_OBJECT: return array_merge( array( - self::ACTION_ADD_CC, - self::ACTION_REMOVE_CC, self::ACTION_EMAIL, self::ACTION_AUDIT, self::ACTION_APPLY_BUILD_PLANS, @@ -99,8 +97,6 @@ final class HeraldCommitAdapter extends HeraldAdapter { case HeraldRuleTypeConfig::RULE_TYPE_PERSONAL: return array_merge( array( - self::ACTION_ADD_CC, - self::ACTION_REMOVE_CC, self::ACTION_EMAIL, self::ACTION_AUDIT, ), diff --git a/src/applications/herald/action/HeraldAction.php b/src/applications/herald/action/HeraldAction.php index bad7a3b6ba..529f3b7471 100644 --- a/src/applications/herald/action/HeraldAction.php +++ b/src/applications/herald/action/HeraldAction.php @@ -41,7 +41,7 @@ abstract class HeraldAction extends Phobject { return new HeraldEmptyFieldValue(); case self::STANDARD_PHID_LIST: $tokenizer = id(new HeraldTokenizerFieldValue()) - ->setKey($this->getHeraldFieldName()) + ->setKey($this->getHeraldActionName()) ->setDatasource($this->getDatasource()); $value_map = $this->getDatasourceValueMap(); @@ -56,6 +56,17 @@ abstract class HeraldAction extends Phobject { } public function willSaveActionValue($value) { + try { + $type = $this->getHeraldActionStandardType(); + } catch (PhutilMethodNotImplementedException $ex) { + return $value; + } + + switch ($type) { + case self::STANDARD_PHID_LIST: + return array_keys($value); + } + return $value; } @@ -162,4 +173,15 @@ abstract class HeraldAction extends Phobject { return idx($map, 'name'); } + protected function renderHandleList($phids) { + if (!is_array($phids)) { + return pht('(Invalid List)'); + } + + return $this->getViewer() + ->renderHandleList($phids) + ->setAsInline(true) + ->render(); + } + } diff --git a/src/applications/herald/adapter/HeraldAdapter.php b/src/applications/herald/adapter/HeraldAdapter.php index 0c630876c6..4e7e49ea66 100644 --- a/src/applications/herald/adapter/HeraldAdapter.php +++ b/src/applications/herald/adapter/HeraldAdapter.php @@ -26,8 +26,6 @@ abstract class HeraldAdapter extends Phobject { const CONDITION_IS_TRUE = 'true'; const CONDITION_IS_FALSE = 'false'; - const ACTION_ADD_CC = 'addcc'; - const ACTION_REMOVE_CC = 'remcc'; const ACTION_EMAIL = 'email'; const ACTION_AUDIT = 'audit'; const ACTION_ASSIGN_TASK = 'assigntask'; @@ -46,9 +44,9 @@ abstract class HeraldAdapter extends Phobject { private $queuedTransactions = array(); private $emailPHIDs = array(); private $forcedEmailPHIDs = array(); - private $unsubscribedPHIDs; private $fieldMap; private $actionMap; + private $edgeCache = array(); public function getEmailPHIDs() { return array_values($this->emailPHIDs); @@ -179,7 +177,7 @@ abstract class HeraldAdapter extends Phobject { return $this->queuedTransactions; } - protected function newTransaction() { + public function newTransaction() { $object = $this->newObject(); if (!($object instanceof PhabricatorApplicationTransactionInterface)) { @@ -723,8 +721,6 @@ abstract class HeraldAdapter extends Phobject { case HeraldRuleTypeConfig::RULE_TYPE_GLOBAL: case HeraldRuleTypeConfig::RULE_TYPE_OBJECT: $standard = array( - self::ACTION_ADD_CC => pht('Add Subscribers'), - self::ACTION_REMOVE_CC => pht('Remove Subscribers'), self::ACTION_EMAIL => pht('Send an email to'), self::ACTION_AUDIT => pht('Trigger an Audit by'), self::ACTION_ASSIGN_TASK => pht('Assign task to'), @@ -739,8 +735,6 @@ abstract class HeraldAdapter extends Phobject { break; case HeraldRuleTypeConfig::RULE_TYPE_PERSONAL: $standard = array( - 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'), self::ACTION_AUDIT => pht('Trigger an Audit by me'), self::ACTION_ASSIGN_TASK => pht('Assign task to me'), @@ -786,8 +780,6 @@ abstract class HeraldAdapter extends Phobject { if ($rule_type == HeraldRuleTypeConfig::RULE_TYPE_PERSONAL) { switch ($action->getAction()) { case self::ACTION_EMAIL: - case self::ACTION_ADD_CC: - case self::ACTION_REMOVE_CC: case self::ACTION_AUDIT: case self::ACTION_ASSIGN_TASK: case self::ACTION_ADD_REVIEWERS: @@ -828,8 +820,6 @@ abstract class HeraldAdapter extends Phobject { if ($is_personal) { switch ($action) { - case self::ACTION_ADD_CC: - case self::ACTION_REMOVE_CC: case self::ACTION_EMAIL: case self::ACTION_AUDIT: case self::ACTION_ASSIGN_TASK: @@ -843,8 +833,6 @@ abstract class HeraldAdapter extends Phobject { } } else { switch ($action) { - case self::ACTION_ADD_CC: - case self::ACTION_REMOVE_CC: case self::ACTION_EMAIL: return $this->buildTokenizerFieldValue( new PhabricatorMetaMTAMailableDatasource()); @@ -1048,17 +1036,24 @@ abstract class HeraldAdapter extends Phobject { PhabricatorUser $viewer) { $field_type = $condition->getFieldName(); + $field = $this->getFieldImplementation($field_type); - $default = pht('(Unknown Field "%s")', $field_type); + if (!$field) { + return pht('Unknown Field: "%s"', $field_type); + } - $field_name = idx($this->getFieldNameMap(), $field_type, $default); + $field_name = $field->getHeraldFieldName(); $condition_type = $condition->getFieldCondition(); $condition_name = idx($this->getConditionNameMap(), $condition_type); $value = $this->renderConditionValueAsText($condition, $handles, $viewer); - return hsprintf(' %s %s %s', $field_name, $condition_name, $value); + return array( + $field_name, + $condition_name, + $value, + ); } private function renderActionAsText( @@ -1068,8 +1063,10 @@ abstract class HeraldAdapter extends Phobject { $impl = $this->getActionImplementation($action->getAction()); if ($impl) { + $impl->setViewer($viewer); + $value = $action->getTarget(); - return $impl->renderActionDescription($viewer, $value); + return $impl->renderActionDescription($value); } $rule_global = HeraldRuleTypeConfig::RULE_TYPE_GLOBAL; @@ -1180,14 +1177,16 @@ abstract class HeraldAdapter extends Phobject { */ protected function applyStandardEffect(HeraldEffect $effect) { $action = $effect->getAction(); + $rule_type = $effect->getRule()->getRuleType(); $impl = $this->getActionImplementation($action); if ($impl) { - $impl->applyEffect($this->getObject(), $effect); - return $impl->getApplyTranscript($effect); + if ($impl->supportsRuleType($rule_type)) { + $impl->applyEffect($this->getObject(), $effect); + return $impl->getApplyTranscript($effect); + } } - $rule_type = $effect->getRule()->getRuleType(); $supported = $this->getActions($rule_type); $supported = array_fuse($supported); if (empty($supported[$action])) { @@ -1205,9 +1204,6 @@ abstract class HeraldAdapter extends Phobject { 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_EMAIL: return $this->applyEmailEffect($effect); default: @@ -1257,96 +1253,6 @@ abstract class HeraldAdapter extends Phobject { 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 @@ -1370,5 +1276,15 @@ abstract class HeraldAdapter extends Phobject { pht('Added mailable to mail targets.')); } + public function loadEdgePHIDs($type) { + if (!isset($this->edgeCache[$type])) { + $phids = PhabricatorEdgeQuery::loadDestinationPHIDs( + $this->getObject()->getPHID(), + $type); + + $this->edgeCache[$type] = array_fuse($phids); + } + return $this->edgeCache[$type]; + } } diff --git a/src/applications/maniphest/herald/HeraldManiphestTaskAdapter.php b/src/applications/maniphest/herald/HeraldManiphestTaskAdapter.php index 7aad56ee24..0ef1399e54 100644 --- a/src/applications/maniphest/herald/HeraldManiphestTaskAdapter.php +++ b/src/applications/maniphest/herald/HeraldManiphestTaskAdapter.php @@ -72,8 +72,6 @@ final class HeraldManiphestTaskAdapter extends HeraldAdapter { case HeraldRuleTypeConfig::RULE_TYPE_GLOBAL: return array_merge( array( - self::ACTION_ADD_CC, - self::ACTION_REMOVE_CC, self::ACTION_EMAIL, self::ACTION_ASSIGN_TASK, ), @@ -81,8 +79,6 @@ final class HeraldManiphestTaskAdapter extends HeraldAdapter { case HeraldRuleTypeConfig::RULE_TYPE_PERSONAL: return array_merge( array( - self::ACTION_ADD_CC, - self::ACTION_REMOVE_CC, self::ACTION_EMAIL, self::ACTION_ASSIGN_TASK, ), diff --git a/src/applications/pholio/herald/HeraldPholioMockAdapter.php b/src/applications/pholio/herald/HeraldPholioMockAdapter.php index 330f4294ad..57fa74c3a1 100644 --- a/src/applications/pholio/herald/HeraldPholioMockAdapter.php +++ b/src/applications/pholio/herald/HeraldPholioMockAdapter.php @@ -47,25 +47,6 @@ final class HeraldPholioMockAdapter extends HeraldAdapter { } } - public function getActions($rule_type) { - switch ($rule_type) { - case HeraldRuleTypeConfig::RULE_TYPE_GLOBAL: - return array_merge( - array( - self::ACTION_ADD_CC, - self::ACTION_REMOVE_CC, - ), - parent::getActions($rule_type)); - case HeraldRuleTypeConfig::RULE_TYPE_PERSONAL: - return array_merge( - array( - self::ACTION_ADD_CC, - self::ACTION_REMOVE_CC, - ), - parent::getActions($rule_type)); - } - } - public function getHeraldName() { return 'M'.$this->getMock()->getID(); } diff --git a/src/applications/phriction/herald/PhrictionDocumentHeraldAdapter.php b/src/applications/phriction/herald/PhrictionDocumentHeraldAdapter.php index 05a09fb877..17128a5773 100644 --- a/src/applications/phriction/herald/PhrictionDocumentHeraldAdapter.php +++ b/src/applications/phriction/herald/PhrictionDocumentHeraldAdapter.php @@ -53,16 +53,12 @@ final class PhrictionDocumentHeraldAdapter extends HeraldAdapter { case HeraldRuleTypeConfig::RULE_TYPE_GLOBAL: return array_merge( array( - self::ACTION_ADD_CC, - self::ACTION_REMOVE_CC, self::ACTION_EMAIL, ), parent::getActions($rule_type)); case HeraldRuleTypeConfig::RULE_TYPE_PERSONAL: return array_merge( array( - self::ACTION_ADD_CC, - self::ACTION_REMOVE_CC, self::ACTION_EMAIL, ), parent::getActions($rule_type)); diff --git a/src/applications/subscriptions/herald/PhabricatorSubscriptionsAddSelfHeraldAction.php b/src/applications/subscriptions/herald/PhabricatorSubscriptionsAddSelfHeraldAction.php new file mode 100644 index 0000000000..44199422b0 --- /dev/null +++ b/src/applications/subscriptions/herald/PhabricatorSubscriptionsAddSelfHeraldAction.php @@ -0,0 +1,37 @@ +getRule()->getAuthorPHID(); + return $this->applySubscribe(array($phid), $is_add = true); + } + + public function getHeraldActionStandardType() { + return self::STANDARD_NONE; + } + + public function renderActionDescription($value) { + return pht('Add rule author as subscriber.'); + } + +} diff --git a/src/applications/subscriptions/herald/PhabricatorSubscriptionsAddSubscribersHeraldAction.php b/src/applications/subscriptions/herald/PhabricatorSubscriptionsAddSubscribersHeraldAction.php new file mode 100644 index 0000000000..7fbec15e9b --- /dev/null +++ b/src/applications/subscriptions/herald/PhabricatorSubscriptionsAddSubscribersHeraldAction.php @@ -0,0 +1,40 @@ +applySubscribe($effect->getTarget(), $is_add = true); + } + + public function getHeraldActionStandardType() { + return self::STANDARD_PHID_LIST; + } + + protected function getDatasource() { + return new PhabricatorMetaMTAMailableDatasource(); + } + + public function renderActionDescription($value) { + return pht('Add subscribers: %s.', $this->renderHandleList($value)); + } + +} diff --git a/src/applications/subscriptions/herald/PhabricatorSubscriptionsHeraldAction.php b/src/applications/subscriptions/herald/PhabricatorSubscriptionsHeraldAction.php new file mode 100644 index 0000000000..66e0d62959 --- /dev/null +++ b/src/applications/subscriptions/herald/PhabricatorSubscriptionsHeraldAction.php @@ -0,0 +1,235 @@ +getAdapter(); + + if ($is_add) { + $kind = '+'; + } else { + $kind = '-'; + } + + $subscriber_phids = array_fuse($phids); + if (!$subscriber_phids) { + $this->logEffect(self::DO_NO_TARGETS); + return; + } + + // The "Add Subscribers" rule only adds subscribers who haven't previously + // unsubscribed from the object explicitly. Filter these subscribers out + // before continuing. + if ($is_add) { + $unsubscribed = $adapter->loadEdgePHIDs( + PhabricatorObjectHasUnsubscriberEdgeType::EDGECONST); + + foreach ($unsubscribed as $phid) { + if (isset($subscriber_phids[$phid])) { + $unsubscribed[$phid] = $phid; + unset($subscriber_phids[$phid]); + } + } + + if ($unsubscribed) { + $this->logEffect( + self::DO_PREVIOUSLY_UNSUBSCRIBED, + array_values($unsubscribed)); + } + } + + if (!$subscriber_phids) { + return; + } + + // 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 ($invalid) { + $this->logEffect(self::DO_INVALID, array_values($invalid)); + } + + if (!$subscriber_phids) { + return; + } + + $auto = array(); + $object = $adapter->getObject(); + foreach ($subscriber_phids as $phid) { + if ($object->isAutomaticallySubscribed($phid)) { + $auto[$phid] = $phid; + unset($subscriber_phids[$phid]); + } + } + + if ($auto) { + $this->logEffect(self::DO_AUTOSUBSCRIBED, array_values($auto)); + } + + if (!$subscriber_phids) { + return; + } + + $current = $adapter->loadEdgePHIDs( + PhabricatorObjectHasSubscriberEdgeType::EDGECONST); + + if ($is_add) { + $already = array(); + foreach ($subscriber_phids as $phid) { + if (isset($current[$phid])) { + $already[$phid] = $phid; + unset($subscriber_phids[$phid]); + } + } + + if ($already) { + $this->logEffect(self::DO_ALREADY_SUBSCRIBED, $already); + } + } else { + $already = array(); + foreach ($subscriber_phids as $phid) { + if (empty($current[$phid])) { + $already[$phid] = $phid; + unset($subscriber_phids[$phid]); + } + } + + if ($already) { + $this->logEffect(self::DO_ALREADY_UNSUBSCRIBED, $already); + } + } + + if (!$subscriber_phids) { + return; + } + + $xaction = $adapter->newTransaction() + ->setTransactionType(PhabricatorTransactions::TYPE_SUBSCRIBERS) + ->setNewValue( + array( + $kind => $subscriber_phids, + )); + + $adapter->queueTransaction($xaction); + + if ($is_add) { + $this->logEffect(self::DO_SUBSCRIBED, $subscriber_phids); + } else { + $this->logEffect(self::DO_UNSUBSCRIBED, $subscriber_phids); + } + } + + protected function getActionEffectMap() { + return array( + self::DO_NO_TARGETS => array( + 'icon' => 'fa-ban', + 'color' => 'grey', + 'name' => pht('No Targets'), + ), + self::DO_PREVIOUSLY_UNSUBSCRIBED => array( + 'icon' => 'fa-minus-circle', + 'color' => 'grey', + 'name' => pht('Previously Unsubscribed'), + ), + self::DO_AUTOSUBSCRIBED => array( + 'icon' => 'fa-envelope', + 'color' => 'grey', + 'name' => pht('Automatically Subscribed'), + ), + self::DO_INVALID => array( + 'icon' => 'fa-ban', + 'color' => 'red', + 'name' => pht('Invalid Targets'), + ), + self::DO_ALREADY_SUBSCRIBED => array( + 'icon' => 'fa-chevron-right', + 'color' => 'grey', + 'name' => pht('Already Subscribed'), + ), + self::DO_ALREADY_UNSUBSCRIBED => array( + 'icon' => 'fa-chevron-right', + 'color' => 'grey', + 'name' => pht('Already Unsubscribed'), + ), + self::DO_SUBSCRIBED => array( + 'icon' => 'fa-envelope', + 'color' => 'green', + 'name' => pht('Added Subscribers'), + ), + self::DO_UNSUBSCRIBED => array( + 'icon' => 'fa-minus-circle', + 'color' => 'green', + 'name' => pht('Removed Subscribers'), + ), + ); + } + + public function renderActionEffectDescription($type, $data) { + switch ($type) { + case self::DO_NO_TARGETS: + return pht('Rule lists no targets.'); + case self::DO_PREVIOUSLY_UNSUBSCRIBED: + return pht( + 'Declined to resubscribe %s target(s) because they previously '. + 'unsubscribed: %s.', + new PhutilNumber(count($data)), + $this->renderHandleList($data)); + case self::DO_INVALID: + return pht( + 'Declined to act on %s invalid target(s): %s.', + new PhutilNumber(count($data)), + $this->renderHandleList($data)); + case self::DO_AUTOSUBSCRIBED: + return pht( + '%s automatically subscribed target(s) were not affected: %s.', + new PhutilNumber(count($data)), + $this->renderHandleList($data)); + case self::DO_ALREADY_SUBSCRIBED: + return pht( + '%s target(s) are already subscribed: %s.', + new PhutilNumber(count($data)), + $this->renderHandleList($data)); + case self::DO_ALREADY_UNSUBSCRIBED: + return pht( + '%s target(s) are not subscribed: %s.', + new PhutilNumber(count($data)), + $this->renderHandleList($data)); + case self::DO_SUBSCRIBED: + return pht( + 'Added %s subscriber(s): %s.', + new PhutilNumber(count($data)), + $this->renderHandleList($data)); + case self::DO_UNSUBSCRIBED: + return pht( + 'Removed %s subscriber(s): %s.', + new PhutilNumber(count($data)), + $this->renderHandleList($data)); + } + } + + +} diff --git a/src/applications/subscriptions/herald/PhabricatorSubscriptionsRemoveSelfHeraldAction.php b/src/applications/subscriptions/herald/PhabricatorSubscriptionsRemoveSelfHeraldAction.php new file mode 100644 index 0000000000..324a04c449 --- /dev/null +++ b/src/applications/subscriptions/herald/PhabricatorSubscriptionsRemoveSelfHeraldAction.php @@ -0,0 +1,37 @@ +getRule()->getAuthorPHID(); + return $this->applySubscribe(array($phid), $is_add = false); + } + + public function getHeraldActionStandardType() { + return self::STANDARD_NONE; + } + + public function renderActionDescription($value) { + return pht('Remove rule author as subscriber.'); + } + +} diff --git a/src/applications/subscriptions/herald/PhabricatorSubscriptionsRemoveSubscribersHeraldAction.php b/src/applications/subscriptions/herald/PhabricatorSubscriptionsRemoveSubscribersHeraldAction.php new file mode 100644 index 0000000000..71f433811e --- /dev/null +++ b/src/applications/subscriptions/herald/PhabricatorSubscriptionsRemoveSubscribersHeraldAction.php @@ -0,0 +1,40 @@ +applySubscribe($effect->getTarget(), $is_add = false); + } + + public function getHeraldActionStandardType() { + return self::STANDARD_PHID_LIST; + } + + protected function getDatasource() { + return new PhabricatorMetaMTAMailableDatasource(); + } + + public function renderActionDescription($value) { + return pht('Remove subscribers: %s.', $this->renderHandleList($value)); + } + +} diff --git a/src/infrastructure/internationalization/translation/PhabricatorUSEnglishTranslation.php b/src/infrastructure/internationalization/translation/PhabricatorUSEnglishTranslation.php index d0466fbb7d..c52137bdf2 100644 --- a/src/infrastructure/internationalization/translation/PhabricatorUSEnglishTranslation.php +++ b/src/infrastructure/internationalization/translation/PhabricatorUSEnglishTranslation.php @@ -1182,6 +1182,7 @@ final class PhabricatorUSEnglishTranslation '%s Task', '%s Tasks', ), + '%s added %s badge(s) for %s: %s.' => array( array( '%s added a badge for %s: %3$s.', @@ -1256,6 +1257,39 @@ final class PhabricatorUSEnglishTranslation ), ), + '%s automatically subscribed target(s) were not affected: %s.' => array( + 'An automatically subscribed target was not affected: %2$s.', + 'Automatically subscribed targets were not affected: %2$s.', + ), + + 'Declined to resubscribe %s target(s) because they previously '. + 'unsubscribed: %s.' => array( + 'Delined to resubscribe a target because they previously '. + 'unsubscribed: %2$s.', + 'Declined to resubscribe targets because they previously '. + 'unsubscribed: %2$s.', + ), + + '%s target(s) are not subscribed: %s.' => array( + 'A target is not subscribed: %2$s.', + 'Targets are not subscribed: %2$s.', + ), + + '%s target(s) are already subscribed: %s.' => array( + 'A target is already subscribed: %2$s.', + 'Targets are already subscribed: %2$s.', + ), + + 'Added %s subscriber(s): %s.' => array( + 'Added a subscriber: %2$s.', + 'Added subscribers: %2$s.', + ), + + 'Removed %s subscriber(s): %s.' => array( + 'Removed a subscriber: %2$s.', + 'Removed subscribers: %2$s.', + ), + ); }