Roughly implement milestone columns on workboards

Summary:
Ref T10010. These aren't perfect but I think (?) they aren't horribly broken.

  - When a project is a parent project, destroy (as far as the user can tell) any custom columns.
  - When a project has milestones, automatically generate columns on the project's workboard (if it has a workboard).
  - When you move tasks between milestones, add the proper milestone tag.
  - When you move tasks out of milestones back into the backlog, add the proper parent project tag.
  - (Plenty of UI / design stuff to adjust.)

Test Plan:
  - Dragged stuff between milestone columns.
  - Used a normal workboard.
  - Wasn't able to find any egregiously bad cases that did anything terrible.

{F1088224}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10010

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

Author: epriestley

resources/builtin/image-200x200.png

@@ -413,7 +413,7 @@
     'rsrc/js/application/phortune/phortune-credit-card-form.js' => '2290aeef',
     'rsrc/js/application/policy/behavior-policy-control.js' => 'ae45872f',
     'rsrc/js/application/policy/behavior-policy-rule-editor.js' => '5e9f347c',
-    'rsrc/js/application/projects/behavior-project-boards.js' => 'c05fb42a',
+    'rsrc/js/application/projects/behavior-project-boards.js' => '48470f95',
     'rsrc/js/application/projects/behavior-project-create.js' => '065227cc',
     'rsrc/js/application/projects/behavior-reorder-columns.js' => 'e1d25dfb',
     'rsrc/js/application/releeph/releeph-preview-branch.js' => 'b2b4fbaf',
@@ -653,7 +653,7 @@
     'javelin-behavior-phui-profile-menu' => '12884df9',
     'javelin-behavior-policy-control' => 'ae45872f',
     'javelin-behavior-policy-rule-editor' => '5e9f347c',
-    'javelin-behavior-project-boards' => 'c05fb42a',
+    'javelin-behavior-project-boards' => '48470f95',
     'javelin-behavior-project-create' => '065227cc',
     'javelin-behavior-quicksand-blacklist' => '7927a7d3',
     'javelin-behavior-recurring-edit' => '5f1c4d5f',
@@ -1151,6 +1151,15 @@
       'javelin-dom',
       'javelin-workflow',
     ),
+    '48470f95' => array(
+      'javelin-behavior',
+      'javelin-dom',
+      'javelin-util',
+      'javelin-vector',
+      'javelin-stratcom',
+      'javelin-workflow',
+      'phabricator-draggable-list',
+    ),
     '49b73b36' => array(
       'javelin-behavior',
       'javelin-dom',
@@ -1779,15 +1788,6 @@
       'javelin-install',
       'javelin-dom',
     ),
-    'c05fb42a' => array(
-      'javelin-behavior',
-      'javelin-dom',
-      'javelin-util',
-      'javelin-vector',
-      'javelin-stratcom',
-      'javelin-workflow',
-      'phabricator-draggable-list',
-    ),
     'c1700f6f' => array(
       'javelin-install',
       'javelin-util',

resources/celerity/map.php

@@ -0,0 +1,2 @@
+ALTER TABLE {$NAMESPACE}_project.project_column
+  ADD proxyPHID VARBINARY(64);

resources/sql/autopatches/20160202.board.1.proxy.sql

@@ -1889,6 +1889,7 @@
     'PhabricatorChatLogQuery' => 'applications/chatlog/query/PhabricatorChatLogQuery.php',
     'PhabricatorChunkedFileStorageEngine' => 'applications/files/engine/PhabricatorChunkedFileStorageEngine.php',
     'PhabricatorClusterConfigOptions' => 'applications/config/option/PhabricatorClusterConfigOptions.php',
+    'PhabricatorColumnProxyInterface' => 'applications/project/interface/PhabricatorColumnProxyInterface.php',
     'PhabricatorCommentEditEngineExtension' => 'applications/transactions/engineextension/PhabricatorCommentEditEngineExtension.php',
     'PhabricatorCommentEditField' => 'applications/transactions/editfield/PhabricatorCommentEditField.php',
     'PhabricatorCommentEditType' => 'applications/transactions/edittype/PhabricatorCommentEditType.php',
@@ -7262,6 +7263,7 @@
       'PhabricatorDestructibleInterface',
       'PhabricatorFulltextInterface',
       'PhabricatorConduitResultInterface',
+      'PhabricatorColumnProxyInterface',
     ),
     'PhabricatorProjectAddHeraldAction' => 'PhabricatorProjectHeraldAction',
     'PhabricatorProjectApplication' => 'PhabricatorApplication',

src/__phutil_library_map__.php

@@ -280,10 +280,23 @@ private function buildCardResponse(ManiphestTask $task) {
       return new Aphront404Response();
     }
 
-    // If the workboard's project has been removed from the card's project
-    // list, we are going to remove it from the board completely.
+    // If the workboard's project and all descendant projects have been removed
+    // from the card's project list, we are going to remove it from the board
+    // completely.
+
+    // TODO: If the user did something sneaky and changed a subproject, we'll
+    // currently leave the card where it was but should really move it to the
+    // proper new column.
+
+    $descendant_projects = id(new PhabricatorProjectQuery())
+      ->setViewer($viewer)
+      ->withAncestorProjectPHIDs(array($column->getProjectPHID()))
+      ->execute();
+    $board_phids = mpull($descendant_projects, 'getPHID', 'getPHID');
+    $board_phids[$column->getProjectPHID()] = $column->getProjectPHID();
+
     $project_map = array_fuse($task->getProjectPHIDs());
-    $remove_card = empty($project_map[$column->getProjectPHID()]);
+    $remove_card = !array_intersect_key($board_phids, $project_map);
 
     $positions = id(new PhabricatorProjectColumnPositionQuery())
       ->setViewer($viewer)

src/applications/maniphest/editor/ManiphestEditEngine.php

@@ -222,12 +222,22 @@ protected function applyCustomExternalTransaction(
         // can't see.
         $omnipotent_viewer = PhabricatorUser::getOmnipotentUser();
 
+        $select_phids = array($board_phid);
+
+        $descendants = id(new PhabricatorProjectQuery())
+          ->setViewer($omnipotent_viewer)
+          ->withAncestorProjectPHIDs($select_phids)
+          ->execute();
+        foreach ($descendants as $descendant) {
+          $select_phids[] = $descendant->getPHID();
+        }
+
         $board_tasks = id(new ManiphestTaskQuery())
           ->setViewer($omnipotent_viewer)
           ->withEdgeLogicPHIDs(
             PhabricatorProjectObjectHasProjectEdgeType::EDGECONST,
-            PhabricatorQueryConstraint::OPERATOR_AND,
-            array($board_phid))
+            PhabricatorQueryConstraint::OPERATOR_ANCESTOR,
+            array($select_phids))
           ->execute();
 
         $object_phids = mpull($board_tasks, 'getPHID');

src/applications/maniphest/editor/ManiphestTransactionEditor.php

@@ -972,7 +972,45 @@ public function testBoardMoves() {
       $task1->getPHID(),
     );
     $this->assertTasksInColumn($expect, $user, $board, $column);
+  }
+
+  public function testMilestoneMoves() {
+    $user = $this->createUser();
+    $user->save();
+
+    $board = $this->createProject($user);
+
+    $backlog = $this->addColumn($user, $board, 0);
+
+    // Create a task into the backlog.
+    $task = $this->newTask($user, array($board));
+    $expect = array(
+      $backlog->getPHID(),
+    );
+    $this->assertColumns($expect, $user, $board, $task);
+
+    $milestone = $this->createProject($user, $board, true);
+
+    $this->addProjectTags($user, $task, array($milestone->getPHID()));
+
+    // We just want the side effect of looking at the board: creation of the
+    // milestone column.
+    $this->loadColumns($user, $board, $task);
+
+    $column = id(new PhabricatorProjectColumnQuery())
+      ->setViewer($user)
+      ->withProjectPHIDs(array($board->getPHID()))
+      ->withProxyPHIDs(array($milestone->getPHID()))
+      ->executeOne();
+
+    $this->assertTrue((bool)$column);
 
+    // Moving the task to the milestone should have moved it to the milestone
+    // column.
+    $expect = array(
+      $column->getPHID(),
+    );
+    $this->assertColumns($expect, $user, $board, $task);
   }
 
   private function moveToColumn(
@@ -1014,7 +1052,14 @@ private function assertColumns(
     PhabricatorUser $viewer,
     PhabricatorProject $board,
     ManiphestTask $task) {
+    $column_phids = $this->loadColumns($viewer, $board, $task);
+    $this->assertEqual($expect, $column_phids);
+  }
 
+  private function loadColumns(
+    PhabricatorUser $viewer,
+    PhabricatorProject $board,
+    ManiphestTask $task) {
     $engine = id(new PhabricatorBoardLayoutEngine())
       ->setViewer($viewer)
       ->setBoardPHIDs(array($board->getPHID()))
@@ -1028,7 +1073,7 @@ private function assertColumns(
     $column_phids = mpull($columns, 'getPHID');
     $column_phids = array_values($column_phids);
 
-    $this->assertEqual($expect, $column_phids);
+    return $column_phids;
   }
 
   private function assertTasksInColumn(
@@ -1236,6 +1281,16 @@ private function createProject(
 
     $this->applyTransactions($project, $user, $xactions);
 
+    // Force these values immediately; they are normally updated by the
+    // index engine.
+    if ($parent) {
+      if ($is_milestone) {
+        $parent->setHasMilestones(1)->save();
+      } else {
+        $parent->setHasSubprojects(1)->save();
+      }
+    }
+
     return $project;
   }
 

src/applications/project/__tests__/PhabricatorProjectCoreTestCase.php

@@ -95,17 +95,27 @@ public function handleRequest(AphrontRequest $request) {
 
     $task_query = $search_engine->buildQueryFromSavedQuery($saved);
 
+    $select_phids = array($project->getPHID());
+    if ($project->getHasSubprojects() || $project->getHasMilestones()) {
+      $descendants = id(new PhabricatorProjectQuery())
+        ->setViewer($viewer)
+        ->withAncestorProjectPHIDs($select_phids)
+        ->execute();
+      foreach ($descendants as $descendant) {
+        $select_phids[] = $descendant->getPHID();
+      }
+    }
+
     $tasks = $task_query
       ->withEdgeLogicPHIDs(
         PhabricatorProjectObjectHasProjectEdgeType::EDGECONST,
-        PhabricatorQueryConstraint::OPERATOR_AND,
-        array($project->getPHID()))
+        PhabricatorQueryConstraint::OPERATOR_ANCESTOR,
+        array($select_phids))
       ->setOrder(ManiphestTaskQuery::ORDER_PRIORITY)
       ->setViewer($viewer)
       ->execute();
     $tasks = mpull($tasks, null, 'getPHID');
 
-
     $board_phid = $project->getPHID();
 
     $layout_engine = id(new PhabricatorBoardLayoutEngine())
@@ -225,6 +235,13 @@ public function handleRequest(AphrontRequest $request) {
         }
       }
 
+      $proxy = $column->getProxy();
+      if ($proxy && !$proxy->isMilestone()) {
+        // TODO: For now, don't show subproject columns because we can't
+        // handle tasks with multiple positions yet.
+        continue;
+      }
+
       $task_phids = $layout_engine->getColumnObjectPHIDs(
         $board_phid,
         $column->getPHID());
@@ -247,6 +264,11 @@ public function handleRequest(AphrontRequest $request) {
         $panel->setHeaderIcon($header_icon);
       }
 
+      $display_class = $column->getDisplayClass();
+      if ($display_class) {
+        $panel->addClass($display_class);
+      }
+
       if ($column->isHidden()) {
         $panel->addClass('project-panel-hidden');
       }
@@ -582,6 +604,12 @@ private function buildColumnMenu(
 
     $column_items = array();
 
+    if ($column->getProxyPHID()) {
+      $default_phid = $column->getProxyPHID();
+    } else {
+      $default_phid = $column->getProjectPHID();
+    }
+
     $column_items[] = id(new PhabricatorActionView())
       ->setIcon('fa-plus')
       ->setName(pht('Create Task...'))
@@ -590,6 +618,7 @@ private function buildColumnMenu(
       ->setMetadata(
         array(
           'columnPHID' => $column->getPHID(),
+          'projectPHID' => $default_phid,
         ));
 
     $batch_edit_uri = $request->getRequestURI();
@@ -738,6 +767,10 @@ private function buildInitializeContent(PhabricatorProject $project) {
       }
     }
 
+    // TODO: Tailor this UI if the project is already a parent project. We
+    // should not offer options for creating a parent project workboard, since
+    // they can't have their own columns.
+
     $new_selector = id(new AphrontFormRadioButtonControl())
       ->setLabel(pht('Columns'))
       ->setName('initialize-type')

src/applications/project/controller/PhabricatorProjectBoardViewController.php

@@ -139,7 +139,33 @@ public function handleRequest(AphrontRequest $request) {
           ->setTransactionType(ManiphestTransaction::TYPE_SUBPRIORITY)
           ->setNewValue($sub);
       }
-   }
+    }
+
+    $proxy = $column->getProxy();
+    if ($proxy) {
+      // We're moving the task into a subproject or milestone column, so add
+      // the subproject or milestone.
+      $add_projects = array($proxy->getPHID());
+    } else if ($project->getHasSubprojects() || $project->getHasMilestones()) {
+      // We're moving the task into the "Backlog" column on the parent project,
+      // so add the parent explicitly. This gets rid of any subproject or
+      // milestone tags.
+      $add_projects = array($project->getPHID());
+    } else {
+      $add_projects = array();
+    }
+
+    if ($add_projects) {
+      $project_type = PhabricatorProjectObjectHasProjectEdgeType::EDGECONST;
+
+      $xactions[] = id(new ManiphestTransaction())
+        ->setTransactionType(PhabricatorTransactions::TYPE_EDGE)
+        ->setMetadataValue('edge:type', $project_type)
+        ->setNewValue(
+          array(
+            '+' => array_fuse($add_projects),
+          ));
+    }
 
     $editor = id(new ManiphestTransactionEditor())
       ->setActor($viewer)
@@ -157,6 +183,18 @@ public function handleRequest(AphrontRequest $request) {
         ->executeOne();
     }
 
+    // Reload the object so it reflects edits which have been applied.
+    $object = id(new ManiphestTaskQuery())
+      ->setViewer($viewer)
+      ->withPHIDs(array($object_phid))
+      ->needProjectPHIDs(true)
+      ->requireCapabilities(
+        array(
+          PhabricatorPolicyCapability::CAN_VIEW,
+          PhabricatorPolicyCapability::CAN_EDIT,
+        ))
+      ->executeOne();
+
     $card = id(new ProjectBoardTaskCard())
       ->setViewer($viewer)
       ->setTask($object)
@@ -169,6 +207,6 @@ public function handleRequest(AphrontRequest $request) {
 
     return id(new AphrontAjaxResponse())->setContent(
       array('task' => $card));
- }
+  }
 
 }

src/applications/project/controller/PhabricatorProjectMoveController.php

@@ -35,7 +35,7 @@ public function handleRequest(AphrontRequest $request) {
 
     $conversion_help = pht(
       "Creating a project's first subproject **moves all ".
-      "members** and **destroys all workboard columns**.".
+      "members** to become members of the subproject instead".
       "\n\n".
       "See [[ %s | Projects User Guide ]] in the documentation for details. ".
       "This process can not be undone.",