Stop reads and writes to projectPHIDs property on ManiphestTask

Summary: Ref T5245. This property predates edges and is unusual in modern applications. Stop writes to it and populate it implicitly from edges when querying.

Test Plan:
  - Viewed task list.
  - Created a task.
  - Added and removed projects from tasks.

Reviewers: joshuaspence, chad, btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T5245

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

src/applications/maniphest/controller/ManiphestTaskEditController.php

@@ -81,7 +81,7 @@ final class ManiphestTaskEditController extends ManiphestController {
             $default_projects = mpull($default_projects, 'getPHID');
 
             if ($default_projects) {
-              $task->setProjectPHIDs($default_projects);
+              $task->attachProjectPHIDs($default_projects);
             }
           }
         }
@@ -215,7 +215,7 @@ final class ManiphestTaskEditController extends ManiphestController {
         $task->setPriority($request->getInt('priority'));
         $task->setOwnerPHID($owner_phid);
         $task->setCCPHIDs($request->getArr('cc'));
-        $task->setProjectPHIDs($request->getArr('projects'));
+        $task->attachProjectPHIDs($request->getArr('projects'));
       } else {
 
         if ($can_edit_priority) {
@@ -438,7 +438,7 @@ final class ManiphestTaskEditController extends ManiphestController {
             ->executeOne();
           if ($template_task) {
             $task->setCCPHIDs($template_task->getCCPHIDs());
-            $task->setProjectPHIDs($template_task->getProjectPHIDs());
+            $task->attachProjectPHIDs($template_task->getProjectPHIDs());
             $task->setOwnerPHID($template_task->getOwnerPHID());
             $task->setPriority($template_task->getPriority());
             $task->setViewPolicy($template_task->getViewPolicy());

src/applications/maniphest/editor/ManiphestTransactionEditor.php

@@ -150,8 +150,9 @@ final class ManiphestTransactionEditor
       case ManiphestTransaction::TYPE_CCS:
         return $object->setCCPHIDs($xaction->getNewValue());
       case ManiphestTransaction::TYPE_PROJECTS:
-        $object->setProjectPHIDs($xaction->getNewValue());
-        ManiphestTaskProject::updateTaskProjects($object);
+        ManiphestTaskProject::updateTaskProjects(
+          $object,
+          $xaction->getNewValue());
         return $object;
       case ManiphestTransaction::TYPE_SUBPRIORITY:
         $data = $xaction->getNewValue();

src/applications/maniphest/query/ManiphestTaskQuery.php

@@ -331,6 +331,27 @@ final class ManiphestTaskQuery extends PhabricatorCursorPagedPolicyAwareQuery {
     return $tasks;
   }
 
+  protected function didFilterPage(array $tasks) {
+    // 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.
+
+    $edge_query = id(new PhabricatorEdgeQuery())
+      ->withSourcePHIDs(mpull($tasks, 'getPHID'))
+      ->withEdgeTypes(
+        array(
+          PhabricatorProjectObjectHasProjectEdgeType::EDGECONST,
+        ));
+    $edge_query->execute();
+
+    foreach ($tasks as $task) {
+      $phids = $edge_query->getDestinationPHIDs(array($task->getPHID()));
+      $task->attachProjectPHIDs($phids);
+    }
+
+    return $tasks;
+  }
+
   private function buildTaskIDsWhereClause(AphrontDatabaseConnection $conn) {
     if (!$this->taskIDs) {
       return null;

src/applications/maniphest/storage/ManiphestTask.php

@@ -37,6 +37,7 @@ final class ManiphestTask extends ManiphestDAO
 
   private $groupByProjectPHID = self::ATTACHABLE;
   private $customFields = self::ATTACHABLE;
+  private $edgeProjectPHIDs = self::ATTACHABLE;
 
   public static function initializeNewTask(PhabricatorUser $actor) {
     $app = id(new PhabricatorApplicationQuery())
@@ -52,7 +53,8 @@ final class ManiphestTask extends ManiphestDAO
       ->setPriority(ManiphestTaskPriority::getDefaultPriority())
       ->setAuthorPHID($actor->getPHID())
       ->setViewPolicy($view_policy)
-      ->setEditPolicy($edit_policy);
+      ->setEditPolicy($edit_policy)
+      ->attachProjectPHIDs(array());
   }
 
   public function getConfiguration() {
@@ -90,13 +92,13 @@ final class ManiphestTask extends ManiphestDAO
     return array_values(nonempty($this->ccPHIDs, array()));
   }
 
-  public function setProjectPHIDs(array $phids) {
-    $this->projectPHIDs = array_values($phids);
-    return $this;
+  public function getProjectPHIDs() {
+    return $this->assertAttached($this->edgeProjectPHIDs);
   }
 
-  public function getProjectPHIDs() {
-    return array_values(nonempty($this->projectPHIDs, array()));
+  public function attachProjectPHIDs(array $phids) {
+    $this->edgeProjectPHIDs = $phids;
+    return $this;
   }
 
   public function setCCPHIDs(array $phids) {

src/applications/maniphest/storage/ManiphestTaskProject.php

@@ -19,13 +19,15 @@ final class ManiphestTaskProject extends ManiphestDAO {
     );
   }
 
-  public static function updateTaskProjects(ManiphestTask $task) {
+  public static function updateTaskProjects(
+    ManiphestTask $task,
+    array $new_phids) {
+
     $edge_type = PhabricatorProjectObjectHasProjectEdgeType::EDGECONST;
 
     $old_phids = PhabricatorEdgeQuery::loadDestinationPHIDs(
       $task->getPHID(),
       $edge_type);
-    $new_phids = $task->getProjectPHIDs();
 
     $add_phids = array_diff($new_phids, $old_phids);
     $rem_phids = array_diff($old_phids, $new_phids);
@@ -34,7 +36,6 @@ final class ManiphestTaskProject extends ManiphestDAO {
       return;
     }
 
-
     $editor = new PhabricatorEdgeEditor();
     foreach ($add_phids as $phid) {
       $editor->addEdge($task->getPHID(), $edge_type, $phid);