diff --git a/src/applications/maniphest/editor/ManiphestTransactionEditor.php b/src/applications/maniphest/editor/ManiphestTransactionEditor.php --- a/src/applications/maniphest/editor/ManiphestTransactionEditor.php +++ b/src/applications/maniphest/editor/ManiphestTransactionEditor.php @@ -289,8 +289,14 @@ 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) diff --git a/src/applications/project/controller/PhabricatorProjectBoardViewController.php b/src/applications/project/controller/PhabricatorProjectBoardViewController.php --- a/src/applications/project/controller/PhabricatorProjectBoardViewController.php +++ b/src/applications/project/controller/PhabricatorProjectBoardViewController.php @@ -124,10 +124,14 @@ $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(); diff --git a/src/applications/project/engine/PhabricatorBoardLayoutEngine.php b/src/applications/project/engine/PhabricatorBoardLayoutEngine.php --- a/src/applications/project/engine/PhabricatorBoardLayoutEngine.php +++ b/src/applications/project/engine/PhabricatorBoardLayoutEngine.php @@ -506,6 +506,7 @@ } } + $view_sequence = 1; foreach ($object_phids as $object_phid) { $positions = idx($position_groups, $object_phid, array()); @@ -554,7 +555,8 @@ ->setBoardPHID($board_phid) ->setColumnPHID($proxy_hit) ->setObjectPHID($object_phid) - ->setSequence(0); + ->setSequence(0) + ->setViewSequence($view_sequence++); $this->addQueue[] = $new_position; @@ -578,7 +580,8 @@ ->setBoardPHID($board_phid) ->setColumnPHID($default_phid) ->setObjectPHID($object_phid) - ->setSequence(0); + ->setSequence(0) + ->setViewSequence($view_sequence++); $this->addQueue[] = $new_position; diff --git a/src/applications/project/storage/PhabricatorProjectColumnPosition.php b/src/applications/project/storage/PhabricatorProjectColumnPosition.php --- a/src/applications/project/storage/PhabricatorProjectColumnPosition.php +++ b/src/applications/project/storage/PhabricatorProjectColumnPosition.php @@ -9,6 +9,7 @@ protected $sequence; private $column = self::ATTACHABLE; + private $viewSequence = 0; protected function getConfiguration() { return array( @@ -40,18 +41,30 @@ return $this; } + public function setViewSequence($view_sequence) { + $this->viewSequence = $view_sequence; + return $this; + } + public function getOrderingKey() { - if (!$this->getID() && !$this->getSequence()) { - return 0; - } + // 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. - // 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. + // 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())); }