From b0a5eee2389ed8444f90636c463f64cb44a70ef0 Mon Sep 17 00:00:00 2001 From: epriestley Date: Tue, 15 Dec 2015 09:11:11 -0800 Subject: [PATCH] Support editing statuses and paths in Owners via Conduit API Summary: Ref T9964. Fixes T9752. Provides API access to enable/disable packages and change their paths. Test Plan: - Changed status via Conduit. - Changed paths via Conduit. - Tried to change a path use a nonsense/bogus repository PHID, got an error. Reviewers: chad Reviewed By: chad Maniphest Tasks: T9752, T9964 Differential Revision: https://secure.phabricator.com/D14790 --- src/__phutil_library_map__.php | 2 + .../PhabricatorOwnersPackageEditEngine.php | 17 +++- ...bricatorOwnersPackageTransactionEditor.php | 83 ++++++++++++++++++- ...catorOwnersPathsSearchEngineAttachment.php | 2 +- .../editfield/PhabricatorConduitEditField.php | 14 ++++ 5 files changed, 115 insertions(+), 3 deletions(-) create mode 100644 src/applications/transactions/editfield/PhabricatorConduitEditField.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 4fb7d43498..abc2a2d16d 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -1887,6 +1887,7 @@ phutil_register_library_map(array( 'PhabricatorConduitConsoleController' => 'applications/conduit/controller/PhabricatorConduitConsoleController.php', 'PhabricatorConduitController' => 'applications/conduit/controller/PhabricatorConduitController.php', 'PhabricatorConduitDAO' => 'applications/conduit/storage/PhabricatorConduitDAO.php', + 'PhabricatorConduitEditField' => 'applications/transactions/editfield/PhabricatorConduitEditField.php', 'PhabricatorConduitListController' => 'applications/conduit/controller/PhabricatorConduitListController.php', 'PhabricatorConduitLogController' => 'applications/conduit/controller/PhabricatorConduitLogController.php', 'PhabricatorConduitLogQuery' => 'applications/conduit/query/PhabricatorConduitLogQuery.php', @@ -6004,6 +6005,7 @@ phutil_register_library_map(array( 'PhabricatorConduitConsoleController' => 'PhabricatorConduitController', 'PhabricatorConduitController' => 'PhabricatorController', 'PhabricatorConduitDAO' => 'PhabricatorLiskDAO', + 'PhabricatorConduitEditField' => 'PhabricatorEditField', 'PhabricatorConduitListController' => 'PhabricatorConduitController', 'PhabricatorConduitLogController' => 'PhabricatorConduitController', 'PhabricatorConduitLogQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', diff --git a/src/applications/owners/editor/PhabricatorOwnersPackageEditEngine.php b/src/applications/owners/editor/PhabricatorOwnersPackageEditEngine.php index 0b6b5988ec..2cab68d7e6 100644 --- a/src/applications/owners/editor/PhabricatorOwnersPackageEditEngine.php +++ b/src/applications/owners/editor/PhabricatorOwnersPackageEditEngine.php @@ -18,7 +18,8 @@ final class PhabricatorOwnersPackageEditEngine } protected function newObjectQuery() { - return id(new PhabricatorOwnersPackageQuery()); + return id(new PhabricatorOwnersPackageQuery()) + ->needPaths(true); } protected function getObjectCreateTitleText($object) { @@ -81,6 +82,20 @@ final class PhabricatorOwnersPackageEditEngine ->setTransactionType( PhabricatorOwnersPackageTransaction::TYPE_DESCRIPTION) ->setValue($object->getDescription()), + id(new PhabricatorSelectEditField()) + ->setKey('status') + ->setLabel(pht('Status')) + ->setDescription(pht('Archive or enable the package.')) + ->setTransactionType(PhabricatorOwnersPackageTransaction::TYPE_STATUS) + ->setIsConduitOnly(true) + ->setValue($object->getStatus()) + ->setOptions($object->getStatusNameMap()), + id(new PhabricatorConduitEditField()) + ->setKey('paths.set') + ->setLabel(pht('Paths')) + ->setDescription(pht('Set paths for this package.')) + ->setIsConduitOnly(true) + ->setTransactionType(PhabricatorOwnersPackageTransaction::TYPE_PATHS), ); } diff --git a/src/applications/owners/editor/PhabricatorOwnersPackageTransactionEditor.php b/src/applications/owners/editor/PhabricatorOwnersPackageTransactionEditor.php index e267a0a0a9..a622129e2d 100644 --- a/src/applications/owners/editor/PhabricatorOwnersPackageTransactionEditor.php +++ b/src/applications/owners/editor/PhabricatorOwnersPackageTransactionEditor.php @@ -54,9 +54,14 @@ final class PhabricatorOwnersPackageTransactionEditor switch ($xaction->getTransactionType()) { case PhabricatorOwnersPackageTransaction::TYPE_NAME: case PhabricatorOwnersPackageTransaction::TYPE_DESCRIPTION: - case PhabricatorOwnersPackageTransaction::TYPE_PATHS: case PhabricatorOwnersPackageTransaction::TYPE_STATUS: return $xaction->getNewValue(); + case PhabricatorOwnersPackageTransaction::TYPE_PATHS: + $new = $xaction->getNewValue(); + foreach ($new as $key => $info) { + $new[$key]['excluded'] = (int)idx($info, 'excluded'); + } + return $new; case PhabricatorOwnersPackageTransaction::TYPE_AUDITING: return (int)$xaction->getNewValue(); case PhabricatorOwnersPackageTransaction::TYPE_OWNERS: @@ -198,6 +203,82 @@ final class PhabricatorOwnersPackageTransactionEditor $errors[] = $error; } break; + case PhabricatorOwnersPackageTransaction::TYPE_PATHS: + $old = mpull($object->getPaths(), 'getRef'); + foreach ($xactions as $xaction) { + $new = $xaction->getNewValue(); + + // Check that we have a list of paths. + if (!is_array($new)) { + $errors[] = new PhabricatorApplicationTransactionValidationError( + $type, + pht('Invalid'), + pht('Path specification must be a list of paths.'), + $xaction); + continue; + } + + // Check that each item in the list is formatted properly. + $type_exception = null; + foreach ($new as $key => $value) { + try { + PhutilTypeSpec::checkMap( + $value, + array( + 'repositoryPHID' => 'string', + 'path' => 'string', + 'excluded' => 'optional wild', + )); + } catch (PhutilTypeCheckException $ex) { + $errors[] = new PhabricatorApplicationTransactionValidationError( + $type, + pht('Invalid'), + pht( + 'Path specification list contains invalid value '. + 'in key "%s": %s.', + $key, + $ex->getMessage()), + $xaction); + $type_exception = $ex; + } + } + + if ($type_exception) { + continue; + } + + // Check that any new paths reference legitimate repositories which + // the viewer has permission to see. + list($rem, $add) = PhabricatorOwnersPath::getTransactionValueChanges( + $old, + $new); + + if ($add) { + $repository_phids = ipull($add, 'repositoryPHID'); + + $repositories = id(new PhabricatorRepositoryQuery()) + ->setViewer($this->getActor()) + ->withPHIDs($repository_phids) + ->execute(); + $repositories = mpull($repositories, null, 'getPHID'); + + foreach ($add as $ref) { + $repository_phid = $ref['repositoryPHID']; + if (isset($repositories[$repository_phid])) { + continue; + } + + $errors[] = new PhabricatorApplicationTransactionValidationError( + $type, + pht('Invalid'), + pht( + 'Path specification list references repository PHID "%s", '. + 'but that is not a valid, visible repository.', + $repository_phid)); + } + } + } + break; } return $errors; diff --git a/src/applications/owners/engineextension/PhabricatorOwnersPathsSearchEngineAttachment.php b/src/applications/owners/engineextension/PhabricatorOwnersPathsSearchEngineAttachment.php index bce6a76b60..ad5415ef69 100644 --- a/src/applications/owners/engineextension/PhabricatorOwnersPathsSearchEngineAttachment.php +++ b/src/applications/owners/engineextension/PhabricatorOwnersPathsSearchEngineAttachment.php @@ -23,7 +23,7 @@ final class PhabricatorOwnersPathsSearchEngineAttachment $list[] = array( 'repositoryPHID' => $path->getRepositoryPHID(), 'path' => $path->getPath(), - 'isExcluded' => (bool)$path->getExcluded(), + 'excluded' => (bool)$path->getExcluded(), ); } diff --git a/src/applications/transactions/editfield/PhabricatorConduitEditField.php b/src/applications/transactions/editfield/PhabricatorConduitEditField.php new file mode 100644 index 0000000000..4a8af48abb --- /dev/null +++ b/src/applications/transactions/editfield/PhabricatorConduitEditField.php @@ -0,0 +1,14 @@ +