diff --git a/src/applications/maniphest/storage/ManiphestTask.php b/src/applications/maniphest/storage/ManiphestTask.php --- a/src/applications/maniphest/storage/ManiphestTask.php +++ b/src/applications/maniphest/storage/ManiphestTask.php @@ -221,6 +221,39 @@ ); } + private function comparePriorityTo(ManiphestTask $other) { + $upri = $this->getPriority(); + $vpri = $other->getPriority(); + + if ($upri != $vpri) { + return ($upri - $vpri); + } + + $usub = $this->getSubpriority(); + $vsub = $other->getSubpriority(); + + if ($usub != $vsub) { + return ($usub - $vsub); + } + + $uid = $this->getID(); + $vid = $other->getID(); + + if ($uid != $vid) { + return ($uid - $vid); + } + + return 0; + } + + public function isLowerPriorityThan(ManiphestTask $other) { + return ($this->comparePriorityTo($other) < 0); + } + + public function isHigherPriorityThan(ManiphestTask $other) { + return ($this->comparePriorityTo($other) > 0); + } + public function getWorkboardProperties() { return array( 'status' => $this->getStatus(), diff --git a/src/applications/project/controller/PhabricatorProjectMoveController.php b/src/applications/project/controller/PhabricatorProjectMoveController.php --- a/src/applications/project/controller/PhabricatorProjectMoveController.php +++ b/src/applications/project/controller/PhabricatorProjectMoveController.php @@ -90,55 +90,13 @@ 'projectPHID' => $column->getProjectPHID(), )); - $task_phids = array(); - if ($after_phid) { - $task_phids[] = $after_phid; - } - if ($before_phid) { - $task_phids[] = $before_phid; - } - - if ($task_phids && ($order == PhabricatorProjectColumn::ORDER_PRIORITY)) { - $tasks = id(new ManiphestTaskQuery()) - ->setViewer($viewer) - ->withPHIDs($task_phids) - ->needProjectPHIDs(true) - ->requireCapabilities( - array( - PhabricatorPolicyCapability::CAN_VIEW, - PhabricatorPolicyCapability::CAN_EDIT, - )) - ->execute(); - if (count($tasks) != count($task_phids)) { - return new Aphront404Response(); - } - $tasks = mpull($tasks, null, 'getPHID'); - - $try = array( - array($after_phid, true), - array($before_phid, false), - ); - - $pri = null; - $sub = null; - foreach ($try as $spec) { - list($task_phid, $is_after) = $spec; - $task = idx($tasks, $task_phid); - if ($task) { - list($pri, $sub) = ManiphestTransactionEditor::getAdjacentSubpriority( - $task, - $is_after); - break; - } - } - - if ($pri !== null) { - $xactions[] = id(new ManiphestTransaction()) - ->setTransactionType(ManiphestTransaction::TYPE_PRIORITY) - ->setNewValue($pri); - $xactions[] = id(new ManiphestTransaction()) - ->setTransactionType(ManiphestTransaction::TYPE_SUBPRIORITY) - ->setNewValue($sub); + if ($order == PhabricatorProjectColumn::ORDER_PRIORITY) { + $priority_xactions = $this->getPriorityTransactions( + $object, + $after_phid, + $before_phid); + foreach ($priority_xactions as $xaction) { + $xactions[] = $xaction; } } @@ -179,4 +137,97 @@ return $this->newCardResponse($board_phid, $object_phid); } + private function getPriorityTransactions( + ManiphestTask $task, + $after_phid, + $before_phid) { + + list($after_task, $before_task) = $this->loadPriorityTasks( + $after_phid, + $before_phid); + + $must_move = false; + if ($after_task && !$task->isLowerPriorityThan($after_task)) { + $must_move = true; + } + + if ($before_task && !$task->isHigherPriorityThan($before_task)) { + $must_move = true; + } + + // The move doesn't require a priority change to be valid, so don't + // change the priority since we are not being forced to. + if (!$must_move) { + return array(); + } + + $try = array( + array($after_task, true), + array($before_task, false), + ); + + $pri = null; + $sub = null; + foreach ($try as $spec) { + list($task, $is_after) = $spec; + + if (!$task) { + continue; + } + + list($pri, $sub) = ManiphestTransactionEditor::getAdjacentSubpriority( + $task, + $is_after); + } + + $xactions = array(); + if ($pri !== null) { + $xactions[] = id(new ManiphestTransaction()) + ->setTransactionType(ManiphestTransaction::TYPE_PRIORITY) + ->setNewValue($pri); + $xactions[] = id(new ManiphestTransaction()) + ->setTransactionType(ManiphestTransaction::TYPE_SUBPRIORITY) + ->setNewValue($sub); + } + + return $xactions; + } + + private function loadPriorityTasks($after_phid, $before_phid) { + $viewer = $this->getViewer(); + + $task_phids = array(); + + if ($after_phid) { + $task_phids[] = $after_phid; + } + if ($before_phid) { + $task_phids[] = $before_phid; + } + + if (!$task_phids) { + return array(null, null); + } + + $tasks = id(new ManiphestTaskQuery()) + ->setViewer($viewer) + ->withPHIDs($task_phids) + ->execute(); + $tasks = mpull($tasks, null, 'getPHID'); + + if ($after_phid) { + $after_task = idx($tasks, $after_phid); + } else { + $after_task = null; + } + + if ($before_phid) { + $before_task = idx($tasks, $before_phid); + } else { + $before_task = null; + } + + return array($after_task, $before_task); + } + }