Diffusion - make projects work properly with commits

Summary: Fixes T3189. Now if you say #projects in a commit message they will associate nicely with the commit. Also we record transactions about all this project editing fun.

Test Plan: tested migration by associating some projects with commits and verifying they still showed up post migration. tested adding / removing projects by doing so from the UI, noting transactions written nicely as well

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Projects: #projects

Maniphest Tasks: T3189

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

resources/sql/autopatches/20141119.commitpedge.sql

@@ -0,0 +1,11 @@
+INSERT IGNORE INTO {$NAMESPACE}_repository.edge
+  (src, type, dst, dateCreated, seq)
+  SELECT src, 41, dst, dateCreated, seq
+  FROM {$NAMESPACE}_repository.edge
+  WHERE type = 15;
+
+INSERT IGNORE INTO {$NAMESPACE}_project.edge
+  (src, type, dst, dateCreated, seq)
+  SELECT src, 42, dst, dateCreated, seq
+  FROM {$NAMESPACE}_project.edge
+  WHERE type = 16;

src/applications/diffusion/controller/DiffusionCommitController.php

@@ -420,7 +420,6 @@ final class DiffusionCommitController extends DiffusionController {
       ->withSourcePHIDs(array($commit_phid))
       ->withEdgeTypes(array(
         DiffusionCommitHasTaskEdgeType::EDGECONST,
-        PhabricatorEdgeConfig::TYPE_COMMIT_HAS_PROJECT,
         PhabricatorEdgeConfig::TYPE_COMMIT_HAS_DREV,
       ));
 
@@ -428,8 +427,6 @@ final class DiffusionCommitController extends DiffusionController {
 
     $task_phids = array_keys(
       $edges[$commit_phid][DiffusionCommitHasTaskEdgeType::EDGECONST]);
-    $proj_phids = array_keys(
-      $edges[$commit_phid][PhabricatorEdgeConfig::TYPE_COMMIT_HAS_PROJECT]);
     $revision_phid = key(
       $edges[$commit_phid][PhabricatorEdgeConfig::TYPE_COMMIT_HAS_DREV]);
 
@@ -629,15 +626,6 @@ final class DiffusionCommitController extends DiffusionController {
       $props['Tasks'] = $task_list;
     }
 
-    if ($proj_phids) {
-      $proj_list = array();
-      foreach ($proj_phids as $phid) {
-        $proj_list[] = $handles[$phid]->renderLink();
-      }
-      $proj_list = phutil_implode_html(phutil_tag('br'), $proj_list);
-      $props['Projects'] = $proj_list;
-    }
-
     return $props;
   }
 

src/applications/diffusion/controller/DiffusionCommitEditController.php

@@ -22,7 +22,7 @@ final class DiffusionCommitEditController extends DiffusionController {
     }
 
     $commit_phid        = $commit->getPHID();
-    $edge_type          = PhabricatorEdgeConfig::TYPE_COMMIT_HAS_PROJECT;
+    $edge_type          = PhabricatorProjectObjectHasProjectEdgeType::EDGECONST;
     $current_proj_phids = PhabricatorEdgeQuery::loadDestinationPHIDs(
       $commit_phid,
       $edge_type);
@@ -30,23 +30,17 @@ final class DiffusionCommitEditController extends DiffusionController {
     $proj_t_values = $handles;
 
     if ($request->isFormPost()) {
+      $xactions = array();
       $proj_phids = $request->getArr('projects');
-      $new_proj_phids = array_values($proj_phids);
+      $xactions[] = id(new PhabricatorAuditTransaction())
-      $rem_proj_phids = array_diff($current_proj_phids,
+        ->setTransactionType(PhabricatorTransactions::TYPE_EDGE)
-                                   $new_proj_phids);
+        ->setMetadataValue('edge:type', $edge_type)
-
+        ->setNewValue(array('=' => array_fuse($proj_phids)));
-      $editor = id(new PhabricatorEdgeEditor());
+      $editor = id(new PhabricatorAuditEditor())
-      foreach ($rem_proj_phids as $phid) {
+        ->setActor($user)
-        $editor->removeEdge($commit_phid, $edge_type, $phid);
+        ->setContinueOnNoEffect(true)
-      }
+        ->setContentSourceFromRequest($request);
-      foreach ($new_proj_phids as $phid) {
+      $xactions = $editor->applyTransactions($commit, $xactions);
-        $editor->addEdge($commit_phid, $edge_type, $phid);
-      }
-      $editor->save();
-
-      id(new PhabricatorSearchIndexer())
-        ->queueDocumentForIndexing($commit->getPHID());
-
       return id(new AphrontRedirectResponse())
       ->setURI('/r'.$callsign.$commit->getCommitIdentifier());
     }

src/applications/maniphest/search/ManiphestSearchIndexer.php

@@ -39,14 +39,6 @@ final class ManiphestSearchIndexer extends PhabricatorSearchDocumentIndexer {
       new ManiphestTransactionQuery(),
       array($phid));
 
-    foreach ($task->getProjectPHIDs() as $phid) {
-      $doc->addRelationship(
-        PhabricatorSearchRelationship::RELATIONSHIP_PROJECT,
-        $phid,
-        PhabricatorProjectProjectPHIDType::TYPECONST,
-        $task->getDateModified()); // Bogus.
-    }
-
     $owner = $task->getOwnerPHID();
     if ($owner) {
       $doc->addRelationship(

src/applications/repository/search/PhabricatorRepositoryCommitSearchIndexer.php

@@ -47,19 +47,6 @@ final class PhabricatorRepositoryCommitSearchIndexer
         $date_created);
     }
 
-    $project_phids = PhabricatorEdgeQuery::loadDestinationPHIDs(
-      $commit->getPHID(),
-      PhabricatorEdgeConfig::TYPE_COMMIT_HAS_PROJECT);
-    if ($project_phids) {
-      foreach ($project_phids as $project_phid) {
-        $doc->addRelationship(
-          PhabricatorSearchRelationship::RELATIONSHIP_PROJECT,
-          $project_phid,
-          PhabricatorProjectProjectPHIDType::TYPECONST,
-          $date_created);
-      }
-    }
-
     $doc->addRelationship(
       PhabricatorSearchRelationship::RELATIONSHIP_REPOSITORY,
       $repository->getPHID(),

src/applications/repository/storage/PhabricatorRepositoryCommit.php

@@ -5,6 +5,7 @@ final class PhabricatorRepositoryCommit
   implements
     PhabricatorPolicyInterface,
     PhabricatorFlaggableInterface,
+    PhabricatorProjectInterface,
     PhabricatorTokenReceiverInterface,
     PhabricatorSubscribableInterface,
     PhabricatorMentionableInterface,

src/applications/search/index/PhabricatorSearchDocumentIndexer.php

@@ -47,6 +47,11 @@ abstract class PhabricatorSearchDocumentIndexer {
         $this->indexSubscribers($document);
       }
 
+      // Automatically build project relationships
+      if ($object instanceof PhabricatorProjectInterface) {
+        $this->indexProjects($document, $object);
+      }
+
       $engine = PhabricatorSearchEngineSelector::newSelector()->newEngine();
       try {
         $engine->reindexAbstractDocument($document);
@@ -93,6 +98,24 @@ abstract class PhabricatorSearchDocumentIndexer {
     }
   }
 
+  protected function indexProjects(
+    PhabricatorSearchAbstractDocument $doc,
+    PhabricatorProjectInterface $object) {
+
+    $project_phids = PhabricatorEdgeQuery::loadDestinationPHIDs(
+      $object->getPHID(),
+      PhabricatorProjectObjectHasProjectEdgeType::EDGECONST);
+    if ($project_phids) {
+      foreach ($project_phids as $project_phid) {
+        $doc->addRelationship(
+          PhabricatorSearchRelationship::RELATIONSHIP_PROJECT,
+          $project_phid,
+          PhabricatorProjectProjectPHIDType::TYPECONST,
+          $doc->getDocumentModified()); // Bogus timestamp.
+      }
+    }
+  }
+
   protected function indexTransactions(
     PhabricatorSearchAbstractDocument $doc,
     PhabricatorApplicationTransactionQuery $query,

src/infrastructure/edges/constants/PhabricatorEdgeConfig.php

@@ -19,9 +19,6 @@ final class PhabricatorEdgeConfig extends PhabricatorEdgeConstants {
   const TYPE_PROJ_MEMBER                = 13;
   const TYPE_MEMBER_OF_PROJ             = 14;
 
-  const TYPE_COMMIT_HAS_PROJECT         = 15;
-  const TYPE_PROJECT_HAS_COMMIT         = 16;
-
   const TYPE_QUESTION_HAS_VOTING_USER   = 17;
   const TYPE_VOTING_USER_HAS_QUESTION   = 18;
   const TYPE_ANSWER_HAS_VOTING_USER     = 19;
@@ -100,6 +97,9 @@ final class PhabricatorEdgeConfig extends PhabricatorEdgeConstants {
       array(9000),
       range(80000, 80005));
 
+    $exclude[] = 15; // Was TYPE_COMMIT_HAS_PROJECT
+    $exclude[] = 16; // Was TYPE_PROJECT_HAS_COMMIT
+
     $exclude[] = 27; // Was TYPE_ACCOUNT_HAS_MEMBER
     $exclude[] = 28; // Was TYPE_MEMBER_HAS_ACCOUNT
 
@@ -145,9 +145,6 @@ final class PhabricatorEdgeConfig extends PhabricatorEdgeConstants {
       self::TYPE_PROJ_MEMBER => self::TYPE_MEMBER_OF_PROJ,
       self::TYPE_MEMBER_OF_PROJ => self::TYPE_PROJ_MEMBER,
 
-      self::TYPE_COMMIT_HAS_PROJECT => self::TYPE_PROJECT_HAS_COMMIT,
-      self::TYPE_PROJECT_HAS_COMMIT => self::TYPE_COMMIT_HAS_PROJECT,
-
       self::TYPE_QUESTION_HAS_VOTING_USER =>
         self::TYPE_VOTING_USER_HAS_QUESTION,
       self::TYPE_VOTING_USER_HAS_QUESTION =>
@@ -239,7 +236,6 @@ final class PhabricatorEdgeConfig extends PhabricatorEdgeConstants {
 
   public static function getEditStringForEdgeType($type) {
     switch ($type) {
-      case self::TYPE_PROJECT_HAS_COMMIT:
       case self::TYPE_DREV_HAS_COMMIT:
         return '%s edited commit(s), added %d: %s; removed %d: %s.';
       case self::TYPE_TASK_DEPENDS_ON_TASK:
@@ -261,7 +257,6 @@ final class PhabricatorEdgeConfig extends PhabricatorEdgeConstants {
       case self::TYPE_PROJ_MEMBER:
         return '%s edited member(s), added %d: %s; removed %d: %s.';
       case self::TYPE_MEMBER_OF_PROJ:
-      case self::TYPE_COMMIT_HAS_PROJECT:
         return '%s edited project(s), added %d: %s; removed %d: %s.';
       case self::TYPE_QUESTION_HAS_VOTING_USER:
       case self::TYPE_ANSWER_HAS_VOTING_USER:
@@ -307,7 +302,6 @@ final class PhabricatorEdgeConfig extends PhabricatorEdgeConstants {
 
   public static function getAddStringForEdgeType($type) {
     switch ($type) {
-      case self::TYPE_PROJECT_HAS_COMMIT:
       case self::TYPE_DREV_HAS_COMMIT:
         return '%s added %d commit(s): %s.';
       case self::TYPE_TASK_DEPENDS_ON_TASK:
@@ -332,7 +326,6 @@ final class PhabricatorEdgeConfig extends PhabricatorEdgeConstants {
       case self::TYPE_PROJ_MEMBER:
         return '%s added %d member(s): %s.';
       case self::TYPE_MEMBER_OF_PROJ:
-      case self::TYPE_COMMIT_HAS_PROJECT:
         return '%s added %d project(s): %s.';
       case self::TYPE_QUESTION_HAS_VOTING_USER:
       case self::TYPE_ANSWER_HAS_VOTING_USER:
@@ -377,7 +370,6 @@ final class PhabricatorEdgeConfig extends PhabricatorEdgeConstants {
 
   public static function getRemoveStringForEdgeType($type) {
     switch ($type) {
-      case self::TYPE_PROJECT_HAS_COMMIT:
       case self::TYPE_DREV_HAS_COMMIT:
         return '%s removed %d commit(s): %s.';
       case self::TYPE_TASK_DEPENDS_ON_TASK:
@@ -401,7 +393,6 @@ final class PhabricatorEdgeConfig extends PhabricatorEdgeConstants {
       case self::TYPE_PROJ_MEMBER:
         return '%s removed %d member(s): %s.';
       case self::TYPE_MEMBER_OF_PROJ:
-      case self::TYPE_COMMIT_HAS_PROJECT:
         return '%s removed %d project(s): %s.';
       case self::TYPE_QUESTION_HAS_VOTING_USER:
       case self::TYPE_ANSWER_HAS_VOTING_USER:
@@ -444,7 +435,6 @@ final class PhabricatorEdgeConfig extends PhabricatorEdgeConstants {
 
   public static function getFeedStringForEdgeType($type) {
     switch ($type) {
-      case self::TYPE_PROJECT_HAS_COMMIT:
       case self::TYPE_DREV_HAS_COMMIT:
         return '%s updated commits of %s.';
       case self::TYPE_TASK_DEPENDS_ON_TASK:
@@ -466,7 +456,6 @@ final class PhabricatorEdgeConfig extends PhabricatorEdgeConstants {
       case self::TYPE_PROJ_MEMBER:
         return '%s updated members of %s.';
       case self::TYPE_MEMBER_OF_PROJ:
-      case self::TYPE_COMMIT_HAS_PROJECT:
         return '%s updated projects of %s.';
       case self::TYPE_QUESTION_HAS_VOTING_USER:
       case self::TYPE_ANSWER_HAS_VOTING_USER: