diff --git a/resources/sql/autopatches/20151207.editengine.1.sql b/resources/sql/autopatches/20151207.editengine.1.sql new file mode 100644 index 0000000000..5111ec99ed --- /dev/null +++ b/resources/sql/autopatches/20151207.editengine.1.sql @@ -0,0 +1,11 @@ +ALTER TABLE {$NAMESPACE}_search.search_editengineconfiguration + DROP editPolicy; + +ALTER TABLE {$NAMESPACE}_search.search_editengineconfiguration + ADD isEdit BOOL NOT NULL; + +ALTER TABLE {$NAMESPACE}_search.search_editengineconfiguration + ADD createOrder INT UNSIGNED NOT NULL; + +ALTER TABLE {$NAMESPACE}_search.search_editengineconfiguration + ADD editOrder INT UNSIGNED NOT NULL; diff --git a/src/applications/base/PhabricatorApplication.php b/src/applications/base/PhabricatorApplication.php index c1cda07710..0a6a2b4cb5 100644 --- a/src/applications/base/PhabricatorApplication.php +++ b/src/applications/base/PhabricatorApplication.php @@ -624,7 +624,7 @@ abstract class PhabricatorApplication '(?P[0-9]\d*)/)?'. '(?:'. '(?:'. - '(?Pparameters|nodefault|comment)'. + '(?Pparameters|nodefault|nocreate|nomanage|comment)'. '|'. '(?:form/(?P[^/]+))'. ')'. diff --git a/src/applications/transactions/controller/PhabricatorEditEngineConfigurationViewController.php b/src/applications/transactions/controller/PhabricatorEditEngineConfigurationViewController.php index a1e37f96c3..2248e7d359 100644 --- a/src/applications/transactions/controller/PhabricatorEditEngineConfigurationViewController.php +++ b/src/applications/transactions/controller/PhabricatorEditEngineConfigurationViewController.php @@ -10,7 +10,7 @@ final class PhabricatorEditEngineConfigurationViewController public function handleRequest(AphrontRequest $request) { $viewer = $this->getViewer(); - $config = $this->loadConfigForEdit(); + $config = $this->loadConfigForView(); if (!$config) { return id(new Aphront404Response()); } diff --git a/src/applications/transactions/controller/PhabricatorEditEngineController.php b/src/applications/transactions/controller/PhabricatorEditEngineController.php index b7a7d39483..e7306c7866 100644 --- a/src/applications/transactions/controller/PhabricatorEditEngineController.php +++ b/src/applications/transactions/controller/PhabricatorEditEngineController.php @@ -35,6 +35,14 @@ abstract class PhabricatorEditEngineController } protected function loadConfigForEdit() { + return $this->loadConfig($need_edit = true); + } + + protected function loadConfigForView() { + return $this->loadConfig($need_edit = false); + } + + private function loadConfig($need_edit) { $request = $this->getRequest(); $viewer = $this->getViewer(); @@ -43,17 +51,23 @@ abstract class PhabricatorEditEngineController $key = $request->getURIData('key'); + if ($need_edit) { + $capabilities = array( + PhabricatorPolicyCapability::CAN_VIEW, + PhabricatorPolicyCapability::CAN_EDIT, + ); + } else { + $capabilities = array( + PhabricatorPolicyCapability::CAN_VIEW, + ); + } + $config = id(new PhabricatorEditEngineConfigurationQuery()) ->setViewer($viewer) ->withEngineKeys(array($engine_key)) ->withIdentifiers(array($key)) - ->requireCapabilities( - array( - PhabricatorPolicyCapability::CAN_VIEW, - PhabricatorPolicyCapability::CAN_EDIT, - )) + ->requireCapabilities($capabilities) ->executeOne(); - if ($config) { $engine = $config->getEngine(); diff --git a/src/applications/transactions/editengine/PhabricatorEditEngine.php b/src/applications/transactions/editengine/PhabricatorEditEngine.php index ff8407c1d3..a5f6cd4cc1 100644 --- a/src/applications/transactions/editengine/PhabricatorEditEngine.php +++ b/src/applications/transactions/editengine/PhabricatorEditEngine.php @@ -628,6 +628,7 @@ abstract class PhabricatorEditEngine $capabilities = array(); $use_default = false; + $require_create = true; switch ($action) { case 'comment': $capabilities = array( @@ -638,6 +639,11 @@ abstract class PhabricatorEditEngine case 'parameters': $use_default = true; break; + case 'nodefault': + case 'nocreate': + case 'nomanage': + $require_create = false; + break; default: break; } @@ -661,6 +667,12 @@ abstract class PhabricatorEditEngine return new Aphront404Response(); } } else { + // Make sure the viewer has permission to create new objects of + // this type if we're going to create a new object. + if ($require_create) { + $this->requireCreateCapability(); + } + $this->setIsCreate(true); $object = $this->newEditableObject(); } @@ -672,6 +684,10 @@ abstract class PhabricatorEditEngine return $this->buildParametersResponse($object); case 'nodefault': return $this->buildNoDefaultResponse($object); + case 'nocreate': + return $this->buildNoCreateResponse($object); + case 'nomanage': + return $this->buildNoManageResponse($object); case 'comment': return $this->buildCommentResponse($object); default: @@ -757,8 +773,7 @@ abstract class PhabricatorEditEngine $editor->applyTransactions($object, $xactions); - return id(new AphrontRedirectResponse()) - ->setURI($this->getObjectViewURI($object)); + return $this->newEditResponse($request, $object, $xactions); } catch (PhabricatorApplicationTransactionValidationException $ex) { $validation_exception = $ex; @@ -847,6 +862,14 @@ abstract class PhabricatorEditEngine ->appendChild($box); } + protected function newEditResponse( + AphrontRequest $request, + $object, + array $xactions) { + return id(new AphrontRedirectResponse()) + ->setURI($this->getObjectViewURI($object)); + } + private function buildEditForm($object, array $fields) { $viewer = $this->getViewer(); @@ -896,32 +919,54 @@ abstract class PhabricatorEditEngine private function buildEditFormActions($object) { $actions = array(); - if ($this->supportsEditEngineConfiguration()) { - $engine_key = $this->getEngineKey(); - $config = $this->getEditEngineConfiguration(); - - $actions[] = id(new PhabricatorActionView()) - ->setName(pht('Manage Form Configurations')) - ->setIcon('fa-list-ul') - ->setHref("/transactions/editengine/{$engine_key}/"); - $actions[] = id(new PhabricatorActionView()) - ->setName(pht('Edit Form Configuration')) - ->setIcon('fa-pencil') - ->setHref($config->getURI()); - } - $actions[] = id(new PhabricatorActionView()) ->setName(pht('Show HTTP Parameters')) ->setIcon('fa-crosshairs') ->setHref($this->getEditURI($object, 'parameters/')); + if ($this->supportsEditEngineConfiguration()) { + $engine_key = $this->getEngineKey(); + $config = $this->getEditEngineConfiguration(); + + $can_manage = PhabricatorPolicyFilter::hasCapability( + $this->getViewer(), + $config, + PhabricatorPolicyCapability::CAN_EDIT); + + if ($can_manage) { + $manage_uri = $config->getURI(); + } else { + $manage_uri = $this->getEditURI(null, 'nomanage/'); + } + + $view_uri = "/transactions/editengine/{$engine_key}/"; + + $actions[] = id(new PhabricatorActionView()) + ->setName(pht('View Form Configurations')) + ->setIcon('fa-list-ul') + ->setHref($view_uri); + + $actions[] = id(new PhabricatorActionView()) + ->setName(pht('Edit Form Configuration')) + ->setIcon('fa-pencil') + ->setHref($manage_uri) + ->setDisabled(!$can_manage) + ->setWorkflow(!$can_manage); + } + + return $actions; } final public function addActionToCrumbs(PHUICrumbsView $crumbs) { $viewer = $this->getViewer(); - $configs = $this->loadUsableConfigurationsForCreate(); + $can_create = $this->hasCreateCapability(); + if ($can_create) { + $configs = $this->loadUsableConfigurationsForCreate(); + } else { + $configs = array(); + } $dropdown = null; $disabled = false; @@ -938,7 +983,12 @@ abstract class PhabricatorEditEngine $disabled = false; } $workflow = true; - $create_uri = $this->getEditURI(null, 'nodefault/'); + + if ($can_create) { + $create_uri = $this->getEditURI(null, 'nodefault/'); + } else { + $create_uri = $this->getEditURI(null, 'nocreate/'); + } } else { $config = head($configs); $form_key = $config->getIdentifier(); @@ -1109,6 +1159,31 @@ abstract class PhabricatorEditEngine ->addCancelButton($cancel_uri); } + private function buildNoCreateResponse($object) { + $cancel_uri = $this->getObjectCreateCancelURI($object); + + return $this->getController() + ->newDialog() + ->setTitle(pht('No Create Permission')) + ->appendParagraph( + pht( + 'You do not have permission to create these objects.')) + ->addCancelButton($cancel_uri); + } + + private function buildNoManageResponse($object) { + $cancel_uri = $this->getObjectCreateCancelURI($object); + + return $this->getController() + ->newDialog() + ->setTitle(pht('No Manage Permission')) + ->appendParagraph( + pht( + 'You do not have permission to configure forms for this '. + 'application.')) + ->addCancelButton($cancel_uri); + } + private function buildCommentResponse($object) { $viewer = $this->getViewer(); @@ -1266,6 +1341,8 @@ abstract class PhabricatorEditEngine $this->setIsCreate(false); $object = $this->newObjectFromIdentifier($identifier); } else { + $this->requireCreateCapability(); + $this->setIsCreate(true); $object = $this->newEditableObject(); } @@ -1429,10 +1506,14 @@ abstract class PhabricatorEditEngine } public function loadQuickCreateItems() { - $configs = $this->loadUsableConfigurationsForCreate(); - $items = array(); + if (!$this->hasCreateCapability()) { + return $items; + } + + $configs = $this->loadUsableConfigurationsForCreate(); + if (!$configs) { // No items to add. } else if (count($configs) == 1) { @@ -1456,12 +1537,16 @@ abstract class PhabricatorEditEngine private function loadUsableConfigurationsForCreate() { $viewer = $this->getViewer(); - return id(new PhabricatorEditEngineConfigurationQuery()) + $configs = id(new PhabricatorEditEngineConfigurationQuery()) ->setViewer($viewer) ->withEngineKeys(array($this->getEngineKey())) ->withIsDefault(true) ->withIsDisabled(false) ->execute(); + + $configs = msort($configs, 'getCreateSortKey'); + + return $configs; } private function newQuickCreateItem( @@ -1478,6 +1563,24 @@ abstract class PhabricatorEditEngine ->setHref($item_uri); } + protected function getCreateNewObjectPolicy() { + return PhabricatorPolicies::POLICY_USER; + } + + private function requireCreateCapability() { + PhabricatorPolicyFilter::requireCapability( + $this->getViewer(), + $this, + PhabricatorPolicyCapability::CAN_EDIT); + } + + private function hasCreateCapability() { + return PhabricatorPolicyFilter::hasCapability( + $this->getViewer(), + $this, + PhabricatorPolicyCapability::CAN_EDIT); + } + /* -( PhabricatorPolicyInterface )----------------------------------------- */ @@ -1489,6 +1592,7 @@ abstract class PhabricatorEditEngine public function getCapabilities() { return array( PhabricatorPolicyCapability::CAN_VIEW, + PhabricatorPolicyCapability::CAN_EDIT, ); } @@ -1496,6 +1600,8 @@ abstract class PhabricatorEditEngine switch ($capability) { case PhabricatorPolicyCapability::CAN_VIEW: return PhabricatorPolicies::getMostOpenPolicy(); + case PhabricatorPolicyCapability::CAN_EDIT: + return $this->getCreateNewObjectPolicy(); } } diff --git a/src/applications/transactions/editor/PhabricatorEditEngineConfigurationEditEngine.php b/src/applications/transactions/editor/PhabricatorEditEngineConfigurationEditEngine.php index 2c340e8649..de667ace4e 100644 --- a/src/applications/transactions/editor/PhabricatorEditEngineConfigurationEditEngine.php +++ b/src/applications/transactions/editor/PhabricatorEditEngineConfigurationEditEngine.php @@ -19,6 +19,12 @@ final class PhabricatorEditEngineConfigurationEditEngine return $this->targetEngine; } + protected function getCreateNewObjectPolicy() { + return $this->getTargetEngine() + ->getApplication() + ->getPolicy(PhabricatorPolicyCapability::CAN_EDIT); + } + public function getEngineName() { return pht('Edit Configurations'); } diff --git a/src/applications/transactions/editor/PhabricatorEditEngineConfigurationEditor.php b/src/applications/transactions/editor/PhabricatorEditEngineConfigurationEditor.php index 8b3f446c90..0b9edd7b63 100644 --- a/src/applications/transactions/editor/PhabricatorEditEngineConfigurationEditor.php +++ b/src/applications/transactions/editor/PhabricatorEditEngineConfigurationEditor.php @@ -15,7 +15,6 @@ final class PhabricatorEditEngineConfigurationEditor $types = parent::getTransactionTypes(); $types[] = PhabricatorTransactions::TYPE_VIEW_POLICY; - $types[] = PhabricatorTransactions::TYPE_EDIT_POLICY; $types[] = PhabricatorEditEngineConfigurationTransaction::TYPE_NAME; $types[] = PhabricatorEditEngineConfigurationTransaction::TYPE_PREAMBLE; diff --git a/src/applications/transactions/storage/PhabricatorEditEngineConfiguration.php b/src/applications/transactions/storage/PhabricatorEditEngineConfiguration.php index 5d80d95c5e..a2793d626f 100644 --- a/src/applications/transactions/storage/PhabricatorEditEngineConfiguration.php +++ b/src/applications/transactions/storage/PhabricatorEditEngineConfiguration.php @@ -10,10 +10,12 @@ final class PhabricatorEditEngineConfiguration protected $builtinKey; protected $name; protected $viewPolicy; - protected $editPolicy; protected $properties = array(); protected $isDisabled = 0; protected $isDefault = 0; + protected $isEdit = 0; + protected $createOrder = 0; + protected $editOrder = 0; private $engine = self::ATTACHABLE; @@ -29,14 +31,10 @@ final class PhabricatorEditEngineConfiguration PhabricatorUser $actor, PhabricatorEditEngine $engine) { - // TODO: This should probably be controlled by a new default capability. - $edit_policy = PhabricatorPolicies::POLICY_ADMIN; - return id(new PhabricatorEditEngineConfiguration()) ->setEngineKey($engine->getEngineKey()) ->attachEngine($engine) - ->setViewPolicy(PhabricatorPolicies::getMostOpenPolicy()) - ->setEditPolicy($edit_policy); + ->setViewPolicy(PhabricatorPolicies::getMostOpenPolicy()); } public function generatePHID() { @@ -44,6 +42,33 @@ final class PhabricatorEditEngineConfiguration PhabricatorEditEngineConfigurationPHIDType::TYPECONST); } + public function getCreateSortKey() { + return $this->getSortKey($this->createOrder); + } + + public function getEditSortKey() { + return $this->getSortKey($this->editOrder); + } + + private function getSortKey($order) { + // Put objects at the bottom by default if they haven't previously been + // reordered. When they're explicitly reordered, the smallest sort key we + // assign is 1, so if the object has a value of 0 it means it hasn't been + // ordered yet. + if ($order != 0) { + $group = 'A'; + } else { + $group = 'B'; + } + + return sprintf( + "%s%012d%s\0%012d", + $group, + $order, + $this->getName(), + $this->getID()); + } + protected function getConfiguration() { return array( self::CONFIG_AUX_PHID => true, @@ -56,6 +81,9 @@ final class PhabricatorEditEngineConfiguration 'name' => 'text255', 'isDisabled' => 'bool', 'isDefault' => 'bool', + 'isEdit' => 'bool', + 'createOrder' => 'uint32', + 'editOrder' => 'uint32', ), self::CONFIG_KEY_SCHEMA => array( 'key_engine' => array( @@ -65,6 +93,9 @@ final class PhabricatorEditEngineConfiguration 'key_default' => array( 'columns' => array('engineKey', 'isDefault', 'isDisabled'), ), + 'key_edit' => array( + 'columns' => array('engineKey', 'isEdit', 'isDisabled'), + ), ), ) + parent::getConfiguration(); } @@ -233,11 +264,21 @@ final class PhabricatorEditEngineConfiguration case PhabricatorPolicyCapability::CAN_VIEW: return $this->getViewPolicy(); case PhabricatorPolicyCapability::CAN_EDIT: - return $this->getEditPolicy(); + return $this->getEngine() + ->getApplication() + ->getPolicy($capability); } } public function hasAutomaticCapability($capability, PhabricatorUser $viewer) { + switch ($capability) { + case PhabricatorPolicyCapability::CAN_VIEW: + return PhabricatorPolicyFilter::hasCapability( + $viewer, + $this->getEngine()->getApplication(), + PhabricatorPolicyCapability::CAN_EDIT); + } + return false; }