Page MenuHomePhabricator
Files F13226524

D10182.id24491.diff
No OneTemporary
Actions

D10182.id24491.diff
View Options

diff --git a/src/applications/maniphest/controller/ManiphestTaskEditController.php b/src/applications/maniphest/controller/ManiphestTaskEditController.php
--- a/src/applications/maniphest/controller/ManiphestTaskEditController.php
+++ b/src/applications/maniphest/controller/ManiphestTaskEditController.php
@@ -387,10 +387,23 @@
->withPHIDs($task_phids)
->execute();
- $sort_map = mpull(
- $column_tasks,
- 'getPrioritySortVector',
- 'getPHID');
+ if ($order == PhabricatorProjectColumn::ORDER_NATURAL) {
+ // TODO: This is a little bit awkward, because PHP and JS use
+ // slightly different sort order parameters to achieve the same
+ // effect. It would be unify this a bit at some point.
+ $sort_map = array();
+ foreach ($positions as $position) {
+ $sort_map[$position->getObjectPHID()] = array(
+ -$position->getSequence(),
+ $position->getID(),
+ );
+ }
+ } else {
+ $sort_map = mpull(
+ $column_tasks,
+ 'getPrioritySortVector',
+ 'getPHID');
+ }
$data = array(
'sortMap' => $sort_map,
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
@@ -190,34 +190,144 @@
pht("Expected 'projectPHID' in column transaction."));
}
+ $old_phids = idx($xaction->getOldValue(), 'columnPHIDs', array());
$new_phids = idx($xaction->getNewValue(), 'columnPHIDs', array());
if (count($new_phids) !== 1) {
throw new Exception(
pht("Expected exactly one 'columnPHIDs' in column transaction."));
}
+ $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');
+
+ 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
+ // (like priority). We can leave the positions untouched and just
+ // bail, there's no work to be done.
+ return;
+ }
+
+ // Otherwise, we're either moving between columns or adjusting the
+ // 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();
}
- // Add the new column position.
+ // Add the new column positions.
foreach ($new_phids as $phid) {
- id(new PhabricatorProjectColumnPosition())
+ $column = idx($columns, $phid);
+ if (!$column) {
+ throw new Exception(
+ pht('No such column "%s" exists!', $phid));
+ }
+
+ // 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.
+
+ $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($phid)
- ->setObjectPHID($object->getPHID())
- // TODO: Do real sequence stuff.
- ->setSequence(0)
- ->save();
+ ->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++;
+ }
+
+ 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++;
+ }
+ }
+
+ // 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!'));
+ }
+
+ // 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:
diff --git a/src/applications/maniphest/storage/ManiphestTransaction.php b/src/applications/maniphest/storage/ManiphestTransaction.php
--- a/src/applications/maniphest/storage/ManiphestTransaction.php
+++ b/src/applications/maniphest/storage/ManiphestTransaction.php
@@ -125,6 +125,16 @@
break;
case self::TYPE_SUBPRIORITY:
return true;
+ case self::TYPE_PROJECT_COLUMN:
+ $old_cols = idx($this->getOldValue(), 'columnPHIDs');
+ $new_cols = idx($this->getNewValue(), 'columnPHIDs');
+
+ $old_cols = array_values($old_cols);
+ $new_cols = array_values($new_cols);
+ sort($old_cols);
+ sort($new_cols);
+
+ return ($old_cols === $new_cols);
}
return parent::shouldHide();
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
@@ -178,6 +178,23 @@
$task_map[$column_phid][] = $task_phid;
}
+ // If we're showing the board in "natural" order, sort columns by their
+ // column positions.
+ if ($this->sortKey == PhabricatorProjectColumn::ORDER_NATURAL) {
+ foreach ($task_map as $column_phid => $task_phids) {
+ $order = array();
+ foreach ($task_phids as $task_phid) {
+ if (isset($positions[$task_phid])) {
+ $order[$task_phid] = $positions[$task_phid]->getOrderingKey();
+ } else {
+ $order[$task_phid] = 0;
+ }
+ }
+ asort($order);
+ $task_map[$column_phid] = array_keys($order);
+ }
+ }
+
$task_can_edit_map = id(new PhabricatorPolicyFilter())
->setViewer($viewer)
->requireCapabilities(array(PhabricatorPolicyCapability::CAN_EDIT))
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
@@ -17,6 +17,8 @@
$object_phid = $request->getStr('objectPHID');
$after_phid = $request->getStr('afterPHID');
$before_phid = $request->getStr('beforePHID');
+ $order = $request->getStr('order', PhabricatorProjectColumn::DEFAULT_ORDER);
+
$project = id(new PhabricatorProjectQuery())
->setViewer($viewer)
@@ -65,13 +67,22 @@
$xactions = array();
+ if ($order == PhabricatorProjectColumn::ORDER_NATURAL) {
+ $order_params = array(
+ 'afterPHID' => $after_phid,
+ 'beforePHID' => $before_phid,
+ );
+ } else {
+ $order_params = array();
+ }
+
$xactions[] = id(new ManiphestTransaction())
->setTransactionType(ManiphestTransaction::TYPE_PROJECT_COLUMN)
->setNewValue(
array(
'columnPHIDs' => array($column->getPHID()),
'projectPHID' => $column->getProjectPHID(),
- ))
+ ) + $order_params)
->setOldValue(
array(
'columnPHIDs' => mpull($positions, 'getColumnPHID'),
@@ -86,7 +97,7 @@
$task_phids[] = $before_phid;
}
- if ($task_phids) {
+ if ($task_phids && ($order == PhabricatorProjectColumn::ORDER_PRIORITY)) {
$tasks = id(new ManiphestTaskQuery())
->setViewer($viewer)
->withPHIDs($task_phids)
diff --git a/src/applications/project/query/PhabricatorProjectColumnPositionQuery.php b/src/applications/project/query/PhabricatorProjectColumnPositionQuery.php
--- a/src/applications/project/query/PhabricatorProjectColumnPositionQuery.php
+++ b/src/applications/project/query/PhabricatorProjectColumnPositionQuery.php
@@ -9,6 +9,7 @@
private $columns;
private $needColumns;
+ private $skipImplicitCreate;
public function withIDs(array $ids) {
$this->ids = $ids;
@@ -46,6 +47,21 @@
return $this;
}
+
+ /**
+ * Skip implicit creation of column positions which are implied but do not
+ * yet exist.
+ *
+ * This is primarily useful internally.
+ *
+ * @param bool True to skip implicit creation of column positions.
+ * @return this
+ */
+ public function setSkipImplicitCreate($skip) {
+ $this->skipImplicitCreate = $skip;
+ return $this;
+ }
+
// NOTE: For now, boards are always attached to projects. However, they might
// not be in the future. This generalization just anticipates a future where
// we let other types of objects (like users) have boards, or let boards
@@ -93,7 +109,7 @@
// column and put it in the default column.
$must_type_filter = false;
- if ($this->columns) {
+ if ($this->columns && !$this->skipImplicitCreate) {
$default_map = array();
foreach ($this->columns as $column) {
if ($column->isDefaultColumn()) {
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
@@ -25,6 +25,17 @@
return $this;
}
+ public function getOrderingKey() {
+ // 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.
+
+ return sprintf(
+ '~%012d%012d',
+ $this->getSequence(),
+ ((1 << 31) - $this->getID()));
+ }
+
/* -( PhabricatorPolicyInterface )----------------------------------------- */
public function getCapabilities() {

File Metadata

Mime Type
text/plain
Expires
Mon, May 20, 9:17 PM (3 w, 5 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6287513
Default Alt Text
D10182.id24491.diff (13 KB)

Event Timeline

Phabricator · Built by Phacility