diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 4a0e093071..112fac5b7a 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -595,7 +595,6 @@ phutil_register_library_map(array( 'HarbormasterRunnerWorker' => 'applications/harbormaster/worker/HarbormasterRunnerWorker.php', 'HarbormasterScratchTable' => 'applications/harbormaster/storage/HarbormasterScratchTable.php', 'HeraldAction' => 'applications/herald/storage/HeraldAction.php', - 'HeraldActionConfig' => 'applications/herald/config/HeraldActionConfig.php', 'HeraldAdapter' => 'applications/herald/adapter/HeraldAdapter.php', 'HeraldApplyTranscript' => 'applications/herald/storage/transcript/HeraldApplyTranscript.php', 'HeraldCommitAdapter' => 'applications/herald/adapter/HeraldCommitAdapter.php', @@ -609,13 +608,13 @@ phutil_register_library_map(array( 'HeraldEditLogQuery' => 'applications/herald/query/HeraldEditLogQuery.php', 'HeraldEffect' => 'applications/herald/engine/HeraldEffect.php', 'HeraldEngine' => 'applications/herald/engine/HeraldEngine.php', - 'HeraldFieldConfig' => 'applications/herald/config/HeraldFieldConfig.php', - 'HeraldInvalidConditionException' => 'applications/herald/engine/engine/HeraldInvalidConditionException.php', - 'HeraldInvalidFieldException' => 'applications/herald/engine/engine/HeraldInvalidFieldException.php', + 'HeraldInvalidActionException' => 'applications/herald/engine/exception/HeraldInvalidActionException.php', + 'HeraldInvalidConditionException' => 'applications/herald/engine/exception/HeraldInvalidConditionException.php', + 'HeraldInvalidFieldException' => 'applications/herald/engine/exception/HeraldInvalidFieldException.php', 'HeraldNewController' => 'applications/herald/controller/HeraldNewController.php', 'HeraldObjectTranscript' => 'applications/herald/storage/transcript/HeraldObjectTranscript.php', 'HeraldPHIDTypeRule' => 'applications/herald/phid/HeraldPHIDTypeRule.php', - 'HeraldRecursiveConditionsException' => 'applications/herald/engine/engine/HeraldRecursiveConditionsException.php', + 'HeraldRecursiveConditionsException' => 'applications/herald/engine/exception/HeraldRecursiveConditionsException.php', 'HeraldRepetitionPolicyConfig' => 'applications/herald/config/HeraldRepetitionPolicyConfig.php', 'HeraldRule' => 'applications/herald/storage/HeraldRule.php', 'HeraldRuleController' => 'applications/herald/controller/HeraldRuleController.php', @@ -2617,6 +2616,7 @@ phutil_register_library_map(array( 'HeraldDifferentialRevisionAdapter' => 'HeraldAdapter', 'HeraldDryRunAdapter' => 'HeraldAdapter', 'HeraldEditLogQuery' => 'PhabricatorOffsetPagedQuery', + 'HeraldInvalidActionException' => 'Exception', 'HeraldInvalidConditionException' => 'Exception', 'HeraldInvalidFieldException' => 'Exception', 'HeraldNewController' => 'HeraldController', diff --git a/src/applications/herald/adapter/HeraldAdapter.php b/src/applications/herald/adapter/HeraldAdapter.php index 17e1e424bd..8977af13cd 100644 --- a/src/applications/herald/adapter/HeraldAdapter.php +++ b/src/applications/herald/adapter/HeraldAdapter.php @@ -435,6 +435,48 @@ abstract class HeraldAdapter { } } + public function willSaveAction( + HeraldRule $rule, + HeraldAction $action) { + + $target = $action->getTarget(); + if (is_array($target)) { + $target = array_keys($target); + } + + $author_phid = $rule->getAuthorPHID(); + + $rule_type = $rule->getRuleType(); + 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: + // For personal rules, force these actions to target the rule owner. + $target = array($author_phid); + break; + case self::ACTION_FLAG: + // Make sure flag color is valid; set to blue if not. + $color_map = PhabricatorFlagColor::getColorNameMap(); + if (empty($color_map[$target])) { + $target = PhabricatorFlagColor::COLOR_BLUE; + } + break; + case self::ACTION_NOTHING: + break; + default: + throw new HeraldInvalidActionException( + pht( + 'Unrecognized action type "%s"!', + $action->getAction())); + } + } + + $action->setTarget($target); + } + + /* -( Values )------------------------------------------------------------- */ diff --git a/src/applications/herald/adapter/HeraldCommitAdapter.php b/src/applications/herald/adapter/HeraldCommitAdapter.php index 608a1f3cb6..639410d753 100644 --- a/src/applications/herald/adapter/HeraldCommitAdapter.php +++ b/src/applications/herald/adapter/HeraldCommitAdapter.php @@ -299,13 +299,13 @@ final class HeraldCommitAdapter extends HeraldAdapter { foreach ($effects as $effect) { $action = $effect->getAction(); switch ($action) { - case HeraldActionConfig::ACTION_NOTHING: + case self::ACTION_NOTHING: $result[] = new HeraldApplyTranscript( $effect, true, pht('Great success at doing nothing.')); break; - case HeraldActionConfig::ACTION_EMAIL: + case self::ACTION_EMAIL: foreach ($effect->getTarget() as $phid) { $this->emailPHIDs[$phid] = true; } @@ -314,7 +314,7 @@ final class HeraldCommitAdapter extends HeraldAdapter { true, pht('Added address to email targets.')); break; - case HeraldActionConfig::ACTION_AUDIT: + case self::ACTION_AUDIT: foreach ($effect->getTarget() as $phid) { if (empty($this->auditMap[$phid])) { $this->auditMap[$phid] = array(); @@ -326,7 +326,7 @@ final class HeraldCommitAdapter extends HeraldAdapter { true, pht('Triggered an audit.')); break; - case HeraldActionConfig::ACTION_FLAG: + case self::ACTION_FLAG: $result[] = parent::applyFlagEffect( $effect, $this->commit->getPHID()); diff --git a/src/applications/herald/adapter/HeraldDifferentialRevisionAdapter.php b/src/applications/herald/adapter/HeraldDifferentialRevisionAdapter.php index 63f14ab727..f59271db0f 100644 --- a/src/applications/herald/adapter/HeraldDifferentialRevisionAdapter.php +++ b/src/applications/herald/adapter/HeraldDifferentialRevisionAdapter.php @@ -269,7 +269,7 @@ final class HeraldDifferentialRevisionAdapter extends HeraldAdapter { $result = array(); if ($this->explicitCCs) { $effect = new HeraldEffect(); - $effect->setAction(HeraldActionConfig::ACTION_ADD_CC); + $effect->setAction(self::ACTION_ADD_CC); $effect->setTarget(array_keys($this->explicitCCs)); $effect->setReason( pht('CCs provided explicitly by revision author or carried over '. @@ -287,20 +287,20 @@ final class HeraldDifferentialRevisionAdapter extends HeraldAdapter { foreach ($effects as $effect) { $action = $effect->getAction(); switch ($action) { - case HeraldActionConfig::ACTION_NOTHING: + case self::ACTION_NOTHING: $result[] = new HeraldApplyTranscript( $effect, true, pht('OK, did nothing.')); break; - case HeraldActionConfig::ACTION_FLAG: + case self::ACTION_FLAG: $result[] = parent::applyFlagEffect( $effect, $this->revision->getPHID()); break; - case HeraldActionConfig::ACTION_EMAIL: - case HeraldActionConfig::ACTION_ADD_CC: - $op = ($action == HeraldActionConfig::ACTION_EMAIL) ? 'email' : 'CC'; + case self::ACTION_EMAIL: + case self::ACTION_ADD_CC: + $op = ($action == self::ACTION_EMAIL) ? 'email' : 'CC'; $base_target = $effect->getTarget(); $forbidden = array(); foreach ($base_target as $key => $fbid) { @@ -308,7 +308,7 @@ final class HeraldDifferentialRevisionAdapter extends HeraldAdapter { $forbidden[] = $fbid; unset($base_target[$key]); } else { - if ($action == HeraldActionConfig::ACTION_EMAIL) { + if ($action == self::ACTION_EMAIL) { $this->emailPHIDs[$fbid] = true; } else { $this->newCCs[$fbid] = true; @@ -338,7 +338,7 @@ final class HeraldDifferentialRevisionAdapter extends HeraldAdapter { pht('Added addresses to %s list.', $op)); } break; - case HeraldActionConfig::ACTION_REMOVE_CC: + case self::ACTION_REMOVE_CC: foreach ($effect->getTarget() as $fbid) { $this->remCCs[$fbid] = true; } diff --git a/src/applications/herald/config/HeraldActionConfig.php b/src/applications/herald/config/HeraldActionConfig.php deleted file mode 100644 index f7bb0b0c39..0000000000 --- a/src/applications/herald/config/HeraldActionConfig.php +++ /dev/null @@ -1,60 +0,0 @@ - - * 1 => array() - * ) - */ - public static function willSaveAction($rule_type, - $author_phid, - $data) { - $obj = new HeraldAction(); - $obj->setAction($data[0]); - - // for personal rule types, set the target to be the owner of the rule - if ($rule_type == HeraldRuleTypeConfig::RULE_TYPE_PERSONAL) { - switch ($obj->getAction()) { - case HeraldActionConfig::ACTION_EMAIL: - case HeraldActionConfig::ACTION_ADD_CC: - case HeraldActionConfig::ACTION_REMOVE_CC: - case HeraldActionConfig::ACTION_AUDIT: - $data[1] = array($author_phid => $author_phid); - break; - case HeraldActionConfig::ACTION_FLAG: - // Make sure flag color is valid; set to blue if not. - $color_map = PhabricatorFlagColor::getColorNameMap(); - if (empty($color_map[$data[1]])) { - $data[1] = PhabricatorFlagColor::COLOR_BLUE; - } - break; - case HeraldActionConfig::ACTION_NOTHING: - break; - default: - throw new Exception('Unrecognized action type: ' . - $obj->getAction()); - } - } - - if (is_array($data[1])) { - $obj->setTarget(array_keys($data[1])); - } else { - $obj->setTarget($data[1]); - } - - return $obj; - } - -} diff --git a/src/applications/herald/config/HeraldFieldConfig.php b/src/applications/herald/config/HeraldFieldConfig.php deleted file mode 100644 index b4939fd762..0000000000 --- a/src/applications/herald/config/HeraldFieldConfig.php +++ /dev/null @@ -1,24 +0,0 @@ -getRuleType(), - $rule->getAuthorPHID(), - $action); + $obj = new HeraldAction(); + $obj->setAction($action[0]); + $obj->setTarget($action[1]); + + try { + $adapter->willSaveAction($rule, $obj); + } catch (HeraldInvalidActionException $ex) { + $errors[] = $ex; + } + + $actions[] = $obj; } $rule->attachConditions($conditions); @@ -328,7 +336,7 @@ final class HeraldRuleController extends HeraldController { foreach ($rule->getActions() as $action) { switch ($action->getAction()) { - case HeraldActionConfig::ACTION_FLAG: + case HeraldAdapter::ACTION_FLAG: $current_value = $action->getTarget(); break; default: diff --git a/src/applications/herald/controller/HeraldTranscriptController.php b/src/applications/herald/controller/HeraldTranscriptController.php index fafb404902..5f596c180a 100644 --- a/src/applications/herald/controller/HeraldTranscriptController.php +++ b/src/applications/herald/controller/HeraldTranscriptController.php @@ -273,10 +273,10 @@ final class HeraldTranscriptController extends HeraldController { $target = $apply_xscript->getTarget(); switch ($apply_xscript->getAction()) { - case HeraldActionConfig::ACTION_NOTHING: + case HeraldAdapter::ACTION_NOTHING: $target = ''; break; - case HeraldActionConfig::ACTION_FLAG: + case HeraldAdapter::ACTION_FLAG: $target = PhabricatorFlagColor::getColorName($target); break; default: diff --git a/src/applications/herald/engine/exception/HeraldInvalidActionException.php b/src/applications/herald/engine/exception/HeraldInvalidActionException.php new file mode 100644 index 0000000000..c7040f2aab --- /dev/null +++ b/src/applications/herald/engine/exception/HeraldInvalidActionException.php @@ -0,0 +1,3 @@ +