Move "close tasks on commit" code out of field specification stuff

Summary: Ref T2222. There's some magic here, just port it forward in a mostly-reasonable way. This could use some refinement eventually.

Test Plan: Pushed commits with "Fixes" and "Ref" language, used `reparse.php` to trigger the new code. Saw expected updates in Maniphest.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T2222

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

src/applications/differential/field/specification/DifferentialFieldSpecification.php

@@ -578,25 +578,6 @@ abstract class DifferentialFieldSpecification {
   }
 
 
-  /**
-   * This method allows you to take action when a commit appears in a tracked
-   * branch (for example, by closing tasks associated with the commit).
-   *
-   * @param PhabricatorRepository The repository the commit appeared in.
-   * @param PhabricatorRepositoryCommit The commit itself.
-   * @param PhabricatorRepostioryCommitData Commit data.
-   * @return void
-   *
-   * @task commit
-   */
-  public function didParseCommit(
-    PhabricatorRepository $repo,
-    PhabricatorRepositoryCommit $commit,
-    PhabricatorRepositoryCommitData $data) {
-    return;
-  }
-
-
 
 /* -(  Loading Additional Data  )-------------------------------------------- */
 

src/applications/differential/field/specification/DifferentialFreeformFieldSpecification.php

@@ -3,162 +3,4 @@
 abstract class DifferentialFreeformFieldSpecification
   extends DifferentialFieldSpecification {
 
-  private function findMentionedTasks($message) {
-    $maniphest = 'PhabricatorApplicationManiphest';
-    if (!PhabricatorApplication::isClassInstalled($maniphest)) {
-      return array();
-    }
-
-    $prefixes = ManiphestTaskStatus::getStatusPrefixMap();
-    $suffixes = ManiphestTaskStatus::getStatusSuffixMap();
-
-    $matches = id(new ManiphestCustomFieldStatusParser())
-      ->parseCorpus($message);
-
-    $task_statuses = array();
-    foreach ($matches as $match) {
-      $prefix = phutil_utf8_strtolower($match['prefix']);
-      $suffix = phutil_utf8_strtolower($match['suffix']);
-
-      $status = idx($suffixes, $suffix);
-      if (!$status) {
-        $status = idx($prefixes, $prefix);
-      }
-
-      foreach ($match['monograms'] as $task_monogram) {
-        $task_id = (int)trim($task_monogram, 'tT');
-        $task_statuses[$task_id] = $status;
-      }
-    }
-
-    return $task_statuses;
-  }
-
-  private function findDependentRevisions($message) {
-    $matches = id(new DifferentialCustomFieldDependsOnParser())
-      ->parseCorpus($message);
-
-    $dependents = array();
-    foreach ($matches as $match) {
-      foreach ($match['monograms'] as $monogram) {
-        $id = (int)trim($monogram, 'dD');
-        $dependents[$id] = $id;
-      }
-    }
-
-    return $dependents;
-  }
-
-  public static function findRevertedCommits($message) {
-    $matches = id(new DifferentialCustomFieldRevertsParser())
-      ->parseCorpus($message);
-
-    $result = array();
-    foreach ($matches as $match) {
-      foreach ($match['monograms'] as $monogram) {
-        $result[$monogram] = $monogram;
-      }
-    }
-
-    return $result;
-  }
-
-  private function saveFieldEdges(
-    DifferentialRevision $revision,
-    $edge_type,
-    array $add_phids) {
-
-    $revision_phid = $revision->getPHID();
-
-    $old_phids = PhabricatorEdgeQuery::loadDestinationPHIDs(
-      $revision_phid,
-      $edge_type);
-
-    $add_phids = array_diff($add_phids, $old_phids);
-    if (!$add_phids) {
-      return;
-    }
-
-    $edge_editor = id(new PhabricatorEdgeEditor())->setActor($this->getUser());
-    foreach ($add_phids as $phid) {
-      $edge_editor->addEdge($revision_phid, $edge_type, $phid);
-    }
-    // NOTE: Deletes only through the fields.
-    $edge_editor->save();
-  }
-
-  public function didParseCommit(
-    PhabricatorRepository $repository,
-    PhabricatorRepositoryCommit $commit,
-    PhabricatorRepositoryCommitData $data) {
-
-    $message = $this->renderValueForCommitMessage($is_edit = false);
-
-    $user = id(new PhabricatorUser())->loadOneWhere(
-      'phid = %s',
-      $data->getCommitDetail('authorPHID'));
-    if (!$user) {
-      // TODO: Maybe after grey users, we should find a way to proceed even
-      // if we don't know who the author is.
-      return;
-    }
-
-    $commit_names = self::findRevertedCommits($message);
-    if ($commit_names) {
-      $reverts = id(new DiffusionCommitQuery())
-        ->setViewer($user)
-        ->withIdentifiers($commit_names)
-        ->withDefaultRepository($repository)
-        ->execute();
-      foreach ($reverts as $revert) {
-        // TODO: Do interesting things here.
-      }
-    }
-
-    $tasks_statuses = $this->findMentionedTasks($message);
-    if (!$tasks_statuses) {
-      return;
-    }
-
-    $tasks = id(new ManiphestTaskQuery())
-      ->setViewer($user)
-      ->withIDs(array_keys($tasks_statuses))
-      ->execute();
-
-    foreach ($tasks as $task_id => $task) {
-      id(new PhabricatorEdgeEditor())
-        ->setActor($user)
-        ->addEdge(
-          $task->getPHID(),
-          PhabricatorEdgeConfig::TYPE_TASK_HAS_COMMIT,
-          $commit->getPHID())
-        ->save();
-
-      $status = $tasks_statuses[$task_id];
-      if (!$status) {
-        // Text like "Ref T123", don't change the task status.
-        continue;
-      }
-
-      if ($task->getStatus() == $status) {
-        // Task is already in the specified status, so skip updating it.
-        continue;
-      }
-
-      $commit_name = $repository->formatCommitName(
-        $commit->getCommitIdentifier());
-
-      $call = new ConduitCall(
-        'maniphest.update',
-        array(
-          'id'        => $task->getID(),
-          'status'    => $status,
-          'comments'  => "Closed by commit {$commit_name}.",
-        ));
-
-      $call->setUser($user);
-      $call->execute();
-    }
-  }
-
 }

src/applications/releeph/differential/DifferentialReleephRequestFieldSpecification.php

@@ -243,6 +243,8 @@ final class DifferentialReleephRequestFieldSpecification
                                  PhabricatorRepositoryCommit $commit,
                                  PhabricatorRepositoryCommitData $data) {
 
+    // NOTE: This is currently dead code. See T2222.
+
     $releeph_requests = $this->loadReleephRequests();
 
     if (!$releeph_requests) {

src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryCommitMessageParserWorker.php

@@ -85,10 +85,6 @@ abstract class PhabricatorRepositoryCommitMessageParserWorker
         ->needReviewerStatus(true)
         ->needActiveDiffs(true);
 
-      // TODO: Remove this once we swap to new CustomFields. This is only
-      // required by the old FieldSpecifications, lower on in this worker.
-      $revision_query->needRelationships(true);
-
       $revision = $revision_query->executeOne();
 
       if ($revision) {
@@ -200,21 +196,9 @@ abstract class PhabricatorRepositoryCommitMessageParserWorker
     }
 
     if ($should_autoclose) {
-      // TODO: When this is moved to CustomFields, remove the additional
+      // TODO: This isn't as general as it could be.
-      // call above in query construction.
+      if ($user->getPHID()) {
-      $fields = DifferentialFieldSelector::newSelector()
+        $this->closeTasks($user, $repository, $commit, $message);
-        ->getFieldSpecifications();
-      foreach ($fields as $key => $field) {
-        if (!$field->shouldAppearOnCommitMessage()) {
-          continue;
-        }
-        $field->setUser($user);
-        $value = idx($field_values, $field->getCommitMessageKey());
-        $field->setValueFromParsedCommitMessage($value);
-        if ($revision) {
-          $field->setRevision($revision);
-        }
-        $field->didParseCommit($repository, $commit, $data);
       }
     }
 
@@ -418,4 +402,109 @@ abstract class PhabricatorRepositoryCommitMessageParserWorker
       ->execute();
   }
 
+  private function closeTasks(
+    PhabricatorUser $actor,
+    PhabricatorRepository $repository,
+    PhabricatorRepositoryCommit $commit,
+    $message) {
+
+    $maniphest = 'PhabricatorApplicationManiphest';
+    if (!PhabricatorApplication::isClassInstalled($maniphest)) {
+      return;
+    }
+
+    $prefixes = ManiphestTaskStatus::getStatusPrefixMap();
+    $suffixes = ManiphestTaskStatus::getStatusSuffixMap();
+
+    $matches = id(new ManiphestCustomFieldStatusParser())
+      ->parseCorpus($message);
+
+    $task_statuses = array();
+    foreach ($matches as $match) {
+      $prefix = phutil_utf8_strtolower($match['prefix']);
+      $suffix = phutil_utf8_strtolower($match['suffix']);
+
+      $status = idx($suffixes, $suffix);
+      if (!$status) {
+        $status = idx($prefixes, $prefix);
+      }
+
+      foreach ($match['monograms'] as $task_monogram) {
+        $task_id = (int)trim($task_monogram, 'tT');
+        $task_statuses[$task_id] = $status;
+      }
+    }
+
+    if (!$task_statuses) {
+      return;
+    }
+
+    $tasks = id(new ManiphestTaskQuery())
+      ->setViewer($actor)
+      ->withIDs(array_keys($task_statuses))
+      ->execute();
+
+    foreach ($tasks as $task_id => $task) {
+      $xactions = array();
+
+      // TODO: Swap this for a real edge transaction once the weirdness in
+      // Maniphest edges is sorted out. Currently, Maniphest reacts to an edge
+      // edit on this edge.
+      id(new PhabricatorEdgeEditor())
+        ->setActor($actor)
+        ->addEdge(
+          $task->getPHID(),
+          PhabricatorEdgeConfig::TYPE_TASK_HAS_COMMIT,
+          $commit->getPHID())
+        ->save();
+
+      /* TODO: Do this instead of the above.
+
+      $xactions[] = id(new ManiphestTransaction())
+        ->setTransactionType(PhabricatorTransactions::TYPE_EDGE)
+        ->setMetadataValue('edge:type', $edge_task_has_commit)
+        ->setNewValue(
+          array(
+            '+' => array(
+              $commit->getPHID() => $commit->getPHID(),
+            ),
+          ));
+      */
+
+      $status = $task_statuses[$task_id];
+      if ($status) {
+        if ($task->getStatus() != $status) {
+          $xactions[] = id(new ManiphestTransaction())
+            ->setTransactionType(ManiphestTransaction::TYPE_STATUS)
+            ->setNewValue($status);
+
+          $commit_name = $repository->formatCommitName(
+            $commit->getCommitIdentifier());
+
+          $status_message = pht(
+            'Closed by commit %s.',
+            $commit_name);
+
+          $xactions[] = id(new ManiphestTransaction())
+            ->setTransactionType(PhabricatorTransactions::TYPE_COMMENT)
+            ->attachComment(
+              id(new ManiphestTransactionComment())
+                ->setContent($status_message));
+        }
+      }
+
+      $content_source = PhabricatorContentSource::newForSource(
+        PhabricatorContentSource::SOURCE_DAEMON,
+        array());
+
+      $editor = id(new ManiphestTransactionEditor())
+        ->setActor($actor)
+        ->setContinueOnNoEffect(true)
+        ->setContinueOnMissingFields(true)
+        ->setContentSource($content_source);
+
+      $editor->applyTransactions($task, $xactions);
+    }
+  }
+
 }