Clean up some EditEngine meta-policies

Summary:
Ref T9908. Simplify some of the policies here:

  - If you can edit an application (currently, always "Administrators"), you can view and edit all of its forms.
  - You must be able to edit an application to create new forms.
  - Improve some error messages.
  - Get about halfway through letting users reorder forms in the "Create" menu if they want to sort by something weird since it'll need schema changes and I can do them all in one go here.

Test Plan:
  - Tried to create and edit forms as an unprivileged user.
  - Created and edited forms as an administrator.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T9908

Differential Revision: https://secure.phabricator.com/D14700

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;

src/applications/base/PhabricatorApplication.php

@@ -624,7 +624,7 @@ abstract class PhabricatorApplication
       '(?P<id>[0-9]\d*)/)?'.
       '(?:'.
         '(?:'.
-          '(?P<editAction>parameters|nodefault|comment)'.
+          '(?P<editAction>parameters|nodefault|nocreate|nomanage|comment)'.
           '|'.
           '(?:form/(?P<formKey>[^/]+))'.
         ')'.

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());
     }

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(
+      ->requireCapabilities($capabilities)
-        array(
-          PhabricatorPolicyCapability::CAN_VIEW,
-          PhabricatorPolicyCapability::CAN_EDIT,
-        ))
       ->executeOne();
-
     if ($config) {
       $engine = $config->getEngine();
 

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())
+        return $this->newEditResponse($request, $object, $xactions);
-          ->setURI($this->getObjectViewURI($object));
       } 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();
     }
   }
 

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');
   }

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;

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())
+      ->setViewPolicy(PhabricatorPolicies::getMostOpenPolicy());
-      ->setEditPolicy($edit_policy);
   }
 
   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;
   }