src/applications/project/controller/PhabricatorProjectMoveController.php
95	This should maybe be `if (count($tasks) != count($task_phids))`? That is, if we get one of the tasks but not the other, 404?
103	I think these two conditions might need to be a little more complex -- something like: if ((task.priority < object.priority) \|\| (task.priority == object.priority && task.subpriority < object.subpriority)) { e.g., if we drag a "medium" task below another "medium" task, we might still need to update the subpriority even if we don't update the priority.
120–124	I would expect one of these two transactions to end up with the dropped tasks on the wrong side of the adjacent task? e.g., you drag "X" between "A" and "B", and end up with "XAB" or "ABX", with the right priority but the wrong subpriority? This transaction doesn't know whether the subpriority should be smaller or larger than the base.

This revision is now accepted and ready to land.Mar 26 2014, 10:59 PM

tighten subpriority case

Can you take another gander? I fond the subpriority stuff to be tricky.

src/applications/maniphest/editor/ManiphestTransactionEditor.php
445–463	I feel low confidence here despite this seeming to work now in testing.

That seems like it's OK to me, and I don't see any obvious ways to simplify it. It would also be kind of a pain in the butt to unit test, although we could do it with a fixture. Let's just go with this, and if we hit issues we can put some tests on it to cover whatever weird cases we end up hitting.

This revision is now accepted and ready to land.Mar 27 2014, 1:17 AM

(The subpriority stuff is kind of hacky anyway, and if this does hit issues it might be best to try to refactor it and come up with something a little better. Particularly, the float loses precision after something like 17 drag operations, so if you have "ABC" and repeatedly drag the top one to the middle over and over again, everything eventually stops making sense.)