diff --git a/resources/celerity/map.php b/resources/celerity/map.php --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -382,7 +382,7 @@ 'rsrc/js/application/doorkeeper/behavior-doorkeeper-tag.js' => 'e5822781', 'rsrc/js/application/files/behavior-icon-composer.js' => '8ef9ab58', 'rsrc/js/application/files/behavior-launch-icon-composer.js' => '48086888', - 'rsrc/js/application/herald/HeraldRuleEditor.js' => '52684226', + 'rsrc/js/application/herald/HeraldRuleEditor.js' => '91a6031b', 'rsrc/js/application/herald/PathTypeahead.js' => 'f7fc67ec', 'rsrc/js/application/herald/herald-rule-editor.js' => '7ebaeed3', 'rsrc/js/application/maniphest/behavior-batch-editor.js' => '782ab6e7', @@ -538,7 +538,7 @@ 'global-drag-and-drop-css' => '697324ad', 'harbormaster-css' => '49d64eb4', 'herald-css' => '826075fa', - 'herald-rule-editor' => '52684226', + 'herald-rule-editor' => '91a6031b', 'herald-test-css' => '778b008e', 'inline-comment-summary-css' => '51efda3a', 'javelin-aphlict' => '5359e785', @@ -1172,15 +1172,6 @@ 'javelin-dom', 'javelin-reactor-dom', ), - 52684226 => array( - 'multirow-row-manager', - 'javelin-install', - 'javelin-util', - 'javelin-dom', - 'javelin-stratcom', - 'javelin-json', - 'phabricator-prefab', - ), '5359e785' => array( 'javelin-install', 'javelin-util', @@ -1539,6 +1530,15 @@ 'javelin-dom', 'javelin-stratcom', ), + '91a6031b' => array( + 'multirow-row-manager', + 'javelin-install', + 'javelin-util', + 'javelin-dom', + 'javelin-stratcom', + 'javelin-json', + 'phabricator-prefab', + ), '93d0c9e3' => array( 'javelin-behavior', 'javelin-stratcom', diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -1007,6 +1007,8 @@ 'HarbormasterUploadArtifactBuildStepImplementation' => 'applications/harbormaster/step/HarbormasterUploadArtifactBuildStepImplementation.php', 'HarbormasterWaitForPreviousBuildStepImplementation' => 'applications/harbormaster/step/HarbormasterWaitForPreviousBuildStepImplementation.php', 'HarbormasterWorker' => 'applications/harbormaster/worker/HarbormasterWorker.php', + 'HeraldAction' => 'applications/herald/action/HeraldAction.php', + 'HeraldActionGroup' => 'applications/herald/action/HeraldActionGroup.php', 'HeraldActionRecord' => 'applications/herald/storage/HeraldActionRecord.php', 'HeraldAdapter' => 'applications/herald/adapter/HeraldAdapter.php', 'HeraldAlwaysField' => 'applications/herald/field/HeraldAlwaysField.php', @@ -1024,6 +1026,7 @@ 'HeraldDifferentialDiffAdapter' => 'applications/differential/herald/HeraldDifferentialDiffAdapter.php', 'HeraldDifferentialRevisionAdapter' => 'applications/differential/herald/HeraldDifferentialRevisionAdapter.php', 'HeraldDisableController' => 'applications/herald/controller/HeraldDisableController.php', + 'HeraldDoNothingAction' => 'applications/herald/action/HeraldDoNothingAction.php', 'HeraldEditFieldGroup' => 'applications/herald/field/HeraldEditFieldGroup.php', 'HeraldEffect' => 'applications/herald/engine/HeraldEffect.php', 'HeraldEmptyFieldValue' => 'applications/herald/value/HeraldEmptyFieldValue.php', @@ -1032,17 +1035,20 @@ 'HeraldFieldGroup' => 'applications/herald/field/HeraldFieldGroup.php', 'HeraldFieldTestCase' => 'applications/herald/field/__tests__/HeraldFieldTestCase.php', 'HeraldFieldValue' => 'applications/herald/value/HeraldFieldValue.php', + 'HeraldGroup' => 'applications/herald/group/HeraldGroup.php', 'HeraldInvalidActionException' => 'applications/herald/engine/exception/HeraldInvalidActionException.php', 'HeraldInvalidConditionException' => 'applications/herald/engine/exception/HeraldInvalidConditionException.php', 'HeraldManageGlobalRulesCapability' => 'applications/herald/capability/HeraldManageGlobalRulesCapability.php', 'HeraldManiphestTaskAdapter' => 'applications/maniphest/herald/HeraldManiphestTaskAdapter.php', 'HeraldNewController' => 'applications/herald/controller/HeraldNewController.php', 'HeraldNewObjectField' => 'applications/herald/field/HeraldNewObjectField.php', + 'HeraldNotifyActionGroup' => 'applications/herald/action/HeraldNotifyActionGroup.php', 'HeraldObjectTranscript' => 'applications/herald/storage/transcript/HeraldObjectTranscript.php', 'HeraldPholioMockAdapter' => 'applications/pholio/herald/HeraldPholioMockAdapter.php', 'HeraldPreCommitAdapter' => 'applications/diffusion/herald/HeraldPreCommitAdapter.php', 'HeraldPreCommitContentAdapter' => 'applications/diffusion/herald/HeraldPreCommitContentAdapter.php', 'HeraldPreCommitRefAdapter' => 'applications/diffusion/herald/HeraldPreCommitRefAdapter.php', + 'HeraldPreventActionGroup' => 'applications/herald/action/HeraldPreventActionGroup.php', 'HeraldProjectsField' => 'applications/project/herald/HeraldProjectsField.php', 'HeraldRecursiveConditionsException' => 'applications/herald/engine/exception/HeraldRecursiveConditionsException.php', 'HeraldRelatedFieldGroup' => 'applications/herald/field/HeraldRelatedFieldGroup.php', @@ -1065,6 +1071,7 @@ 'HeraldSelectFieldValue' => 'applications/herald/value/HeraldSelectFieldValue.php', 'HeraldSpaceField' => 'applications/spaces/herald/HeraldSpaceField.php', 'HeraldSubscribersField' => 'applications/subscriptions/herald/HeraldSubscribersField.php', + 'HeraldSupportActionGroup' => 'applications/herald/action/HeraldSupportActionGroup.php', 'HeraldSupportFieldGroup' => 'applications/herald/field/HeraldSupportFieldGroup.php', 'HeraldTestConsoleController' => 'applications/herald/controller/HeraldTestConsoleController.php', 'HeraldTextFieldValue' => 'applications/herald/value/HeraldTextFieldValue.php', @@ -1077,6 +1084,7 @@ 'HeraldTranscriptQuery' => 'applications/herald/query/HeraldTranscriptQuery.php', 'HeraldTranscriptSearchEngine' => 'applications/herald/query/HeraldTranscriptSearchEngine.php', 'HeraldTranscriptTestCase' => 'applications/herald/storage/__tests__/HeraldTranscriptTestCase.php', + 'HeraldUtilityActionGroup' => 'applications/herald/action/HeraldUtilityActionGroup.php', 'Javelin' => 'infrastructure/javelin/Javelin.php', 'JavelinReactorUIExample' => 'applications/uiexample/examples/JavelinReactorUIExample.php', 'JavelinUIExample' => 'applications/uiexample/examples/JavelinUIExample.php', @@ -4684,6 +4692,8 @@ 'HarbormasterUploadArtifactBuildStepImplementation' => 'HarbormasterBuildStepImplementation', 'HarbormasterWaitForPreviousBuildStepImplementation' => 'HarbormasterBuildStepImplementation', 'HarbormasterWorker' => 'PhabricatorWorker', + 'HeraldAction' => 'Phobject', + 'HeraldActionGroup' => 'HeraldGroup', 'HeraldActionRecord' => 'HeraldDAO', 'HeraldAdapter' => 'Phobject', 'HeraldAlwaysField' => 'HeraldField', @@ -4701,25 +4711,29 @@ 'HeraldDifferentialDiffAdapter' => 'HeraldDifferentialAdapter', 'HeraldDifferentialRevisionAdapter' => 'HeraldDifferentialAdapter', 'HeraldDisableController' => 'HeraldController', + 'HeraldDoNothingAction' => 'HeraldAction', 'HeraldEditFieldGroup' => 'HeraldFieldGroup', 'HeraldEffect' => 'Phobject', 'HeraldEmptyFieldValue' => 'HeraldFieldValue', 'HeraldEngine' => 'Phobject', 'HeraldField' => 'Phobject', - 'HeraldFieldGroup' => 'Phobject', + 'HeraldFieldGroup' => 'HeraldGroup', 'HeraldFieldTestCase' => 'PhutilTestCase', 'HeraldFieldValue' => 'Phobject', + 'HeraldGroup' => 'Phobject', 'HeraldInvalidActionException' => 'Exception', 'HeraldInvalidConditionException' => 'Exception', 'HeraldManageGlobalRulesCapability' => 'PhabricatorPolicyCapability', 'HeraldManiphestTaskAdapter' => 'HeraldAdapter', 'HeraldNewController' => 'HeraldController', 'HeraldNewObjectField' => 'HeraldField', + 'HeraldNotifyActionGroup' => 'HeraldActionGroup', 'HeraldObjectTranscript' => 'Phobject', 'HeraldPholioMockAdapter' => 'HeraldAdapter', 'HeraldPreCommitAdapter' => 'HeraldAdapter', 'HeraldPreCommitContentAdapter' => 'HeraldPreCommitAdapter', 'HeraldPreCommitRefAdapter' => 'HeraldPreCommitAdapter', + 'HeraldPreventActionGroup' => 'HeraldActionGroup', 'HeraldProjectsField' => 'HeraldField', 'HeraldRecursiveConditionsException' => 'Exception', 'HeraldRelatedFieldGroup' => 'HeraldFieldGroup', @@ -4748,6 +4762,7 @@ 'HeraldSelectFieldValue' => 'HeraldFieldValue', 'HeraldSpaceField' => 'HeraldField', 'HeraldSubscribersField' => 'HeraldField', + 'HeraldSupportActionGroup' => 'HeraldActionGroup', 'HeraldSupportFieldGroup' => 'HeraldFieldGroup', 'HeraldTestConsoleController' => 'HeraldController', 'HeraldTextFieldValue' => 'HeraldFieldValue', @@ -4764,6 +4779,7 @@ 'HeraldTranscriptQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', 'HeraldTranscriptSearchEngine' => 'PhabricatorApplicationSearchEngine', 'HeraldTranscriptTestCase' => 'PhabricatorTestCase', + 'HeraldUtilityActionGroup' => 'HeraldActionGroup', 'Javelin' => 'Phobject', 'JavelinReactorUIExample' => 'PhabricatorUIExample', 'JavelinUIExample' => 'PhabricatorUIExample', diff --git a/src/applications/differential/herald/HeraldDifferentialDiffAdapter.php b/src/applications/differential/herald/HeraldDifferentialDiffAdapter.php --- a/src/applications/differential/herald/HeraldDifferentialDiffAdapter.php +++ b/src/applications/differential/herald/HeraldDifferentialDiffAdapter.php @@ -75,7 +75,6 @@ return array_merge( array( self::ACTION_BLOCK, - self::ACTION_NOTHING, ), parent::getActions($rule_type)); } diff --git a/src/applications/differential/herald/HeraldDifferentialRevisionAdapter.php b/src/applications/differential/herald/HeraldDifferentialRevisionAdapter.php --- a/src/applications/differential/herald/HeraldDifferentialRevisionAdapter.php +++ b/src/applications/differential/herald/HeraldDifferentialRevisionAdapter.php @@ -169,7 +169,6 @@ self::ACTION_ADD_BLOCKING_REVIEWERS, self::ACTION_APPLY_BUILD_PLANS, self::ACTION_REQUIRE_SIGNATURE, - self::ACTION_NOTHING, ), parent::getActions($rule_type)); case HeraldRuleTypeConfig::RULE_TYPE_PERSONAL: @@ -181,7 +180,6 @@ self::ACTION_FLAG, self::ACTION_ADD_REVIEWERS, self::ACTION_ADD_BLOCKING_REVIEWERS, - self::ACTION_NOTHING, ), parent::getActions($rule_type)); } diff --git a/src/applications/diffusion/herald/HeraldCommitAdapter.php b/src/applications/diffusion/herald/HeraldCommitAdapter.php --- a/src/applications/diffusion/herald/HeraldCommitAdapter.php +++ b/src/applications/diffusion/herald/HeraldCommitAdapter.php @@ -94,7 +94,6 @@ self::ACTION_EMAIL, self::ACTION_AUDIT, self::ACTION_APPLY_BUILD_PLANS, - self::ACTION_NOTHING, ), parent::getActions($rule_type)); case HeraldRuleTypeConfig::RULE_TYPE_PERSONAL: @@ -105,7 +104,6 @@ self::ACTION_EMAIL, self::ACTION_FLAG, self::ACTION_AUDIT, - self::ACTION_NOTHING, ), parent::getActions($rule_type)); } diff --git a/src/applications/diffusion/herald/HeraldPreCommitAdapter.php b/src/applications/diffusion/herald/HeraldPreCommitAdapter.php --- a/src/applications/diffusion/herald/HeraldPreCommitAdapter.php +++ b/src/applications/diffusion/herald/HeraldPreCommitAdapter.php @@ -81,14 +81,12 @@ array( self::ACTION_BLOCK, self::ACTION_EMAIL, - self::ACTION_NOTHING, ), parent::getActions($rule_type)); case HeraldRuleTypeConfig::RULE_TYPE_PERSONAL: return array_merge( array( self::ACTION_EMAIL, - self::ACTION_NOTHING, ), parent::getActions($rule_type)); } diff --git a/src/applications/herald/action/HeraldAction.php b/src/applications/herald/action/HeraldAction.php new file mode 100644 --- /dev/null +++ b/src/applications/herald/action/HeraldAction.php @@ -0,0 +1,118 @@ +getActionConstant() => $this); + } + + protected function getDatasource() { + throw new PhutilMethodNotImplementedException(); + } + + protected function getDatasourceValueMap() { + return null; + } + + public function getHeraldActionStandardType() { + throw new PhutilMethodNotImplementedException(); + } + + public function getHeraldActionValueType() { + switch ($this->getHeraldActionStandardType()) { + case self::STANDARD_NONE: + return new HeraldEmptyFieldValue(); + case self::STANDARD_PHID_LIST: + $tokenizer = id(new HeraldTokenizerFieldValue()) + ->setKey($this->getHeraldFieldName()) + ->setDatasource($this->getDatasource()); + + $value_map = $this->getDatasourceValueMap(); + if ($value_map !== null) { + $tokenizer->setValueMap($value_map); + } + + return $tokenizer; + } + + throw new PhutilMethodNotImplementedException(); + } + + public function willSaveActionValue($value) { + return $value; + } + + final public function setAdapter(HeraldAdapter $adapter) { + $this->adapter = $adapter; + return $this; + } + + final public function getAdapter() { + return $this->adapter; + } + + final public function getActionConstant() { + $class = new ReflectionClass($this); + + $const = $class->getConstant('ACTIONCONST'); + if ($const === false) { + throw new Exception( + pht( + '"%s" class "%s" must define a "%s" property.', + __CLASS__, + get_class($this), + 'ACTIONCONST')); + } + + $limit = self::getActionConstantByteLimit(); + if (!is_string($const) || (strlen($const) > $limit)) { + throw new Exception( + pht( + '"%s" class "%s" has an invalid "%s" property. Action constants '. + 'must be strings and no more than %s bytes in length.', + __CLASS__, + get_class($this), + 'ACTIONCONST', + new PhutilNumber($limit))); + } + + return $const; + } + + final public static function getActionConstantByteLimit() { + return 64; + } + + final public static function getAllActions() { + return id(new PhutilClassMapQuery()) + ->setAncestorClass(__CLASS__) + ->setUniqueMethod('getActionConstant') + ->execute(); + } + + protected function logEffect($type, $data = null) { + return; + } + + final public function getApplyTranscript(HeraldEffect $effect) { + $context = 'v2/'.phutil_json_encode($this->applyLog); + $this->applyLog = array(); + return new HeraldApplyTranscript($effect, true, $context); + } + +} diff --git a/src/applications/herald/field/HeraldFieldGroup.php b/src/applications/herald/action/HeraldActionGroup.php copy from src/applications/herald/field/HeraldFieldGroup.php copy to src/applications/herald/action/HeraldActionGroup.php --- a/src/applications/herald/field/HeraldFieldGroup.php +++ b/src/applications/herald/action/HeraldActionGroup.php @@ -1,34 +1,24 @@ getConstant('FIELDGROUPKEY'); + $const = $class->getConstant('ACTIONGROUPKEY'); if ($const === false) { throw new Exception( pht( '"%s" class "%s" must define a "%s" property.', __CLASS__, get_class($this), - 'FIELDCONST')); + 'ACTIONGROUPKEY')); } return $const; } - public function getSortKey() { - return sprintf('A%08d%s', $this->getGroupOrder(), $this->getGroupLabel()); - } - - final public static function getAllFieldGroups() { + final public static function getAllActionGroups() { return id(new PhutilClassMapQuery()) ->setAncestorClass(__CLASS__) ->setUniqueMethod('getGroupKey') diff --git a/src/applications/herald/action/HeraldDoNothingAction.php b/src/applications/herald/action/HeraldDoNothingAction.php new file mode 100644 --- /dev/null +++ b/src/applications/herald/action/HeraldDoNothingAction.php @@ -0,0 +1,32 @@ +logEffect($effect, self::DO_NOTHING); + } + + public function getHeraldActionStandardType() { + return self::STANDARD_NONE; + } + +} diff --git a/src/applications/herald/action/HeraldNotifyActionGroup.php b/src/applications/herald/action/HeraldNotifyActionGroup.php new file mode 100644 --- /dev/null +++ b/src/applications/herald/action/HeraldNotifyActionGroup.php @@ -0,0 +1,15 @@ +emailPHIDs); @@ -615,6 +615,78 @@ /* -( Actions )------------------------------------------------------------ */ + private function getActionImplementationMap() { + if ($this->actionMap === null) { + // We can't use PhutilClassMapQuery here because action expansion + // depends on the adapter and object. + + $object = $this->getObject(); + + $map = array(); + $all = HeraldAction::getAllActions(); + foreach ($all as $key => $action) { + $action = id(clone $action)->setAdapter($this); + + if (!$action->supportsObject($object)) { + continue; + } + + $subactions = $action->getActionsForObject($object); + foreach ($subactions as $subkey => $subaction) { + if (isset($map[$subkey])) { + throw new Exception( + pht( + 'Two HeraldActions (of classes "%s" and "%s") have the same '. + 'action key ("%s") after expansion for an object of class '. + '"%s" inside adapter "%s". Each action must have a unique '. + 'action key.', + get_class($subaction), + get_class($map[$subkey]), + $subkey, + get_class($object), + get_class($this))); + } + + $subaction = id(clone $subaction)->setAdapter($this); + + $map[$subkey] = $subaction; + } + } + $this->actionMap = $map; + } + + return $this->actionMap; + } + + private function getActionsForRuleType($rule_type) { + $actions = $this->getActionImplementationMap(); + + foreach ($actions as $key => $action) { + if (!$action->supportsRuleType($rule_type)) { + unset($actions[$key]); + } + } + + return $actions; + } + + private function getActionImplementation($key) { + return idx($this->getActionImplementationMap(), $key); + } + + public function getActionKeys() { + return array_keys($this->getActionImplementationMap()); + } + + public function getActionGroupKey($action_key) { + $action = $this->getActionImplementation($action_key); + if (!$action) { + return null; + } + + return $action->getActionGroupKey(); + } + public function getCustomActionsForRuleType($rule_type) { $results = array(); foreach ($this->getCustomActions() as $custom_action) { @@ -640,6 +712,10 @@ } } + foreach ($this->getActionsForRuleType($rule_type) as $key => $action) { + $actions[] = $key; + } + return $actions; } @@ -648,7 +724,6 @@ case HeraldRuleTypeConfig::RULE_TYPE_GLOBAL: case HeraldRuleTypeConfig::RULE_TYPE_OBJECT: $standard = array( - self::ACTION_NOTHING => pht('Do nothing'), self::ACTION_ADD_CC => pht('Add Subscribers'), self::ACTION_REMOVE_CC => pht('Remove Subscribers'), self::ACTION_EMAIL => pht('Send an email to'), @@ -666,7 +741,6 @@ break; case HeraldRuleTypeConfig::RULE_TYPE_PERSONAL: $standard = array( - self::ACTION_NOTHING => pht('Do nothing'), 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'), @@ -685,6 +759,10 @@ $custom_actions = $this->getCustomActionsForRuleType($rule_type); $standard += mpull($custom_actions, 'getActionName', 'getActionKey'); + foreach ($this->getActionsForRuleType($rule_type) as $key => $action) { + $standard[$key] = $action->getHeraldActionName(); + } + return $standard; } @@ -692,6 +770,14 @@ HeraldRule $rule, HeraldActionRecord $action) { + $impl = $this->getActionImplementation($action->getAction()); + if ($impl) { + $target = $action->getTarget(); + $target = $impl->willSaveActionValue($target); + $action->setTarget($target); + return; + } + $target = $action->getTarget(); if (is_array($target)) { $target = array_keys($target); @@ -720,7 +806,6 @@ } break; case self::ACTION_BLOCK: - case self::ACTION_NOTHING: break; default: throw new HeraldInvalidActionException( @@ -744,6 +829,11 @@ } public function getValueTypeForAction($action, $rule_type) { + $impl = $this->getActionImplementation($action); + if ($impl) { + return $impl->getHeraldActionValueType(); + } + $is_personal = ($rule_type == HeraldRuleTypeConfig::RULE_TYPE_PERSONAL); if ($is_personal) { @@ -751,7 +841,6 @@ case self::ACTION_ADD_CC: case self::ACTION_REMOVE_CC: case self::ACTION_EMAIL: - case self::ACTION_NOTHING: case self::ACTION_AUDIT: case self::ACTION_ASSIGN_TASK: case self::ACTION_ADD_REVIEWERS: @@ -771,8 +860,6 @@ case self::ACTION_EMAIL: return $this->buildTokenizerFieldValue( new PhabricatorMetaMTAMailableDatasource()); - case self::ACTION_NOTHING: - return new HeraldEmptyFieldValue(); case self::ACTION_ADD_PROJECTS: case self::ACTION_REMOVE_PROJECTS: return $this->buildTokenizerFieldValue( @@ -1108,6 +1195,12 @@ protected function applyStandardEffect(HeraldEffect $effect) { $action = $effect->getAction(); + $impl = $this->getActionImplementation($action); + if ($impl) { + $impl->applyEffect($this->getObject(), $effect); + return $impl->getApplyTranscript($effect); + } + $rule_type = $effect->getRule()->getRuleType(); $supported = $this->getActions($rule_type); $supported = array_fuse($supported); @@ -1133,8 +1226,6 @@ return $this->applyFlagEffect($effect); case self::ACTION_EMAIL: return $this->applyEmailEffect($effect); - case self::ACTION_NOTHING: - return $this->applyNothingEffect($effect); default: break; } @@ -1153,13 +1244,6 @@ return $result; } - private function applyNothingEffect(HeraldEffect $effect) { - return new HeraldApplyTranscript( - $effect, - true, - pht('Did nothing.')); - } - /** * @task apply */ diff --git a/src/applications/herald/controller/HeraldRuleController.php b/src/applications/herald/controller/HeraldRuleController.php --- a/src/applications/herald/controller/HeraldRuleController.php +++ b/src/applications/herald/controller/HeraldRuleController.php @@ -427,40 +427,10 @@ } } - $group_map = array(); - foreach ($field_map as $field_key => $field_name) { - $group_key = $adapter->getFieldGroupKey($field_key); - $group_map[$group_key][$field_key] = $field_name; - } - - $field_groups = HeraldFieldGroup::getAllFieldGroups(); - - $groups = array(); - foreach ($group_map as $group_key => $options) { - asort($options); - - $field_group = idx($field_groups, $group_key); - if ($field_group) { - $group_label = $field_group->getGroupLabel(); - $group_order = $field_group->getSortKey(); - } else { - $group_label = nonempty($group_key, pht('Other')); - $group_order = 'Z'; - } - - $groups[] = array( - 'label' => $group_label, - 'options' => $options, - 'order' => $group_order, - ); - } - - $groups = array_values(isort($groups, 'order')); - $config_info = array(); - $config_info['fields'] = $groups; + $config_info['fields'] = $this->getFieldGroups($adapter, $field_map); $config_info['conditions'] = $all_conditions; - $config_info['actions'] = $action_map; + $config_info['actions'] = $this->getActionGroups($adapter, $action_map); $config_info['valueMap'] = array(); foreach ($field_map as $field => $name) { @@ -492,7 +462,7 @@ $config_info['rule_type'] = $rule->getRuleType(); - foreach ($config_info['actions'] as $action => $name) { + foreach ($action_map as $action => $name) { try { $value_key = $adapter->getValueTypeForAction( $action, @@ -659,4 +629,55 @@ return $all_rules; } + private function getFieldGroups(HeraldAdapter $adapter, array $field_map) { + $group_map = array(); + foreach ($field_map as $field_key => $field_name) { + $group_key = $adapter->getFieldGroupKey($field_key); + $group_map[$group_key][$field_key] = $field_name; + } + + return $this->getGroups( + $group_map, + HeraldFieldGroup::getAllFieldGroups()); + } + + private function getActionGroups(HeraldAdapter $adapter, array $action_map) { + $group_map = array(); + foreach ($action_map as $action_key => $action_name) { + $group_key = $adapter->getActionGroupKey($action_key); + $group_map[$group_key][$action_key] = $action_name; + } + + return $this->getGroups( + $group_map, + HeraldActionGroup::getAllActionGroups()); + } + + private function getGroups(array $item_map, array $group_list) { + assert_instances_of($group_list, 'HeraldGroup'); + + $groups = array(); + foreach ($item_map as $group_key => $options) { + asort($options); + + $group_object = idx($group_list, $group_key); + if ($group_object) { + $group_label = $group_object->getGroupLabel(); + $group_order = $group_object->getSortKey(); + } else { + $group_label = nonempty($group_key, pht('Other')); + $group_order = 'Z'; + } + + $groups[] = array( + 'label' => $group_label, + 'options' => $options, + 'order' => $group_order, + ); + } + + return array_values(isort($groups, 'order')); + } + + } diff --git a/src/applications/herald/controller/HeraldTranscriptController.php b/src/applications/herald/controller/HeraldTranscriptController.php --- a/src/applications/herald/controller/HeraldTranscriptController.php +++ b/src/applications/herald/controller/HeraldTranscriptController.php @@ -347,9 +347,6 @@ $target = $apply_xscript->getTarget(); switch ($apply_xscript->getAction()) { - case HeraldAdapter::ACTION_NOTHING: - $target = null; - break; case HeraldAdapter::ACTION_FLAG: $target = PhabricatorFlagColor::getColorName($target); break; @@ -358,6 +355,8 @@ $target = $target; break; default: + // TODO: This should be driven by HeraldActions. + if (is_array($target) && $target) { foreach ($target as $k => $phid) { if (isset($handles[$phid])) { @@ -388,6 +387,9 @@ $item->setHeader(pht('%s: %s', $rule, $target)); $item->addAttribute($apply_xscript->getReason()); + + // TODO: This is a bit of a mess while actions convert. + $item->addAttribute( pht('Outcome: %s', $apply_xscript->getAppliedReason())); diff --git a/src/applications/herald/field/HeraldFieldGroup.php b/src/applications/herald/field/HeraldFieldGroup.php --- a/src/applications/herald/field/HeraldFieldGroup.php +++ b/src/applications/herald/field/HeraldFieldGroup.php @@ -1,12 +1,6 @@ getGroupOrder(), $this->getGroupLabel()); - } - final public static function getAllFieldGroups() { return id(new PhutilClassMapQuery()) ->setAncestorClass(__CLASS__) diff --git a/src/applications/herald/group/HeraldGroup.php b/src/applications/herald/group/HeraldGroup.php new file mode 100644 --- /dev/null +++ b/src/applications/herald/group/HeraldGroup.php @@ -0,0 +1,15 @@ +getGroupOrder(), $this->getGroupLabel()); + } + +} diff --git a/src/applications/maniphest/herald/HeraldManiphestTaskAdapter.php b/src/applications/maniphest/herald/HeraldManiphestTaskAdapter.php --- a/src/applications/maniphest/herald/HeraldManiphestTaskAdapter.php +++ b/src/applications/maniphest/herald/HeraldManiphestTaskAdapter.php @@ -76,7 +76,6 @@ self::ACTION_REMOVE_CC, self::ACTION_EMAIL, self::ACTION_ASSIGN_TASK, - self::ACTION_NOTHING, ), parent::getActions($rule_type)); case HeraldRuleTypeConfig::RULE_TYPE_PERSONAL: @@ -87,7 +86,6 @@ self::ACTION_EMAIL, self::ACTION_FLAG, self::ACTION_ASSIGN_TASK, - self::ACTION_NOTHING, ), parent::getActions($rule_type)); } diff --git a/src/applications/pholio/herald/HeraldPholioMockAdapter.php b/src/applications/pholio/herald/HeraldPholioMockAdapter.php --- a/src/applications/pholio/herald/HeraldPholioMockAdapter.php +++ b/src/applications/pholio/herald/HeraldPholioMockAdapter.php @@ -54,7 +54,6 @@ array( self::ACTION_ADD_CC, self::ACTION_REMOVE_CC, - self::ACTION_NOTHING, ), parent::getActions($rule_type)); case HeraldRuleTypeConfig::RULE_TYPE_PERSONAL: @@ -63,7 +62,6 @@ self::ACTION_ADD_CC, self::ACTION_REMOVE_CC, self::ACTION_FLAG, - self::ACTION_NOTHING, ), parent::getActions($rule_type)); } diff --git a/src/applications/phriction/herald/PhrictionDocumentHeraldAdapter.php b/src/applications/phriction/herald/PhrictionDocumentHeraldAdapter.php --- a/src/applications/phriction/herald/PhrictionDocumentHeraldAdapter.php +++ b/src/applications/phriction/herald/PhrictionDocumentHeraldAdapter.php @@ -56,7 +56,6 @@ self::ACTION_ADD_CC, self::ACTION_REMOVE_CC, self::ACTION_EMAIL, - self::ACTION_NOTHING, ), parent::getActions($rule_type)); case HeraldRuleTypeConfig::RULE_TYPE_PERSONAL: @@ -66,7 +65,6 @@ self::ACTION_REMOVE_CC, self::ACTION_EMAIL, self::ACTION_FLAG, - self::ACTION_NOTHING, ), parent::getActions($rule_type)); } diff --git a/webroot/rsrc/js/application/herald/HeraldRuleEditor.js b/webroot/rsrc/js/application/herald/HeraldRuleEditor.js --- a/webroot/rsrc/js/application/herald/HeraldRuleEditor.js +++ b/webroot/rsrc/js/application/herald/HeraldRuleEditor.js @@ -322,21 +322,11 @@ _renderCondition : function(row_id) { var groups = this._config.info.fields; - var optgroups = []; - for (var ii = 0; ii < groups.length; ii++) { - var group = groups[ii]; - var options = []; - for (var k in group.options) { - options.push(JX.$N('option', {value: k}, group.options[k])); - } - optgroups.push(JX.$N('optgroup', {label: group.label}, options)); - } - var attrs = { sigil: 'field-select' }; - var field_select = JX.$N('select', attrs, optgroups); + var field_select = this._renderGroupSelect(groups, attrs); field_select.value = this._config.conditions[row_id][0]; var field_cell = JX.$N('td', {sigil: 'field-cell'}, field_select); @@ -352,6 +342,21 @@ delete actions[k]; } }, + + _renderGroupSelect: function(groups, attrs) { + var optgroups = []; + for (var ii = 0; ii < groups.length; ii++) { + var group = groups[ii]; + var options = []; + for (var k in group.options) { + options.push(JX.$N('option', {value: k}, group.options[k])); + } + optgroups.push(JX.$N('optgroup', {label: group.label}, options)); + } + + return JX.$N('select', attrs, optgroups); + }, + _newAction : function(data) { data = data || []; var temprow = this._actionsRowManager.addRow([]); @@ -361,11 +366,16 @@ this._renderAction(data)); this._onactionchange(r); }, + _renderAction : function(action) { - var action_select = this._renderSelect( - this._config.info.actions, - action[0], - 'action-select'); + var groups = this._config.info.actions; + var attrs = { + sigil: 'action-select' + }; + + var action_select = this._renderGroupSelect(groups, attrs); + action_select.value = action[0]; + var action_cell = JX.$N('td', {sigil: 'action-cell'}, action_select); var target_cell = JX.$N(