Use standard infrastructure to attach commits to other objects

Summary:
Ref T4896. Now that we have a transaction editor, we can delete a giant block of hacks.

I believe this also resolves the commit/task attachment issues @joshuaspence and @mbishopim3 mentioned.

Test Plan: Attached and detached commits and tasks.

Reviewers: btrahan, joshuaspence, mbishopim3

Reviewed By: mbishopim3

Subscribers: mbishopim3, epriestley, joshuaspence

Maniphest Tasks: T4896

Differential Revision: https://secure.phabricator.com/D10138

src/__phutil_library_map__.php

@@ -4851,6 +4851,7 @@ phutil_register_library_map(array(
       'PhabricatorSubscribableInterface',
       'HarbormasterBuildableInterface',
       'PhabricatorCustomFieldInterface',
+      'PhabricatorApplicationTransactionInterface',
     ),
     'PhabricatorRepositoryCommitChangeParserWorker' => 'PhabricatorRepositoryCommitParserWorker',
     'PhabricatorRepositoryCommitData' => 'PhabricatorRepositoryDAO',

src/applications/audit/editor/PhabricatorAuditEditor.php

@@ -7,6 +7,7 @@ final class PhabricatorAuditEditor
     $types = parent::getTransactionTypes();
 
     $types[] = PhabricatorTransactions::TYPE_COMMENT;
+    $types[] = PhabricatorTransactions::TYPE_EDGE;
 
     // TODO: These will get modernized eventually, but that can happen one
     // at a time later on.
@@ -66,6 +67,7 @@ final class PhabricatorAuditEditor
     switch ($xaction->getTransactionType()) {
       case PhabricatorTransactions::TYPE_COMMENT:
       case PhabricatorTransactions::TYPE_SUBSCRIBERS:
+      case PhabricatorTransactions::TYPE_EDGE:
       case PhabricatorAuditActionConstants::ACTION:
       case PhabricatorAuditActionConstants::INLINE:
       case PhabricatorAuditActionConstants::ADD_AUDITORS:
@@ -82,6 +84,7 @@ final class PhabricatorAuditEditor
     switch ($xaction->getTransactionType()) {
       case PhabricatorTransactions::TYPE_COMMENT:
       case PhabricatorTransactions::TYPE_SUBSCRIBERS:
+      case PhabricatorTransactions::TYPE_EDGE:
       case PhabricatorAuditActionConstants::ACTION:
       case PhabricatorAuditActionConstants::INLINE:
         return;
@@ -130,6 +133,20 @@ final class PhabricatorAuditEditor
     PhabricatorLiskDAO $object,
     array $xactions) {
 
+    // Load auditors explicitly; we may not have them if the caller was a
+    // generic piece of infrastructure.
+
+    $commit = id(new DiffusionCommitQuery())
+      ->setViewer($this->requireActor())
+      ->withIDs(array($object->getID()))
+      ->needAuditRequests(true)
+      ->executeOne();
+    if (!$commit) {
+      throw new Exception(
+        pht('Failed to load commit during transaction finalization!'));
+    }
+    $object->attachAudits($commit->getAudits());
+
     $status_concerned = PhabricatorAuditStatusConstants::CONCERNED;
     $status_closed = PhabricatorAuditStatusConstants::CLOSED;
     $status_resigned = PhabricatorAuditStatusConstants::RESIGNED;

src/applications/diffusion/edge/DiffusionCommitHasTaskEdgeType.php

@@ -5,9 +5,11 @@ final class DiffusionCommitHasTaskEdgeType extends PhabricatorEdgeType {
   const EDGECONST = 2;
 
   public function shouldWriteInverseTransactions() {
-    // TODO: This should happen after T4896, but Diffusion does not support
+    return true;
-    // transactions yet.
+  }
-    return false;
+
+  public function getInverseEdgeConstant() {
+    return ManiphestTaskHasCommitEdgeType::EDGECONST;
   }
 
   public function getTransactionAddString(

src/applications/maniphest/edge/ManiphestTaskHasCommitEdgeType.php

@@ -5,9 +5,11 @@ final class ManiphestTaskHasCommitEdgeType extends PhabricatorEdgeType {
   const EDGECONST = 1;
 
   public function shouldWriteInverseTransactions() {
-    // TODO: This should happen after T4896, but Diffusion does not support
+    return true;
-    // transactions yet.
+  }
-    return false;
+
+  public function getInverseEdgeConstant() {
+    return DiffusionCommitHasTaskEdgeType::EDGECONST;
   }
 
   public function getTransactionAddString(

src/applications/repository/storage/PhabricatorRepositoryCommit.php

@@ -8,7 +8,8 @@ final class PhabricatorRepositoryCommit
     PhabricatorTokenReceiverInterface,
     PhabricatorSubscribableInterface,
     HarbormasterBuildableInterface,
-    PhabricatorCustomFieldInterface {
+    PhabricatorCustomFieldInterface,
+    PhabricatorApplicationTransactionInterface {
 
   protected $repositoryID;
   protected $phid;
@@ -351,4 +352,20 @@ final class PhabricatorRepositoryCommit
     return true;
   }
 
+
+/* -(  PhabricatorApplicationTransactionInterface  )------------------------- */
+
+
+  public function getApplicationTransactionEditor() {
+    return new PhabricatorAuditEditor();
+  }
+
+  public function getApplicationTransactionObject() {
+    return $this;
+  }
+
+  public function getApplicationTransactionTemplate() {
+    return new PhabricatorAuditTransaction();
+  }
+
 }

src/applications/search/controller/PhabricatorSearchAttachController.php

@@ -57,69 +57,35 @@ final class PhabricatorSearchAttachController
       $phids = array_values($phids);
 
       if ($edge_type) {
-        if ($object instanceof PhabricatorRepositoryCommit) {
+        if (!$object instanceof PhabricatorApplicationTransactionInterface) {
-          // TODO: Remove this entire branch of special cased grossness
+          throw new Exception(
-          // after T4896.
+            pht(
-
+              'Expected object ("%s") to implement interface "%s".',
-          $old_phids = PhabricatorEdgeQuery::loadDestinationPHIDs(
+              get_class($object),
-            $this->phid,
+              'PhabricatorApplicationTransactionInterface'));
-            $edge_type);
-          $add_phids = $phids;
-          $rem_phids = array_diff($old_phids, $add_phids);
-
-          // Doing this correctly (in a way that writes edge transactions) would
-          // be a huge mess and we don't get the commit half of the transaction
-          // anyway until T4896, so just write the edges themselves and skip
-          // the transactions for now.
-
-          $editor = new PhabricatorEdgeEditor();
-          foreach ($add_phids as $phid) {
-            $editor->addEdge(
-              $object->getPHID(),
-              DiffusionCommitHasTaskEdgeType::EDGECONST,
-              $phid);
-          }
-
-          foreach ($rem_phids as $phid) {
-            $editor->removeEdge(
-              $object->getPHID(),
-              DiffusionCommitHasTaskEdgeType::EDGECONST,
-              $phid);
-          }
-
-          $editor->save();
-
-        } else {
-          if (!$object instanceof PhabricatorApplicationTransactionInterface) {
-            throw new Exception(
-              pht(
-                'Expected object ("%s") to implement interface "%s".',
-                get_class($object),
-                'PhabricatorApplicationTransactionInterface'));
-          }
-
-          $old_phids = PhabricatorEdgeQuery::loadDestinationPHIDs(
-            $this->phid,
-            $edge_type);
-          $add_phids = $phids;
-          $rem_phids = array_diff($old_phids, $add_phids);
-
-          $txn_editor = $object->getApplicationTransactionEditor()
-            ->setActor($user)
-            ->setContentSourceFromRequest($request)
-            ->setContinueOnMissingFields(true);
-
-          $txn_template = $object->getApplicationTransactionTemplate()
-            ->setTransactionType(PhabricatorTransactions::TYPE_EDGE)
-            ->setMetadataValue('edge:type', $edge_type)
-            ->setNewValue(array(
-              '+' => array_fuse($add_phids),
-              '-' => array_fuse($rem_phids)));
-          $txn_editor->applyTransactions(
-            $object->getApplicationTransactionObject(),
-            array($txn_template));
         }
 
+        $old_phids = PhabricatorEdgeQuery::loadDestinationPHIDs(
+          $this->phid,
+          $edge_type);
+        $add_phids = $phids;
+        $rem_phids = array_diff($old_phids, $add_phids);
+
+        $txn_editor = $object->getApplicationTransactionEditor()
+          ->setActor($user)
+          ->setContentSourceFromRequest($request)
+          ->setContinueOnMissingFields(true);
+
+        $txn_template = $object->getApplicationTransactionTemplate()
+          ->setTransactionType(PhabricatorTransactions::TYPE_EDGE)
+          ->setMetadataValue('edge:type', $edge_type)
+          ->setNewValue(array(
+            '+' => array_fuse($add_phids),
+            '-' => array_fuse($rem_phids)));
+        $txn_editor->applyTransactions(
+          $object->getApplicationTransactionObject(),
+          array($txn_template));
+
         return id(new AphrontReloadResponse())->setURI($handle->getURI());
       } else {
         return $this->performMerge($object, $handle, $phids);