From f97d120c3fe6f560b05e94075fa2bde6c4a07855 Mon Sep 17 00:00:00 2001 From: epriestley Date: Sun, 5 Jun 2016 15:57:12 -0700 Subject: [PATCH] When a task is removed from projects, remove its position on proxy columns for those projects Summary: Fixes T11088. When a task is removed from a project, we don't normally delete its column positions. If you accidentally remove a project and then restore the project, it's nice for the task to stay where you put it. However, we do need to remove its positions in proxy columns to avoid the issue in T11088. Test Plan: - Added a failing unit test, made it pass. - Added a task to "X > Milestone 1", loaded workboard, used "Edit Projects" to move it to "X" instead, loaded workboard. - Before, it stayed in the "Milestone 1" column. - After, it moves to the "Backlog" column. Reviewers: chad Reviewed By: chad Maniphest Tasks: T11088 Differential Revision: https://secure.phabricator.com/D16052 --- .../PhabricatorProjectCoreTestCase.php | 15 ++++++ ...habricatorApplicationTransactionEditor.php | 46 +++++++++++++++++++ 2 files changed, 61 insertions(+) diff --git a/src/applications/project/__tests__/PhabricatorProjectCoreTestCase.php b/src/applications/project/__tests__/PhabricatorProjectCoreTestCase.php index 0c3f51ef6b..e157211f7b 100644 --- a/src/applications/project/__tests__/PhabricatorProjectCoreTestCase.php +++ b/src/applications/project/__tests__/PhabricatorProjectCoreTestCase.php @@ -1044,6 +1044,21 @@ final class PhabricatorProjectCoreTestCase extends PhabricatorTestCase { $this->moveToColumn($user, $board, $task_a, $column, $column, $a_options); $new_projects = $this->getTaskProjects($task_a); $this->assertEqual($old_projects, $new_projects); + + + // Add the parent project to the task. This should move it out of the + // milestone column and into the parent's backlog. + $this->addProjectTags($user, $task, array($board->getPHID())); + $expect_columns = array( + $backlog->getPHID(), + ); + $this->assertColumns($expect_columns, $user, $board, $task); + + $new_projects = $this->getTaskProjects($task); + $expect_projects = array( + $board->getPHID(), + ); + $this->assertEqual($expect_projects, $new_projects); } public function testColumnExtendedPolicies() { diff --git a/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php b/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php index 7570507c9f..ae58787503 100644 --- a/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php +++ b/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php @@ -677,6 +677,8 @@ abstract class PhabricatorApplicationTransactionEditor } $editor->save(); + + $this->updateWorkboardColumns($object, $const, $old, $new); break; case PhabricatorTransactions::TYPE_VIEW_POLICY: case PhabricatorTransactions::TYPE_SPACE: @@ -3603,4 +3605,48 @@ abstract class PhabricatorApplicationTransactionEditor return true; } + private function updateWorkboardColumns($object, $const, $old, $new) { + // If an object is removed from a project, remove it from any proxy + // columns for that project. This allows a task which is moved up from a + // milestone to the parent to move back into the "Backlog" column on the + // parent workboard. + + if ($const != PhabricatorProjectObjectHasProjectEdgeType::EDGECONST) { + return; + } + + // TODO: This should likely be some future WorkboardInterface. + $appears_on_workboards = ($object instanceof ManiphestTask); + if (!$appears_on_workboards) { + return; + } + + $removed_phids = array_keys(array_diff_key($old, $new)); + if (!$removed_phids) { + return; + } + + // Find any proxy columns for the removed projects. + $proxy_columns = id(new PhabricatorProjectColumnQuery()) + ->setViewer(PhabricatorUser::getOmnipotentUser()) + ->withProxyPHIDs($removed_phids) + ->execute(); + if (!$proxy_columns) { + return array(); + } + + $proxy_phids = mpull($proxy_columns, 'getPHID'); + + $position_table = new PhabricatorProjectColumnPosition(); + $conn_w = $position_table->establishConnection('w'); + + queryfx( + $conn_w, + 'DELETE FROM %T WHERE objectPHID = %s AND columnPHID IN (%Ls)', + $position_table->getTableName(), + $object->getPHID(), + $proxy_phids); + } + + }