From a407b83dc276c5f67ddaa8e495d72bcd2fdd4436 Mon Sep 17 00:00:00 2001 From: epriestley Date: Sun, 29 Nov 2015 09:33:36 -0800 Subject: [PATCH] Move Owners to EditEngine Summary: Ref T9132. Paste is in fairly good shape so Owners is up next. Reasoning: - One install wants API access for it. - It's a simple application for getting CustomFields working with EditEngine. This only does the EditEngine part, so CustomFields are no longer editable until I make that work. That will be up next, and I'll hold this until that's ready. Test Plan: - Created and edited packages via web UI. - Created and edited package editing forms via web UI. - Created and edited packages via Conduit. Reviewers: chad Reviewed By: chad Maniphest Tasks: T9132 Differential Revision: https://secure.phabricator.com/D14598 --- src/__phutil_library_map__.php | 4 + .../PhabricatorOwnersApplication.php | 4 +- .../conduit/OwnersEditConduitAPIMethod.php | 20 ++ .../PhabricatorOwnersEditController.php | 198 +----------------- .../PhabricatorOwnersListController.php | 40 +--- .../PhabricatorOwnersPackageEditEngine.php | 93 ++++++++ .../storage/PhabricatorOwnersPackage.php | 49 +++-- 7 files changed, 159 insertions(+), 249 deletions(-) create mode 100644 src/applications/owners/conduit/OwnersEditConduitAPIMethod.php create mode 100644 src/applications/owners/editor/PhabricatorOwnersPackageEditEngine.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 9cb07a0259..b066e65b06 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -1403,6 +1403,7 @@ phutil_register_library_map(array( 'NuanceSourceViewController' => 'applications/nuance/controller/NuanceSourceViewController.php', 'NuanceTransaction' => 'applications/nuance/storage/NuanceTransaction.php', 'OwnersConduitAPIMethod' => 'applications/owners/conduit/OwnersConduitAPIMethod.php', + 'OwnersEditConduitAPIMethod' => 'applications/owners/conduit/OwnersEditConduitAPIMethod.php', 'OwnersPackageReplyHandler' => 'applications/owners/mail/OwnersPackageReplyHandler.php', 'OwnersQueryConduitAPIMethod' => 'applications/owners/conduit/OwnersQueryConduitAPIMethod.php', 'PHIDConduitAPIMethod' => 'applications/phid/conduit/PHIDConduitAPIMethod.php', @@ -2564,6 +2565,7 @@ phutil_register_library_map(array( 'PhabricatorOwnersOwner' => 'applications/owners/storage/PhabricatorOwnersOwner.php', 'PhabricatorOwnersPackage' => 'applications/owners/storage/PhabricatorOwnersPackage.php', 'PhabricatorOwnersPackageDatasource' => 'applications/owners/typeahead/PhabricatorOwnersPackageDatasource.php', + 'PhabricatorOwnersPackageEditEngine' => 'applications/owners/editor/PhabricatorOwnersPackageEditEngine.php', 'PhabricatorOwnersPackageFunctionDatasource' => 'applications/owners/typeahead/PhabricatorOwnersPackageFunctionDatasource.php', 'PhabricatorOwnersPackageOwnerDatasource' => 'applications/owners/typeahead/PhabricatorOwnersPackageOwnerDatasource.php', 'PhabricatorOwnersPackagePHIDType' => 'applications/owners/phid/PhabricatorOwnersPackagePHIDType.php', @@ -5398,6 +5400,7 @@ phutil_register_library_map(array( 'NuanceSourceViewController' => 'NuanceController', 'NuanceTransaction' => 'PhabricatorApplicationTransaction', 'OwnersConduitAPIMethod' => 'ConduitAPIMethod', + 'OwnersEditConduitAPIMethod' => 'PhabricatorEditEngineAPIMethod', 'OwnersPackageReplyHandler' => 'PhabricatorMailReplyHandler', 'OwnersQueryConduitAPIMethod' => 'OwnersConduitAPIMethod', 'PHIDConduitAPIMethod' => 'ConduitAPIMethod', @@ -6741,6 +6744,7 @@ phutil_register_library_map(array( 'PhabricatorCustomFieldInterface', ), 'PhabricatorOwnersPackageDatasource' => 'PhabricatorTypeaheadDatasource', + 'PhabricatorOwnersPackageEditEngine' => 'PhabricatorEditEngine', 'PhabricatorOwnersPackageFunctionDatasource' => 'PhabricatorTypeaheadCompositeDatasource', 'PhabricatorOwnersPackageOwnerDatasource' => 'PhabricatorTypeaheadCompositeDatasource', 'PhabricatorOwnersPackagePHIDType' => 'PhabricatorPHIDType', diff --git a/src/applications/owners/application/PhabricatorOwnersApplication.php b/src/applications/owners/application/PhabricatorOwnersApplication.php index 79f63b26b8..39933f3dce 100644 --- a/src/applications/owners/application/PhabricatorOwnersApplication.php +++ b/src/applications/owners/application/PhabricatorOwnersApplication.php @@ -43,10 +43,12 @@ final class PhabricatorOwnersApplication extends PhabricatorApplication { return array( '/owners/' => array( '(?:query/(?P[^/]+)/)?' => 'PhabricatorOwnersListController', - 'edit/(?P[1-9]\d*)/' => 'PhabricatorOwnersEditController', 'new/' => 'PhabricatorOwnersEditController', 'package/(?P[1-9]\d*)/' => 'PhabricatorOwnersDetailController', 'paths/(?P[1-9]\d*)/' => 'PhabricatorOwnersPathsController', + + $this->getEditRoutePattern('edit/') + => 'PhabricatorOwnersEditController', ), ); } diff --git a/src/applications/owners/conduit/OwnersEditConduitAPIMethod.php b/src/applications/owners/conduit/OwnersEditConduitAPIMethod.php new file mode 100644 index 0000000000..4715f7695d --- /dev/null +++ b/src/applications/owners/conduit/OwnersEditConduitAPIMethod.php @@ -0,0 +1,20 @@ +getUser(); - - $id = $request->getURIData('id'); - if ($id) { - $package = id(new PhabricatorOwnersPackageQuery()) - ->setViewer($viewer) - ->withIDs(array($id)) - ->requireCapabilities( - array( - PhabricatorPolicyCapability::CAN_VIEW, - // TODO: Support this capability. - // PhabricatorPolicyCapability::CAN_EDIT, - )) - ->needOwners(true) - ->executeOne(); - if (!$package) { - return new Aphront404Response(); - } - $is_new = false; - } else { - $package = PhabricatorOwnersPackage::initializeNewPackage($viewer); - $is_new = true; - } - - $e_name = true; - - $v_name = $package->getName(); - $v_owners = mpull($package->getOwners(), 'getUserPHID'); - $v_auditing = $package->getAuditingEnabled(); - $v_description = $package->getDescription(); - $v_status = $package->getStatus(); - - $field_list = PhabricatorCustomField::getObjectFields( - $package, - PhabricatorCustomField::ROLE_EDIT); - $field_list->setViewer($viewer); - $field_list->readFieldsFromStorage($package); - - $errors = array(); - if ($request->isFormPost()) { - $xactions = array(); - - $v_name = $request->getStr('name'); - $v_owners = $request->getArr('owners'); - $v_auditing = ($request->getStr('auditing') == 'enabled'); - $v_description = $request->getStr('description'); - $v_status = $request->getStr('status'); - - $type_name = PhabricatorOwnersPackageTransaction::TYPE_NAME; - $type_owners = PhabricatorOwnersPackageTransaction::TYPE_OWNERS; - $type_auditing = PhabricatorOwnersPackageTransaction::TYPE_AUDITING; - $type_description = PhabricatorOwnersPackageTransaction::TYPE_DESCRIPTION; - $type_status = PhabricatorOwnersPackageTransaction::TYPE_STATUS; - - $xactions[] = id(new PhabricatorOwnersPackageTransaction()) - ->setTransactionType($type_name) - ->setNewValue($v_name); - - $xactions[] = id(new PhabricatorOwnersPackageTransaction()) - ->setTransactionType($type_owners) - ->setNewValue($v_owners); - - $xactions[] = id(new PhabricatorOwnersPackageTransaction()) - ->setTransactionType($type_auditing) - ->setNewValue($v_auditing); - - $xactions[] = id(new PhabricatorOwnersPackageTransaction()) - ->setTransactionType($type_description) - ->setNewValue($v_description); - - if (!$is_new) { - $xactions[] = id(new PhabricatorOwnersPackageTransaction()) - ->setTransactionType($type_status) - ->setNewValue($v_status); - } - - $field_xactions = $field_list->buildFieldTransactionsFromRequest( - new PhabricatorOwnersPackageTransaction(), - $request); - - $xactions = array_merge($xactions, $field_xactions); - - $editor = id(new PhabricatorOwnersPackageTransactionEditor()) - ->setActor($viewer) - ->setContentSourceFromRequest($request) - ->setContinueOnNoEffect(true); - - try { - $editor->applyTransactions($package, $xactions); - - $id = $package->getID(); - if ($is_new) { - $next_uri = '/owners/paths/'.$id.'/'; - } else { - $next_uri = '/owners/package/'.$id.'/'; - } - - return id(new AphrontRedirectResponse())->setURI($next_uri); - } catch (AphrontDuplicateKeyQueryException $ex) { - $e_name = pht('Duplicate'); - $errors[] = pht('Package name must be unique.'); - } catch (PhabricatorApplicationTransactionValidationException $ex) { - $validation_exception = $ex; - - $e_name = $ex->getShortMessage($type_name); - } - } - - if ($is_new) { - $cancel_uri = '/owners/'; - $title = pht('New Package'); - $button_text = pht('Continue'); - } else { - $cancel_uri = '/owners/package/'.$package->getID().'/'; - $title = pht('Edit Package'); - $button_text = pht('Save Package'); - } - - $form = id(new AphrontFormView()) - ->setUser($viewer) - ->appendChild( - id(new AphrontFormTextControl()) - ->setLabel(pht('Name')) - ->setName('name') - ->setValue($v_name) - ->setError($e_name)) - ->appendControl( - id(new AphrontFormTokenizerControl()) - ->setDatasource(new PhabricatorProjectOrUserDatasource()) - ->setLabel(pht('Owners')) - ->setName('owners') - ->setValue($v_owners)); - - if (!$is_new) { - $form->appendChild( - id(new AphrontFormSelectControl()) - ->setLabel(pht('Status')) - ->setName('status') - ->setValue($v_status) - ->setOptions($package->getStatusNameMap())); - } - - $form->appendChild( - id(new AphrontFormSelectControl()) - ->setName('auditing') - ->setLabel(pht('Auditing')) - ->setCaption( - pht( - 'With auditing enabled, all future commits that touch '. - 'this package will be reviewed to make sure an owner '. - 'of the package is involved and the commit message has '. - 'a valid revision, reviewed by, and author.')) - ->setOptions( - array( - 'disabled' => pht('Disabled'), - 'enabled' => pht('Enabled'), - )) - ->setValue(($v_auditing ? 'enabled' : 'disabled'))) - ->appendChild( - id(new PhabricatorRemarkupControl()) - ->setUser($viewer) - ->setLabel(pht('Description')) - ->setName('description') - ->setValue($v_description)); - - $field_list->appendFieldsToForm($form); - - $form->appendChild( - id(new AphrontFormSubmitControl()) - ->addCancelButton($cancel_uri) - ->setValue($button_text)); - - $form_box = id(new PHUIObjectBoxView()) - ->setHeaderText($title) - ->setFormErrors($errors) - ->setForm($form); - - $crumbs = $this->buildApplicationCrumbs(); - if ($package->getID()) { - $crumbs->addTextCrumb( - $package->getName(), - $this->getApplicationURI('package/'.$package->getID().'/')); - $crumbs->addTextCrumb(pht('Edit')); - } else { - $crumbs->addTextCrumb(pht('New Package')); - } - - return $this->buildApplicationPage( - array( - $crumbs, - $form_box, - ), - array( - 'title' => $title, - )); + return id(new PhabricatorOwnersPackageEditEngine()) + ->setController($this) + ->buildResponse(); } } diff --git a/src/applications/owners/controller/PhabricatorOwnersListController.php b/src/applications/owners/controller/PhabricatorOwnersListController.php index 7beb842c82..6392922577 100644 --- a/src/applications/owners/controller/PhabricatorOwnersListController.php +++ b/src/applications/owners/controller/PhabricatorOwnersListController.php @@ -8,45 +8,17 @@ final class PhabricatorOwnersListController } public function handleRequest(AphrontRequest $request) { - $controller = id(new PhabricatorApplicationSearchController()) - ->setQueryKey($request->getURIData('queryKey')) - ->setSearchEngine(new PhabricatorOwnersPackageSearchEngine()) - ->setNavigation($this->buildSideNavView()); - - return $this->delegateToController($controller); - } - - public function buildSideNavView($for_app = false) { - $viewer = $this->getViewer(); - - $nav = new AphrontSideNavFilterView(); - $nav->setBaseURI(new PhutilURI($this->getApplicationURI())); - - if ($for_app) { - $nav->addFilter('new/', pht('Create Package')); - } - - id(new PhabricatorOwnersPackageSearchEngine()) - ->setViewer($viewer) - ->addNavigationItems($nav->getMenu()); - - $nav->selectFilter(null); - - return $nav; - } - - public function buildApplicationMenu() { - return $this->buildSideNavView(true)->getMenu(); + return id(new PhabricatorOwnersPackageSearchEngine()) + ->setController($this) + ->buildResponse(); } protected function buildApplicationCrumbs() { $crumbs = parent::buildApplicationCrumbs(); - $crumbs->addAction( - id(new PHUIListItemView()) - ->setName(pht('Create Package')) - ->setHref($this->getApplicationURI('new/')) - ->setIcon('fa-plus-square')); + id(new PhabricatorOwnersPackageEditEngine()) + ->setViewer($this->getViewer()) + ->addActionToCrumbs($crumbs); return $crumbs; } diff --git a/src/applications/owners/editor/PhabricatorOwnersPackageEditEngine.php b/src/applications/owners/editor/PhabricatorOwnersPackageEditEngine.php new file mode 100644 index 0000000000..c753556e89 --- /dev/null +++ b/src/applications/owners/editor/PhabricatorOwnersPackageEditEngine.php @@ -0,0 +1,93 @@ +getViewer()); + } + + protected function newObjectQuery() { + return id(new PhabricatorOwnersPackageQuery()) + ->needOwners(true); + } + + protected function getObjectCreateTitleText($object) { + return pht('Create New Package'); + } + + protected function getObjectEditTitleText($object) { + return pht('Edit Package %s', $object->getName()); + } + + protected function getObjectEditShortText($object) { + return pht('Package %d', $object->getID()); + } + + protected function getObjectCreateShortText() { + return pht('Create Package'); + } + + protected function getObjectViewURI($object) { + $id = $object->getID(); + return "/owners/package/{$id}/"; + } + + protected function buildCustomEditFields($object) { + return array( + id(new PhabricatorTextEditField()) + ->setKey('name') + ->setLabel(pht('Name')) + ->setDescription(pht('Name of the package.')) + ->setTransactionType(PhabricatorOwnersPackageTransaction::TYPE_NAME) + ->setIsRequired(true) + ->setValue($object->getName()), + id(new PhabricatorDatasourceEditField()) + ->setKey('owners') + ->setLabel(pht('Owners')) + ->setDescription(pht('Users and projects which own the package.')) + ->setTransactionType(PhabricatorOwnersPackageTransaction::TYPE_OWNERS) + ->setDatasource(new PhabricatorProjectOrUserDatasource()) + ->setValue($object->getOwnerPHIDs()), + id(new PhabricatorSelectEditField()) + ->setKey('status') + ->setLabel(pht('Status')) + ->setDescription(pht('Archive or enable the package.')) + ->setTransactionType(PhabricatorOwnersPackageTransaction::TYPE_STATUS) + ->setValue($object->getStatus()) + ->setOptions($object->getStatusNameMap()), + id(new PhabricatorSelectEditField()) + ->setKey('auditing') + ->setLabel(pht('Auditing')) + ->setDescription( + pht( + 'Automatically trigger audits for commits affecting files in '. + 'this package.')) + ->setTransactionType(PhabricatorOwnersPackageTransaction::TYPE_AUDITING) + ->setValue($object->getAuditingEnabled()) + ->setOptions( + array( + '' => pht('Disabled'), + '1' => pht('Enabled'), + )), + id(new PhabricatorRemarkupEditField()) + ->setKey('description') + ->setLabel(pht('Description')) + ->setDescription(pht('Human-readable description of the package.')) + ->setTransactionType( + PhabricatorOwnersPackageTransaction::TYPE_DESCRIPTION) + ->setValue($object->getDescription()), + ); + } + +} diff --git a/src/applications/owners/storage/PhabricatorOwnersPackage.php b/src/applications/owners/storage/PhabricatorOwnersPackage.php index 1a26bb6814..09e2a3be26 100644 --- a/src/applications/owners/storage/PhabricatorOwnersPackage.php +++ b/src/applications/owners/storage/PhabricatorOwnersPackage.php @@ -27,25 +27,8 @@ final class PhabricatorOwnersPackage ->setAuditingEnabled(0) ->attachPaths(array()) ->setStatus(self::STATUS_ACTIVE) - ->attachOwners(array()); - } - - public function getCapabilities() { - return array( - PhabricatorPolicyCapability::CAN_VIEW, - ); - } - - public function getPolicy($capability) { - return PhabricatorPolicies::POLICY_USER; - } - - public function hasAutomaticCapability($capability, PhabricatorUser $viewer) { - return false; - } - - public function describeAutomaticCapability($capability) { - return null; + ->attachOwners(array()) + ->setDescription(''); } public static function getStatusNameMap() { @@ -284,6 +267,34 @@ final class PhabricatorOwnersPackage return $this->assertAttached($this->owners); } + public function getOwnerPHIDs() { + return mpull($this->getOwners(), 'getUserPHID'); + } + + +/* -( PhabricatorPolicyInterface )----------------------------------------- */ + + + public function getCapabilities() { + return array( + PhabricatorPolicyCapability::CAN_VIEW, + PhabricatorPolicyCapability::CAN_EDIT, + ); + } + + public function getPolicy($capability) { + // TODO: Implement proper policies. + return PhabricatorPolicies::POLICY_USER; + } + + public function hasAutomaticCapability($capability, PhabricatorUser $viewer) { + return false; + } + + public function describeAutomaticCapability($capability) { + return null; + } + /* -( PhabricatorApplicationTransactionInterface )------------------------- */