Page MenuHomePhabricator

D15634.id37673.diff
No OneTemporary

D15634.id37673.diff

diff --git a/src/applications/maniphest/editor/ManiphestEditEngine.php b/src/applications/maniphest/editor/ManiphestEditEngine.php
--- a/src/applications/maniphest/editor/ManiphestEditEngine.php
+++ b/src/applications/maniphest/editor/ManiphestEditEngine.php
@@ -98,11 +98,13 @@
id(new PhabricatorHandlesEditField())
->setKey('column')
->setLabel(pht('Column'))
- ->setDescription(pht('Workboard column to create this task into.'))
- ->setConduitDescription(pht('Create into a workboard column.'))
- ->setConduitTypeDescription(pht('PHID of workboard column.'))
- ->setAliases(array('columnPHID'))
- ->setTransactionType(ManiphestTransaction::TYPE_COLUMN)
+ ->setDescription(pht('Create a task in a workboard column.'))
+ ->setConduitDescription(
+ pht('Move a task to one or more workboard columns.'))
+ ->setConduitTypeDescription(
+ pht('PHID or PHIDs of workboard columns.'))
+ ->setAliases(array('columnPHID', 'columns', 'columnPHIDs'))
+ ->setTransactionType(PhabricatorTransactions::TYPE_COLUMNS)
->setSingleValue(null)
->setIsInvisible(true)
->setIsReorderable(false)
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
@@ -3,6 +3,8 @@
final class ManiphestTransactionEditor
extends PhabricatorApplicationTransactionEditor {
+ private $moreValidationErrors = array();
+
public function getEditorApplicationClass() {
return 'PhabricatorManiphestApplication';
}
@@ -22,14 +24,13 @@
$types[] = ManiphestTransaction::TYPE_DESCRIPTION;
$types[] = ManiphestTransaction::TYPE_OWNER;
$types[] = ManiphestTransaction::TYPE_SUBPRIORITY;
- $types[] = ManiphestTransaction::TYPE_PROJECT_COLUMN;
$types[] = ManiphestTransaction::TYPE_MERGED_INTO;
$types[] = ManiphestTransaction::TYPE_MERGED_FROM;
$types[] = ManiphestTransaction::TYPE_UNBLOCK;
$types[] = ManiphestTransaction::TYPE_PARENT;
- $types[] = ManiphestTransaction::TYPE_COLUMN;
$types[] = ManiphestTransaction::TYPE_COVER_IMAGE;
$types[] = ManiphestTransaction::TYPE_POINTS;
+ $types[] = PhabricatorTransactions::TYPE_COLUMNS;
$types[] = PhabricatorTransactions::TYPE_VIEW_POLICY;
$types[] = PhabricatorTransactions::TYPE_EDIT_POLICY;
@@ -63,9 +64,6 @@
return $object->getDescription();
case ManiphestTransaction::TYPE_OWNER:
return nonempty($object->getOwnerPHID(), null);
- case ManiphestTransaction::TYPE_PROJECT_COLUMN:
- // These are pre-populated.
- return $xaction->getOldValue();
case ManiphestTransaction::TYPE_SUBPRIORITY:
return $object->getSubpriority();
case ManiphestTransaction::TYPE_COVER_IMAGE:
@@ -80,7 +78,7 @@
case ManiphestTransaction::TYPE_MERGED_FROM:
return null;
case ManiphestTransaction::TYPE_PARENT:
- case ManiphestTransaction::TYPE_COLUMN:
+ case PhabricatorTransactions::TYPE_COLUMNS:
return null;
}
}
@@ -98,14 +96,13 @@
case ManiphestTransaction::TYPE_TITLE:
case ManiphestTransaction::TYPE_DESCRIPTION:
case ManiphestTransaction::TYPE_SUBPRIORITY:
- case ManiphestTransaction::TYPE_PROJECT_COLUMN:
case ManiphestTransaction::TYPE_MERGED_INTO:
case ManiphestTransaction::TYPE_MERGED_FROM:
case ManiphestTransaction::TYPE_UNBLOCK:
case ManiphestTransaction::TYPE_COVER_IMAGE:
return $xaction->getNewValue();
case ManiphestTransaction::TYPE_PARENT:
- case ManiphestTransaction::TYPE_COLUMN:
+ case PhabricatorTransactions::TYPE_COLUMNS:
return $xaction->getNewValue();
case ManiphestTransaction::TYPE_POINTS:
$value = $xaction->getNewValue();
@@ -127,12 +124,8 @@
$new = $xaction->getNewValue();
switch ($xaction->getTransactionType()) {
- case ManiphestTransaction::TYPE_PROJECT_COLUMN:
- $new_column_phids = $new['columnPHIDs'];
- $old_column_phids = $old['columnPHIDs'];
- sort($new_column_phids);
- sort($old_column_phids);
- return ($old !== $new);
+ case PhabricatorTransactions::TYPE_COLUMNS:
+ return (bool)$new;
}
return parent::transactionHasEffect($object, $xaction);
@@ -175,9 +168,6 @@
case ManiphestTransaction::TYPE_SUBPRIORITY:
$object->setSubpriority($xaction->getNewValue());
return;
- case ManiphestTransaction::TYPE_PROJECT_COLUMN:
- // these do external (edge) updates
- return;
case ManiphestTransaction::TYPE_MERGED_INTO:
$object->setStatus(ManiphestTaskStatus::getDuplicateStatus());
return;
@@ -212,7 +202,7 @@
return;
case ManiphestTransaction::TYPE_MERGED_FROM:
case ManiphestTransaction::TYPE_PARENT:
- case ManiphestTransaction::TYPE_COLUMN:
+ case PhabricatorTransactions::TYPE_COLUMNS:
return;
}
}
@@ -231,125 +221,10 @@
->addEdge($parent_phid, $parent_type, $task_phid)
->save();
break;
- case ManiphestTransaction::TYPE_PROJECT_COLUMN:
- $board_phid = idx($xaction->getNewValue(), 'projectPHID');
- if (!$board_phid) {
- throw new Exception(
- pht(
- "Expected '%s' in column transaction.",
- 'projectPHID'));
+ case PhabricatorTransactions::TYPE_COLUMNS:
+ foreach ($xaction->getNewValue() as $move) {
+ $this->applyBoardMove($object, $move);
}
-
- $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 '%s' in column transaction.",
- 'columnPHIDs'));
- }
-
- $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.
-
- $object_phid = $object->getPHID();
-
- // 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();
-
- $select_phids = array($board_phid);
-
- $descendants = id(new PhabricatorProjectQuery())
- ->setViewer($omnipotent_viewer)
- ->withAncestorProjectPHIDs($select_phids)
- ->execute();
- foreach ($descendants as $descendant) {
- $select_phids[] = $descendant->getPHID();
- }
-
- $board_tasks = id(new ManiphestTaskQuery())
- ->setViewer($omnipotent_viewer)
- ->withEdgeLogicPHIDs(
- PhabricatorProjectObjectHasProjectEdgeType::EDGECONST,
- PhabricatorQueryConstraint::OPERATOR_ANCESTOR,
- array($select_phids))
- ->execute();
-
- $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)
- ->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);
- }
-
- // Remove all existing column positions on the board.
- foreach ($old_phids as $column_phid) {
- $engine->queueRemovePosition(
- $board_phid,
- $column_phid,
- $object_phid);
- }
-
- // 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);
- }
- }
-
- $engine->applyPositionUpdates();
-
break;
default:
break;
@@ -492,13 +367,12 @@
$board_phids = array();
- $type_column = ManiphestTransaction::TYPE_PROJECT_COLUMN;
+ $type_columns = PhabricatorTransactions::TYPE_COLUMNS;
foreach ($xactions as $xaction) {
- if ($xaction->getTransactionType() == $type_column) {
- $new = $xaction->getNewValue();
- $project_phid = idx($new, 'projectPHID');
- if ($project_phid) {
- $board_phids[] = $project_phid;
+ if ($xaction->getTransactionType() == $type_columns) {
+ $moves = $xaction->getNewValue();
+ foreach ($moves as $move) {
+ $board_phids[] = $move['boardPHID'];
}
}
}
@@ -815,38 +689,6 @@
last($with_effect));
}
break;
- case ManiphestTransaction::TYPE_COLUMN:
- $with_effect = array();
- foreach ($xactions as $xaction) {
- $column_phid = $xaction->getNewValue();
- if (!$column_phid) {
- continue;
- }
-
- $with_effect[] = $xaction;
-
- $column = $this->loadProjectColumn($column_phid);
- if (!$column) {
- $errors[] = new PhabricatorApplicationTransactionValidationError(
- $type,
- pht('Invalid'),
- pht(
- 'Column PHID "%s" does not identify a visible column.',
- $column_phid),
- $xaction);
- }
- }
-
- if ($with_effect && !$this->getIsNewObject()) {
- $errors[] = new PhabricatorApplicationTransactionValidationError(
- $type,
- pht('Invalid'),
- pht(
- 'You can only put a task into an initial column during task '.
- 'creation.'),
- last($with_effect));
- }
- break;
case ManiphestTransaction::TYPE_OWNER:
foreach ($xactions as $xaction) {
$old = $xaction->getOldValue();
@@ -938,6 +780,19 @@
return $errors;
}
+ protected function validateAllTransactions(
+ PhabricatorLiskDAO $object,
+ array $xactions) {
+
+ $errors = parent::validateAllTransactions($object, $xactions);
+
+ if ($this->moreValidationErrors) {
+ $errors = array_merge($errors, $this->moreValidationErrors);
+ }
+
+ return $errors;
+ }
+
protected function expandTransactions(
PhabricatorLiskDAO $object,
array $xactions) {
@@ -1009,28 +864,36 @@
$results = parent::expandTransaction($object, $xaction);
- switch ($xaction->getTransactionType()) {
- case ManiphestTransaction::TYPE_COLUMN:
- $column_phid = $xaction->getNewValue();
- if (!$column_phid) {
- break;
+ $type = $xaction->getTransactionType();
+ switch ($type) {
+ case PhabricatorTransactions::TYPE_COLUMNS:
+ try {
+ $this->buildMoveTransaction($object, $xaction);
+
+ // Implicilty add the task to any boards that we're moving it
+ // on, since moves on a board the task isn't part of are not
+ // meaningful.
+ $board_phids = ipull($xaction->getNewValue(), 'boardPHID');
+ if ($board_phids) {
+ $results[] = id(new ManiphestTransaction())
+ ->setTransactionType(PhabricatorTransactions::TYPE_EDGE)
+ ->setMetadataValue(
+ 'edge:type',
+ PhabricatorProjectObjectHasProjectEdgeType::EDGECONST)
+ ->setIgnoreOnNoEffect(true)
+ ->setNewValue(
+ array(
+ '+' => array_fuse($board_phids),
+ ));
+ }
+ } catch (Exception $ex) {
+ $error = new PhabricatorApplicationTransactionValidationError(
+ $type,
+ pht('Invalid'),
+ $ex->getMessage(),
+ $xaction);
+ $this->moreValidationErrors[] = $error;
}
-
- // When a task is created into a column, we also generate a transaction
- // to actually put it in that column.
- $column = $this->loadProjectColumn($column_phid);
- $results[] = id(new ManiphestTransaction())
- ->setTransactionType(ManiphestTransaction::TYPE_PROJECT_COLUMN)
- ->setOldValue(
- array(
- 'projectPHID' => $column->getProjectPHID(),
- 'columnPHIDs' => array(),
- ))
- ->setNewValue(
- array(
- 'projectPHID' => $column->getProjectPHID(),
- 'columnPHIDs' => array($column->getPHID()),
- ));
break;
case ManiphestTransaction::TYPE_OWNER:
// If this is a no-op update, don't expand it.
@@ -1057,13 +920,6 @@
return $results;
}
- private function loadProjectColumn($column_phid) {
- return id(new PhabricatorProjectColumnQuery())
- ->setViewer($this->getActor())
- ->withPHIDs(array($column_phid))
- ->executeOne();
- }
-
protected function extractFilePHIDsFromCustomTransaction(
PhabricatorLiskDAO $object,
PhabricatorApplicationTransaction $xaction) {
@@ -1078,5 +934,264 @@
return $phids;
}
+ private function buildMoveTransaction(
+ PhabricatorLiskDAO $object,
+ PhabricatorApplicationTransaction $xaction) {
+
+ $new = $xaction->getNewValue();
+ if (!is_array($new)) {
+ $this->validateColumnPHID($new);
+ $new = array($new);
+ }
+
+ $nearby_phids = array();
+ foreach ($new as $key => $value) {
+ if (!is_array($value)) {
+ $this->validateColumnPHID($value);
+ $value = array(
+ 'columnPHID' => $value,
+ );
+ }
+
+ PhutilTypeSpec::checkMap(
+ $value,
+ array(
+ 'columnPHID' => 'string',
+ 'beforePHID' => 'optional string',
+ 'afterPHID' => 'optional string',
+ ));
+
+ $new[$key] = $value;
+
+ if (!empty($value['beforePHID'])) {
+ $nearby_phids[] = $value['beforePHID'];
+ }
+
+ if (!empty($value['afterPHID'])) {
+ $nearby_phids[] = $value['afterPHID'];
+ }
+ }
+
+ if ($nearby_phids) {
+ $nearby_objects = id(new PhabricatorObjectQuery())
+ ->setViewer($this->getActor())
+ ->withPHIDs($nearby_phids)
+ ->execute();
+ $nearby_objects = mpull($nearby_objects, null, 'getPHID');
+ } else {
+ $nearby_objects = array();
+ }
+
+ $column_phids = ipull($new, 'columnPHID');
+ if ($column_phids) {
+ $columns = id(new PhabricatorProjectColumnQuery())
+ ->setViewer($this->getActor())
+ ->withPHIDs($column_phids)
+ ->execute();
+ $columns = mpull($columns, null, 'getPHID');
+ } else {
+ $columns = array();
+ }
+
+ $board_phids = mpull($columns, 'getProjectPHID');
+ $object_phid = $object->getPHID();
+
+ $object_phids = $nearby_phids;
+
+ // Note that we may not have an object PHID if we're creating a new
+ // object.
+ if ($object_phid) {
+ $object_phids[] = $object_phid;
+ }
+
+ if ($object_phids) {
+ $layout_engine = id(new PhabricatorBoardLayoutEngine())
+ ->setViewer($this->getActor())
+ ->setBoardPHIDs($board_phids)
+ ->setObjectPHIDs($object_phids)
+ ->setFetchAllBoards(true)
+ ->executeLayout();
+ }
+
+ foreach ($new as $key => $spec) {
+ $column_phid = $spec['columnPHID'];
+ $column = idx($columns, $column_phid);
+ if (!$column) {
+ throw new Exception(
+ pht(
+ 'Column move transaction specifies column PHID "%s", but there '.
+ 'is no corresponding column with this PHID.',
+ $column_phid));
+ }
+
+ $board_phid = $column->getProjectPHID();
+
+ $nearby = array();
+
+ if (!empty($spec['beforePHID'])) {
+ $nearby['beforePHID'] = $spec['beforePHID'];
+ }
+
+ if (!empty($spec['afterPHID'])) {
+ $nearby['afterPHID'] = $spec['afterPHID'];
+ }
+
+ if (count($nearby) > 1) {
+ throw new Exception(
+ pht(
+ 'Column move transaction moves object to multiple positions. '.
+ 'Specify only "beforePHID" or "afterPHID", not both.'));
+ }
+
+ foreach ($nearby as $where => $nearby_phid) {
+ if (empty($nearby_objects[$nearby_phid])) {
+ throw new Exception(
+ pht(
+ 'Column move transaction specifies object "%s" as "%s", but '.
+ 'there is no corresponding object with this PHID.',
+ $object_phid,
+ $where));
+ }
+
+ $nearby_columns = $layout_engine->getObjectColumns(
+ $board_phid,
+ $nearby_phid);
+ $nearby_columns = mpull($nearby_columns, null, 'getPHID');
+
+ if (empty($nearby_columns[$column_phid])) {
+ throw new Exception(
+ pht(
+ 'Column move transaction specifies object "%s" as "%s" in '.
+ 'column "%s", but this object is not in that column!',
+ $nearby_phid,
+ $where,
+ $column_phid));
+ }
+ }
+
+ if ($object_phid) {
+ $old_columns = $layout_engine->getObjectColumns(
+ $board_phid,
+ $object_phid);
+ $old_column_phids = mpull($old_columns, 'getPHID');
+ } else {
+ $old_column_phids = array();
+ }
+
+ $spec += array(
+ 'boardPHID' => $board_phid,
+ 'fromColumnPHIDs' => $old_column_phids,
+ );
+
+ // Check if the object is already in this column, and isn't being moved.
+ // We can just drop this column change if it has no effect.
+ $from_map = array_fuse($spec['fromColumnPHIDs']);
+ $already_here = isset($from_map[$column_phid]);
+ $is_reordering = (bool)$nearby;
+
+ if ($already_here && !$is_reordering) {
+ unset($new[$key]);
+ } else {
+ $new[$key] = $spec;
+ }
+ }
+
+ $new = array_values($new);
+ $xaction->setNewValue($new);
+ }
+
+ private function applyBoardMove($object, array $move) {
+ $board_phid = $move['boardPHID'];
+ $column_phid = $move['columnPHID'];
+ $before_phid = idx($move, 'beforePHID');
+ $after_phid = idx($move, 'afterPHID');
+
+ $object_phid = $object->getPHID();
+
+ // 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();
+
+ $select_phids = array($board_phid);
+
+ $descendants = id(new PhabricatorProjectQuery())
+ ->setViewer($omnipotent_viewer)
+ ->withAncestorProjectPHIDs($select_phids)
+ ->execute();
+ foreach ($descendants as $descendant) {
+ $select_phids[] = $descendant->getPHID();
+ }
+
+ $board_tasks = id(new ManiphestTaskQuery())
+ ->setViewer($omnipotent_viewer)
+ ->withEdgeLogicPHIDs(
+ PhabricatorProjectObjectHasProjectEdgeType::EDGECONST,
+ PhabricatorQueryConstraint::OPERATOR_ANCESTOR,
+ array($select_phids))
+ ->execute();
+
+ $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)
+ ->setBoardPHIDs(array($board_phid))
+ ->setObjectPHIDs($object_phids)
+ ->executeLayout();
+
+ // TODO: This logic needs to be revised when we legitimately support
+ // multiple column positions.
+ $columns = $engine->getObjectColumns($board_phid, $object_phid);
+ foreach ($columns as $column) {
+ $engine->queueRemovePosition(
+ $board_phid,
+ $column->getPHID(),
+ $object_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);
+ }
+
+ $engine->applyPositionUpdates();
+ }
+
+
+ private function validateColumnPHID($value) {
+ if (phid_get_type($value) == PhabricatorProjectColumnPHIDType::TYPECONST) {
+ return;
+ }
+
+ throw new Exception(
+ pht(
+ 'When moving objects between columns on a board, columns must '.
+ 'be identified by PHIDs. This transaction uses "%s" to identify '.
+ 'a column, but that is not a valid column PHID.',
+ $value));
+ }
+
+
}
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
@@ -10,12 +10,10 @@
const TYPE_PRIORITY = 'priority';
const TYPE_EDGE = 'edge';
const TYPE_SUBPRIORITY = 'subpriority';
- const TYPE_PROJECT_COLUMN = 'projectcolumn';
const TYPE_MERGED_INTO = 'mergedinto';
const TYPE_MERGED_FROM = 'mergedfrom';
const TYPE_UNBLOCK = 'unblock';
const TYPE_PARENT = 'parent';
- const TYPE_COLUMN = 'column';
const TYPE_COVER_IMAGE = 'cover-image';
const TYPE_POINTS = 'points';
@@ -48,7 +46,6 @@
public function shouldGenerateOldValue() {
switch ($this->getTransactionType()) {
- case self::TYPE_PROJECT_COLUMN:
case self::TYPE_EDGE:
case self::TYPE_UNBLOCK:
return false;
@@ -85,10 +82,6 @@
$phids[] = $old;
}
break;
- case self::TYPE_PROJECT_COLUMN:
- $phids[] = $new['projectPHID'];
- $phids[] = head($new['columnPHIDs']);
- break;
case self::TYPE_MERGED_INTO:
$phids[] = $new;
break;
@@ -152,18 +145,7 @@
break;
case self::TYPE_SUBPRIORITY:
case self::TYPE_PARENT:
- case self::TYPE_COLUMN:
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);
case self::TYPE_COVER_IMAGE:
// At least for now, don't show these.
return true;
@@ -308,7 +290,7 @@
return pht('Reassigned');
}
- case self::TYPE_PROJECT_COLUMN:
+ case PhabricatorTransactions::TYPE_COLUMNS:
return pht('Changed Project Column');
case self::TYPE_PRIORITY:
@@ -378,7 +360,7 @@
case self::TYPE_DESCRIPTION:
return 'fa-pencil';
- case self::TYPE_PROJECT_COLUMN:
+ case PhabricatorTransactions::TYPE_COLUMNS:
return 'fa-columns';
case self::TYPE_MERGED_INTO:
@@ -616,16 +598,6 @@
$this->renderHandleList($removed));
}
- case self::TYPE_PROJECT_COLUMN:
- $project_phid = $new['projectPHID'];
- $column_phid = head($new['columnPHIDs']);
- return pht(
- '%s moved this task to %s on the %s workboard.',
- $this->renderHandleLink($author_phid),
- $this->renderHandleLink($column_phid),
- $this->renderHandleLink($project_phid));
- break;
-
case self::TYPE_MERGED_INTO:
return pht(
'%s closed this task as a duplicate of %s.',
@@ -887,16 +859,6 @@
$this->renderHandleList($removed));
}
- case self::TYPE_PROJECT_COLUMN:
- $project_phid = $new['projectPHID'];
- $column_phid = head($new['columnPHIDs']);
- return pht(
- '%s moved %s to %s on the %s workboard.',
- $this->renderHandleLink($author_phid),
- $this->renderHandleLink($object_phid),
- $this->renderHandleLink($column_phid),
- $this->renderHandleLink($project_phid));
-
case self::TYPE_MERGED_INTO:
return pht(
'%s merged task %s into %s.',
@@ -961,7 +923,7 @@
case self::TYPE_UNBLOCK:
$tags[] = self::MAILTAG_UNBLOCK;
break;
- case self::TYPE_PROJECT_COLUMN:
+ case PhabricatorTransactions::TYPE_COLUMNS:
$tags[] = self::MAILTAG_COLUMN;
break;
case PhabricatorTransactions::TYPE_COMMENT:
diff --git a/src/applications/project/__tests__/PhabricatorProjectCoreTestCase.php b/src/applications/project/__tests__/PhabricatorProjectCoreTestCase.php
--- a/src/applications/project/__tests__/PhabricatorProjectCoreTestCase.php
+++ b/src/applications/project/__tests__/PhabricatorProjectCoreTestCase.php
@@ -1072,18 +1072,13 @@
$options = array();
}
+ $value = array(
+ 'columnPHID' => $dst->getPHID(),
+ ) + $options;
+
$xactions[] = id(new ManiphestTransaction())
- ->setTransactionType(ManiphestTransaction::TYPE_PROJECT_COLUMN)
- ->setOldValue(
- array(
- 'projectPHID' => $board->getPHID(),
- 'columnPHIDs' => array($src->getPHID()),
- ))
- ->setNewValue(
- array(
- 'projectPHID' => $board->getPHID(),
- 'columnPHIDs' => array($dst->getPHID()),
- ) + $options);
+ ->setTransactionType(PhabricatorTransactions::TYPE_COLUMNS)
+ ->setNewValue(array($value));
$editor = id(new ManiphestTransactionEditor())
->setActor($viewer)
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
@@ -68,26 +68,22 @@
$xactions = array();
+ $order_params = array();
if ($order == PhabricatorProjectColumn::ORDER_NATURAL) {
- $order_params = array(
- 'afterPHID' => $after_phid,
- 'beforePHID' => $before_phid,
- );
- } else {
- $order_params = array();
+ if ($after_phid) {
+ $order_params['afterPHID'] = $after_phid;
+ } else if ($before_phid) {
+ $order_params['beforePHID'] = $before_phid;
+ }
}
$xactions[] = id(new ManiphestTransaction())
- ->setTransactionType(ManiphestTransaction::TYPE_PROJECT_COLUMN)
+ ->setTransactionType(PhabricatorTransactions::TYPE_COLUMNS)
->setNewValue(
array(
- 'columnPHIDs' => array($column->getPHID()),
- 'projectPHID' => $column->getProjectPHID(),
- ) + $order_params)
- ->setOldValue(
- array(
- 'columnPHIDs' => $old_column_phids,
- 'projectPHID' => $column->getProjectPHID(),
+ array(
+ 'columnPHID' => $column->getPHID(),
+ ) + $order_params,
));
if ($order == PhabricatorProjectColumn::ORDER_PRIORITY) {
diff --git a/src/applications/transactions/constants/PhabricatorTransactions.php b/src/applications/transactions/constants/PhabricatorTransactions.php
--- a/src/applications/transactions/constants/PhabricatorTransactions.php
+++ b/src/applications/transactions/constants/PhabricatorTransactions.php
@@ -14,6 +14,7 @@
const TYPE_INLINESTATE = 'core:inlinestate';
const TYPE_SPACE = 'core:space';
const TYPE_CREATE = 'core:create';
+ const TYPE_COLUMNS = 'core:columns';
const COLOR_RED = 'red';
const COLOR_ORANGE = 'orange';

File Metadata

Mime Type
text/plain
Expires
Sat, Mar 8, 7:25 PM (2 d, 3 h ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7228162
Default Alt Text
D15634.id37673.diff (28 KB)

Event Timeline