From 102e431feb01804379893d7d32b9d7bb90a88d48 Mon Sep 17 00:00:00 2001 From: Alex Monk Date: Sun, 28 Dec 2014 06:10:49 -0800 Subject: [PATCH] Migrate Maniphest task blockers to modern EdgeType classes Summary: Prevents "edited tasks, added: 1; removed: 1" Fixes T6757, using D9839 as an example Test Plan: Added and removed blockers to/from tasks, saw the expected history entries. Reviewers: epriestley, #blessed_reviewers Reviewed By: epriestley, #blessed_reviewers Subscribers: Korvin, epriestley Maniphest Tasks: T6757 Differential Revision: https://secure.phabricator.com/D11045 --- .../migrate-maniphest-dependencies.php | 2 +- src/__phutil_library_map__.php | 4 + .../conduit/ManiphestConduitAPIMethod.php | 4 +- .../ManiphestTaskDetailController.php | 8 +- .../ManiphestTaskEditController.php | 2 +- .../ManiphestTaskDependedOnByTaskEdgeType.php | 101 +++++++++++++++++ .../ManiphestTaskDependsOnTaskEdgeType.php | 106 ++++++++++++++++++ .../editor/ManiphestTransactionEditor.php | 2 +- .../event/ManiphestHovercardEventListener.php | 4 +- .../maniphest/storage/ManiphestTask.php | 4 +- .../PhabricatorSearchAttachController.php | 2 +- .../edges/constants/PhabricatorEdgeConfig.php | 19 ---- .../PhabricatorBaseEnglishTranslation.php | 50 ++++++++- 13 files changed, 270 insertions(+), 38 deletions(-) create mode 100644 src/applications/maniphest/edge/ManiphestTaskDependedOnByTaskEdgeType.php create mode 100644 src/applications/maniphest/edge/ManiphestTaskDependsOnTaskEdgeType.php diff --git a/resources/sql/patches/migrate-maniphest-dependencies.php b/resources/sql/patches/migrate-maniphest-dependencies.php index 72222c7c41..394c98a95f 100644 --- a/resources/sql/patches/migrate-maniphest-dependencies.php +++ b/resources/sql/patches/migrate-maniphest-dependencies.php @@ -18,7 +18,7 @@ foreach (new LiskMigrationIterator($table) as $task) { foreach ($deps as $dep) { $editor->addEdge( $task->getPHID(), - PhabricatorEdgeConfig::TYPE_TASK_DEPENDS_ON_TASK, + ManiphestTaskDependsOnTaskEdgeType::EDGECONST, $dep); } $editor->save(); diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 8ce914c374..19b88ebcc6 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -1013,6 +1013,8 @@ phutil_register_library_map(array( 'ManiphestStatusConfigOptionType' => 'applications/maniphest/config/ManiphestStatusConfigOptionType.php', 'ManiphestSubpriorityController' => 'applications/maniphest/controller/ManiphestSubpriorityController.php', 'ManiphestTask' => 'applications/maniphest/storage/ManiphestTask.php', + 'ManiphestTaskDependedOnByTaskEdgeType' => 'applications/maniphest/edge/ManiphestTaskDependedOnByTaskEdgeType.php', + 'ManiphestTaskDependsOnTaskEdgeType' => 'applications/maniphest/edge/ManiphestTaskDependsOnTaskEdgeType.php', 'ManiphestTaskDescriptionPreviewController' => 'applications/maniphest/controller/ManiphestTaskDescriptionPreviewController.php', 'ManiphestTaskDetailController' => 'applications/maniphest/controller/ManiphestTaskDetailController.php', 'ManiphestTaskEditController' => 'applications/maniphest/controller/ManiphestTaskEditController.php', @@ -4121,6 +4123,8 @@ phutil_register_library_map(array( 'PhabricatorApplicationTransactionInterface', 'PhabricatorProjectInterface', ), + 'ManiphestTaskDependedOnByTaskEdgeType' => 'PhabricatorEdgeType', + 'ManiphestTaskDependsOnTaskEdgeType' => 'PhabricatorEdgeType', 'ManiphestTaskDescriptionPreviewController' => 'ManiphestController', 'ManiphestTaskDetailController' => 'ManiphestController', 'ManiphestTaskEditController' => 'ManiphestController', diff --git a/src/applications/maniphest/conduit/ManiphestConduitAPIMethod.php b/src/applications/maniphest/conduit/ManiphestConduitAPIMethod.php index aaaf4c377b..b89369f692 100644 --- a/src/applications/maniphest/conduit/ManiphestConduitAPIMethod.php +++ b/src/applications/maniphest/conduit/ManiphestConduitAPIMethod.php @@ -251,7 +251,7 @@ abstract class ManiphestConduitAPIMethod extends ConduitAPIMethod { $all_deps = id(new PhabricatorEdgeQuery()) ->withSourcePHIDs($task_phids) - ->withEdgeTypes(array(PhabricatorEdgeConfig::TYPE_TASK_DEPENDS_ON_TASK)); + ->withEdgeTypes(array(ManiphestTaskDependsOnTaskEdgeType::EDGECONST)); $all_deps->execute(); $result = array(); @@ -269,7 +269,7 @@ abstract class ManiphestConduitAPIMethod extends ConduitAPIMethod { $task_deps = $all_deps->getDestinationPHIDs( array($task->getPHID()), - array(PhabricatorEdgeConfig::TYPE_TASK_DEPENDS_ON_TASK)); + array(ManiphestTaskDependsOnTaskEdgeType::EDGECONST)); $result[$task->getPHID()] = array( 'id' => $task->getID(), diff --git a/src/applications/maniphest/controller/ManiphestTaskDetailController.php b/src/applications/maniphest/controller/ManiphestTaskDetailController.php index 16514a6a82..a5d1e2200d 100644 --- a/src/applications/maniphest/controller/ManiphestTaskDetailController.php +++ b/src/applications/maniphest/controller/ManiphestTaskDetailController.php @@ -46,8 +46,8 @@ final class ManiphestTaskDetailController extends ManiphestController { ->readFieldsFromStorage($task); $e_commit = ManiphestTaskHasCommitEdgeType::EDGECONST; - $e_dep_on = PhabricatorEdgeConfig::TYPE_TASK_DEPENDS_ON_TASK; - $e_dep_by = PhabricatorEdgeConfig::TYPE_TASK_DEPENDED_ON_BY_TASK; + $e_dep_on = ManiphestTaskDependsOnTaskEdgeType::EDGECONST; + $e_dep_by = ManiphestTaskDependedOnByTaskEdgeType::EDGECONST; $e_rev = ManiphestTaskHasRevisionEdgeType::EDGECONST; $e_mock = PhabricatorEdgeConfig::TYPE_TASK_HAS_MOCK; @@ -482,9 +482,9 @@ final class ManiphestTaskDetailController extends ManiphestController { } $edge_types = array( - PhabricatorEdgeConfig::TYPE_TASK_DEPENDED_ON_BY_TASK + ManiphestTaskDependedOnByTaskEdgeType::EDGECONST => pht('Blocks'), - PhabricatorEdgeConfig::TYPE_TASK_DEPENDS_ON_TASK + ManiphestTaskDependsOnTaskEdgeType::EDGECONST => pht('Blocked By'), ManiphestTaskHasRevisionEdgeType::EDGECONST => pht('Differential Revisions'), diff --git a/src/applications/maniphest/controller/ManiphestTaskEditController.php b/src/applications/maniphest/controller/ManiphestTaskEditController.php index cfd121e59b..b51a3cada4 100644 --- a/src/applications/maniphest/controller/ManiphestTaskEditController.php +++ b/src/applications/maniphest/controller/ManiphestTaskEditController.php @@ -349,7 +349,7 @@ final class ManiphestTaskEditController extends ManiphestController { id(new PhabricatorEdgeEditor()) ->addEdge( $parent_task->getPHID(), - PhabricatorEdgeConfig::TYPE_TASK_DEPENDS_ON_TASK, + ManiphestTaskDependsOnTaskEdgeType::EDGECONST, $task->getPHID()) ->save(); $workflow = $parent_task->getID(); diff --git a/src/applications/maniphest/edge/ManiphestTaskDependedOnByTaskEdgeType.php b/src/applications/maniphest/edge/ManiphestTaskDependedOnByTaskEdgeType.php new file mode 100644 index 0000000000..ec879cb32e --- /dev/null +++ b/src/applications/maniphest/edge/ManiphestTaskDependedOnByTaskEdgeType.php @@ -0,0 +1,101 @@ +getPHID(), - PhabricatorEdgeConfig::TYPE_TASK_DEPENDED_ON_BY_TASK); + ManiphestTaskDependedOnByTaskEdgeType::EDGECONST); if ($blocked_phids) { // In theory we could apply these through policies, but that seems a // little bit surprising. For now, use the actor's vision. diff --git a/src/applications/maniphest/event/ManiphestHovercardEventListener.php b/src/applications/maniphest/event/ManiphestHovercardEventListener.php index 197e5ddd19..f1e747de16 100644 --- a/src/applications/maniphest/event/ManiphestHovercardEventListener.php +++ b/src/applications/maniphest/event/ManiphestHovercardEventListener.php @@ -27,8 +27,8 @@ final class ManiphestHovercardEventListener extends PhabricatorEventListener { $e_project = PhabricatorProjectObjectHasProjectEdgeType::EDGECONST; // Fun with "Unbeta Pholio", hua hua - $e_dep_on = PhabricatorEdgeConfig::TYPE_TASK_DEPENDS_ON_TASK; - $e_dep_by = PhabricatorEdgeConfig::TYPE_TASK_DEPENDED_ON_BY_TASK; + $e_dep_on = ManiphestTaskDependsOnTaskEdgeType::EDGECONST; + $e_dep_by = ManiphestTaskDependedOnByTaskEdgeType::EDGECONST; $edge_query = id(new PhabricatorEdgeQuery()) ->withSourcePHIDs(array($phid)) diff --git a/src/applications/maniphest/storage/ManiphestTask.php b/src/applications/maniphest/storage/ManiphestTask.php index 0d8c51695e..8d19640191 100644 --- a/src/applications/maniphest/storage/ManiphestTask.php +++ b/src/applications/maniphest/storage/ManiphestTask.php @@ -125,13 +125,13 @@ final class ManiphestTask extends ManiphestDAO public function loadDependsOnTaskPHIDs() { return PhabricatorEdgeQuery::loadDestinationPHIDs( $this->getPHID(), - PhabricatorEdgeConfig::TYPE_TASK_DEPENDS_ON_TASK); + ManiphestTaskDependsOnTaskEdgeType::EDGECONST); } public function loadDependedOnByTaskPHIDs() { return PhabricatorEdgeQuery::loadDestinationPHIDs( $this->getPHID(), - PhabricatorEdgeConfig::TYPE_TASK_DEPENDED_ON_BY_TASK); + ManiphestTaskDependedOnByTaskEdgeType::EDGECONST); } public function getAttachedPHIDs($type) { diff --git a/src/applications/search/controller/PhabricatorSearchAttachController.php b/src/applications/search/controller/PhabricatorSearchAttachController.php index 360319ce6f..0eb558cc50 100644 --- a/src/applications/search/controller/PhabricatorSearchAttachController.php +++ b/src/applications/search/controller/PhabricatorSearchAttachController.php @@ -292,7 +292,7 @@ final class PhabricatorSearchAttachController ), $t_task => array( $t_cmit => ManiphestTaskHasCommitEdgeType::EDGECONST, - $t_task => PhabricatorEdgeConfig::TYPE_TASK_DEPENDS_ON_TASK, + $t_task => ManiphestTaskDependsOnTaskEdgeType::EDGECONST, $t_drev => ManiphestTaskHasRevisionEdgeType::EDGECONST, $t_mock => PhabricatorEdgeConfig::TYPE_TASK_HAS_MOCK, ), diff --git a/src/infrastructure/edges/constants/PhabricatorEdgeConfig.php b/src/infrastructure/edges/constants/PhabricatorEdgeConfig.php index 9738dec2a8..42ec7e875f 100644 --- a/src/infrastructure/edges/constants/PhabricatorEdgeConfig.php +++ b/src/infrastructure/edges/constants/PhabricatorEdgeConfig.php @@ -5,9 +5,6 @@ final class PhabricatorEdgeConfig extends PhabricatorEdgeConstants { const TABLE_NAME_EDGE = 'edge'; const TABLE_NAME_EDGEDATA = 'edgedata'; - const TYPE_TASK_DEPENDS_ON_TASK = 3; - const TYPE_TASK_DEPENDED_ON_BY_TASK = 4; - const TYPE_DREV_DEPENDS_ON_DREV = 5; const TYPE_DREV_DEPENDED_ON_BY_DREV = 6; @@ -131,9 +128,6 @@ final class PhabricatorEdgeConfig extends PhabricatorEdgeConstants { private static function getInverse($edge_type) { static $map = array( - self::TYPE_TASK_DEPENDS_ON_TASK => self::TYPE_TASK_DEPENDED_ON_BY_TASK, - self::TYPE_TASK_DEPENDED_ON_BY_TASK => self::TYPE_TASK_DEPENDS_ON_TASK, - self::TYPE_DREV_DEPENDS_ON_DREV => self::TYPE_DREV_DEPENDED_ON_BY_DREV, self::TYPE_DREV_DEPENDED_ON_BY_DREV => self::TYPE_DREV_DEPENDS_ON_DREV, @@ -203,7 +197,6 @@ final class PhabricatorEdgeConfig extends PhabricatorEdgeConstants { private static function shouldPreventCycles($edge_type) { static $map = array( self::TYPE_TEST_NO_CYCLE => true, - self::TYPE_TASK_DEPENDS_ON_TASK => true, self::TYPE_DREV_DEPENDS_ON_DREV => true, ); return isset($map[$edge_type]); @@ -238,8 +231,6 @@ final class PhabricatorEdgeConfig extends PhabricatorEdgeConstants { switch ($type) { case self::TYPE_DREV_HAS_COMMIT: return '%s edited commit(s), added %d: %s; removed %d: %s.'; - case self::TYPE_TASK_DEPENDS_ON_TASK: - case self::TYPE_TASK_DEPENDED_ON_BY_TASK: case self::TYPE_MOCK_HAS_TASK: return '%s edited task(s), added %d: %s; removed %d: %s.'; case self::TYPE_DREV_DEPENDS_ON_DREV: @@ -304,12 +295,8 @@ final class PhabricatorEdgeConfig extends PhabricatorEdgeConstants { switch ($type) { case self::TYPE_DREV_HAS_COMMIT: return '%s added %d commit(s): %s.'; - case self::TYPE_TASK_DEPENDS_ON_TASK: - return '%s added %d blocking task(s): %s.'; case self::TYPE_DREV_DEPENDS_ON_DREV: return '%s added %d dependencie(s): %s.'; - case self::TYPE_TASK_DEPENDED_ON_BY_TASK: - return '%s added %d blocked task(s): %s.'; case self::TYPE_MOCK_HAS_TASK: return '%s added %d task(s): %s.'; case self::TYPE_DREV_DEPENDED_ON_BY_DREV: @@ -372,10 +359,6 @@ final class PhabricatorEdgeConfig extends PhabricatorEdgeConstants { switch ($type) { case self::TYPE_DREV_HAS_COMMIT: return '%s removed %d commit(s): %s.'; - case self::TYPE_TASK_DEPENDS_ON_TASK: - return '%s removed %d blocking task(s): %s.'; - case self::TYPE_TASK_DEPENDED_ON_BY_TASK: - return '%s removed %d blocked task(s): %s.'; case self::TYPE_MOCK_HAS_TASK: return '%s removed %d task(s): %s.'; case self::TYPE_DREV_DEPENDS_ON_DREV: @@ -437,8 +420,6 @@ final class PhabricatorEdgeConfig extends PhabricatorEdgeConstants { switch ($type) { case self::TYPE_DREV_HAS_COMMIT: return '%s updated commits of %s.'; - case self::TYPE_TASK_DEPENDS_ON_TASK: - case self::TYPE_TASK_DEPENDED_ON_BY_TASK: case self::TYPE_MOCK_HAS_TASK: return '%s updated tasks of %s.'; case self::TYPE_DREV_DEPENDS_ON_DREV: diff --git a/src/infrastructure/internationalization/translation/PhabricatorBaseEnglishTranslation.php b/src/infrastructure/internationalization/translation/PhabricatorBaseEnglishTranslation.php index 0360a400c2..a899746bb7 100644 --- a/src/infrastructure/internationalization/translation/PhabricatorBaseEnglishTranslation.php +++ b/src/infrastructure/internationalization/translation/PhabricatorBaseEnglishTranslation.php @@ -382,34 +382,74 @@ abstract class PhabricatorBaseEnglishTranslation ), ), - '%s added %d blocking task(s): %s.' => array( + '%s added %s blocking task(s): %s.' => array( array( '%s added a blocking task: %3$s.', '%s added blocking tasks: %3$s.', ), ), - '%s added %d blocked task(s): %s.' => array( + '%s added %s blocked task(s): %s.' => array( array( '%s added a blocked task: %3$s.', '%s added blocked tasks: %3$s.', ), ), - '%s removed %d blocking task(s): %s.' => array( + '%s removed %s blocking task(s): %s.' => array( array( '%s removed a blocking task: %3$s.', '%s removed blocking tasks: %3$s.', ), ), - '%s removed %d blocked task(s): %s.' => array( + '%s removed %s blocked task(s): %s.' => array( array( '%s removed a blocked task: %3$s.', '%s removed blocked tasks: %3$s.', ), ), + '%s added %s blocking task(s) for %s: %s.' => array( + array( + '%s added a blocking task for %3$s: %4$s.', + '%s added blocking tasks for %3$s: %4$s.', + ), + ), + + '%s added %s blocked task(s) for %s: %s.' => array( + array( + '%s added a blocked task for %3$s: %4$s.', + '%s added blocked tasks for %3$s: %4$s.', + ), + ), + + '%s removed %s blocking task(s) for %s: %s.' => array( + array( + '%s removed a blocking task for %3$s: %4$s.', + '%s removed blocking tasks for %3$s: %4$s.', + ), + ), + + '%s removed %s blocked task(s) for %s: %s.' => array( + array( + '%s removed a blocked task for %3$s: %4$s.', + '%s removed blocked tasks for %3$s: %4$s.', + ), + ), + + '%s edited blocking task(s), added %s: %s; removed %s: %s.' => + '%s edited blocking tasks, added: %3$s; removed: %5$s', + + '%s edited blocking task(s) for %s, added %s: %s; removed %s: %s.' => + '%s edited blocking tasks for %s, added: %4$s; removed: %6$s', + + '%s edited blocked task(s), added %s: %s; removed %s: %s.' => + '%s edited blocked tasks, added: %3$s; removed: %5$s', + + '%s edited blocked task(s) for %s, added %s: %s; removed %s: %s.' => + '%s edited blocked tasks for %s, added: %4$s; removed: %6$s', + '%s edited answer(s), added %d: %s; removed %d: %s.' => '%s edited answers, added: %3$s; removed: %5$s', @@ -461,7 +501,7 @@ abstract class PhabricatorBaseEnglishTranslation ), ), - '%s edited task(s), added %d: %s; removed %d: %s.' => + '%s edited task(s), added %d: %s; removed %s: %s.' => '%s edited tasks, added: %3$s; removed: %5$s', '%s added %d task(s): %s.' => array(