From d7693a93b3c4e34e3d6898a8e2ed7c35dc4bf2b0 Mon Sep 17 00:00:00 2001 From: epriestley Date: Tue, 15 Dec 2015 06:57:32 -0800 Subject: [PATCH] Provide "Change Projects" and "Change Subscribers" (instead of "Add ...") in comment actions Summary: Ref T9908. Fixes T6205. This is largely some refactoring to improve the code. The new structure is: - Each EditField has zero or one "submit" (normal edit form) controls. - Each EditField has zero or one "comment" (stacked actions) controls. - If we want more than one in the future, we'd just add two fields. - Each EditField can have multiple EditTypes which provide Conduit transactions. - EditTypes are now lower-level and less involved on the Submit/Comment pathways. Test Plan: - Added and removed projects and subscribers. - Changed task statuses. - In two windows: added some subscribers in one, removed different ones in the other. The changes did not conflict. - Applied changes via Conduit. Reviewers: chad Reviewed By: chad Maniphest Tasks: T6205, T9908 Differential Revision: https://secure.phabricator.com/D14789 --- resources/celerity/map.php | 20 +-- src/__phutil_library_map__.php | 6 + .../maniphest/editor/ManiphestEditEngine.php | 4 +- ...PhabricatorProjectsEditEngineExtension.php | 2 +- ...icatorSubscriptionsEditEngineExtension.php | 2 +- .../PhabricatorEditEngineCommentAction.php | 49 +++++++ ...abricatorEditEngineSelectCommentAction.php | 29 ++++ ...icatorEditEngineTokenizerCommentAction.php | 55 ++++++++ .../editengine/PhabricatorEditEngine.php | 115 +++++++-------- .../editfield/PhabricatorCommentEditField.php | 4 + .../editfield/PhabricatorEditField.php | 131 ++++++++++++++++-- .../PhabricatorPHIDListEditField.php | 8 -- .../editfield/PhabricatorSelectEditField.php | 35 +---- .../PhabricatorTokenizerEditField.php | 58 +++----- .../edittype/PhabricatorEditType.php | 12 -- .../edittype/PhabricatorPHIDListEditType.php | 59 -------- .../edittype/PhabricatorSimpleEditType.php | 18 --- ...catorApplicationTransactionCommentView.php | 83 +++++------ .../control/AphrontTokenizerTemplateView.php | 24 ++-- .../control/AphrontFormTokenizerControl.php | 16 +-- .../transactions/behavior-comment-actions.js | 8 +- 21 files changed, 417 insertions(+), 321 deletions(-) create mode 100644 src/applications/transactions/commentaction/PhabricatorEditEngineCommentAction.php create mode 100644 src/applications/transactions/commentaction/PhabricatorEditEngineSelectCommentAction.php create mode 100644 src/applications/transactions/commentaction/PhabricatorEditEngineTokenizerCommentAction.php diff --git a/resources/celerity/map.php b/resources/celerity/map.php index 93b528376b..099bafaee6 100644 --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -426,7 +426,7 @@ return array( 'rsrc/js/application/repository/repository-crossreference.js' => 'e5339c43', 'rsrc/js/application/search/behavior-reorder-queries.js' => 'e9581f08', 'rsrc/js/application/slowvote/behavior-slowvote-embed.js' => '887ad43f', - 'rsrc/js/application/transactions/behavior-comment-actions.js' => '6de53e91', + 'rsrc/js/application/transactions/behavior-comment-actions.js' => 'bb0d2d0c', 'rsrc/js/application/transactions/behavior-reorder-configs.js' => 'd7a74243', 'rsrc/js/application/transactions/behavior-reorder-fields.js' => 'b59e1e96', 'rsrc/js/application/transactions/behavior-show-older-transactions.js' => 'dbbf48b6', @@ -571,7 +571,7 @@ return array( 'javelin-behavior-audit-preview' => 'd835b03a', 'javelin-behavior-bulk-job-reload' => 'edf8a145', 'javelin-behavior-choose-control' => '6153c708', - 'javelin-behavior-comment-actions' => '6de53e91', + 'javelin-behavior-comment-actions' => 'bb0d2d0c', 'javelin-behavior-config-reorder-fields' => 'b6993408', 'javelin-behavior-conpherence-drag-and-drop-photo' => 'cf86d16a', 'javelin-behavior-conpherence-menu' => '1d45c74d', @@ -1335,14 +1335,6 @@ return array( 'phabricator-drag-and-drop-file-upload', 'phabricator-textareautils', ), - '6de53e91' => array( - 'javelin-behavior', - 'javelin-stratcom', - 'javelin-workflow', - 'javelin-dom', - 'phuix-form-control-view', - 'phuix-icon-view', - ), '6eff08aa' => array( 'javelin-install', 'javelin-util', @@ -1739,6 +1731,14 @@ return array( 'javelin-workflow', 'phabricator-draggable-list', ), + 'bb0d2d0c' => array( + 'javelin-behavior', + 'javelin-stratcom', + 'javelin-workflow', + 'javelin-dom', + 'phuix-form-control-view', + 'phuix-icon-view', + ), 'bd4c8dca' => array( 'javelin-install', 'javelin-util', diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 03f2f47062..4fb7d43498 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -2150,6 +2150,7 @@ phutil_register_library_map(array( 'PhabricatorEdgeTypeTestCase' => 'infrastructure/edges/type/__tests__/PhabricatorEdgeTypeTestCase.php', 'PhabricatorEditEngine' => 'applications/transactions/editengine/PhabricatorEditEngine.php', 'PhabricatorEditEngineAPIMethod' => 'applications/transactions/editengine/PhabricatorEditEngineAPIMethod.php', + 'PhabricatorEditEngineCommentAction' => 'applications/transactions/commentaction/PhabricatorEditEngineCommentAction.php', 'PhabricatorEditEngineConfiguration' => 'applications/transactions/storage/PhabricatorEditEngineConfiguration.php', 'PhabricatorEditEngineConfigurationDefaultCreateController' => 'applications/transactions/controller/PhabricatorEditEngineConfigurationDefaultCreateController.php', 'PhabricatorEditEngineConfigurationDefaultsController' => 'applications/transactions/controller/PhabricatorEditEngineConfigurationDefaultsController.php', @@ -2175,6 +2176,8 @@ phutil_register_library_map(array( 'PhabricatorEditEngineListController' => 'applications/transactions/controller/PhabricatorEditEngineListController.php', 'PhabricatorEditEngineQuery' => 'applications/transactions/query/PhabricatorEditEngineQuery.php', 'PhabricatorEditEngineSearchEngine' => 'applications/transactions/query/PhabricatorEditEngineSearchEngine.php', + 'PhabricatorEditEngineSelectCommentAction' => 'applications/transactions/commentaction/PhabricatorEditEngineSelectCommentAction.php', + 'PhabricatorEditEngineTokenizerCommentAction' => 'applications/transactions/commentaction/PhabricatorEditEngineTokenizerCommentAction.php', 'PhabricatorEditField' => 'applications/transactions/editfield/PhabricatorEditField.php', 'PhabricatorEditType' => 'applications/transactions/edittype/PhabricatorEditType.php', 'PhabricatorEditor' => 'infrastructure/PhabricatorEditor.php', @@ -6308,6 +6311,7 @@ phutil_register_library_map(array( 'PhabricatorPolicyInterface', ), 'PhabricatorEditEngineAPIMethod' => 'ConduitAPIMethod', + 'PhabricatorEditEngineCommentAction' => 'Phobject', 'PhabricatorEditEngineConfiguration' => array( 'PhabricatorSearchDAO', 'PhabricatorApplicationTransactionInterface', @@ -6337,6 +6341,8 @@ phutil_register_library_map(array( 'PhabricatorEditEngineListController' => 'PhabricatorEditEngineController', 'PhabricatorEditEngineQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', 'PhabricatorEditEngineSearchEngine' => 'PhabricatorApplicationSearchEngine', + 'PhabricatorEditEngineSelectCommentAction' => 'PhabricatorEditEngineCommentAction', + 'PhabricatorEditEngineTokenizerCommentAction' => 'PhabricatorEditEngineCommentAction', 'PhabricatorEditField' => 'Phobject', 'PhabricatorEditType' => 'Phobject', 'PhabricatorEditor' => 'Phobject', diff --git a/src/applications/maniphest/editor/ManiphestEditEngine.php b/src/applications/maniphest/editor/ManiphestEditEngine.php index 2b3655677c..276eaa2a98 100644 --- a/src/applications/maniphest/editor/ManiphestEditEngine.php +++ b/src/applications/maniphest/editor/ManiphestEditEngine.php @@ -110,7 +110,7 @@ final class ManiphestEditEngine ->setIsCopyable(true) ->setSingleValue($object->getOwnerPHID()) ->setCommentActionLabel(pht('Assign / Claim')) - ->setCommentActionDefaultValue($owner_value), + ->setCommentActionValue($owner_value), id(new PhabricatorSelectEditField()) ->setKey('status') ->setLabel(pht('Status')) @@ -120,7 +120,7 @@ final class ManiphestEditEngine ->setValue($object->getStatus()) ->setOptions($status_map) ->setCommentActionLabel(pht('Change Status')) - ->setCommentActionDefaultValue($default_status), + ->setCommentActionValue($default_status), id(new PhabricatorSelectEditField()) ->setKey('priority') ->setLabel(pht('Priority')) diff --git a/src/applications/project/engineextension/PhabricatorProjectsEditEngineExtension.php b/src/applications/project/engineextension/PhabricatorProjectsEditEngineExtension.php index 4aadf75e5e..0775b24464 100644 --- a/src/applications/project/engineextension/PhabricatorProjectsEditEngineExtension.php +++ b/src/applications/project/engineextension/PhabricatorProjectsEditEngineExtension.php @@ -54,7 +54,7 @@ final class PhabricatorProjectsEditEngineExtension pht('Add projects.'), pht('Remove projects.'), pht('Set associated projects, overwriting current value.')) - ->setCommentActionLabel(pht('Add Projects')) + ->setCommentActionLabel(pht('Change Projects')) ->setTransactionType($edge_type) ->setMetadataValue('edge:type', $project_edge_type) ->setValue($project_phids); diff --git a/src/applications/subscriptions/engineextension/PhabricatorSubscriptionsEditEngineExtension.php b/src/applications/subscriptions/engineextension/PhabricatorSubscriptionsEditEngineExtension.php index ea7c2d4d4b..ae67caee12 100644 --- a/src/applications/subscriptions/engineextension/PhabricatorSubscriptionsEditEngineExtension.php +++ b/src/applications/subscriptions/engineextension/PhabricatorSubscriptionsEditEngineExtension.php @@ -49,7 +49,7 @@ final class PhabricatorSubscriptionsEditEngineExtension pht('Add subscribers.'), pht('Remove subscribers.'), pht('Set subscribers, overwriting current value.')) - ->setCommentActionLabel(pht('Add Subscribers')) + ->setCommentActionLabel(pht('Change Subscribers')) ->setTransactionType($subscribers_type) ->setValue($sub_phids); diff --git a/src/applications/transactions/commentaction/PhabricatorEditEngineCommentAction.php b/src/applications/transactions/commentaction/PhabricatorEditEngineCommentAction.php new file mode 100644 index 0000000000..548039ce15 --- /dev/null +++ b/src/applications/transactions/commentaction/PhabricatorEditEngineCommentAction.php @@ -0,0 +1,49 @@ +key = $key; + return $this; + } + + public function getKey() { + return $this->key; + } + + public function setLabel($label) { + $this->label = $label; + return $this; + } + + public function getLabel() { + return $this->label; + } + + public function setValue($value) { + $this->value = $value; + return $this; + } + + public function getValue() { + return $this->value; + } + + public function setInitialValue($initial_value) { + $this->initialValue = $initial_value; + return $this; + } + + public function getInitialValue() { + return $this->initialValue; + } + +} diff --git a/src/applications/transactions/commentaction/PhabricatorEditEngineSelectCommentAction.php b/src/applications/transactions/commentaction/PhabricatorEditEngineSelectCommentAction.php new file mode 100644 index 0000000000..03b5cf88fe --- /dev/null +++ b/src/applications/transactions/commentaction/PhabricatorEditEngineSelectCommentAction.php @@ -0,0 +1,29 @@ +options = $options; + return $this; + } + + public function getOptions() { + return $this->options; + } + + public function getPHUIXControlType() { + return 'select'; + } + + public function getPHUIXControlSpecification() { + return array( + 'options' => $this->getOptions(), + 'order' => array_keys($this->getOptions()), + 'value' => $this->getValue(), + ); + } + +} diff --git a/src/applications/transactions/commentaction/PhabricatorEditEngineTokenizerCommentAction.php b/src/applications/transactions/commentaction/PhabricatorEditEngineTokenizerCommentAction.php new file mode 100644 index 0000000000..d754592eb5 --- /dev/null +++ b/src/applications/transactions/commentaction/PhabricatorEditEngineTokenizerCommentAction.php @@ -0,0 +1,55 @@ +datasource = $datasource; + return $this; + } + + public function getDatasource() { + return $this->datasource; + } + + public function setLimit($limit) { + $this->limit = $limit; + return $this; + } + + public function getLimit() { + return $this->limit; + } + + public function getPHUIXControlType() { + return 'tokenizer'; + } + + public function getPHUIXControlSpecification() { + $template = new AphrontTokenizerTemplateView(); + + $datasource = $this->getDatasource(); + $limit = $this->getLimit(); + + $value = $this->getValue(); + if (!$value) { + $value = array(); + } + $value = $datasource->getWireTokens($value); + + return array( + 'markup' => $template->render(), + 'config' => array( + 'src' => $datasource->getDatasourceURI(), + 'browseURI' => $datasource->getBrowseURI(), + 'placeholder' => $datasource->getPlaceholderText(), + 'limit' => $limit, + ), + 'value' => $value, + ); + } + +} diff --git a/src/applications/transactions/editengine/PhabricatorEditEngine.php b/src/applications/transactions/editengine/PhabricatorEditEngine.php index 1526dfc1b9..02e226d7b4 100644 --- a/src/applications/transactions/editengine/PhabricatorEditEngine.php +++ b/src/applications/transactions/editengine/PhabricatorEditEngine.php @@ -792,10 +792,17 @@ abstract class PhabricatorEditEngine $validation_exception = null; if ($request->isFormPost()) { - foreach ($fields as $field) { + $submit_fields = $fields; + + foreach ($submit_fields as $key => $field) { + if (!$field->shouldGenerateTransactionsFromSubmit()) { + unset($submit_fields[$key]); + continue; + } + $field->setIsSubmittedForm(true); - if ($field->getIsLocked() || $field->getIsHidden()) { + if (!$field->shouldReadValueFromSubmit()) { continue; } @@ -803,22 +810,15 @@ abstract class PhabricatorEditEngine } $xactions = array(); - foreach ($fields as $field) { - $types = $field->getWebEditTypes(); - foreach ($types as $type) { - $type_xactions = $type->generateTransactions( - clone $template, - array( - 'value' => $field->getValueForTransaction(), - )); + foreach ($submit_fields as $field) { + $type_xactions = $field->generateTransactions( + clone $template, + array( + 'value' => $field->getValueForTransaction(), + )); - if (!$type_xactions) { - continue; - } - - foreach ($type_xactions as $type_xaction) { - $xactions[] = $type_xaction; - } + foreach ($type_xactions as $type_xaction) { + $xactions[] = $type_xaction; } } @@ -879,7 +879,7 @@ abstract class PhabricatorEditEngine } foreach ($fields as $field) { - if ($field->getIsLocked() || $field->getIsHidden()) { + if (!$field->shouldReadValueFromRequest()) { continue; } @@ -1171,19 +1171,25 @@ abstract class PhabricatorEditEngine $fields = $this->buildEditFields($object); - $all_types = array(); + $comment_actions = array(); foreach ($fields as $field) { - if (!$this->isCommentField($field)) { + if (!$field->shouldGenerateTransactionsFromComment()) { continue; } - $types = $field->getCommentEditTypes(); - foreach ($types as $type) { - $all_types[] = $type; + $comment_action = $field->getCommentAction(); + if (!$comment_action) { + continue; } + + $key = $comment_action->getKey(); + + // TODO: Validate these better. + + $comment_actions[$key] = $comment_action; } - $view->setEditTypes($all_types); + $view->setCommentActions($comment_actions); return $view; } @@ -1378,39 +1384,35 @@ abstract class PhabricatorEditEngine $xactions = array(); if ($actions) { - $type_map = array(); - foreach ($fields as $field) { - if (!$this->isCommentField($field)) { - continue; - } - - $types = $field->getCommentEditTypes(); - foreach ($types as $type) { - $type_map[$type->getEditType()] = array( - 'type' => $type, - 'field' => $field, - ); - } - } - + $action_map = array(); foreach ($actions as $action) { $type = idx($action, 'type'); if (!$type) { continue; } - $spec = idx($type_map, $type); - if (!$spec) { + if (empty($fields[$type])) { continue; } - $edit_type = $spec['type']; - $field = $spec['field']; + $action_map[$type] = $action; + } - $field->readValueFromComment($action); + foreach ($action_map as $type => $action) { + $field = $fields[$type]; - $type_xactions = $edit_type->generateTransactions( - $template, + if (!$field->shouldGenerateTransactionsFromComment()) { + continue; + } + + if (array_key_exists('initialValue', $action)) { + $field->setInitialValue($action['initialValue']); + } + + $field->readValueFromComment(idx($action, 'value')); + + $type_xactions = $field->generateTransactions( + clone $template, array( 'value' => $field->getValueForTransaction(), )); @@ -1509,6 +1511,10 @@ abstract class PhabricatorEditEngine ->setContentSourceFromConduitRequest($request) ->setContinueOnNoEffect(true); + if (!$this->getIsCreate()) { + $editor->setContinueOnMissingFields(true); + } + $xactions = $editor->applyTransactions($object, $xactions); $xactions_struct = array(); @@ -1729,23 +1735,6 @@ abstract class PhabricatorEditEngine PhabricatorPolicyCapability::CAN_EDIT); } - private function isCommentField(PhabricatorEditField $field) { - // TODO: This is a little bit hacky. - if ($field->getKey() == 'comment') { - return true; - } - - if ($field->getIsLocked()) { - return false; - } - - if ($field->getIsHidden()) { - return false; - } - - return true; - } - /* -( PhabricatorPolicyInterface )----------------------------------------- */ diff --git a/src/applications/transactions/editfield/PhabricatorCommentEditField.php b/src/applications/transactions/editfield/PhabricatorCommentEditField.php index 7a7e671e8c..d3ff1374b3 100644 --- a/src/applications/transactions/editfield/PhabricatorCommentEditField.php +++ b/src/applications/transactions/editfield/PhabricatorCommentEditField.php @@ -11,4 +11,8 @@ final class PhabricatorCommentEditField return new PhabricatorCommentEditType(); } + public function shouldGenerateTransactionsFromComment() { + return true; + } + } diff --git a/src/applications/transactions/editfield/PhabricatorEditField.php b/src/applications/transactions/editfield/PhabricatorEditField.php index 44807de725..77a90f2d03 100644 --- a/src/applications/transactions/editfield/PhabricatorEditField.php +++ b/src/applications/transactions/editfield/PhabricatorEditField.php @@ -15,7 +15,10 @@ abstract class PhabricatorEditField extends Phobject { private $description; private $editTypeKey; private $isRequired; + private $commentActionLabel; + private $commentActionValue; + private $hasCommentActionValue; private $isLocked; private $isHidden; @@ -202,6 +205,16 @@ abstract class PhabricatorEditField extends Phobject { return $this->commentActionLabel; } + public function setCommentActionValue($comment_action_value) { + $this->hasCommentActionValue = true; + $this->commentActionValue = $comment_action_value; + return $this; + } + + public function getCommentActionValue() { + return $this->commentActionValue; + } + protected function newControl() { throw new PhutilMethodNotImplementedException(); } @@ -345,8 +358,8 @@ abstract class PhabricatorEditField extends Phobject { return $this; } - public function readValueFromComment($action) { - $this->value = $this->getValueFromComment(idx($action, 'value')); + public function readValueFromComment($value) { + $this->value = $this->getValueFromComment($value); return $this; } @@ -424,6 +437,11 @@ abstract class PhabricatorEditField extends Phobject { return $this->initialValue; } + public function setInitialValue($initial_value) { + $this->initialValue = $initial_value; + return $this; + } + public function readValueFromSubmit(AphrontRequest $request) { $key = $this->getKey(); if ($this->getValueExistsInSubmit($request, $key)) { @@ -548,22 +566,115 @@ abstract class PhabricatorEditField extends Phobject { return array($edit_type); } - public function getWebEditTypes() { + public function getCommentAction() { + $label = $this->getCommentActionLabel(); + if ($label === null) { + return null; + } + + $action = $this->newCommentAction(); + if ($action === null) { + return null; + } + + if ($this->hasCommentActionValue) { + $value = $this->getCommentActionValue(); + } else { + $value = $this->getValue(); + } + + $action + ->setKey($this->getKey()) + ->setLabel($label) + ->setValue($this->getValueForCommentAction($value)); + + return $action; + } + + protected function newCommentAction() { + return null; + } + + protected function getValueForCommentAction($value) { + return $value; + } + + public function shouldGenerateTransactionsFromSubmit() { if ($this->getIsConduitOnly()) { - return array(); + return false; } $edit_type = $this->getEditType(); - - if ($edit_type === null) { - return array(); + if (!$edit_type) { + return false; } - return array($edit_type); + return true; } - public function getCommentEditTypes() { - return array(); + public function shouldReadValueFromRequest() { + if ($this->getIsConduitOnly()) { + return false; + } + + if ($this->getIsLocked()) { + return false; + } + + if ($this->getIsHidden()) { + return false; + } + + return true; + } + + public function shouldReadValueFromSubmit() { + if ($this->getIsConduitOnly()) { + return false; + } + + if ($this->getIsLocked()) { + return false; + } + + if ($this->getIsHidden()) { + return false; + } + + return true; + } + + public function shouldGenerateTransactionsFromComment() { + if ($this->getIsConduitOnly()) { + return false; + } + + if ($this->getIsLocked()) { + return false; + } + + if ($this->getIsHidden()) { + return false; + } + + return true; + } + + public function generateTransactions( + PhabricatorApplicationTransaction $template, + array $spec) { + + $edit_type = $this->getEditType(); + if (!$edit_type) { + throw new Exception( + pht( + 'EditField (with key "%s", of class "%s") is generating '. + 'transactions, but has no EditType.', + $this->getKey(), + get_class($this))); + } + + return $edit_type->generateTransactions($template, $spec); } } diff --git a/src/applications/transactions/editfield/PhabricatorPHIDListEditField.php b/src/applications/transactions/editfield/PhabricatorPHIDListEditField.php index b42cf806d4..f2663d2063 100644 --- a/src/applications/transactions/editfield/PhabricatorPHIDListEditField.php +++ b/src/applications/transactions/editfield/PhabricatorPHIDListEditField.php @@ -44,14 +44,6 @@ abstract class PhabricatorPHIDListEditField return new AphrontPHIDListHTTPParameterType(); } - public function readValueFromComment($value) { - // TODO: This is really hacky -- make sure we pass a plain PHID list to - // the edit type. This method probably needs to move down to EditType, and - // maybe more additional logic does too. - $this->setUseEdgeTransactions(false); - return parent::readValueFromComment($value); - } - protected function getValueFromRequest(AphrontRequest $request, $key) { $value = parent::getValueFromRequest($request, $key); if ($this->getIsSingleValue()) { diff --git a/src/applications/transactions/editfield/PhabricatorSelectEditField.php b/src/applications/transactions/editfield/PhabricatorSelectEditField.php index b7693cb625..921e5b3d11 100644 --- a/src/applications/transactions/editfield/PhabricatorSelectEditField.php +++ b/src/applications/transactions/editfield/PhabricatorSelectEditField.php @@ -4,7 +4,6 @@ final class PhabricatorSelectEditField extends PhabricatorEditField { private $options; - private $commentActionDefaultValue; public function setOptions(array $options) { $this->options = $options; @@ -18,15 +17,6 @@ final class PhabricatorSelectEditField return $this->options; } - public function setCommentActionDefaultValue($default) { - $this->commentActionDefaultValue = $default; - return $this; - } - - public function getCommentActionDefaultValue() { - return $this->commentActionDefaultValue; - } - protected function newControl() { return id(new AphrontFormSelectControl()) ->setOptions($this->getOptions()); @@ -36,28 +26,9 @@ final class PhabricatorSelectEditField return new AphrontSelectHTTPParameterType(); } - public function getCommentEditTypes() { - $label = $this->getCommentActionLabel(); - if ($label === null) { - return array(); - } - - $default_value = $this->getCommentActionDefaultValue(); - if ($default_value === null) { - $default_value = $this->getValue(); - } - - $edit = $this->getEditType() - ->setLabel($label) - ->setPHUIXControlType('select') - ->setPHUIXControlSpecification( - array( - 'options' => $this->getOptions(), - 'order' => array_keys($this->getOptions()), - 'value' => $default_value, - )); - - return array($edit); + protected function newCommentAction() { + return id(new PhabricatorEditEngineSelectCommentAction()) + ->setOptions($this->getOptions()); } } diff --git a/src/applications/transactions/editfield/PhabricatorTokenizerEditField.php b/src/applications/transactions/editfield/PhabricatorTokenizerEditField.php index e252465ac9..237f315b51 100644 --- a/src/applications/transactions/editfield/PhabricatorTokenizerEditField.php +++ b/src/applications/transactions/editfield/PhabricatorTokenizerEditField.php @@ -3,26 +3,15 @@ abstract class PhabricatorTokenizerEditField extends PhabricatorPHIDListEditField { - private $commentActionDefaultValue; - abstract protected function newDatasource(); - public function setCommentActionDefaultValue(array $default) { - $this->commentActionDefaultValue = $default; - return $this; - } - - public function getCommentActionDefaultValue() { - return $this->commentActionDefaultValue; - } - protected function newControl() { $control = id(new AphrontFormTokenizerControl()) ->setDatasource($this->newDatasource()); $initial_value = $this->getInitialValue(); if ($initial_value !== null) { - $control->setOriginalValue($initial_value); + $control->setInitialValue($initial_value); } if ($this->getIsSingleValue()) { @@ -33,7 +22,7 @@ abstract class PhabricatorTokenizerEditField } protected function getInitialValueFromSubmit(AphrontRequest $request, $key) { - return $request->getArr($key.'.original'); + return $request->getArr($key.'.initial'); } protected function newEditType() { @@ -46,38 +35,25 @@ abstract class PhabricatorTokenizerEditField return $type; } - public function getCommentEditTypes() { - $label = $this->getCommentActionLabel(); - if ($label === null) { - return array(); + protected function newCommentAction() { + $viewer = $this->getViewer(); + + $datasource = $this->newDatasource() + ->setViewer($viewer); + + $action = id(new PhabricatorEditEngineTokenizerCommentAction()) + ->setDatasource($datasource); + + if ($this->getIsSingleValue()) { + $action->setLimit(1); } - $transaction_type = $this->getTransactionType(); - if ($transaction_type === null) { - return array(); + $initial_value = $this->getInitialValue(); + if ($initial_value !== null) { + $action->setInitialValue($initial_value); } - if ($this->getUseEdgeTransactions()) { - $type_key = $this->getEditTypeKey(); - $base = $this->getEditType(); - - $add = id(clone $base) - ->setEditType($type_key.'.add') - ->setEdgeOperation('+') - ->setLabel($label); - - return array($add); - } - - $edit = $this->getEditType() - ->setLabel($label); - - $default = $this->getCommentActionDefaultValue(); - if ($default) { - $edit->setDefaultValue($default); - } - - return array($edit); + return $action; } } diff --git a/src/applications/transactions/edittype/PhabricatorEditType.php b/src/applications/transactions/edittype/PhabricatorEditType.php index 562e54b7b5..c376046374 100644 --- a/src/applications/transactions/edittype/PhabricatorEditType.php +++ b/src/applications/transactions/edittype/PhabricatorEditType.php @@ -96,16 +96,4 @@ abstract class PhabricatorEditType extends Phobject { return $xaction; } - public function getPHUIXControlType() { - return null; - } - - public function getPHUIXControlSpecification() { - return null; - } - - public function getCommentActionValueFromDraftValue($value) { - return $value; - } - } diff --git a/src/applications/transactions/edittype/PhabricatorPHIDListEditType.php b/src/applications/transactions/edittype/PhabricatorPHIDListEditType.php index 2913b210b5..742daff0df 100644 --- a/src/applications/transactions/edittype/PhabricatorPHIDListEditType.php +++ b/src/applications/transactions/edittype/PhabricatorPHIDListEditType.php @@ -42,63 +42,4 @@ abstract class PhabricatorPHIDListEditType } } - public function getPHUIXControlType() { - $datasource = $this->getDatasource(); - - if (!$datasource) { - return null; - } - - return 'tokenizer'; - } - - public function getPHUIXControlSpecification() { - $datasource = $this->getDatasource(); - - if (!$datasource) { - return null; - } - - $template = new AphrontTokenizerTemplateView(); - - if ($this->getIsSingleValue()) { - $limit = 1; - } else { - $limit = null; - } - - $default = $this->getDefaultValue(); - if ($default) { - $value = $datasource->getWireTokens($default); - } else { - $value = array(); - } - - return array( - 'markup' => $template->render(), - 'config' => array( - 'src' => $datasource->getDatasourceURI(), - 'browseURI' => $datasource->getBrowseURI(), - 'placeholder' => $datasource->getPlaceholderText(), - 'limit' => $limit, - ), - 'value' => $value, - ); - } - - public function getCommentActionValueFromDraftValue($value) { - $datasource = $this->getDatasource(); - - if (!$datasource) { - return array(); - } - - if (!is_array($value)) { - return array(); - } - - return $datasource->getWireTokens($value); - } - - } diff --git a/src/applications/transactions/edittype/PhabricatorSimpleEditType.php b/src/applications/transactions/edittype/PhabricatorSimpleEditType.php index dcc18f64f1..d41d33f5c3 100644 --- a/src/applications/transactions/edittype/PhabricatorSimpleEditType.php +++ b/src/applications/transactions/edittype/PhabricatorSimpleEditType.php @@ -35,22 +35,4 @@ final class PhabricatorSimpleEditType extends PhabricatorEditType { return $this->valueDescription; } - public function setPHUIXControlType($type) { - $this->phuixControlType = $type; - return $this; - } - - public function getPHUIXControlType() { - return $this->phuixControlType; - } - - public function setPHUIXControlSpecification(array $spec) { - $this->phuixControlSpecification = $spec; - return $this; - } - - public function getPHUIXControlSpecification() { - return $this->phuixControlSpecification; - } - } diff --git a/src/applications/transactions/view/PhabricatorApplicationTransactionCommentView.php b/src/applications/transactions/view/PhabricatorApplicationTransactionCommentView.php index c33ea44be2..819ab6c8e4 100644 --- a/src/applications/transactions/view/PhabricatorApplicationTransactionCommentView.php +++ b/src/applications/transactions/view/PhabricatorApplicationTransactionCommentView.php @@ -20,11 +20,9 @@ class PhabricatorApplicationTransactionCommentView extends AphrontView { private $headerText; private $noPermission; - - private $currentVersion; private $versionedDraft; - private $editTypes; + private $commentActions; private $transactionTimeline; public function setObjectPHID($object_phid) { @@ -104,13 +102,14 @@ class PhabricatorApplicationTransactionCommentView extends AphrontView { return $this; } - public function setEditTypes($edit_types) { - $this->editTypes = $edit_types; + public function setCommentActions(array $comment_actions) { + assert_instances_of($comment_actions, 'PhabricatorEditEngineCommentAction'); + $this->commentActions = $comment_actions; return $this; } - public function getEditTypes() { - return $this->editTypes; + public function getCommentActions() { + return $this->commentActions; } public function setNoPermission($no_permission) { @@ -166,7 +165,7 @@ class PhabricatorApplicationTransactionCommentView extends AphrontView { $preview = null; } - if (!$this->getEditTypes()) { + if (!$this->getCommentActions()) { Javelin::initBehavior( 'phabricator-transaction-comment-form', array( @@ -219,21 +218,47 @@ class PhabricatorApplicationTransactionCommentView extends AphrontView { ->addHiddenInput('__draft__', $draft_key) ->addHiddenInput($version_key, $version_value); - $edit_types = $this->getEditTypes(); - if ($edit_types) { - + $comment_actions = $this->getCommentActions(); + if ($comment_actions) { $action_map = array(); $type_map = array(); - foreach ($edit_types as $edit_type) { - $key = $edit_type->getEditType(); + + $comment_actions = mpull($comment_actions, null, 'getKey'); + + $draft_actions = array(); + $draft_keys = array(); + if ($versioned_draft) { + $draft_actions = $versioned_draft->getProperty('actions', array()); + + if (!is_array($draft_actions)) { + $draft_actions = array(); + } + + foreach ($draft_actions as $action) { + $type = idx($action, 'type'); + $comment_action = idx($comment_actions, $type); + if (!$comment_action) { + continue; + } + + $value = idx($action, 'value'); + $comment_action->setValue($value); + + $draft_keys[] = $type; + } + } + + foreach ($comment_actions as $key => $comment_action) { + $key = $comment_action->getKey(); $action_map[$key] = array( 'key' => $key, - 'label' => $edit_type->getLabel(), - 'type' => $edit_type->getPHUIXControlType(), - 'spec' => $edit_type->getPHUIXControlSpecification(), + 'label' => $comment_action->getLabel(), + 'type' => $comment_action->getPHUIXControlType(), + 'spec' => $comment_action->getPHUIXControlSpecification(), + 'initialValue' => $comment_action->getInitialValue(), ); - $type_map[$key] = $edit_type; + $type_map[$key] = $comment_action; } $options = array(); @@ -270,28 +295,6 @@ class PhabricatorApplicationTransactionCommentView extends AphrontView { 'id' => $place_id, ))); - $draft_actions = array(); - if ($versioned_draft) { - $draft_actions = $versioned_draft->getProperty('actions', array()); - foreach ($draft_actions as $key => $action) { - $type = idx($action, 'type'); - if (!$type) { - unset($draft_actions[$key]); - continue; - } - - $edit_type = idx($type_map, $type); - if (!$edit_type) { - unset($draft_actions[$key]); - continue; - } - - $value = idx($action, 'value'); - $value = $edit_type->getCommentActionValueFromDraftValue($value); - $draft_actions[$key]['value'] = $value; - } - } - Javelin::initBehavior( 'comment-actions', array( @@ -304,7 +307,7 @@ class PhabricatorApplicationTransactionCommentView extends AphrontView { 'actions' => $action_map, 'showPreview' => $this->getShowPreview(), 'actionURI' => $this->getAction(), - 'drafts' => $draft_actions, + 'drafts' => $draft_keys, )); } diff --git a/src/view/control/AphrontTokenizerTemplateView.php b/src/view/control/AphrontTokenizerTemplateView.php index b8c2c6f2cc..74456e84b8 100644 --- a/src/view/control/AphrontTokenizerTemplateView.php +++ b/src/view/control/AphrontTokenizerTemplateView.php @@ -6,7 +6,7 @@ final class AphrontTokenizerTemplateView extends AphrontView { private $name; private $id; private $browseURI; - private $originalValue; + private $initialValue; public function setBrowseURI($browse_uri) { $this->browseURI = $browse_uri; @@ -37,13 +37,13 @@ final class AphrontTokenizerTemplateView extends AphrontView { return $this->name; } - public function setOriginalValue(array $original_value) { - $this->originalValue = $original_value; + public function setInitialValue(array $initial_value) { + $this->initialValue = $initial_value; return $this; } - public function getOriginalValue() { - return $this->originalValue; + public function getInitialValue() { + return $this->initialValue; } public function render() { @@ -95,15 +95,15 @@ final class AphrontTokenizerTemplateView extends AphrontView { $classes[] = 'has-browse'; } - $original = array(); - $original_value = $this->getOriginalValue(); - if ($original_value) { - foreach ($this->getOriginalValue() as $value) { - $original[] = phutil_tag( + $initial = array(); + $initial_value = $this->getInitialValue(); + if ($initial_value) { + foreach ($this->getInitialValue() as $value) { + $initial[] = phutil_tag( 'input', array( 'type' => 'hidden', - 'name' => $name.'.original[]', + 'name' => $name.'.initial[]', 'value' => $value, )); } @@ -118,7 +118,7 @@ final class AphrontTokenizerTemplateView extends AphrontView { array( $container, $browse, - $original, + $initial, )); return $frame; diff --git a/src/view/form/control/AphrontFormTokenizerControl.php b/src/view/form/control/AphrontFormTokenizerControl.php index 60c8962e78..4994d89834 100644 --- a/src/view/form/control/AphrontFormTokenizerControl.php +++ b/src/view/form/control/AphrontFormTokenizerControl.php @@ -7,7 +7,7 @@ final class AphrontFormTokenizerControl extends AphrontFormControl { private $limit; private $placeholder; private $handles; - private $originalValue; + private $initialValue; public function setDatasource(PhabricatorTypeaheadDatasource $datasource) { $this->datasource = $datasource; @@ -33,13 +33,13 @@ final class AphrontFormTokenizerControl extends AphrontFormControl { return $this; } - public function setOriginalValue(array $original_value) { - $this->originalValue = $original_value; + public function setInitialValue(array $initial_value) { + $this->initialValue = $initial_value; return $this; } - public function getOriginalValue() { - return $this->originalValue; + public function getInitialValue() { + return $this->initialValue; } public function willRender() { @@ -84,9 +84,9 @@ final class AphrontFormTokenizerControl extends AphrontFormControl { ->setID($id) ->setValue($tokens); - $original_value = $this->getOriginalValue(); - if ($original_value !== null) { - $template->setOriginalValue($original_value); + $initial_value = $this->getInitialValue(); + if ($initial_value !== null) { + $template->setInitialValue($initial_value); } $username = null; diff --git a/webroot/rsrc/js/application/transactions/behavior-comment-actions.js b/webroot/rsrc/js/application/transactions/behavior-comment-actions.js index 1240c71bed..c121111077 100644 --- a/webroot/rsrc/js/application/transactions/behavior-comment-actions.js +++ b/webroot/rsrc/js/application/transactions/behavior-comment-actions.js @@ -80,7 +80,8 @@ JX.behavior('comment-actions', function(config) { for (var k in rows) { data.push({ type: k, - value: rows[k].getValue() + value: rows[k].getValue(), + initialValue: action_map[k].initialValue || null }); } @@ -104,13 +105,12 @@ JX.behavior('comment-actions', function(config) { for (var ii = 0; ii < drafts.length; ii++) { draft = drafts[ii]; - option = find_option(draft.type); + option = find_option(draft); if (!option) { continue; } control = add_row(option); - control.setValue(draft.value); } } @@ -133,6 +133,7 @@ JX.behavior('comment-actions', function(config) { input_node.value = serialize_actions(); }); + if (config.showPreview) { var request = new JX.PhabricatorShapedRequest( config.actionURI, @@ -154,5 +155,4 @@ JX.behavior('comment-actions', function(config) { } restore_draft_actions(config.drafts || []); - });