diff --git a/resources/celerity/map.php b/resources/celerity/map.php index eae4b0ab75..8379ef7071 100644 --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -382,7 +382,7 @@ return array( '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' => '0f85bdfd', + 'rsrc/js/application/herald/HeraldRuleEditor.js' => '797e4876', '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 @@ return array( 'global-drag-and-drop-css' => '697324ad', 'harbormaster-css' => '49d64eb4', 'herald-css' => '826075fa', - 'herald-rule-editor' => '0f85bdfd', + 'herald-rule-editor' => '797e4876', 'herald-test-css' => '778b008e', 'inline-comment-summary-css' => '51efda3a', 'javelin-aphlict' => '5359e785', @@ -918,15 +918,6 @@ return array( 'javelin-install', 'javelin-util', ), - '0f85bdfd' => array( - 'multirow-row-manager', - 'javelin-install', - 'javelin-util', - 'javelin-dom', - 'javelin-stratcom', - 'javelin-json', - 'phabricator-prefab', - ), '13c739ea' => array( 'javelin-behavior', 'javelin-stratcom', @@ -1425,6 +1416,15 @@ return array( 'javelin-behavior', 'javelin-quicksand', ), + '797e4876' => array( + 'multirow-row-manager', + 'javelin-install', + 'javelin-util', + 'javelin-dom', + 'javelin-stratcom', + 'javelin-json', + 'phabricator-prefab', + ), '7a68dda3' => array( 'owners-path-editor', 'javelin-behavior', diff --git a/src/applications/diffusion/herald/DiffusionPreCommitRefChangeHeraldField.php b/src/applications/diffusion/herald/DiffusionPreCommitRefChangeHeraldField.php index 61942409ba..8de278e449 100644 --- a/src/applications/diffusion/herald/DiffusionPreCommitRefChangeHeraldField.php +++ b/src/applications/diffusion/herald/DiffusionPreCommitRefChangeHeraldField.php @@ -21,23 +21,11 @@ final class DiffusionPreCommitRefChangeHeraldField } public function getHeraldFieldValueType($condition) { - return HeraldPreCommitRefAdapter::VALUE_REF_CHANGE; - } - - public function renderConditionValue( - PhabricatorUser $viewer, - $value) { - - $change_map = - PhabricatorRepositoryPushLog::getHeraldChangeFlagConditionOptions(); - foreach ($value as $index => $val) { - $name = idx($change_map, $val); - if ($name) { - $value[$index] = $name; - } - } - - return phutil_implode_html(', ', $value); + return id(new HeraldSelectFieldValue()) + ->setKey(self::FIELDCONST) + ->setOptions( + PhabricatorRepositoryPushLog::getHeraldChangeFlagConditionOptions()) + ->setDefault(PhabricatorRepositoryPushLog::CHANGEFLAG_ADD); } } diff --git a/src/applications/diffusion/herald/DiffusionPreCommitRefTypeHeraldField.php b/src/applications/diffusion/herald/DiffusionPreCommitRefTypeHeraldField.php index 1daae7bc4b..ff71ae8713 100644 --- a/src/applications/diffusion/herald/DiffusionPreCommitRefTypeHeraldField.php +++ b/src/applications/diffusion/herald/DiffusionPreCommitRefTypeHeraldField.php @@ -21,7 +21,16 @@ final class DiffusionPreCommitRefTypeHeraldField } public function getHeraldFieldValueType($condition) { - return HeraldPreCommitRefAdapter::VALUE_REF_TYPE; + $types = array( + PhabricatorRepositoryPushLog::REFTYPE_BRANCH => pht('branch (git/hg)'), + PhabricatorRepositoryPushLog::REFTYPE_TAG => pht('tag (git)'), + PhabricatorRepositoryPushLog::REFTYPE_BOOKMARK => pht('bookmark (hg)'), + ); + + return id(new HeraldSelectFieldValue()) + ->setKey(self::FIELDCONST) + ->setOptions($types) + ->setDefault(PhabricatorRepositoryPushLog::REFTYPE_BRANCH); } } diff --git a/src/applications/diffusion/herald/HeraldPreCommitRefAdapter.php b/src/applications/diffusion/herald/HeraldPreCommitRefAdapter.php index bdb19f2e53..76b360b84d 100644 --- a/src/applications/diffusion/herald/HeraldPreCommitRefAdapter.php +++ b/src/applications/diffusion/herald/HeraldPreCommitRefAdapter.php @@ -2,9 +2,6 @@ final class HeraldPreCommitRefAdapter extends HeraldPreCommitAdapter { - const VALUE_REF_TYPE = 'value-ref-type'; - const VALUE_REF_CHANGE = 'value-ref-change'; - public function getAdapterContentName() { return pht('Commit Hook: Branches/Tags/Bookmarks'); } diff --git a/src/applications/herald/adapter/HeraldAdapter.php b/src/applications/herald/adapter/HeraldAdapter.php index 31b5863dde..708a51b828 100644 --- a/src/applications/herald/adapter/HeraldAdapter.php +++ b/src/applications/herald/adapter/HeraldAdapter.php @@ -41,7 +41,6 @@ abstract class HeraldAdapter extends Phobject { const ACTION_BLOCK = 'block'; const ACTION_REQUIRE_SIGNATURE = 'signature'; - const VALUE_TEXT = 'text'; const VALUE_NONE = 'none'; const VALUE_EMAIL = 'email'; const VALUE_USER = 'user'; @@ -49,8 +48,6 @@ abstract class HeraldAdapter extends Phobject { const VALUE_REPOSITORY = 'repository'; const VALUE_OWNERS_PACKAGE = 'package'; const VALUE_PROJECT = 'project'; - const VALUE_FLAG_COLOR = 'flagcolor'; - const VALUE_CONTENT_SOURCE = 'contentsource'; const VALUE_USER_OR_PROJECT = 'userorproject'; const VALUE_BUILD_PLAN = 'buildplan'; const VALUE_TASK_PRIORITY = 'taskpriority'; @@ -766,7 +763,7 @@ abstract class HeraldAdapter extends Phobject { case self::ACTION_ADD_BLOCKING_REVIEWERS: return self::VALUE_NONE; case self::ACTION_FLAG: - return self::VALUE_FLAG_COLOR; + return $this->buildFlagColorFieldValue(); case self::ACTION_ADD_PROJECTS: case self::ACTION_REMOVE_PROJECTS: return self::VALUE_PROJECT; @@ -783,7 +780,7 @@ abstract class HeraldAdapter extends Phobject { case self::ACTION_REMOVE_PROJECTS: return self::VALUE_PROJECT; case self::ACTION_FLAG: - return self::VALUE_FLAG_COLOR; + return $this->buildFlagColorFieldValue(); case self::ACTION_ASSIGN_TASK: return self::VALUE_USER; case self::ACTION_AUDIT: @@ -795,7 +792,7 @@ abstract class HeraldAdapter extends Phobject { case self::ACTION_REQUIRE_SIGNATURE: return self::VALUE_LEGAL_DOCUMENTS; case self::ACTION_BLOCK: - return self::VALUE_TEXT; + return new HeraldTextFieldValue(); } } @@ -807,6 +804,12 @@ abstract class HeraldAdapter extends Phobject { throw new Exception(pht("Unknown or invalid action '%s'.", $action)); } + private function buildFlagColorFieldValue() { + return id(new HeraldSelectFieldValue()) + ->setKey('flag.color') + ->setOptions(PhabricatorFlagColor::getColorNameMap()) + ->setDefault(PhabricatorFlagColor::COLOR_BLUE); + } /* -( Repetition )--------------------------------------------------------- */ @@ -1019,6 +1022,7 @@ abstract class HeraldAdapter extends Phobject { if ($impl) { return $impl->renderConditionValue( $viewer, + $condition->getFieldCondition(), $condition->getValue()); } diff --git a/src/applications/herald/controller/HeraldRuleController.php b/src/applications/herald/controller/HeraldRuleController.php index d624cdc85a..09e5c2fbfa 100644 --- a/src/applications/herald/controller/HeraldRuleController.php +++ b/src/applications/herald/controller/HeraldRuleController.php @@ -471,45 +471,28 @@ final class HeraldRuleController extends HeraldController { foreach ($config_info['actions'] as $action => $name) { try { - $action_value = $adapter->getValueTypeForAction( + $value_key = $adapter->getValueTypeForAction( $action, $rule->getRuleType()); } catch (Exception $ex) { - $action_value = array(HeraldAdapter::VALUE_NONE); + $value_key = new HeraldEmptyFieldValue(); } - $config_info['targets'][$action] = $action_value; + if ($value_key instanceof HeraldFieldValue) { + $spec = $value_key->getControlSpecificationDictionary(); + $value_key = $value_key->getFieldValueKey(); + $config_info['valueMap'][$value_key] = $spec; + } + + $config_info['targets'][$action] = $value_key; } - $changeflag_options = - PhabricatorRepositoryPushLog::getHeraldChangeFlagConditionOptions(); Javelin::initBehavior( 'herald-rule-editor', array( 'root' => 'herald-rule-edit-form', 'conditions' => (object)$serial_conditions, 'actions' => (object)$serial_actions, - 'select' => array( - HeraldAdapter::VALUE_FLAG_COLOR => array( - 'options' => PhabricatorFlagColor::getColorNameMap(), - 'default' => PhabricatorFlagColor::COLOR_BLUE, - ), - HeraldPreCommitRefAdapter::VALUE_REF_TYPE => array( - 'options' => array( - PhabricatorRepositoryPushLog::REFTYPE_BRANCH - => pht('branch (git/hg)'), - PhabricatorRepositoryPushLog::REFTYPE_TAG - => pht('tag (git)'), - PhabricatorRepositoryPushLog::REFTYPE_BOOKMARK - => pht('bookmark (hg)'), - ), - 'default' => PhabricatorRepositoryPushLog::REFTYPE_BRANCH, - ), - HeraldPreCommitRefAdapter::VALUE_REF_CHANGE => array( - 'options' => $changeflag_options, - 'default' => PhabricatorRepositoryPushLog::CHANGEFLAG_ADD, - ), - ), 'template' => $this->buildTokenizerTemplates($handles) + array( 'rules' => $all_rules, ), diff --git a/src/applications/herald/field/HeraldField.php b/src/applications/herald/field/HeraldField.php index 46fd21f422..c78d925fcb 100644 --- a/src/applications/herald/field/HeraldField.php +++ b/src/applications/herald/field/HeraldField.php @@ -108,8 +108,14 @@ abstract class HeraldField extends Phobject { public function renderConditionValue( PhabricatorUser $viewer, + $condition, $value) { + $value_type = $this->getHeraldFieldValueType($condition); + if ($value_type instanceof HeraldFieldValue) { + return $value_type->renderValue($viewer, $value); + } + // TODO: While this is less of a mess than it used to be, it would still // be nice to push this down into individual fields better eventually and // stop guessing which values are PHIDs and which aren't. diff --git a/src/applications/herald/value/HeraldEmptyFieldValue.php b/src/applications/herald/value/HeraldEmptyFieldValue.php index b0d7b774de..565bf69006 100644 --- a/src/applications/herald/value/HeraldEmptyFieldValue.php +++ b/src/applications/herald/value/HeraldEmptyFieldValue.php @@ -11,4 +11,8 @@ final class HeraldEmptyFieldValue return self::CONTROL_NONE; } + public function renderValue(PhabricatorUser $viewer, $value) { + return null; + } + } diff --git a/src/applications/herald/value/HeraldFieldValue.php b/src/applications/herald/value/HeraldFieldValue.php index beea1766bd..915ebbe0f9 100644 --- a/src/applications/herald/value/HeraldFieldValue.php +++ b/src/applications/herald/value/HeraldFieldValue.php @@ -9,6 +9,7 @@ abstract class HeraldFieldValue extends Phobject { abstract public function getFieldValueKey(); abstract public function getControlType(); + abstract public function renderValue(PhabricatorUser $viewer, $value); final public function getControlSpecificationDictionary() { return array( diff --git a/src/applications/herald/value/HeraldSelectFieldValue.php b/src/applications/herald/value/HeraldSelectFieldValue.php index e29ec029ad..ac0706082e 100644 --- a/src/applications/herald/value/HeraldSelectFieldValue.php +++ b/src/applications/herald/value/HeraldSelectFieldValue.php @@ -56,4 +56,9 @@ final class HeraldSelectFieldValue ); } + public function renderValue(PhabricatorUser $viewer, $value) { + $options = $this->getOptions(); + return idx($options, $value, $value); + } + } diff --git a/src/applications/herald/value/HeraldTextFieldValue.php b/src/applications/herald/value/HeraldTextFieldValue.php index 33de188185..8cf3308184 100644 --- a/src/applications/herald/value/HeraldTextFieldValue.php +++ b/src/applications/herald/value/HeraldTextFieldValue.php @@ -11,4 +11,9 @@ final class HeraldTextFieldValue return self::CONTROL_TEXT; } + + public function renderValue(PhabricatorUser $viewer, $value) { + return $value; + } + } diff --git a/src/applications/maniphest/herald/ManiphestTaskPriorityHeraldField.php b/src/applications/maniphest/herald/ManiphestTaskPriorityHeraldField.php index 22c9a53a62..403cc5669e 100644 --- a/src/applications/maniphest/herald/ManiphestTaskPriorityHeraldField.php +++ b/src/applications/maniphest/herald/ManiphestTaskPriorityHeraldField.php @@ -23,6 +23,7 @@ final class ManiphestTaskPriorityHeraldField public function renderConditionValue( PhabricatorUser $viewer, + $condition, $value) { $priority_map = ManiphestTaskPriority::getTaskPriorityMap(); diff --git a/src/applications/maniphest/herald/ManiphestTaskStatusHeraldField.php b/src/applications/maniphest/herald/ManiphestTaskStatusHeraldField.php index 49f6b210af..caadbd644c 100644 --- a/src/applications/maniphest/herald/ManiphestTaskStatusHeraldField.php +++ b/src/applications/maniphest/herald/ManiphestTaskStatusHeraldField.php @@ -23,6 +23,7 @@ final class ManiphestTaskStatusHeraldField public function renderConditionValue( PhabricatorUser $viewer, + $condition, $value) { $status_map = ManiphestTaskStatus::getTaskStatusMap(); diff --git a/webroot/rsrc/js/application/herald/HeraldRuleEditor.js b/webroot/rsrc/js/application/herald/HeraldRuleEditor.js index 33aeb6c1f9..525d82fe0d 100644 --- a/webroot/rsrc/js/application/herald/HeraldRuleEditor.js +++ b/webroot/rsrc/js/application/herald/HeraldRuleEditor.js @@ -269,19 +269,6 @@ JX.install('HeraldRuleEditor', { get_fn = tokenizer[1]; set_fn = tokenizer[2]; break; - case 'none': - input = ''; - get_fn = JX.bag; - set_fn = JX.bag; - break; - case 'flagcolor': - case 'value-ref-type': - case 'value-ref-change': - input = this._renderSelect(this._config.select[type].options); - get_fn = function() { return input.value; }; - set_fn = function(v) { input.value = v; }; - set_fn(this._config.select[type]['default']); - break; default: input = JX.$N('input', {type: 'text'}); get_fn = function() { return input.value; };