From 043e0db8d36b85fcb58f9d71838e5f8d6a9af9a7 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Fri, 8 Aug 2014 08:10:49 -0700
Subject: [PATCH] When selecting implicit column positions, actually create
 them

Summary:
Ref T4807. This is an alternative to D10179. The problem these diffs solve is that I want to be able to reorder a column's positions without having to load the actual objects, but that's difficutl because two positions may have the same sequence number (and I think it's good that we allow that, since it makes a bunch of other stuff way easier).

Instead of using the object ID (e.g., the task ID) to reorder positions with the same sequence, use the position itself. This is a little easier, is less ambiguous if columns eventually have several types of objects, and produces a better behavior when old objects are freshly added to a board. For example, if you tag `T300` with `#project`, this new rule will push it to the top of "Backlog" while the old rule might have buried it deep. I think this behavior is desirable and more "natural".

When creating a group of new rows, we do order the batch by ID, so a group of freshly-tagged objects float to the top togehter in ID order. This seems like the most natural rule, too.

Test Plan:
  - Loaded some boards with implicit objects on them (freshly tagged tasks) and saw rows create.
  - Verified new rows created in the right order.
  - Dragged some tasks around.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T4807

Differential Revision: https://secure.phabricator.com/D10180
---
 .../editor/ManiphestTransactionEditor.php     |  4 ---
 .../PhabricatorProjectColumnPositionQuery.php | 34 ++++++++++++++++++-
 2 files changed, 33 insertions(+), 5 deletions(-)

diff --git a/src/applications/maniphest/editor/ManiphestTransactionEditor.php b/src/applications/maniphest/editor/ManiphestTransactionEditor.php
index 2c8f7b8c05..f4069bf183 100644
--- a/src/applications/maniphest/editor/ManiphestTransactionEditor.php
+++ b/src/applications/maniphest/editor/ManiphestTransactionEditor.php
@@ -205,10 +205,6 @@ final class ManiphestTransactionEditor
         // Remove all existing column positions on the board.
 
         foreach ($positions as $position) {
-          if (!$position->getID()) {
-            // This is an ephemeral position, so don't try to destroy it.
-            continue;
-          }
           $position->delete();
         }
 
diff --git a/src/applications/project/query/PhabricatorProjectColumnPositionQuery.php b/src/applications/project/query/PhabricatorProjectColumnPositionQuery.php
index 27786737e0..60e9b2de58 100644
--- a/src/applications/project/query/PhabricatorProjectColumnPositionQuery.php
+++ b/src/applications/project/query/PhabricatorProjectColumnPositionQuery.php
@@ -178,14 +178,46 @@ final class PhabricatorProjectColumnPositionQuery
 
     $positions = $table->loadAllFromArray($data);
 
+    // Find the implied positions which don't exist yet. If there are any,
+    // we're going to create them.
+    $create = array();
     foreach ($positions as $position) {
       if ($position->getColumnPHID() === null) {
-        $position->makeEphemeral();
         $column_phid = idx($default_map, $position->getBoardPHID());
         $position->setColumnPHID($column_phid);
+
+        $create[] = $position;
       }
     }
 
+    if ($create) {
+      // If we're adding several objects to a column, insert the column
+      // position objects in object ID order. This means that newly added
+      // objects float to the top, and when a group of newly added objects
+      // float up at the same time, the most recently created ones end up
+      // highest in the list.
+
+      $objects = id(new PhabricatorObjectQuery())
+        ->setViewer(PhabricatorUser::getOmnipotentUser())
+        ->withPHIDs(mpull($create, 'getObjectPHID'))
+        ->execute();
+      $objects = mpull($objects, null, 'getPHID');
+      $objects = msort($objects, 'getID');
+
+      $create = mgroup($create, 'getObjectPHID');
+      $create = array_select_keys($create, array_keys($objects)) + $create;
+
+      $unguarded = AphrontWriteGuard::beginScopedUnguardedWrites();
+
+        foreach ($create as $object_phid => $create_positions) {
+          foreach ($create_positions as $create_position) {
+            $create_position->save();
+          }
+        }
+
+      unset($unguarded);
+    }
+
     return $positions;
   }