Page MenuHomePhabricator
Files F13965185

D15175.id36633.diff
No OneTemporary
Actions

D15175.id36633.diff
View Options

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;
}

File Metadata

Mime Type
text/plain
Expires
Oct 17 2024, 12:28 AM (4 w, 5 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6719833
Default Alt Text
D15175.id36633.diff (15 KB)

Event Timeline

Phabricator · Built by Phacility