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 @@ -200,21 +200,18 @@ 'columnPHIDs')); } - $columns = id(new PhabricatorProjectColumnQuery()) - ->setViewer($this->requireActor()) - ->withPHIDs($new_phids) - ->execute(); - $columns = mpull($columns, null, 'getPHID'); - - $positions = id(new PhabricatorProjectColumnPositionQuery()) - ->setViewer($this->requireActor()) - ->withObjectPHIDs(array($object->getPHID())) - ->withBoardPHIDs(array($board_phid)) - ->execute(); - $before_phid = idx($xaction->getNewValue(), 'beforePHID'); $after_phid = idx($xaction->getNewValue(), 'afterPHID'); + // throw new Exception( + // print_r( + // array( + // 'before' => $before_phid, + // 'after' => $after_phid, + // 'old' => $old_phids, + // 'new' => $new_phids, + // ), true)); + if (!$before_phid && !$after_phid && ($old_phids == $new_phids)) { // If we are not moving the object between columns and also not // reordering the position, this is a move on some other order @@ -227,111 +224,76 @@ // object's position in the "natural" ordering, so we do need to update // some rows. - // Remove all existing column positions on the board. - - foreach ($positions as $position) { - $position->delete(); - } + $object_phid = $object->getPHID(); - // Add the new column positions. + // We're doing layout with the ominpotent viewer to make sure we don't + // remove positions in columns that exist, but which the actual actor + // can't see. + $omnipotent_viewer = PhabricatorUser::getOmnipotentUser(); - foreach ($new_phids as $phid) { - $column = idx($columns, $phid); - if (!$column) { - throw new Exception( - pht('No such column "%s" exists!', $phid)); - } + $board_tasks = id(new ManiphestTaskQuery()) + ->setViewer($omnipotent_viewer) + ->withEdgeLogicPHIDs( + PhabricatorProjectObjectHasProjectEdgeType::EDGECONST, + PhabricatorQueryConstraint::OPERATOR_AND, + array($board_phid)) + ->execute(); - // Load the other object positions in the column. Note that we must - // skip implicit column creation to avoid generating a new position - // if the target column is a backlog column. + $object_phids = mpull($board_tasks, 'getPHID'); + $object_phids[] = $object_phid; + + $engine = id(new PhabricatorBoardLayoutEngine()) + ->setViewer($omnipotent_viewer) + ->setBoardPHIDs(array($board_phid)) + ->setObjectPHIDs($object_phids) + ->executeLayout(); + + // TODO: This logic needs to be revised if we legitimately support + // multiple column positions. + + // NOTE: When a task is newly created, it's implicitly added to the + // backlog but we don't currently record that in the "$old_phids". Just + // clean it up for now. + $columns = $engine->getObjectColumns($board_phid, $object_phid); + foreach ($columns as $column) { + $engine->queueRemovePosition( + $board_phid, + $column->getPHID(), + $object_phid); + } - $other_positions = id(new PhabricatorProjectColumnPositionQuery()) - ->setViewer($this->requireActor()) - ->withColumns(array($column)) - ->withBoardPHIDs(array($board_phid)) - ->setSkipImplicitCreate(true) - ->execute(); - $other_positions = msort($other_positions, 'getOrderingKey'); - - // Set up the new position object. We're going to figure out the - // right sequence number and then persist this object with that - // sequence number. - $new_position = id(new PhabricatorProjectColumnPosition()) - ->setBoardPHID($board_phid) - ->setColumnPHID($column->getPHID()) - ->setObjectPHID($object->getPHID()); - - $updates = array(); - $sequence = 0; - - // If we're just dropping this into the column without any specific - // position information, put it at the top. - if (!$before_phid && !$after_phid) { - $new_position->setSequence($sequence)->save(); - $sequence++; - } + // Remove all existing column positions on the board. + foreach ($old_phids as $column_phid) { + $engine->queueRemovePosition( + $board_phid, + $column_phid, + $object_phid); + } - foreach ($other_positions as $position) { - $object_phid = $position->getObjectPHID(); - - // If this is the object we're moving before and we haven't - // saved yet, insert here. - if (($before_phid == $object_phid) && !$new_position->getID()) { - $new_position->setSequence($sequence)->save(); - $sequence++; - } - - // This object goes here in the sequence; we might need to update - // the row. - if ($sequence != $position->getSequence()) { - $updates[$position->getID()] = $sequence; - } - $sequence++; - - // If this is the object we're moving after and we haven't saved - // yet, insert here. - if (($after_phid == $object_phid) && !$new_position->getID()) { - $new_position->setSequence($sequence)->save(); - $sequence++; - } + // Add new positions. + foreach ($new_phids as $column_phid) { + if ($before_phid) { + $engine->queueAddPositionBefore( + $board_phid, + $column_phid, + $object_phid, + $before_phid); + } else if ($after_phid) { + $engine->queueAddPositionAfter( + $board_phid, + $column_phid, + $object_phid, + $after_phid); + } else { + $engine->queueAddPosition( + $board_phid, + $column_phid, + $object_phid); } + } - // We should have found a place to put it. - if (!$new_position->getID()) { - throw new Exception( - pht('Unable to find a place to insert object on column!')); - } + $engine->applyPositionUpdates(); - // If we changed other objects' column positions, bulk reorder them. - - if ($updates) { - $position = new PhabricatorProjectColumnPosition(); - $conn_w = $position->establishConnection('w'); - - $pairs = array(); - foreach ($updates as $id => $sequence) { - // This is ugly because MySQL gets upset with us if it is - // configured strictly and we attempt inserts which can't work. - // We'll never actually do these inserts since they'll always - // collide (triggering the ON DUPLICATE KEY logic), so we just - // provide dummy values in order to get there. - - $pairs[] = qsprintf( - $conn_w, - '(%d, %d, "", "", "")', - $id, - $sequence); - } - - queryfx( - $conn_w, - 'INSERT INTO %T (id, sequence, boardPHID, columnPHID, objectPHID) - VALUES %Q ON DUPLICATE KEY UPDATE sequence = VALUES(sequence)', - $position->getTableName(), - implode(', ', $pairs)); - } - } break; default: break; 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 @@ -10,6 +10,9 @@ private $objectColumnMap = array(); private $boardLayout = array(); + private $remQueue = array(); + private $addQueue = array(); + public function setViewer(PhabricatorUser $viewer) { $this->viewer = $viewer; return $this; @@ -76,6 +79,207 @@ return array_select_keys($this->columnMap, $column_phids); } + public function queueRemovePosition( + $board_phid, + $column_phid, + $object_phid) { + + $board_layout = idx($this->boardLayout, $board_phid, array()); + $positions = idx($board_layout, $column_phid, array()); + $position = idx($positions, $object_phid); + + if ($position) { + $this->remQueue[] = $position; + + // If this position hasn't been saved yet, get it out of the add queue. + if (!$position->getID()) { + foreach ($this->addQueue as $key => $add_position) { + if ($add_position === $position) { + unset($this->addQueue[$key]); + } + } + } + } + + unset($this->boardLayout[$board_phid][$column_phid][$object_phid]); + + return $this; + } + + public function queueAddPositionBefore( + $board_phid, + $column_phid, + $object_phid, + $before_phid) { + + return $this->queueAddPositionRelative( + $board_phid, + $column_phid, + $object_phid, + $before_phid, + true); + } + + public function queueAddPositionAfter( + $board_phid, + $column_phid, + $object_phid, + $after_phid) { + + return $this->queueAddPositionRelative( + $board_phid, + $column_phid, + $object_phid, + $after_phid, + false); + } + + public function queueAddPosition( + $board_phid, + $column_phid, + $object_phid) { + return $this->queueAddPositionRelative( + $board_phid, + $column_phid, + $object_phid, + null, + true); + } + + private function queueAddPositionRelative( + $board_phid, + $column_phid, + $object_phid, + $relative_phid, + $is_before) { + + $board_layout = idx($this->boardLayout, $board_phid, array()); + $positions = idx($board_layout, $column_phid, array()); + + // Check if the object is already in the column, and remove it if it is. + $object_position = idx($positions, $object_phid); + unset($positions[$object_phid]); + + if (!$object_position) { + $object_position = id(new PhabricatorProjectColumnPosition()) + ->setBoardPHID($board_phid) + ->setColumnPHID($column_phid) + ->setObjectPHID($object_phid); + } + + $found = false; + if (!$positions) { + $object_position->setSequence(0); + } else { + foreach ($positions as $position) { + if (!$found) { + if ($relative_phid === null) { + $is_match = true; + } else { + $position_phid = $position->getObjectPHID(); + $is_match = ($relative_phid == $position_phid); + } + + if ($is_match) { + $found = true; + + $sequence = $position->getSequence(); + + if (!$is_before) { + $sequence++; + } + + $object_position->setSequence($sequence++); + + if (!$is_before) { + // If we're inserting after this position, continue the loop so + // we don't update it. + continue; + } + } + } + + if ($found) { + $position->setSequence($sequence++); + $this->addQueue[] = $position; + } + } + } + + if ($relative_phid && !$found) { + throw new Exception( + pht( + 'Unable to find object "%s" in column "%s" on board "%s".', + $relative_phid, + $column_phid, + $board_phid)); + } + + $this->addQueue[] = $object_position; + + $positions[$object_phid] = $object_position; + $positions = msort($positions, 'getOrderingKey'); + + $this->boardLayout[$board_phid][$column_phid] = $positions; + + return $this; + } + + public function applyPositionUpdates() { + foreach ($this->remQueue as $position) { + if ($position->getID()) { + $position->delete(); + } + } + $this->remQueue = array(); + + $adds = array(); + $updates = array(); + + foreach ($this->addQueue as $position) { + $id = $position->getID(); + if ($id) { + $updates[$id] = $position; + } else { + $adds[] = $position; + } + } + $this->addQueue = array(); + + $table = new PhabricatorProjectColumnPosition(); + $conn_w = $table->establishConnection('w'); + + $pairs = array(); + foreach ($updates as $id => $position) { + // This is ugly because MySQL gets upset with us if it is configured + // strictly and we attempt inserts which can't work. We'll never actually + // do these inserts since they'll always collide (triggering the ON + // DUPLICATE KEY logic), so we just provide dummy values in order to get + // there. + + $pairs[] = qsprintf( + $conn_w, + '(%d, %d, "", "", "")', + $id, + $position->getSequence()); + } + + if ($pairs) { + queryfx( + $conn_w, + 'INSERT INTO %T (id, sequence, boardPHID, columnPHID, objectPHID) + VALUES %Q ON DUPLICATE KEY UPDATE sequence = VALUES(sequence)', + $table->getTableName(), + implode(', ', $pairs)); + } + + foreach ($adds as $position) { + $position->save(); + } + + return $this; + } + private function loadBoards() { $viewer = $this->getViewer(); $board_phids = $this->getBoardPHIDs(); @@ -114,10 +318,15 @@ private function loadPositions(array $boards) { $viewer = $this->getViewer(); + $object_phids = $this->getObjectPHIDs(); + if (!$object_phids) { + return array(); + } + $positions = id(new PhabricatorProjectColumnPositionQuery()) ->setViewer($viewer) ->withBoardPHIDs(array_keys($boards)) - ->withObjectPHIDs($this->getObjectPHIDs()) + ->withObjectPHIDs($object_phids) ->execute(); $positions = msort($positions, 'getOrderingKey'); $positions = mgroup($positions, 'getBoardPHID'); @@ -150,6 +359,7 @@ foreach ($positions as $key => $position) { $column_phid = $position->getColumnPHID(); if (empty($columns[$column_phid])) { + $this->remQueue[] = $position; unset($positions[$key]); } } @@ -161,6 +371,9 @@ ->setColumnPHID($default_phid) ->setObjectPHID($object_phid) ->setSequence(0); + + $this->addQueue[] = $new_position; + $positions = array( $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 @@ -41,7 +41,7 @@ } public function getOrderingKey() { - if (!$this->getID()) { + if (!$this->getID() && !$this->getSequence()) { return 0; }