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

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();

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',

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(),

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_on = ManiphestTaskDependsOnTaskEdgeType::EDGECONST;
-    $e_dep_by = PhabricatorEdgeConfig::TYPE_TASK_DEPENDED_ON_BY_TASK;
+    $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'),

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();

src/applications/maniphest/edge/ManiphestTaskDependedOnByTaskEdgeType.php

@@ -0,0 +1,101 @@
+<?php
+
+final class ManiphestTaskDependedOnByTaskEdgeType extends PhabricatorEdgeType {
+  const EDGECONST = 4;
+
+  public function getInverseEdgeConstant() {
+    return ManiphestTaskDependsOnTaskEdgeType::EDGECONST;
+  }
+
+  public function shouldWriteInverseTransaction() {
+    return true;
+  }
+
+  public function getTransactionAddString(
+    $actor,
+    $add_count,
+    $add_edges) {
+
+    return pht(
+      '%s added %s blocked task(s): %s.',
+      $actor,
+      $add_count,
+      $add_edges);
+  }
+
+  public function getTransactionRemoveString(
+    $actor,
+    $rem_count,
+    $rem_edges) {
+
+    return pht(
+      '%s removed %s blocked task(s): %s.',
+      $actor,
+      $rem_count,
+      $rem_edges);
+  }
+
+  public function getTransactionEditString(
+    $actor,
+    $total_count,
+    $add_count,
+    $add_edges,
+    $rem_count,
+    $rem_edges) {
+
+    return pht(
+      '%s edited blocked task(s), added %s: %s; removed %s: %s.',
+      $actor,
+      $add_count,
+      $add_edges,
+      $rem_count,
+      $rem_edges);
+  }
+
+  public function getFeedAddString(
+    $actor,
+    $object,
+    $add_count,
+    $add_edges) {
+
+    return pht(
+      '%s added %s blocked task(s) for %s: %s.',
+      $actor,
+      $add_count,
+      $object,
+      $add_edges);
+  }
+
+  public function getFeedRemoveString(
+    $actor,
+    $object,
+    $rem_count,
+    $rem_edges) {
+
+    return pht(
+      '%s removed %s blocked task(s) for %s: %s.',
+      $actor,
+      $rem_count,
+      $object,
+      $rem_edges);
+  }
+
+  public function getFeedEditString(
+    $actor,
+    $object,
+    $total_count,
+    $add_count,
+    $add_edges,
+    $rem_count,
+    $rem_edges) {
+
+    return pht(
+      '%s edited blocked task(s) for %s, added %s: %s; removed %s: %s.',
+      $actor,
+      $object,
+      $add_count,
+      $add_edges,
+      $rem_count,
+      $rem_edges);
+  }
+}

src/applications/maniphest/edge/ManiphestTaskDependsOnTaskEdgeType.php

@@ -0,0 +1,106 @@
+<?php
+
+final class ManiphestTaskDependsOnTaskEdgeType extends PhabricatorEdgeType {
+
+  const EDGECONST = 3;
+
+  public function getInverseEdgeConstant() {
+    return ManiphestTaskDependedOnByTaskEdgeType::EDGECONST;
+  }
+
+  public function shouldWriteInverseTransaction() {
+    return true;
+  }
+
+  public function shouldPreventCycles() {
+    return true;
+  }
+
+  public function getTransactionAddString(
+    $actor,
+    $add_count,
+    $add_edges) {
+
+    return pht(
+      '%s added %s blocking task(s): %s.',
+      $actor,
+      $add_count,
+      $add_edges);
+  }
+
+  public function getTransactionRemoveString(
+    $actor,
+    $rem_count,
+    $rem_edges) {
+
+    return pht(
+      '%s removed %s blocking task(s): %s.',
+      $actor,
+      $rem_count,
+      $rem_edges);
+  }
+
+  public function getTransactionEditString(
+    $actor,
+    $total_count,
+    $add_count,
+    $add_edges,
+    $rem_count,
+    $rem_edges) {
+
+    return pht(
+      '%s edited blocking task(s), added %s: %s; removed %s: %s.',
+      $actor,
+      $add_count,
+      $add_edges,
+      $rem_count,
+      $rem_edges);
+  }
+
+  public function getFeedAddString(
+    $actor,
+    $object,
+    $add_count,
+    $add_edges) {
+
+    return pht(
+      '%s added %s blocking task(s) for %s: %s.',
+      $actor,
+      $add_count,
+      $object,
+      $add_edges);
+  }
+
+  public function getFeedRemoveString(
+    $actor,
+    $object,
+    $rem_count,
+    $rem_edges) {
+
+    return pht(
+      '%s removed %s blocking task(s) for %s: %s.',
+      $actor,
+      $rem_count,
+      $object,
+      $rem_edges);
+  }
+
+  public function getFeedEditString(
+    $actor,
+    $object,
+    $total_count,
+    $add_count,
+    $add_edges,
+    $rem_count,
+    $rem_edges) {
+
+    return pht(
+      '%s edited blocking task(s) for %s, added %s: %s; removed %s: %s.',
+      $actor,
+      $object,
+      $add_count,
+      $add_edges,
+      $rem_count,
+      $rem_edges);
+  }
+}

src/applications/maniphest/editor/ManiphestTransactionEditor.php

@@ -362,7 +362,7 @@ final class ManiphestTransactionEditor
     if ($unblock_xaction !== null) {
       $blocked_phids = PhabricatorEdgeQuery::loadDestinationPHIDs(
         $object->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.

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_on = ManiphestTaskDependsOnTaskEdgeType::EDGECONST;
-    $e_dep_by = PhabricatorEdgeConfig::TYPE_TASK_DEPENDED_ON_BY_TASK;
+    $e_dep_by = ManiphestTaskDependedOnByTaskEdgeType::EDGECONST;
 
     $edge_query = id(new PhabricatorEdgeQuery())
       ->withSourcePHIDs(array($phid))

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) {

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,
       ),

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:

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(