Smooth out milestone creation workflow

Summary:
Ref T10010.

  - Default name to "Milestone X".
  - Remove policy controls, which have no effect.
  - Don't generate slugs for milestones since this is a big pain where they all generate as `#milestone_1` by default (you can add one if you want). I plan to add some kind of syntax like `#parent/32` to mean "Milestone 32 in Parent" later.
  - Don't require projects to have unique names (again, 900 copies of "Milestone X"). I think we can trust users to sort this out for themselves since modern Phabricator has "Can Create Projects" permission, etc.

Test Plan: Created some milestones, had a less awful experience.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10010

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

src/applications/project/controller/PhabricatorProjectProfileController.php

@@ -186,7 +186,9 @@ final class PhabricatorProjectProfileController
         ->setName('#'.$slug->getSlug());
     }
 
+    if ($hashtags) {
       $view->addProperty(pht('Hashtags'), phutil_implode_html(' ', $hashtags));
+    }
 
     $view->addProperty(
       pht('Members'),

src/applications/project/editor/PhabricatorProjectTransactionEditor.php

@@ -3,6 +3,17 @@
 final class PhabricatorProjectTransactionEditor
   extends PhabricatorApplicationTransactionEditor {
 
+  private $isMilestone;
+
+  private function setIsMilestone($is_milestone) {
+    $this->isMilestone = $is_milestone;
+    return $this;
+  }
+
+  private function getIsMilestone() {
+    return $this->isMilestone;
+  }
+
   public function getEditorApplicationClass() {
     return 'PhabricatorProjectApplication';
   }
@@ -91,7 +102,9 @@ final class PhabricatorProjectTransactionEditor
       case PhabricatorProjectTransaction::TYPE_NAME:
         $name = $xaction->getNewValue();
         $object->setName($name);
+        if ($this->getIsMilestone()) {
           $object->setPrimarySlug(PhabricatorSlug::normalizeProjectSlug($name));
+        }
         return;
       case PhabricatorProjectTransaction::TYPE_SLUGS:
         return;
@@ -114,19 +127,7 @@ final class PhabricatorProjectTransactionEditor
         $object->setParentProjectPHID($xaction->getNewValue());
         return;
       case PhabricatorProjectTransaction::TYPE_MILESTONE:
-        $current = queryfx_one(
-          $object->establishConnection('w'),
-          'SELECT MAX(milestoneNumber) n
-            FROM %T
-            WHERE parentProjectPHID = %s',
-          $object->getTableName(),
-          $object->getParentProject()->getPHID());
-        if (!$current) {
-          $number = 1;
-        } else {
-          $number = (int)$current['n'] + 1;
-        }
-
+        $number = $object->getParentProject()->loadNextMilestoneNumber();
         $object->setMilestoneNumber($number);
         $object->setParentProjectPHID($xaction->getNewValue());
         return;
@@ -275,16 +276,7 @@ final class PhabricatorProjectTransactionEditor
       }
     }
 
-    $is_milestone = $object->isMilestone();
-    foreach ($xactions as $xaction) {
-      switch ($xaction->getTransactionType()) {
-        case PhabricatorProjectTransaction::TYPE_MILESTONE:
-          if ($xaction->getNewValue() !== null) {
-            $is_milestone = true;
-          }
-          break;
-      }
-    }
+    $is_milestone = $this->getIsMilestone();
 
     $is_parent = $object->getHasSubprojects();
 
@@ -346,6 +338,10 @@ final class PhabricatorProjectTransactionEditor
           break;
         }
 
+        if ($this->getIsMilestone()) {
+          break;
+        }
+
         $name = last($xactions)->getNewValue();
 
         if (!PhabricatorSlug::isValidProjectSlug($name)) {
@@ -358,20 +354,6 @@ final class PhabricatorProjectTransactionEditor
           break;
         }
 
-        $name_used_already = id(new PhabricatorProjectQuery())
-          ->setViewer($this->getActor())
-          ->withNames(array($name))
-          ->executeOne();
-        if ($name_used_already &&
-           ($name_used_already->getPHID() != $object->getPHID())) {
-          $error = new PhabricatorApplicationTransactionValidationError(
-            $type,
-            pht('Duplicate'),
-            pht('Project name is already used.'),
-            nonempty(last($xactions), null));
-          $errors[] = $error;
-        }
-
         $slug = PhabricatorSlug::normalizeProjectSlug($name);
 
         $slug_used_already = id(new PhabricatorProjectSlug())
@@ -381,7 +363,10 @@ final class PhabricatorProjectTransactionEditor
           $error = new PhabricatorApplicationTransactionValidationError(
             $type,
             pht('Duplicate'),
-            pht('Project name can not be used due to hashtag collision.'),
+            pht(
+              'Project name generates the same hashtag ("%s") as another '.
+              'existing project. Choose a unique name.',
+              '#'.$slug),
             nonempty(last($xactions), null));
           $errors[] = $error;
         }
@@ -883,6 +868,19 @@ final class PhabricatorProjectTransactionEditor
       }
     }
 
+    $is_milestone = $object->isMilestone();
+    foreach ($xactions as $xaction) {
+      switch ($xaction->getTransactionType()) {
+        case PhabricatorProjectTransaction::TYPE_MILESTONE:
+          if ($xaction->getNewValue() !== null) {
+            $is_milestone = true;
+          }
+          break;
+      }
+    }
+
+    $this->setIsMilestone($is_milestone);
+
     return $results;
   }
 

src/applications/project/engine/PhabricatorProjectEditEngine.php

@@ -43,7 +43,17 @@ final class PhabricatorProjectEditEngine
   }
 
   protected function newEditableObject() {
-    return PhabricatorProject::initializeNewProject($this->getViewer());
+    $project = PhabricatorProject::initializeNewProject($this->getViewer());
+
+    $milestone = $this->getMilestoneProject();
+    if ($milestone) {
+      $default_name = pht(
+        'Milestone %s',
+        new PhutilNumber($milestone->loadNextMilestoneNumber()));
+      $project->setName($default_name);
+    }
+
+    return $project;
   }
 
   protected function newObjectQuery() {
@@ -92,6 +102,28 @@ final class PhabricatorProjectEditEngine
       ProjectCreateProjectsCapability::CAPABILITY);
   }
 
+  protected function willConfigureFields($object, array $fields) {
+    $is_milestone = ($this->getMilestoneProject() || $object->isMilestone());
+
+    $unavailable = array(
+      PhabricatorTransactions::TYPE_VIEW_POLICY,
+      PhabricatorTransactions::TYPE_EDIT_POLICY,
+      PhabricatorTransactions::TYPE_JOIN_POLICY,
+    );
+    $unavailable = array_fuse($unavailable);
+
+    if ($is_milestone) {
+      foreach ($fields as $key => $field) {
+        $xaction_type = $field->getTransactionType();
+        if (isset($unavailable[$xaction_type])) {
+          unset($fields[$key]);
+        }
+      }
+    }
+
+    return $fields;
+  }
+
   protected function newBuiltinEngineConfigurations() {
     $configuration = head(parent::newBuiltinEngineConfigurations());
 

src/applications/project/storage/PhabricatorProject.php

@@ -211,21 +211,12 @@ final class PhabricatorProject extends PhabricatorProjectDAO
         'projectPathKey' => 'bytes4',
       ),
       self::CONFIG_KEY_SCHEMA => array(
-        'key_phid' => null,
-        'phid' => array(
-          'columns' => array('phid'),
-          'unique' => true,
-        ),
         'key_icon' => array(
           'columns' => array('icon'),
         ),
         'key_color' => array(
           'columns' => array('color'),
         ),
-        'name' => array(
-          'columns' => array('name'),
-          'unique' => true,
-        ),
         'key_milestone' => array(
           'columns' => array('parentProjectPHID', 'milestoneNumber'),
           'unique' => true,
@@ -475,6 +466,24 @@ final class PhabricatorProject extends PhabricatorProjectDAO
     return true;
   }
 
+  public function loadNextMilestoneNumber() {
+    $current = queryfx_one(
+      $this->establishConnection('w'),
+      'SELECT MAX(milestoneNumber) n
+        FROM %T
+        WHERE parentProjectPHID = %s',
+      $this->getTableName(),
+      $this->getPHID());
+
+    if (!$current) {
+      $number = 1;
+    } else {
+      $number = (int)$current['n'] + 1;
+    }
+
+    return $number;
+  }
+
 
 /* -(  PhabricatorSubscribableInterface  )----------------------------------- */
 

src/applications/transactions/editengine/PhabricatorEditEngine.php

@@ -64,10 +64,6 @@ abstract class PhabricatorEditEngine
   abstract public function getEngineApplicationClass();
   abstract protected function buildCustomEditFields($object);
 
-  protected function didBuildCustomEditFields($object, array $fields) {
-    return;
-  }
-
   public function getFieldsForConfig(
     PhabricatorEditEngineConfiguration $config) {
 
@@ -93,7 +89,6 @@ abstract class PhabricatorEditEngine
     }
 
     $fields = mpull($fields, null, 'getKey');
-    $this->didBuildCustomEditFields($object, $fields);
 
     $extensions = PhabricatorEditEngineExtension::getAllEnabledExtensions();
     foreach ($extensions as $extension) {
@@ -115,7 +110,6 @@ abstract class PhabricatorEditEngine
       }
 
       $extension_fields = mpull($extension_fields, null, 'getKey');
-      $extension->didBuildCustomEditFields($this, $object, $extension_fields);
 
       foreach ($extension_fields as $key => $field) {
         $fields[$key] = $field;
@@ -123,11 +117,16 @@ abstract class PhabricatorEditEngine
     }
 
     $config = $this->getEditEngineConfiguration();
+    $fields = $this->willConfigureFields($object, $fields);
     $fields = $config->applyConfigurationToFields($this, $object, $fields);
 
     return $fields;
   }
 
+  protected function willConfigureFields($object, array $fields) {
+    return $fields;
+  }
+
 
 /* -(  Display Text  )------------------------------------------------------- */
 

src/applications/transactions/engineextension/PhabricatorEditEngineExtension.php

@@ -32,13 +32,6 @@ abstract class PhabricatorEditEngineExtension extends Phobject {
     PhabricatorEditEngine $engine,
     PhabricatorApplicationTransactionInterface $object);
 
-  public function didBuildCustomEditFields(
-    PhabricatorEditEngine $engine,
-    PhabricatorApplicationTransactionInterface $object,
-    array $fields) {
-    return;
-  }
-
   final public static function getAllExtensions() {
     return id(new PhutilClassMapQuery())
       ->setAncestorClass(__CLASS__)