From ca908d7cc4a8836b8ea0b291601b8aa8450a05e0 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Fri, 12 Feb 2016 10:57:13 -0800
Subject: [PATCH] Don't autoname milestones, but do show the previous milestone
 name as a hint

Summary: Fixes T10347. In the long run maybe we'll try to guess this better, but for now get rid of the "Milestone X" hardcode and just show what the last one was called.

Test Plan:
  - Created the first milestone for a project.
  - Created the nth milestone for a project.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10347

Differential Revision: https://secure.phabricator.com/D15262
---
 .../engine/PhabricatorProjectEditEngine.php   | 39 +++++++++++++------
 .../project/query/PhabricatorProjectQuery.php | 23 +++++++++++
 .../PhabricatorEditEngineConfiguration.php    | 14 ++++++-
 3 files changed, 64 insertions(+), 12 deletions(-)

diff --git a/src/applications/project/engine/PhabricatorProjectEditEngine.php b/src/applications/project/engine/PhabricatorProjectEditEngine.php
index 71f91b7bc1..ee101cb2e4 100644
--- a/src/applications/project/engine/PhabricatorProjectEditEngine.php
+++ b/src/applications/project/engine/PhabricatorProjectEditEngine.php
@@ -43,17 +43,7 @@ final class PhabricatorProjectEditEngine
   }
 
   protected function newEditableObject() {
-    $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;
+    return PhabricatorProject::initializeNewProject($this->getViewer());
   }
 
   protected function newObjectQuery() {
@@ -139,6 +129,7 @@ final class PhabricatorProjectEditEngine
         array(
           'parent',
           'milestone',
+          'milestone.previous',
           'name',
           'std:project:internal:description',
           'icon',
@@ -166,8 +157,26 @@ final class PhabricatorProjectEditEngine
       $parent_phid = null;
     }
 
+    $previous_milestone_phid = null;
     if ($milestone) {
       $milestone_phid = $milestone->getPHID();
+
+      // Load the current milestone so we can show the user a hint about what
+      // it was called, so they don't have to remember if the next one should
+      // be "Sprint 287" or "Sprint 278".
+
+      $number = ($milestone->loadNextMilestoneNumber() - 1);
+      if ($number > 0) {
+        $previous_milestone = id(new PhabricatorProjectQuery())
+          ->setViewer($this->getViewer())
+          ->withParentProjectPHIDs(array($milestone->getPHID()))
+          ->withIsMilestone(true)
+          ->withMilestoneNumberBetween($number, $number)
+          ->executeOne();
+        if ($previous_milestone) {
+          $previous_milestone_phid = $previous_milestone->getPHID();
+        }
+      }
     } else {
       $milestone_phid = null;
     }
@@ -203,6 +212,14 @@ final class PhabricatorProjectEditEngine
         ->setIsDefaultable(false)
         ->setIsLockable(false)
         ->setIsLocked(true),
+      id(new PhabricatorHandlesEditField())
+        ->setKey('milestone.previous')
+        ->setLabel(pht('Previous Milestone'))
+        ->setSingleValue($previous_milestone_phid)
+        ->setIsReorderable(false)
+        ->setIsDefaultable(false)
+        ->setIsLockable(false)
+        ->setIsLocked(true),
       id(new PhabricatorTextEditField())
         ->setKey('name')
         ->setLabel(pht('Name'))
diff --git a/src/applications/project/query/PhabricatorProjectQuery.php b/src/applications/project/query/PhabricatorProjectQuery.php
index f4711bb4f7..d12e66e392 100644
--- a/src/applications/project/query/PhabricatorProjectQuery.php
+++ b/src/applications/project/query/PhabricatorProjectQuery.php
@@ -20,6 +20,8 @@ final class PhabricatorProjectQuery
   private $hasSubprojects;
   private $minDepth;
   private $maxDepth;
+  private $minMilestoneNumber;
+  private $maxMilestoneNumber;
 
   private $status       = 'status-any';
   const STATUS_ANY      = 'status-any';
@@ -111,6 +113,12 @@ final class PhabricatorProjectQuery
     return $this;
   }
 
+  public function withMilestoneNumberBetween($min, $max) {
+    $this->minMilestoneNumber = $min;
+    $this->maxMilestoneNumber = $max;
+    return $this;
+  }
+
   public function needMembers($need_members) {
     $this->needMembers = $need_members;
     return $this;
@@ -494,6 +502,7 @@ final class PhabricatorProjectQuery
       }
     }
 
+
     if ($this->hasSubprojects !== null) {
       $where[] = qsprintf(
         $conn,
@@ -515,6 +524,20 @@ final class PhabricatorProjectQuery
         $this->maxDepth);
     }
 
+    if ($this->minMilestoneNumber !== null) {
+      $where[] = qsprintf(
+        $conn,
+        'milestoneNumber >= %d',
+        $this->minMilestoneNumber);
+    }
+
+    if ($this->maxMilestoneNumber !== null) {
+      $where[] = qsprintf(
+        $conn,
+        'milestoneNumber <= %d',
+        $this->maxMilestoneNumber);
+    }
+
     return $where;
   }
 
diff --git a/src/applications/transactions/storage/PhabricatorEditEngineConfiguration.php b/src/applications/transactions/storage/PhabricatorEditEngineConfiguration.php
index a2793d626f..38a4fda0a6 100644
--- a/src/applications/transactions/storage/PhabricatorEditEngineConfiguration.php
+++ b/src/applications/transactions/storage/PhabricatorEditEngineConfiguration.php
@@ -175,8 +175,20 @@ final class PhabricatorEditEngineConfiguration
   }
 
   private function reorderFields(array $fields) {
+    // Fields which can not be reordered are fixed in order at the top of the
+    // form. These are used to show instructions or contextual information.
+
+    $fixed = array();
+    foreach ($fields as $key => $field) {
+      if (!$field->getIsReorderable()) {
+        $fixed[$key] = $field;
+      }
+    }
+
     $keys = $this->getFieldOrder();
-    $fields = array_select_keys($fields, $keys) + $fields;
+
+    $fields = $fixed + array_select_keys($fields, $keys) + $fields;
+
     return $fields;
   }