diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -1402,6 +1402,7 @@ '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', @@ -2559,6 +2560,7 @@ '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', @@ -5385,6 +5387,7 @@ 'NuanceSourceViewController' => 'NuanceController', 'NuanceTransaction' => 'PhabricatorApplicationTransaction', 'OwnersConduitAPIMethod' => 'ConduitAPIMethod', + 'OwnersEditConduitAPIMethod' => 'PhabricatorEditEngineAPIMethod', 'OwnersPackageReplyHandler' => 'PhabricatorMailReplyHandler', 'OwnersQueryConduitAPIMethod' => 'OwnersConduitAPIMethod', 'PHIDConduitAPIMethod' => 'ConduitAPIMethod', @@ -6724,6 +6727,7 @@ 'PhabricatorCustomFieldInterface', ), 'PhabricatorOwnersPackageDatasource' => 'PhabricatorTypeaheadDatasource', + 'PhabricatorOwnersPackageEditEngine' => 'PhabricatorEditEngine', 'PhabricatorOwnersPackageFunctionDatasource' => 'PhabricatorTypeaheadCompositeDatasource', 'PhabricatorOwnersPackageOwnerDatasource' => 'PhabricatorTypeaheadCompositeDatasource', 'PhabricatorOwnersPackagePHIDType' => 'PhabricatorPHIDType', diff --git a/src/aphront/httpparametertype/AphrontPHIDListHTTPParameterType.php b/src/aphront/httpparametertype/AphrontPHIDListHTTPParameterType.php --- a/src/aphront/httpparametertype/AphrontPHIDListHTTPParameterType.php +++ b/src/aphront/httpparametertype/AphrontPHIDListHTTPParameterType.php @@ -27,4 +27,8 @@ ); } + protected function getParameterDefault() { + return array(); + } + } diff --git a/src/applications/owners/application/PhabricatorOwnersApplication.php b/src/applications/owners/application/PhabricatorOwnersApplication.php --- a/src/applications/owners/application/PhabricatorOwnersApplication.php +++ b/src/applications/owners/application/PhabricatorOwnersApplication.php @@ -43,10 +43,12 @@ 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 --- /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 --- a/src/applications/owners/controller/PhabricatorOwnersListController.php +++ b/src/applications/owners/controller/PhabricatorOwnersListController.php @@ -8,45 +8,17 @@ } 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 --- /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 --- a/src/applications/owners/storage/PhabricatorOwnersPackage.php +++ b/src/applications/owners/storage/PhabricatorOwnersPackage.php @@ -27,25 +27,8 @@ ->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 @@ 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 )------------------------- */ diff --git a/src/applications/transactions/editengine/PhabricatorEditEngine.php b/src/applications/transactions/editengine/PhabricatorEditEngine.php --- a/src/applications/transactions/editengine/PhabricatorEditEngine.php +++ b/src/applications/transactions/editengine/PhabricatorEditEngine.php @@ -215,7 +215,13 @@ } $xaction = $object->getApplicationTransactionTemplate(); - $comment = $xaction->getApplicationTransactionCommentObject(); + + try { + $comment = $xaction->getApplicationTransactionCommentObject(); + } catch (PhutilMethodNotImplementedException $ex) { + $comment = null; + } + if ($comment) { $comment_type = PhabricatorTransactions::TYPE_COMMENT; @@ -678,6 +684,8 @@ $validation_exception = null; if ($request->isFormPost()) { foreach ($fields as $field) { + $field->setIsSubmittedForm(true); + if ($field->getIsLocked() || $field->getIsHidden()) { continue; } @@ -709,6 +717,20 @@ ->setURI($this->getObjectViewURI($object)); } catch (PhabricatorApplicationTransactionValidationException $ex) { $validation_exception = $ex; + + foreach ($fields as $field) { + $xaction_type = $field->getTransactionType(); + if ($xaction_type === null) { + continue; + } + + $message = $ex->getShortMessage($xaction_type); + if ($message === null) { + continue; + } + + $field->setControlError($message); + } } } else { if ($this->getIsCreate()) { diff --git a/src/applications/transactions/editfield/PhabricatorDatasourceEditField.php b/src/applications/transactions/editfield/PhabricatorDatasourceEditField.php --- a/src/applications/transactions/editfield/PhabricatorDatasourceEditField.php +++ b/src/applications/transactions/editfield/PhabricatorDatasourceEditField.php @@ -11,6 +11,9 @@ } public function getDatasource() { + if (!$this->datasource) { + throw new PhutilInvalidStateException('setDatasource'); + } return $this->datasource; } diff --git a/src/applications/transactions/editfield/PhabricatorEditField.php b/src/applications/transactions/editfield/PhabricatorEditField.php --- a/src/applications/transactions/editfield/PhabricatorEditField.php +++ b/src/applications/transactions/editfield/PhabricatorEditField.php @@ -13,12 +13,15 @@ private $metadata = array(); private $description; private $editTypeKey; + private $isRequired; private $isLocked; private $isHidden; private $isPreview; private $isEditDefaults; + private $isSubmittedForm; + private $controlError; private $isReorderable = true; private $isDefaultable = true; @@ -145,6 +148,33 @@ throw new PhutilMethodNotImplementedException(); } + public function setIsSubmittedForm($is_submitted) { + $this->isSubmittedForm = $is_submitted; + return $this; + } + + public function getIsSubmittedForm() { + return $this->isSubmittedForm; + } + + public function setIsRequired($is_required) { + $this->isRequired = $is_required; + return $this; + } + + public function getIsRequired() { + return $this->isRequired; + } + + public function setControlError($control_error) { + $this->controlError = $control_error; + return $this; + } + + public function getControlError() { + return $this->controlError; + } + protected function renderControl() { $control = $this->newControl(); if ($control === null) { @@ -176,6 +206,16 @@ $control->setDisabled($disabled); + + if ($this->getIsSubmittedForm()) { + $error = $this->getControlError(); + if ($error !== null) { + $control->setError($error); + } + } else if ($this->getIsRequired()) { + $control->setError(true); + } + return $control; }