Move many task status hardcodes into ManiphestTaskStatus

Summary:
Ref T1812. This cleans up most of the easy hard-coded references to specific statuses:

  - The "fixes" language moves into ManiphestTaskStatus.
  - Add a method to list open statuses.
  - Add a method to test if a status is open.
  - Add a method to get default status for new tasks.

Test Plan: Browsed around, lint, grep, created, filtered and updated tasks.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T1812

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

resources/sql/patches/20130913.maniphest.1.migratesearch.php

@@ -13,6 +13,9 @@ $conn_w = $table->establishConnection('w');
 $search_table = new PhabricatorSearchQuery();
 $search_conn_w = $search_table->establishConnection('w');
 
+// See T1812. This is an old status constant from the time of this migration.
+$old_open_status = 0;
+
 echo "Updating saved Maniphest queries...\n";
 $rows = new LiskRawMigrationIterator($conn_w, 'maniphest_savedquery');
 foreach ($rows as $row) {
@@ -132,12 +135,12 @@ foreach ($rows as $row) {
         if ($include_open xor $include_closed) {
           if ($include_open) {
             $new_data['statuses'] = array(
-              ManiphestTaskStatus::STATUS_OPEN,
+              $old_open_status,
             );
           } else {
             $statuses = array();
             foreach (ManiphestTaskStatus::getTaskStatusMap() as $status => $n) {
-              if ($status != ManiphestTaskStatus::STATUS_OPEN) {
+              if ($status != $old_open_status) {
                 $statuses[] = $status;
               }
             }

src/applications/differential/field/specification/DifferentialFreeformFieldSpecification.php

@@ -9,40 +9,8 @@ abstract class DifferentialFreeformFieldSpecification
       return array();
     }
 
-    $prefixes = array(
+    $prefixes = ManiphestTaskStatus::getStatusPrefixMap();
-      'resolve'       => ManiphestTaskStatus::STATUS_CLOSED_RESOLVED,
+    $suffixes = ManiphestTaskStatus::getStatusSuffixMap();
-      'resolves'      => ManiphestTaskStatus::STATUS_CLOSED_RESOLVED,
-      'resolved'      => ManiphestTaskStatus::STATUS_CLOSED_RESOLVED,
-      'fix'           => ManiphestTaskStatus::STATUS_CLOSED_RESOLVED,
-      'fixes'         => ManiphestTaskStatus::STATUS_CLOSED_RESOLVED,
-      'fixed'         => ManiphestTaskStatus::STATUS_CLOSED_RESOLVED,
-      'wontfix'       => ManiphestTaskStatus::STATUS_CLOSED_WONTFIX,
-      'wontfixes'     => ManiphestTaskStatus::STATUS_CLOSED_WONTFIX,
-      'wontfixed'     => ManiphestTaskStatus::STATUS_CLOSED_WONTFIX,
-      'spite'         => ManiphestTaskStatus::STATUS_CLOSED_SPITE,
-      'spites'        => ManiphestTaskStatus::STATUS_CLOSED_SPITE,
-      'spited'        => ManiphestTaskStatus::STATUS_CLOSED_SPITE,
-      'invalidate'    => ManiphestTaskStatus::STATUS_CLOSED_INVALID,
-      'invaldiates'   => ManiphestTaskStatus::STATUS_CLOSED_INVALID,
-      'invalidated'   => ManiphestTaskStatus::STATUS_CLOSED_INVALID,
-      'close'         => ManiphestTaskStatus::STATUS_CLOSED_RESOLVED,
-      'closes'        => ManiphestTaskStatus::STATUS_CLOSED_RESOLVED,
-      'closed'        => ManiphestTaskStatus::STATUS_CLOSED_RESOLVED,
-      'ref'           => null,
-      'refs'          => null,
-      'references'    => null,
-      'cf.'           => null,
-    );
-
-    $suffixes = array(
-      'as resolved'   => ManiphestTaskStatus::STATUS_CLOSED_RESOLVED,
-      'as fixed'      => ManiphestTaskStatus::STATUS_CLOSED_RESOLVED,
-      'as wontfix'    => ManiphestTaskStatus::STATUS_CLOSED_WONTFIX,
-      'as spite'      => ManiphestTaskStatus::STATUS_CLOSED_SPITE,
-      'out of spite'  => ManiphestTaskStatus::STATUS_CLOSED_SPITE,
-      'as invalid'    => ManiphestTaskStatus::STATUS_CLOSED_INVALID,
-      ''              => null,
-    );
 
     $matches = id(new ManiphestCustomFieldStatusParser())
       ->parseCorpus($message);
@@ -200,8 +168,8 @@ abstract class DifferentialFreeformFieldSpecification
         continue;
       }
 
-      if ($task->getStatus() != ManiphestTaskStatus::STATUS_OPEN) {
+      if ($task->getStatus() == $status) {
-        // Task is already closed.
+        // Task is already in the specified status, so skip updating it.
         continue;
       }
 

src/applications/home/controller/PhabricatorHomeMainController.php

@@ -118,7 +118,7 @@ final class PhabricatorHomeMainController
 
     $task_query = id(new ManiphestTaskQuery())
       ->setViewer($user)
-      ->withStatuses(array(ManiphestTaskStatus::STATUS_OPEN))
+      ->withStatuses(ManiphestTaskStatus::getOpenStatusConstants())
       ->withPriorities(array($unbreak_now))
       ->setLimit(10);
 
@@ -157,7 +157,7 @@ final class PhabricatorHomeMainController
     if ($projects) {
       $task_query = id(new ManiphestTaskQuery())
         ->setViewer($user)
-        ->withStatuses(array(ManiphestTaskStatus::STATUS_OPEN))
+        ->withStatuses(ManiphestTaskStatus::getOpenStatusConstants())
         ->withPriorities(array($needs_triage))
         ->withAnyProjects(mpull($projects, 'getPHID'))
         ->setLimit(10);
@@ -250,7 +250,7 @@ final class PhabricatorHomeMainController
 
     $task_query = id(new ManiphestTaskQuery())
       ->setViewer($user)
-      ->withStatus(ManiphestTaskQuery::STATUS_OPEN)
+      ->withStatuses(ManiphestTaskStatus::getOpenStatusConstants())
       ->setGroupBy(ManiphestTaskQuery::GROUP_PRIORITY)
       ->withOwners(array($user_phid))
       ->setLimit(10);

src/applications/maniphest/application/PhabricatorApplicationManiphest.php

@@ -73,7 +73,7 @@ final class PhabricatorApplicationManiphest extends PhabricatorApplication {
 
     $query = id(new ManiphestTaskQuery())
       ->setViewer($user)
-      ->withStatus(ManiphestTaskQuery::STATUS_OPEN)
+      ->withStatuses(ManiphestTaskStatus::getOpenStatusConstants())
       ->withOwners(array($user->getPHID()));
     $count = count($query->execute());
 

src/applications/maniphest/conduit/ConduitAPI_maniphest_Method.php

@@ -63,7 +63,7 @@ abstract class ConduitAPI_maniphest_Method extends ConduitAPIMethod {
       $task->setTitle((string)$request->getValue('title'));
       $task->setDescription((string)$request->getValue('description'));
       $changes[ManiphestTransaction::TYPE_STATUS] =
-        ManiphestTaskStatus::STATUS_OPEN;
+        ManiphestTaskStatus::getDefaultStatus();
     } else {
 
       $comments = $request->getValue('comments');

src/applications/maniphest/constants/ManiphestTaskStatus.php

@@ -98,4 +98,63 @@ final class ManiphestTaskStatus extends ManiphestConstants {
 
     return $tag;
   }
+
+  public static function getDefaultStatus() {
+    return self::STATUS_OPEN;
+  }
+
+  public static function getOpenStatusConstants() {
+    return array(
+      self::STATUS_OPEN,
+    );
+  }
+
+  public static function isOpenStatus($status) {
+    foreach (self::getOpenStatusConstants() as $constant) {
+      if ($status == $constant) {
+        return true;
+      }
+    }
+    return false;
+  }
+
+  public static function getStatusPrefixMap() {
+    return array(
+      'resolve'       => self::STATUS_CLOSED_RESOLVED,
+      'resolves'      => self::STATUS_CLOSED_RESOLVED,
+      'resolved'      => self::STATUS_CLOSED_RESOLVED,
+      'fix'           => self::STATUS_CLOSED_RESOLVED,
+      'fixes'         => self::STATUS_CLOSED_RESOLVED,
+      'fixed'         => self::STATUS_CLOSED_RESOLVED,
+      'wontfix'       => self::STATUS_CLOSED_WONTFIX,
+      'wontfixes'     => self::STATUS_CLOSED_WONTFIX,
+      'wontfixed'     => self::STATUS_CLOSED_WONTFIX,
+      'spite'         => self::STATUS_CLOSED_SPITE,
+      'spites'        => self::STATUS_CLOSED_SPITE,
+      'spited'        => self::STATUS_CLOSED_SPITE,
+      'invalidate'    => self::STATUS_CLOSED_INVALID,
+      'invaldiates'   => self::STATUS_CLOSED_INVALID,
+      'invalidated'   => self::STATUS_CLOSED_INVALID,
+      'close'         => self::STATUS_CLOSED_RESOLVED,
+      'closes'        => self::STATUS_CLOSED_RESOLVED,
+      'closed'        => self::STATUS_CLOSED_RESOLVED,
+      'ref'           => null,
+      'refs'          => null,
+      'references'    => null,
+      'cf.'           => null,
+    );
+  }
+
+  public static function getStatusSuffixMap() {
+    return array(
+      'as resolved'   => self::STATUS_CLOSED_RESOLVED,
+      'as fixed'      => self::STATUS_CLOSED_RESOLVED,
+      'as wontfix'    => self::STATUS_CLOSED_WONTFIX,
+      'as spite'      => self::STATUS_CLOSED_SPITE,
+      'out of spite'  => self::STATUS_CLOSED_SPITE,
+      'as invalid'    => self::STATUS_CLOSED_INVALID,
+    );
+  }
+
+
 }

src/applications/maniphest/controller/ManiphestReportController.php

@@ -380,7 +380,7 @@ final class ManiphestReportController extends ManiphestController {
 
     $query = id(new ManiphestTaskQuery())
       ->setViewer($user)
-      ->withStatus(ManiphestTaskQuery::STATUS_OPEN);
+      ->withStatuses(ManiphestTaskStatus::getOpenStatusConstants());
 
     $project_phid = $request->getStr('project');
     $project_handle = null;

src/applications/maniphest/field/parser/ManiphestCustomFieldStatusParser.php

@@ -4,30 +4,7 @@ final class ManiphestCustomFieldStatusParser
   extends PhabricatorCustomFieldMonogramParser {
 
   protected function getPrefixes() {
-    return array(
+    return array_keys(ManiphestTaskStatus::getStatusPrefixMap());
-      'resolve',
-      'resolves',
-      'resolved',
-      'fix',
-      'fixes',
-      'fixed',
-      'wontfix',
-      'wontfixes',
-      'wontfixed',
-      'spite',
-      'spites',
-      'spited',
-      'invalidate',
-      'invalidates',
-      'invalidated',
-      'close',
-      'closes',
-      'closed',
-      'ref',
-      'refs',
-      'references',
-      'cf.',
-    );
   }
 
   protected function getInfixes() {
@@ -42,14 +19,7 @@ final class ManiphestCustomFieldStatusParser
   }
 
   protected function getSuffixes() {
-    return array(
+    return array_keys(ManiphestTaskStatus::getStatusSuffixMap());
-      'as resolved',
-      'as fixed',
-      'as wontfix',
-      'as spite',
-      'out of spite',
-      'as invalid',
-    );
   }
 
   protected function getMonogramPattern() {

src/applications/maniphest/lipsum/PhabricatorManiphestTaskTestDataGenerator.php

@@ -9,8 +9,7 @@ final class PhabricatorManiphestTaskTestDataGenerator
           ->loadOneWhere('phid = %s', $authorPHID);
     $task = ManiphestTask::initializeNewTask($author)
       ->setSubPriority($this->generateTaskSubPriority())
-      ->setTitle($this->generateTitle())
+      ->setTitle($this->generateTitle());
-      ->setStatus(ManiphestTaskStatus::STATUS_OPEN);
 
     $content_source = PhabricatorContentSource::newForSource(
       PhabricatorContentSource::SOURCE_UNKNOWN,
@@ -101,7 +100,7 @@ final class PhabricatorManiphestTaskTestDataGenerator
     // Make sure 4/5th of all generated Tasks are open
     $random = rand(0, 4);
     if ($random != 0) {
-      return ManiphestTaskStatus::STATUS_OPEN;
+      return ManiphestTaskStatus::getDefaultStatus();
     } else {
       return array_rand($statuses);
     }

src/applications/maniphest/mail/ManiphestReplyHandler.php

@@ -64,7 +64,7 @@ final class ManiphestReplyHandler extends PhabricatorMailReplyHandler {
       // and then set the title and description.
       $xaction = clone $template;
       $xaction->setTransactionType(ManiphestTransaction::TYPE_STATUS);
-      $xaction->setNewValue(ManiphestTaskStatus::STATUS_OPEN);
+      $xaction->setNewValue(ManiphestTaskStatus::getDefaultStatus());
       $xactions[] = $xaction;
 
       $task->setAuthorPHID($user->getPHID());

src/applications/maniphest/phid/ManiphestPHIDTypeTask.php

@@ -38,7 +38,7 @@ final class ManiphestPHIDTypeTask extends PhabricatorPHIDType {
       $handle->setFullName("T{$id}: {$title}");
       $handle->setURI("/T{$id}");
 
-      if ($task->getStatus() != ManiphestTaskStatus::STATUS_OPEN) {
+      if (!ManiphestTaskStatus::isOpenStatus($task->getStatus())) {
         $handle->setStatus(PhabricatorObjectHandleStatus::STATUS_CLOSED);
       }
     }

src/applications/maniphest/query/ManiphestTaskSearchEngine.php

@@ -384,14 +384,20 @@ final class ManiphestTaskSearchEngine
       case 'assigned':
         return $query
           ->setParameter('assignedPHIDs', array($viewer_phid))
-          ->setParameter('statuses', array(ManiphestTaskStatus::STATUS_OPEN));
+          ->setParameter(
+            'statuses',
+            ManiphestTaskStatus::getOpenStatusConstants());
       case 'subscribed':
         return $query
           ->setParameter('subscriberPHIDs', array($viewer_phid))
-          ->setParameter('statuses', array(ManiphestTaskStatus::STATUS_OPEN));
+          ->setParameter(
+            'statuses',
+            ManiphestTaskStatus::getOpenStatusConstants());
       case 'open':
         return $query
-          ->setParameter('statuses', array(ManiphestTaskStatus::STATUS_OPEN));
+          ->setParameter(
+            'statuses',
+            ManiphestTaskStatus::getOpenStatusConstants());
       case 'authored':
         return $query
           ->setParameter('authorPHIDs', array($viewer_phid))

src/applications/maniphest/search/ManiphestSearchIndexer.php

@@ -31,7 +31,7 @@ final class ManiphestSearchIndexer
       $task->getDateCreated());
 
     $doc->addRelationship(
-      ($task->getStatus() == ManiphestTaskStatus::STATUS_OPEN)
+      (ManiphestTaskStatus::isOpenStatus($task->getStatus()))
         ? PhabricatorSearchRelationship::RELATIONSHIP_OPEN
         : PhabricatorSearchRelationship::RELATIONSHIP_CLOSED,
       $task->getPHID(),

src/applications/maniphest/storage/ManiphestTask.php

@@ -15,7 +15,7 @@ final class ManiphestTask extends ManiphestDAO
   protected $ownerPHID;
   protected $ccPHIDs = array();
 
-  protected $status = ManiphestTaskStatus::STATUS_OPEN;
+  protected $status;
   protected $priority;
   protected $subpriority = 0;
 
@@ -47,6 +47,7 @@ final class ManiphestTask extends ManiphestDAO
     $edit_policy = $app->getPolicy(ManiphestCapabilityDefaultEdit::CAPABILITY);
 
     return id(new ManiphestTask())
+      ->setStatus(ManiphestTaskStatus::getDefaultStatus())
       ->setPriority(ManiphestTaskPriority::getDefaultPriority())
       ->setAuthorPHID($actor->getPHID())
       ->setViewPolicy($view_policy)

src/applications/project/controller/PhabricatorProjectBoardController.php

@@ -51,7 +51,7 @@ final class PhabricatorProjectBoardController
     $tasks = id(new ManiphestTaskQuery())
       ->setViewer($viewer)
       ->withAllProjects(array($project->getPHID()))
-      ->withStatus(ManiphestTaskQuery::STATUS_OPEN)
+      ->withStatuses(ManiphestTaskStatus::getOpenStatusConstants())
       ->setOrderBy(ManiphestTaskQuery::ORDER_PRIORITY)
       ->execute();
     $tasks = mpull($tasks, null, 'getPHID');

src/applications/project/controller/PhabricatorProjectProfileController.php

@@ -129,7 +129,7 @@ final class PhabricatorProjectProfileController
     $query = id(new ManiphestTaskQuery())
       ->setViewer($user)
       ->withAnyProjects(array($project->getPHID()))
-      ->withStatus(ManiphestTaskQuery::STATUS_OPEN)
+      ->withStatuses(ManiphestTaskStatus::getOpenStatusConstants())
       ->setOrderBy(ManiphestTaskQuery::ORDER_PRIORITY)
       ->setLimit(10);
     $tasks = $query->execute();