Fix an issue where newly created tasks could appear at the bottom of columns

Summary:
Ref T10349. At HEAD, if you create a task //on a board//, it floats to the top correctly.

If you create a task elsewhere and tag it with the board, you were subject to the whims of the layout engine and it would generally end up on the bottom.

Instead, make the rules consistent so that "virtual" positions (of tasks which haven't been committed to a particular position yet) still float to the top.

Test Plan:
  - Created tasks from a board.
  - Created tasks from Maniphest, then looked at them on a board.
  - Moved tasks around.
  - In all cases, newly created tasks floated to the top.
  - Sorted by natural and priority.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10349

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

src/applications/maniphest/editor/ManiphestTransactionEditor.php

@@ -289,8 +289,14 @@ final class ManiphestTransactionEditor
             array($select_phids))
           ->execute();
 
-        $object_phids = mpull($board_tasks, 'getPHID');
-        $object_phids[] = $object_phid;
+        $board_tasks = mpull($board_tasks, null, 'getPHID');
+        $board_tasks[$object_phid] = $object;
+
+        // Make sure tasks are sorted by ID, so we lay out new positions in
+        // a consistent way.
+        $board_tasks = msort($board_tasks, 'getID');
+
+        $object_phids = array_keys($board_tasks);
 
         $engine = id(new PhabricatorBoardLayoutEngine())
           ->setViewer($omnipotent_viewer)

src/applications/project/controller/PhabricatorProjectBoardViewController.php

@@ -124,10 +124,14 @@ final class PhabricatorProjectBoardViewController
 
     $board_phid = $project->getPHID();
 
+    // Regardless of display order, pass tasks to the layout engine in ID order
+    // so layout is consistent.
+    $board_tasks = msort($tasks, 'getID');
+
     $layout_engine = id(new PhabricatorBoardLayoutEngine())
       ->setViewer($viewer)
       ->setBoardPHIDs(array($board_phid))
-      ->setObjectPHIDs(array_keys($tasks))
+      ->setObjectPHIDs(array_keys($board_tasks))
       ->setFetchAllBoards(true)
       ->executeLayout();
 

src/applications/project/engine/PhabricatorBoardLayoutEngine.php

@@ -506,6 +506,7 @@ final class PhabricatorBoardLayoutEngine extends Phobject {
       }
     }
 
+    $view_sequence = 1;
     foreach ($object_phids as $object_phid) {
       $positions = idx($position_groups, $object_phid, array());
 
@@ -554,7 +555,8 @@ final class PhabricatorBoardLayoutEngine extends Phobject {
             ->setBoardPHID($board_phid)
             ->setColumnPHID($proxy_hit)
             ->setObjectPHID($object_phid)
-            ->setSequence(0);
+            ->setSequence(0)
+            ->setViewSequence($view_sequence++);
 
           $this->addQueue[] = $new_position;
 
@@ -578,7 +580,8 @@ final class PhabricatorBoardLayoutEngine extends Phobject {
             ->setBoardPHID($board_phid)
             ->setColumnPHID($default_phid)
             ->setObjectPHID($object_phid)
-            ->setSequence(0);
+            ->setSequence(0)
+            ->setViewSequence($view_sequence++);
 
           $this->addQueue[] = $new_position;
 

src/applications/project/storage/PhabricatorProjectColumnPosition.php

@@ -9,6 +9,7 @@ final class PhabricatorProjectColumnPosition extends PhabricatorProjectDAO
   protected $sequence;
 
   private $column = self::ATTACHABLE;
+  private $viewSequence = 0;
 
   protected function getConfiguration() {
     return array(
@@ -40,18 +41,30 @@ final class PhabricatorProjectColumnPosition extends PhabricatorProjectDAO
     return $this;
   }
 
-  public function getOrderingKey() {
-    if (!$this->getID() && !$this->getSequence()) {
-      return 0;
+  public function setViewSequence($view_sequence) {
+    $this->viewSequence = $view_sequence;
+    return $this;
   }
 
-    // Low sequence numbers go above high sequence numbers.
-    // High position IDs go above low position IDs.
-    // Broadly, this makes newly added stuff float to the top.
+  public function getOrderingKey() {
+    // We're ordering both real positions and "virtual" positions which we have
+    // created but not saved yet.
+
+    // Low sequence numbers go above high sequence numbers. Virtual positions
+    // will have sequence number 0.
+
+    // High virtual sequence numbers go above low virtual sequence numbers.
+    // The layout engine gets objects in ID order, and this puts them in
+    // reverse ID order.
+
+    // High IDs go above low IDs.
+
+    // Broadly, this collectively makes newly added stuff float to the top.
 
     return sprintf(
-      '~%012d%012d',
+      '~%012d%012d%012d',
       $this->getSequence(),
+      ((1 << 31) - $this->viewSequence),
       ((1 << 31) - $this->getID()));
   }