Maniphest - introduce needProjectPHIDs

Summary: Ref T5245. This is some of the associated cleanup there.

Test Plan:
foreach ManiphestTaskQuery site, I made the change (or not) and tested as follows:

=== Call sites where added needProjectPHIDs ===

- PhabricatorHomeMainController - loaded the home page
- ManiphestBatchEditController - batch edited some tasks (added a project)
- ManiphestConduitAPIMethod - tested implicitly when tested ManiphestUpdateConduitAPIMethod
- ManiphestInfoConduitAPIMethod - used the method via conduit console with input id : 1
- ManiphestQueryConduitAPIMethod - used the method via conduit console with input ids : [1, 2]
- ManiphestUpdateConduitAPIMethod - used the method via conduit with input id : 1 and comment : “asdasds"
- ManiphestReportController - viewed “By User” and “By Project”
- ManiphestSubpriorityController - changed the priority of a task via a drag on manphest home
- ManiphestTaskMailReceiver - updated Task 1 via bin/mail receive-test with a comment that is the README
- ManiphestTaskSearchEngine - loaded Manifest home page
- ManiphestTaskEditController - edited a task
- ManiphestTransactionEditor - closed a blocking task
- ManiphestTransactionSaveController - commented on a task
- PhabricatorProjectProfileController - viewed project with id of 1 that has a few tasks in it
- PhabricatorSearchAttachController - merged tasks together
- DifferentialTransactionEditor - submit a diff that references a task; commit the diff (thus closing the diff) and the task gets updated
- PhabricatorRepositoryCommitMessageParserWorker - submit a diff that references a task; commit the diff (thus closing the diff) and the task gets updated

=== Calls sites where *did not* add needProjectPHIDs (they do not appear in this revision) ===

- PhabricatorManiphestApplication - loaded the home page
- ManiphestGetTaskTransactionsConduitAPIMethod - used the method via conduit console with input ids : [1, 2] ManiphestTaskDetailController - viewed a task with and without associated projects; finished workflow creating a task with a parent
- ManiphestTransactionPreviewController - verified transaction preview showed up properly
- PhabricatorProjectBoardViewController - viewed a board
- PhabricatorProjectMoveController - moved a task around
- ManiphestRemarkupRule - made a task reference like {T123}
- ManiphestTaskQuery - executed a custom query for all tasks with page size of 2 and paginated through some tasks
- ManiphestTaskPHIDType - nothing random seems broken? =D

=== Call sites where had to do something funky ===

- ManiphestHovercardEventListener - loaded hover cards from task mentions

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T5245

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

src/applications/home/controller/PhabricatorHomeMainController.php

@@ -130,6 +130,7 @@ final class PhabricatorHomeMainController extends PhabricatorHomeController {
       ->setViewer($user)
       ->withStatuses(ManiphestTaskStatus::getOpenStatusConstants())
       ->withPriorities(array($unbreak_now))
+      ->needProjectPHIDs(true)
       ->setLimit(10);
 
     $tasks = $task_query->execute();
@@ -173,6 +174,7 @@ final class PhabricatorHomeMainController extends PhabricatorHomeController {
         ->withStatuses(ManiphestTaskStatus::getOpenStatusConstants())
         ->withPriorities(array($needs_triage))
         ->withAnyProjects(mpull($projects, 'getPHID'))
+        ->needProjectPHIDs(true)
         ->setLimit(10);
       $tasks = $task_query->execute();
     } else {
@@ -269,6 +271,7 @@ final class PhabricatorHomeMainController extends PhabricatorHomeController {
       ->withStatuses(ManiphestTaskStatus::getOpenStatusConstants())
       ->setGroupBy(ManiphestTaskQuery::GROUP_PRIORITY)
       ->withOwners(array($user_phid))
+      ->needProjectPHIDs(true)
       ->setLimit(10);
 
     $tasks = $task_query->execute();

src/applications/maniphest/conduit/ManiphestConduitAPIMethod.php

@@ -237,6 +237,7 @@ abstract class ManiphestConduitAPIMethod extends ConduitAPIMethod {
       ->setViewer($request->getUser())
       ->withPHIDs(array($task->getPHID()))
       ->needSubscriberPHIDs(true)
+      ->needProjectPHIDs(true)
       ->executeOne();
   }
 

src/applications/maniphest/conduit/ManiphestInfoConduitAPIMethod.php

@@ -33,6 +33,7 @@ final class ManiphestInfoConduitAPIMethod extends ManiphestConduitAPIMethod {
       ->setViewer($request->getUser())
       ->withIDs(array($task_id))
       ->needSubscriberPHIDs(true)
+      ->needProjectPHIDs(true)
       ->executeOne();
     if (!$task) {
       throw new ConduitException('ERR_BAD_TASK');

src/applications/maniphest/conduit/ManiphestQueryConduitAPIMethod.php

@@ -61,10 +61,10 @@ class ManiphestQueryConduitAPIMethod extends ManiphestConduitAPIMethod {
   }
 
   protected function execute(ConduitAPIRequest $request) {
-    $query = new ManiphestTaskQuery();
-
-    $query->setViewer($request->getUser());
-    $query->needSubscriberPHIDs(true);
+    $query = id(new ManiphestTaskQuery())
+      ->setViewer($request->getUser())
+      ->needProjectPHIDs(true)
+      ->needSubscriberPHIDs(true);
 
     $task_ids = $request->getValue('ids');
     if ($task_ids) {

src/applications/maniphest/conduit/ManiphestUpdateConduitAPIMethod.php

@@ -34,19 +34,16 @@ final class ManiphestUpdateConduitAPIMethod extends ManiphestConduitAPIMethod {
       throw new Exception("Specify exactly one of 'id' and 'phid'.");
     }
 
+    $query = id (new ManiphestTaskQuery())
+      ->setViewer($request->getUser())
+      ->needSubscriberPHIDs(true)
+      ->needProjectPHIDs(true);
     if ($id) {
-      $task = id(new ManiphestTaskQuery())
-        ->setViewer($request->getUser())
-        ->withIDs(array($id))
-        ->needSubscriberPHIDs(true)
-        ->executeOne();
+      $query->withIDs(array($id));
     } else {
-      $task = id(new ManiphestTaskQuery())
-        ->setViewer($request->getUser())
-        ->withPHIDs(array($phid))
-        ->needSubscriberPHIDs(true)
-        ->executeOne();
+      $query->withPHIDs(array($phid));
     }
+    $task = $query->executeOne();
 
     $params = $request->getAllParameters();
     unset($params['id']);

src/applications/maniphest/controller/ManiphestBatchEditController.php

@@ -19,6 +19,7 @@ final class ManiphestBatchEditController extends ManiphestController {
           PhabricatorPolicyCapability::CAN_EDIT,
         ))
       ->needSubscriberPHIDs(true)
+      ->needProjectPHIDs(true)
       ->execute();
 
     $actions = $request->getStr('actions');

src/applications/maniphest/controller/ManiphestReportController.php

@@ -396,6 +396,12 @@ final class ManiphestReportController extends ManiphestController {
       ->setViewer($user)
       ->withStatuses(ManiphestTaskStatus::getOpenStatusConstants());
 
+    switch ($this->view) {
+      case 'project':
+        $query->needProjectPHIDs(true);
+        break;
+    }
+
     $project_phid = $request->getStr('project');
     $project_handle = null;
     if ($project_phid) {

src/applications/maniphest/controller/ManiphestSubpriorityController.php

@@ -13,6 +13,7 @@ final class ManiphestSubpriorityController extends ManiphestController {
     $task = id(new ManiphestTaskQuery())
       ->setViewer($user)
       ->withIDs(array($request->getInt('task')))
+      ->needProjectPHIDs(true)
       ->requireCapabilities(
         array(
           PhabricatorPolicyCapability::CAN_VIEW,

src/applications/maniphest/controller/ManiphestTaskEditController.php

@@ -39,6 +39,7 @@ final class ManiphestTaskEditController extends ManiphestController {
           ))
         ->withIDs(array($this->id))
         ->needSubscriberPHIDs(true)
+        ->needProjectPHIDs(true)
         ->executeOne();
       if (!$task) {
         return new Aphront404Response();
@@ -446,6 +447,7 @@ final class ManiphestTaskEditController extends ManiphestController {
             ->setViewer($user)
             ->withIDs(array($template_id))
             ->needSubscriberPHIDs(true)
+            ->needProjectPHIDs(true)
             ->executeOne();
           if ($template_task) {
             $cc_phids = array_unique(array_merge(

src/applications/maniphest/controller/ManiphestTransactionSaveController.php

@@ -10,6 +10,7 @@ final class ManiphestTransactionSaveController extends ManiphestController {
       ->setViewer($user)
       ->withIDs(array($request->getStr('taskID')))
       ->needSubscriberPHIDs(true)
+      ->needProjectPHIDs(true)
       ->executeOne();
     if (!$task) {
       return new Aphront404Response();

src/applications/maniphest/editor/ManiphestTransactionEditor.php

@@ -370,6 +370,7 @@ final class ManiphestTransactionEditor
           ->setViewer($this->getActor())
           ->withPHIDs($blocked_phids)
           ->needSubscriberPHIDs(true)
+          ->needProjectPHIDs(true)
           ->execute();
 
         $old = $unblock_xaction->getOldValue();

src/applications/maniphest/event/ManiphestHovercardEventListener.php

@@ -25,6 +25,7 @@ final class ManiphestHovercardEventListener extends PhabricatorEventListener {
       return;
     }
 
+    $e_project = PhabricatorProjectObjectHasProjectEdgeType::EDGECONST;
     // Fun with "Unbeta Pholio", hua hua
     $e_dep_on = PhabricatorEdgeConfig::TYPE_TASK_DEPENDS_ON_TASK;
     $e_dep_by = PhabricatorEdgeConfig::TYPE_TASK_DEPENDED_ON_BY_TASK;
@@ -33,6 +34,7 @@ final class ManiphestHovercardEventListener extends PhabricatorEventListener {
       ->withSourcePHIDs(array($phid))
       ->withEdgeTypes(
         array(
+          $e_project,
           $e_dep_on,
           $e_dep_by,
         ));
@@ -40,12 +42,10 @@ final class ManiphestHovercardEventListener extends PhabricatorEventListener {
     $edge_phids = $edge_query->getDestinationPHIDs();
 
     $owner_phid = $task->getOwnerPHID();
-    $project_phids = $task->getProjectPHIDs();
 
     $phids = array_filter(array_merge(
       array($owner_phid),
-      $edge_phids,
-      $project_phids));
+      $edge_phids));
 
     $viewer_handles = $this->loadHandles($phids, $viewer);
 
@@ -58,17 +58,12 @@ final class ManiphestHovercardEventListener extends PhabricatorEventListener {
     }
 
     $hovercard->addField(pht('Assigned to'), $owner);
-    if ($project_phids) {
-      $hovercard->addField(pht('Projects'),
-        implode_selected_handle_links(', ', $viewer_handles, $project_phids));
-    }
 
     if ($edge_phids) {
       $edge_types = array(
-        PhabricatorEdgeConfig::TYPE_TASK_DEPENDED_ON_BY_TASK
-          => pht('Dependent Tasks'),
-        PhabricatorEdgeConfig::TYPE_TASK_DEPENDS_ON_TASK
-          => pht('Depends On'),
+        $e_project => pht('Projects'),
+        $e_dep_by => pht('Dependent Tasks'),
+        $e_dep_on  => pht('Depends On'),
       );
 
       $max_count = 6;

src/applications/maniphest/mail/ManiphestTaskMailReceiver.php

@@ -18,6 +18,7 @@ final class ManiphestTaskMailReceiver extends PhabricatorObjectMailReceiver {
       ->setViewer($viewer)
       ->withIDs(array($id))
       ->needSubscriberPHIDs(true)
+      ->needProjectPHIDs(true)
       ->execute();
 
     return head($results);

src/applications/maniphest/query/ManiphestTaskQuery.php

@@ -51,6 +51,7 @@ final class ManiphestTaskQuery extends PhabricatorCursorPagedPolicyAwareQuery {
   const ORDER_TITLE         = 'order-title';
 
   private $needSubscriberPHIDs;
+  private $needProjectPHIDs;
 
   const DEFAULT_PAGE_SIZE   = 1000;
 
@@ -185,6 +186,11 @@ final class ManiphestTaskQuery extends PhabricatorCursorPagedPolicyAwareQuery {
     return $this;
   }
 
+  public function needProjectPHIDs($bool) {
+    $this->needProjectPHIDs = $bool;
+    return $this;
+  }
+
   public function loadPage() {
     // TODO: (T603) It is possible for a user to find the PHID of a project
     // they can't see, then query for tasks in that project and deduce the
@@ -340,22 +346,20 @@ final class ManiphestTaskQuery extends PhabricatorCursorPagedPolicyAwareQuery {
   protected function didFilterPage(array $tasks) {
     $phids = mpull($tasks, 'getPHID');
 
-    // TODO: Eventually, we should make this optional and introduce a
-    // needProjectPHIDs() method, but for now there's a lot of code which
-    // assumes the data is always populated.
+    if ($this->needProjectPHIDs) {
+      $edge_query = id(new PhabricatorEdgeQuery())
+        ->withSourcePHIDs($phids)
+        ->withEdgeTypes(
+          array(
+            PhabricatorProjectObjectHasProjectEdgeType::EDGECONST,
+          ));
+      $edge_query->execute();
 
-    $edge_query = id(new PhabricatorEdgeQuery())
-      ->withSourcePHIDs($phids)
-      ->withEdgeTypes(
-        array(
-          PhabricatorProjectObjectHasProjectEdgeType::EDGECONST,
-        ));
-    $edge_query->execute();
-
-    foreach ($tasks as $task) {
-      $project_phids = $edge_query->getDestinationPHIDs(
-        array($task->getPHID()));
-      $task->attachProjectPHIDs($project_phids);
+      foreach ($tasks as $task) {
+        $project_phids = $edge_query->getDestinationPHIDs(
+          array($task->getPHID()));
+        $task->attachProjectPHIDs($project_phids);
+      }
     }
 
     if ($this->needSubscriberPHIDs) {

src/applications/maniphest/query/ManiphestTaskSearchEngine.php

@@ -119,7 +119,8 @@ final class ManiphestTaskSearchEngine
   }
 
   public function buildQueryFromSavedQuery(PhabricatorSavedQuery $saved) {
-    $query = id(new ManiphestTaskQuery());
+    $query = id(new ManiphestTaskQuery())
+      ->needProjectPHIDs(true);
 
     $author_phids = $saved->getParameter('authorPHIDs');
     if ($author_phids) {

src/applications/project/controller/PhabricatorProjectProfileController.php

@@ -143,6 +143,7 @@ final class PhabricatorProjectProfileController
       ->withAnyProjects(array($project->getPHID()))
       ->withStatuses(ManiphestTaskStatus::getOpenStatusConstants())
       ->setOrderBy(ManiphestTaskQuery::ORDER_PRIORITY)
+      ->needProjectPHIDs(true)
       ->setLimit(10);
     $tasks = $query->execute();
 

src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryCommitMessageParserWorker.php

@@ -459,6 +459,7 @@ abstract class PhabricatorRepositoryCommitMessageParserWorker
     $tasks = id(new ManiphestTaskQuery())
       ->setViewer($actor)
       ->withIDs(array_keys($task_statuses))
+      ->needProjectPHIDs(true)
       ->execute();
 
     foreach ($tasks as $task_id => $task) {

src/applications/search/controller/PhabricatorSearchAttachController.php

@@ -144,6 +144,7 @@ final class PhabricatorSearchAttachController
       ->setViewer($user)
       ->withPHIDs(array_keys($phids))
       ->needSubscriberPHIDs(true)
+      ->needProjectPHIDs(true)
       ->execute();
 
     if (empty($targets)) {
@@ -158,9 +159,13 @@ final class PhabricatorSearchAttachController
 
     $cc_vector = array();
     // since we loaded this via a generic object query, go ahead and get the
-    // attach the cc phids now
+    // attach the subscriber and project phids now
     $task->attachSubscriberPHIDs(
       PhabricatorSubscribersQuery::loadSubscribersForPHID($task->getPHID()));
+    $task->attachProjectPHIDs(
+      PhabricatorEdgeQuery::loadDestinationPHIDs($task->getPHID(),
+        PhabricatorProjectObjectHasProjectEdgeType::EDGECONST));
+
     $cc_vector[] = $task->getSubscriberPHIDs();
     foreach ($targets as $target) {
       $cc_vector[] = $target->getSubscriberPHIDs();