From add8333b98cb68dd73bdf2d2a32b97211c83ce9c Mon Sep 17 00:00:00 2001 From: epriestley Date: Mon, 11 Jan 2016 03:24:03 -0800 Subject: [PATCH] Improve behavior of "owner" transaction in "maniphest.edit" endpoint Summary: Fixes T10117. - I accidentally broke setting `null` to unassign tasks at some point when I added richer validation. - Raise a better error if the user passes junk. Test Plan: - Unassigned a task via API and web UI. - Reassigned a task via API and web UI. - Tried to do an invalid assign via API, got a sensible error. Reviewers: chad Reviewed By: chad Maniphest Tasks: T10117 Differential Revision: https://secure.phabricator.com/D14992 --- src/__phutil_library_map__.php | 2 + .../ConduitUserParameterType.php | 47 +++++++++++++++++++ .../editor/ManiphestTransactionEditor.php | 27 +++++++++++ .../editengine/PhabricatorEditEngine.php | 3 ++ .../PhabricatorPHIDListEditField.php | 1 + .../editfield/PhabricatorUsersEditField.php | 6 ++- 6 files changed, 85 insertions(+), 1 deletion(-) create mode 100644 src/applications/conduit/parametertype/ConduitUserParameterType.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 2a0de867aa..1782d3dbcb 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -249,6 +249,7 @@ phutil_register_library_map(array( 'ConduitStringParameterType' => 'applications/conduit/parametertype/ConduitStringParameterType.php', 'ConduitTokenGarbageCollector' => 'applications/conduit/garbagecollector/ConduitTokenGarbageCollector.php', 'ConduitUserListParameterType' => 'applications/conduit/parametertype/ConduitUserListParameterType.php', + 'ConduitUserParameterType' => 'applications/conduit/parametertype/ConduitUserParameterType.php', 'ConduitWildParameterType' => 'applications/conduit/parametertype/ConduitWildParameterType.php', 'ConpherenceColumnViewController' => 'applications/conpherence/controller/ConpherenceColumnViewController.php', 'ConpherenceConduitAPIMethod' => 'applications/conpherence/conduit/ConpherenceConduitAPIMethod.php', @@ -4184,6 +4185,7 @@ phutil_register_library_map(array( 'ConduitStringParameterType' => 'ConduitParameterType', 'ConduitTokenGarbageCollector' => 'PhabricatorGarbageCollector', 'ConduitUserListParameterType' => 'ConduitListParameterType', + 'ConduitUserParameterType' => 'ConduitParameterType', 'ConduitWildParameterType' => 'ConduitListParameterType', 'ConpherenceColumnViewController' => 'ConpherenceController', 'ConpherenceConduitAPIMethod' => 'ConduitAPIMethod', diff --git a/src/applications/conduit/parametertype/ConduitUserParameterType.php b/src/applications/conduit/parametertype/ConduitUserParameterType.php new file mode 100644 index 0000000000..3590d1a405 --- /dev/null +++ b/src/applications/conduit/parametertype/ConduitUserParameterType.php @@ -0,0 +1,47 @@ +raiseValidationException( + $request, + $key, + pht('Expected PHID or null, got something else.')); + } + + $user_phids = id(new PhabricatorUserPHIDResolver()) + ->setViewer($this->getViewer()) + ->resolvePHIDs(array($value)); + + return nonempty(head($user_phids), null); + } + + protected function getParameterTypeName() { + return 'phid|string|null'; + } + + protected function getParameterFormatDescriptions() { + return array( + pht('User PHID.'), + pht('Username.'), + pht('Literal null.'), + ); + } + + protected function getParameterExamples() { + return array( + '"PHID-USER-1111"', + '"alincoln"', + 'null', + ); + } + +} diff --git a/src/applications/maniphest/editor/ManiphestTransactionEditor.php b/src/applications/maniphest/editor/ManiphestTransactionEditor.php index 42170d4c53..72979d3a4e 100644 --- a/src/applications/maniphest/editor/ManiphestTransactionEditor.php +++ b/src/applications/maniphest/editor/ManiphestTransactionEditor.php @@ -829,6 +829,33 @@ final class ManiphestTransactionEditor last($with_effect)); } break; + case ManiphestTransaction::TYPE_OWNER: + foreach ($xactions as $xaction) { + $old = $xaction->getOldValue(); + $new = $xaction->getNewValue(); + if (!strlen($new)) { + continue; + } + + if ($new === $old) { + continue; + } + + $assignee_list = id(new PhabricatorPeopleQuery()) + ->setViewer($this->getActor()) + ->withPHIDs(array($new)) + ->execute(); + if (!$assignee_list) { + $errors[] = new PhabricatorApplicationTransactionValidationError( + $type, + pht('Invalid'), + pht( + 'User "%s" is not a valid user.', + $new), + $xaction); + } + } + break; } return $errors; diff --git a/src/applications/transactions/editengine/PhabricatorEditEngine.php b/src/applications/transactions/editengine/PhabricatorEditEngine.php index e89fea3604..6f1a2a669a 100644 --- a/src/applications/transactions/editengine/PhabricatorEditEngine.php +++ b/src/applications/transactions/editengine/PhabricatorEditEngine.php @@ -1632,6 +1632,7 @@ abstract class PhabricatorEditEngine array $types, PhabricatorApplicationTransaction $template) { + $viewer = $request->getUser(); $transactions_key = 'transactions'; $xactions = $request->getValue($transactions_key); @@ -1688,6 +1689,8 @@ abstract class PhabricatorEditEngine // use usernames in list fields, for example. $parameter_type = $type->getConduitParameterType(); + $parameter_type->setViewer($viewer); + try { $xaction['value'] = $parameter_type->getValue($xaction, 'value'); } catch (Exception $ex) { diff --git a/src/applications/transactions/editfield/PhabricatorPHIDListEditField.php b/src/applications/transactions/editfield/PhabricatorPHIDListEditField.php index 4968aef5b2..7195741c43 100644 --- a/src/applications/transactions/editfield/PhabricatorPHIDListEditField.php +++ b/src/applications/transactions/editfield/PhabricatorPHIDListEditField.php @@ -100,6 +100,7 @@ abstract class PhabricatorPHIDListEditField $type = new PhabricatorDatasourceEditType(); $type->setIsSingleValue($this->getIsSingleValue()); + $type->setConduitParameterType($this->newConduitParameterType()); return $type; } diff --git a/src/applications/transactions/editfield/PhabricatorUsersEditField.php b/src/applications/transactions/editfield/PhabricatorUsersEditField.php index 472d84141e..fd8b8384b8 100644 --- a/src/applications/transactions/editfield/PhabricatorUsersEditField.php +++ b/src/applications/transactions/editfield/PhabricatorUsersEditField.php @@ -12,7 +12,11 @@ final class PhabricatorUsersEditField } protected function newConduitParameterType() { - return new ConduitUserListParameterType(); + if ($this->getIsSingleValue()) { + return new ConduitUserParameterType(); + } else { + return new ConduitUserListParameterType(); + } } }