diff --git a/src/applications/diffusion/herald/HeraldPreCommitContentAdapter.php b/src/applications/diffusion/herald/HeraldPreCommitContentAdapter.php index 53840c8029..aeff27ad11 100644 --- a/src/applications/diffusion/herald/HeraldPreCommitContentAdapter.php +++ b/src/applications/diffusion/herald/HeraldPreCommitContentAdapter.php @@ -41,6 +41,17 @@ final class HeraldPreCommitContentAdapter extends HeraldAdapter { "Hook rules can block changes."); } + public function supportsRuleType($rule_type) { + switch ($rule_type) { + case HeraldRuleTypeConfig::RULE_TYPE_GLOBAL: + return true; + case HeraldRuleTypeConfig::RULE_TYPE_PERSONAL: + case HeraldRuleTypeConfig::RULE_TYPE_OBJECT: + default: + return false; + } + } + public function getFieldNameMap() { return array( ) + parent::getFieldNameMap(); diff --git a/src/applications/diffusion/herald/HeraldPreCommitRefAdapter.php b/src/applications/diffusion/herald/HeraldPreCommitRefAdapter.php index 1395ab9eae..90f6748571 100644 --- a/src/applications/diffusion/herald/HeraldPreCommitRefAdapter.php +++ b/src/applications/diffusion/herald/HeraldPreCommitRefAdapter.php @@ -44,6 +44,17 @@ final class HeraldPreCommitRefAdapter extends HeraldAdapter { "Hook rules can block changes."); } + public function supportsRuleType($rule_type) { + switch ($rule_type) { + case HeraldRuleTypeConfig::RULE_TYPE_GLOBAL: + return true; + case HeraldRuleTypeConfig::RULE_TYPE_PERSONAL: + case HeraldRuleTypeConfig::RULE_TYPE_OBJECT: + default: + return false; + } + } + public function getFieldNameMap() { return array( self::FIELD_REF_TYPE => pht('Ref type'), diff --git a/src/applications/herald/adapter/HeraldAdapter.php b/src/applications/herald/adapter/HeraldAdapter.php index e38235fafd..5735321b41 100644 --- a/src/applications/herald/adapter/HeraldAdapter.php +++ b/src/applications/herald/adapter/HeraldAdapter.php @@ -140,6 +140,10 @@ abstract class HeraldAdapter { abstract public function getAdapterApplicationClass(); abstract public function getObject(); + public function supportsRuleType($rule_type) { + return false; + } + public function getAdapterSortKey() { return sprintf( '%08d%s', diff --git a/src/applications/herald/adapter/HeraldCommitAdapter.php b/src/applications/herald/adapter/HeraldCommitAdapter.php index cd6cebd2b0..5206860bae 100644 --- a/src/applications/herald/adapter/HeraldCommitAdapter.php +++ b/src/applications/herald/adapter/HeraldCommitAdapter.php @@ -49,6 +49,17 @@ final class HeraldCommitAdapter extends HeraldAdapter { "and run build plans."); } + public function supportsRuleType($rule_type) { + switch ($rule_type) { + case HeraldRuleTypeConfig::RULE_TYPE_GLOBAL: + case HeraldRuleTypeConfig::RULE_TYPE_PERSONAL: + return true; + case HeraldRuleTypeConfig::RULE_TYPE_OBJECT: + default: + return false; + } + } + public function getFieldNameMap() { return array( self::FIELD_NEED_AUDIT_FOR_PACKAGE => diff --git a/src/applications/herald/adapter/HeraldDifferentialRevisionAdapter.php b/src/applications/herald/adapter/HeraldDifferentialRevisionAdapter.php index 976154634d..a17d6b2e6d 100644 --- a/src/applications/herald/adapter/HeraldDifferentialRevisionAdapter.php +++ b/src/applications/herald/adapter/HeraldDifferentialRevisionAdapter.php @@ -43,6 +43,17 @@ final class HeraldDifferentialRevisionAdapter extends HeraldAdapter { "and run build plans."); } + public function supportsRuleType($rule_type) { + switch ($rule_type) { + case HeraldRuleTypeConfig::RULE_TYPE_GLOBAL: + case HeraldRuleTypeConfig::RULE_TYPE_PERSONAL: + return true; + case HeraldRuleTypeConfig::RULE_TYPE_OBJECT: + default: + return false; + } + } + public function getFields() { return array_merge( array( diff --git a/src/applications/herald/adapter/HeraldManiphestTaskAdapter.php b/src/applications/herald/adapter/HeraldManiphestTaskAdapter.php index 6fec583819..a4e5f8bfc0 100644 --- a/src/applications/herald/adapter/HeraldManiphestTaskAdapter.php +++ b/src/applications/herald/adapter/HeraldManiphestTaskAdapter.php @@ -19,6 +19,17 @@ final class HeraldManiphestTaskAdapter extends HeraldAdapter { 'React to tasks being created or updated.'); } + public function supportsRuleType($rule_type) { + switch ($rule_type) { + case HeraldRuleTypeConfig::RULE_TYPE_GLOBAL: + case HeraldRuleTypeConfig::RULE_TYPE_PERSONAL: + return true; + case HeraldRuleTypeConfig::RULE_TYPE_OBJECT: + default: + return false; + } + } + public function setTask(ManiphestTask $task) { $this->task = $task; return $this; diff --git a/src/applications/herald/adapter/HeraldPholioMockAdapter.php b/src/applications/herald/adapter/HeraldPholioMockAdapter.php index a853120306..5a6404d182 100644 --- a/src/applications/herald/adapter/HeraldPholioMockAdapter.php +++ b/src/applications/herald/adapter/HeraldPholioMockAdapter.php @@ -41,6 +41,17 @@ final class HeraldPholioMockAdapter extends HeraldAdapter { return pht('Pholio Mocks'); } + public function supportsRuleType($rule_type) { + switch ($rule_type) { + case HeraldRuleTypeConfig::RULE_TYPE_GLOBAL: + case HeraldRuleTypeConfig::RULE_TYPE_PERSONAL: + return true; + case HeraldRuleTypeConfig::RULE_TYPE_OBJECT: + default: + return false; + } + } + public function getFields() { return array_merge( array( diff --git a/src/applications/herald/controller/HeraldNewController.php b/src/applications/herald/controller/HeraldNewController.php index 41ed03cf4b..610c74d8ca 100644 --- a/src/applications/herald/controller/HeraldNewController.php +++ b/src/applications/herald/controller/HeraldNewController.php @@ -53,17 +53,13 @@ final class HeraldNewController extends HeraldController { ->setUser($user) ->setAction($this->getApplicationURI('new/')); - $content_types = $this->renderContentTypeControl( - $content_type_map, - $e_type); - - $rule_types = $this->renderRuleTypeControl( - $rule_type_map, - $e_rule); - switch ($step) { case 0: default: + $content_types = $this->renderContentTypeControl( + $content_type_map, + $e_type); + $form ->addHiddenInput('step', 1) ->appendChild($content_types); @@ -72,6 +68,10 @@ final class HeraldNewController extends HeraldController { $cancel_uri = $this->getApplicationURI(); break; case 1: + $rule_types = $this->renderRuleTypeControl( + $rule_type_map, + $e_rule); + $form ->addHiddenInput('content_type', $request->getStr('content_type')) ->addHiddenInput('step', 2) @@ -185,14 +185,31 @@ final class HeraldNewController extends HeraldController { ->setValue($request->getStr('rule_type')) ->setError($e_rule); + $adapter = HeraldAdapter::getAdapterForContentType( + $request->getStr('content_type')); + foreach ($rule_type_map as $value => $name) { + $caption = idx($captions, $value); $disabled = ($value == HeraldRuleTypeConfig::RULE_TYPE_GLOBAL) && (!$can_global); + if (!$adapter->supportsRuleType($value)) { + $disabled = true; + $caption = array( + $caption, + "\n\n", + phutil_tag( + 'em', + array(), + pht( + 'This rule type is not supported by the selected content type.')), + ); + } + $radio->addButton( $value, $name, - idx($captions, $value), + phutil_escape_html_newlines($caption), $disabled ? 'disabled' : null, $disabled); } diff --git a/src/applications/herald/controller/HeraldRuleController.php b/src/applications/herald/controller/HeraldRuleController.php index d4e48c95aa..e3ddabae0e 100644 --- a/src/applications/herald/controller/HeraldRuleController.php +++ b/src/applications/herald/controller/HeraldRuleController.php @@ -59,9 +59,16 @@ final class HeraldRuleController extends HeraldController { $local_version = id(new HeraldRule())->getConfigVersion(); if ($rule->getConfigVersion() > $local_version) { throw new Exception( - "This rule was created with a newer version of Herald. You can not ". - "view or edit it in this older version. Upgrade your Phabricator ". - "deployment."); + pht( + "This rule was created with a newer version of Herald. You can not ". + "view or edit it in this older version. Upgrade your Phabricator ". + "deployment.")); + } + + if (!$adapter->supportsRuleType($rule->getRuleType())) { + throw new Exception( + pht( + "This rule's content type does not support the selected rule type.")); } // Upgrade rule version to our version, since we might add newly-defined