From a9caab49f7b5f2b523e52f3e076482d8e40770ab Mon Sep 17 00:00:00 2001 From: epriestley Date: Thu, 16 Jul 2015 14:12:00 -0700 Subject: [PATCH] Begin modularizing Herald field values Summary: Ref T8726. I'm primarily trying to modularize tokenizer values so we don't have to update JS to add a new one. This is ultimately the blocker for "select" custom fields working in Herald. This inches us toward that. I'm //not// modularizing conditions or control types in this round, but hope to end up with hard-coded conditions (which are highly general and very rarely change), hard-coded control types (which are also highly general and very rarely change) and completely modular fields and values (which have mid-to-low generality and change frequently). Test Plan: Used UI to interact with "none", "text", and new-style "select" controls. No actual support for tokenizers yet. Reviewers: btrahan Reviewed By: btrahan Subscribers: epriestley Maniphest Tasks: T8726 Differential Revision: https://secure.phabricator.com/D13613 --- resources/celerity/map.php | 22 +++---- src/__phutil_library_map__.php | 8 +++ .../controller/HeraldRuleController.php | 20 +++--- .../herald/field/HeraldContentSourceField.php | 8 ++- src/applications/herald/field/HeraldField.php | 4 +- .../herald/value/HeraldEmptyFieldValue.php | 14 ++++ .../herald/value/HeraldFieldValue.php | 24 +++++++ .../herald/value/HeraldSelectFieldValue.php | 59 +++++++++++++++++ .../herald/value/HeraldTextFieldValue.php | 14 ++++ .../js/application/herald/HeraldRuleEditor.js | 64 ++++++++++++++++--- 10 files changed, 204 insertions(+), 33 deletions(-) create mode 100644 src/applications/herald/value/HeraldEmptyFieldValue.php create mode 100644 src/applications/herald/value/HeraldFieldValue.php create mode 100644 src/applications/herald/value/HeraldSelectFieldValue.php create mode 100644 src/applications/herald/value/HeraldTextFieldValue.php diff --git a/resources/celerity/map.php b/resources/celerity/map.php index 345f528c9a..eae4b0ab75 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' => 'b2cae298', + 'rsrc/js/application/herald/HeraldRuleEditor.js' => '0f85bdfd', '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' => 'b2cae298', + 'herald-rule-editor' => '0f85bdfd', 'herald-test-css' => '778b008e', 'inline-comment-summary-css' => '51efda3a', 'javelin-aphlict' => '5359e785', @@ -918,6 +918,15 @@ 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', @@ -1676,15 +1685,6 @@ return array( 'javelin-uri', 'javelin-request', ), - 'b2cae298' => array( - 'multirow-row-manager', - 'javelin-install', - 'javelin-util', - 'javelin-dom', - 'javelin-stratcom', - 'javelin-json', - 'phabricator-prefab', - ), 'b3a4b884' => array( 'javelin-behavior', 'phabricator-prefab', diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 3511f44a37..021cdc319a 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -1018,9 +1018,11 @@ phutil_register_library_map(array( 'HeraldDifferentialRevisionAdapter' => 'applications/differential/herald/HeraldDifferentialRevisionAdapter.php', 'HeraldDisableController' => 'applications/herald/controller/HeraldDisableController.php', 'HeraldEffect' => 'applications/herald/engine/HeraldEffect.php', + 'HeraldEmptyFieldValue' => 'applications/herald/value/HeraldEmptyFieldValue.php', 'HeraldEngine' => 'applications/herald/engine/HeraldEngine.php', 'HeraldField' => 'applications/herald/field/HeraldField.php', 'HeraldFieldTestCase' => 'applications/herald/field/__tests__/HeraldFieldTestCase.php', + 'HeraldFieldValue' => 'applications/herald/value/HeraldFieldValue.php', 'HeraldInvalidActionException' => 'applications/herald/engine/exception/HeraldInvalidActionException.php', 'HeraldInvalidConditionException' => 'applications/herald/engine/exception/HeraldInvalidConditionException.php', 'HeraldManageGlobalRulesCapability' => 'applications/herald/capability/HeraldManageGlobalRulesCapability.php', @@ -1050,9 +1052,11 @@ phutil_register_library_map(array( 'HeraldRuleTypeConfig' => 'applications/herald/config/HeraldRuleTypeConfig.php', 'HeraldRuleViewController' => 'applications/herald/controller/HeraldRuleViewController.php', 'HeraldSchemaSpec' => 'applications/herald/storage/HeraldSchemaSpec.php', + 'HeraldSelectFieldValue' => 'applications/herald/value/HeraldSelectFieldValue.php', 'HeraldSpaceField' => 'applications/spaces/herald/HeraldSpaceField.php', 'HeraldSubscribersField' => 'applications/subscriptions/herald/HeraldSubscribersField.php', 'HeraldTestConsoleController' => 'applications/herald/controller/HeraldTestConsoleController.php', + 'HeraldTextFieldValue' => 'applications/herald/value/HeraldTextFieldValue.php', 'HeraldTransactionQuery' => 'applications/herald/query/HeraldTransactionQuery.php', 'HeraldTranscript' => 'applications/herald/storage/transcript/HeraldTranscript.php', 'HeraldTranscriptController' => 'applications/herald/controller/HeraldTranscriptController.php', @@ -4615,9 +4619,11 @@ phutil_register_library_map(array( 'HeraldDifferentialRevisionAdapter' => 'HeraldDifferentialAdapter', 'HeraldDisableController' => 'HeraldController', 'HeraldEffect' => 'Phobject', + 'HeraldEmptyFieldValue' => 'HeraldFieldValue', 'HeraldEngine' => 'Phobject', 'HeraldField' => 'Phobject', 'HeraldFieldTestCase' => 'PhutilTestCase', + 'HeraldFieldValue' => 'Phobject', 'HeraldInvalidActionException' => 'Exception', 'HeraldInvalidConditionException' => 'Exception', 'HeraldManageGlobalRulesCapability' => 'PhabricatorPolicyCapability', @@ -4653,9 +4659,11 @@ phutil_register_library_map(array( 'HeraldRuleTypeConfig' => 'Phobject', 'HeraldRuleViewController' => 'HeraldController', 'HeraldSchemaSpec' => 'PhabricatorConfigSchemaSpec', + 'HeraldSelectFieldValue' => 'HeraldFieldValue', 'HeraldSpaceField' => 'HeraldField', 'HeraldSubscribersField' => 'HeraldField', 'HeraldTestConsoleController' => 'HeraldController', + 'HeraldTextFieldValue' => 'HeraldFieldValue', 'HeraldTransactionQuery' => 'PhabricatorApplicationTransactionQuery', 'HeraldTranscript' => array( 'HeraldDAO', diff --git a/src/applications/herald/controller/HeraldRuleController.php b/src/applications/herald/controller/HeraldRuleController.php index 994bed9d4a..d624cdc85a 100644 --- a/src/applications/herald/controller/HeraldRuleController.php +++ b/src/applications/herald/controller/HeraldRuleController.php @@ -440,6 +440,7 @@ final class HeraldRuleController extends HeraldController { $config_info['fields'] = $field_map; $config_info['conditions'] = $all_conditions; $config_info['actions'] = $action_map; + $config_info['valueMap'] = array(); foreach ($config_info['fields'] as $field => $name) { try { @@ -452,10 +453,17 @@ final class HeraldRuleController extends HeraldController { foreach ($config_info['fields'] as $field => $fname) { foreach ($config_info['conditionMap'][$field] as $condition) { - $value_type = $adapter->getValueTypeForFieldAndCondition( + $value_key = $adapter->getValueTypeForFieldAndCondition( $field, $condition); - $config_info['values'][$field][$condition] = $value_type; + + if ($value_key instanceof HeraldFieldValue) { + $spec = $value_key->getControlSpecificationDictionary(); + $value_key = $value_key->getFieldValueKey(); + $config_info['valueMap'][$value_key] = $spec; + } + + $config_info['values'][$field][$condition] = $value_key; } } @@ -482,10 +490,6 @@ final class HeraldRuleController extends HeraldController { 'conditions' => (object)$serial_conditions, 'actions' => (object)$serial_actions, 'select' => array( - HeraldAdapter::VALUE_CONTENT_SOURCE => array( - 'options' => PhabricatorContentSource::getSourceNameMap(), - 'default' => PhabricatorContentSource::SOURCE_WEB, - ), HeraldAdapter::VALUE_FLAG_COLOR => array( 'options' => PhabricatorFlagColor::getColorNameMap(), 'default' => PhabricatorFlagColor::COLOR_BLUE, @@ -509,10 +513,6 @@ final class HeraldRuleController extends HeraldController { 'template' => $this->buildTokenizerTemplates($handles) + array( 'rules' => $all_rules, ), - 'author' => array( - $rule->getAuthorPHID() => - $handles[$rule->getAuthorPHID()]->getName(), - ), 'info' => $config_info, )); } diff --git a/src/applications/herald/field/HeraldContentSourceField.php b/src/applications/herald/field/HeraldContentSourceField.php index d19ce0faa1..c716a35c08 100644 --- a/src/applications/herald/field/HeraldContentSourceField.php +++ b/src/applications/herald/field/HeraldContentSourceField.php @@ -20,7 +20,13 @@ final class HeraldContentSourceField extends HeraldField { } public function getHeraldFieldValueType($condition) { - return HeraldAdapter::VALUE_CONTENT_SOURCE; + $map = PhabricatorContentSource::getSourceNameMap(); + asort($map); + + return id(new HeraldSelectFieldValue()) + ->setKey(self::FIELDCONST) + ->setDefault(PhabricatorContentSource::SOURCE_WEB) + ->setOptions($map); } public function supportsObject($object) { diff --git a/src/applications/herald/field/HeraldField.php b/src/applications/herald/field/HeraldField.php index 82c783972e..46fd21f422 100644 --- a/src/applications/herald/field/HeraldField.php +++ b/src/applications/herald/field/HeraldField.php @@ -86,11 +86,11 @@ abstract class HeraldField extends Phobject { switch ($standard_type) { case self::STANDARD_BOOL: case self::STANDARD_PHID_BOOL: - return HeraldAdapter::VALUE_NONE; + return new HeraldEmptyFieldValue(); case self::STANDARD_TEXT: case self::STANDARD_TEXT_LIST: case self::STANDARD_TEXT_MAP: - return HeraldAdapter::VALUE_TEXT; + return new HeraldTextFieldValue(); } throw new Exception( diff --git a/src/applications/herald/value/HeraldEmptyFieldValue.php b/src/applications/herald/value/HeraldEmptyFieldValue.php new file mode 100644 index 0000000000..b0d7b774de --- /dev/null +++ b/src/applications/herald/value/HeraldEmptyFieldValue.php @@ -0,0 +1,14 @@ + $this->getControlType(), + 'template' => $this->getControlTemplate(), + ); + } + + protected function getControlTemplate() { + return array(); + } + +} diff --git a/src/applications/herald/value/HeraldSelectFieldValue.php b/src/applications/herald/value/HeraldSelectFieldValue.php new file mode 100644 index 0000000000..e29ec029ad --- /dev/null +++ b/src/applications/herald/value/HeraldSelectFieldValue.php @@ -0,0 +1,59 @@ +key = $key; + return $this; + } + + public function getKey() { + return $this->key; + } + + public function setOptions(array $options) { + $this->options = $options; + return $this; + } + + public function getOptions() { + return $this->options; + } + + public function setDefault($default) { + $this->default = $default; + return $this; + } + + public function getDefault() { + return $this->default; + } + + public function getFieldValueKey() { + if ($this->getKey() === null) { + throw new PhutilInvalidStateException('setKey'); + } + return 'select.'.$this->getKey(); + } + + public function getControlType() { + return self::CONTROL_SELECT; + } + + protected function getControlTemplate() { + if ($this->getOptions() === null) { + throw new PhutilInvalidStateException('setOptions'); + } + + return array( + 'options' => $this->getOptions(), + 'default' => $this->getDefault(), + ); + } + +} diff --git a/src/applications/herald/value/HeraldTextFieldValue.php b/src/applications/herald/value/HeraldTextFieldValue.php new file mode 100644 index 0000000000..33de188185 --- /dev/null +++ b/src/applications/herald/value/HeraldTextFieldValue.php @@ -0,0 +1,14 @@ +