Page MenuHomePhabricator
Files F14362730

D15834.diff
No OneTemporary
Actions

D15834.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
@@ -868,23 +868,9 @@
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),
- ));
+ $more_xactions = $this->buildMoveTransaction($object, $xaction);
+ foreach ($more_xactions as $more_xaction) {
+ $results[] = $more_xaction;
}
} catch (Exception $ex) {
$error = new PhabricatorApplicationTransactionValidationError(
@@ -1098,6 +1084,89 @@
$new = array_values($new);
$xaction->setNewValue($new);
+
+
+ $more = array();
+
+ // If we're moving the object into a column and it does not already belong
+ // in the column, add the appropriate board. For normal columns, this
+ // is the board PHID. For proxy columns, it is the proxy PHID, unless the
+ // object is already a member of some descendant of the proxy PHID.
+
+ // The major case where this can happen is moves via the API, but it also
+ // happens when a user drags a task from the "Backlog" to a milestone
+ // column.
+
+ if ($object_phid) {
+ $current_phids = PhabricatorEdgeQuery::loadDestinationPHIDs(
+ $object_phid,
+ PhabricatorProjectObjectHasProjectEdgeType::EDGECONST);
+ $current_phids = array_fuse($current_phids);
+ } else {
+ $current_phids = array();
+ }
+
+ $add_boards = array();
+ foreach ($new as $move) {
+ $column_phid = $move['columnPHID'];
+ $board_phid = $move['boardPHID'];
+ $column = $columns[$column_phid];
+ $proxy_phid = $column->getProxyPHID();
+
+ // If this is a normal column, add the board if the object isn't already
+ // associated.
+ if (!$proxy_phid) {
+ if (!isset($current_phids[$board_phid])) {
+ $add_boards[] = $board_phid;
+ }
+ continue;
+ }
+
+ // If this is a proxy column but the object is already associated with
+ // the proxy board, we don't need to do anything.
+ if (isset($current_phids[$proxy_phid])) {
+ continue;
+ }
+
+ // If this a proxy column and the object is already associated with some
+ // descendant of the proxy board, we also don't need to do anything.
+ $descendants = id(new PhabricatorProjectQuery())
+ ->setViewer(PhabricatorUser::getOmnipotentUser())
+ ->withAncestorProjectPHIDs(array($proxy_phid))
+ ->execute();
+
+ $found_descendant = false;
+ foreach ($descendants as $descendant) {
+ if (isset($current_phids[$descendant->getPHID()])) {
+ $found_descendant = true;
+ break;
+ }
+ }
+
+ if ($found_descendant) {
+ continue;
+ }
+
+ // Otherwise, we're moving the object to a proxy column which it is not
+ // a member of yet, so add an association to the column's proxy board.
+
+ $add_boards[] = $proxy_phid;
+ }
+
+ if ($add_boards) {
+ $more[] = id(new ManiphestTransaction())
+ ->setTransactionType(PhabricatorTransactions::TYPE_EDGE)
+ ->setMetadataValue(
+ 'edge:type',
+ PhabricatorProjectObjectHasProjectEdgeType::EDGECONST)
+ ->setIgnoreOnNoEffect(true)
+ ->setNewValue(
+ array(
+ '+' => array_fuse($add_boards),
+ ));
+ }
+
+ return $more;
}
private function applyBoardMove($object, array $move) {
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
@@ -1011,6 +1011,39 @@
$column->getPHID(),
);
$this->assertColumns($expect, $user, $board, $task);
+
+
+ // Move the task within the "Milestone" column. This should not affect
+ // the projects the task is tagged with. See T10912.
+ $task_a = $task;
+
+ $task_b = $this->newTask($user, array($backlog));
+ $this->moveToColumn($user, $board, $task_b, $backlog, $column);
+
+ $a_options = array(
+ 'beforePHID' => $task_b->getPHID(),
+ );
+
+ $b_options = array(
+ 'beforePHID' => $task_a->getPHID(),
+ );
+
+ $old_projects = $this->getTaskProjects($task);
+
+ // Move the target task to the top.
+ $this->moveToColumn($user, $board, $task_a, $column, $column, $a_options);
+ $new_projects = $this->getTaskProjects($task_a);
+ $this->assertEqual($old_projects, $new_projects);
+
+ // Move the other task.
+ $this->moveToColumn($user, $board, $task_b, $column, $column, $b_options);
+ $new_projects = $this->getTaskProjects($task_a);
+ $this->assertEqual($old_projects, $new_projects);
+
+ // Move the target task again.
+ $this->moveToColumn($user, $board, $task_a, $column, $column, $a_options);
+ $new_projects = $this->getTaskProjects($task_a);
+ $this->assertEqual($old_projects, $new_projects);
}
public function testColumnExtendedPolicies() {
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
@@ -96,32 +96,6 @@
}
}
- $proxy = $column->getProxy();
- if ($proxy) {
- // We're moving the task into a subproject or milestone column, so add
- // the subproject or milestone.
- $add_projects = array($proxy->getPHID());
- } else if ($project->getHasSubprojects() || $project->getHasMilestones()) {
- // We're moving the task into the "Backlog" column on the parent project,
- // so add the parent explicitly. This gets rid of any subproject or
- // milestone tags.
- $add_projects = array($project->getPHID());
- } else {
- $add_projects = array();
- }
-
- if ($add_projects) {
- $project_type = PhabricatorProjectObjectHasProjectEdgeType::EDGECONST;
-
- $xactions[] = id(new ManiphestTransaction())
- ->setTransactionType(PhabricatorTransactions::TYPE_EDGE)
- ->setMetadataValue('edge:type', $project_type)
- ->setNewValue(
- array(
- '+' => array_fuse($add_projects),
- ));
- }
-
$editor = id(new ManiphestTransactionEditor())
->setActor($viewer)
->setContinueOnMissingFields(true)

File Metadata

Mime Type
text/plain
Expires
Sat, Dec 21, 1:21 PM (18 h, 46 m)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6914113
Default Alt Text
D15834.diff (7 KB)

Event Timeline

Phabricator · Built by Phacility