Move Maniphest to modular mail commands

Summary:
Ref T7199. This fully modularizes mail command handling in Maniphest.

I had to add a couple of minor not-totally-solid-feeling tricks to deal with the "create" case, but they feel not-too-bad, and a million times better than what came before.

Test Plan: Used all commands with `receive-test`.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7199

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

src/__phutil_library_map__.php

@@ -1000,8 +1000,11 @@ phutil_register_library_map(array(
     'MacroConduitAPIMethod' => 'applications/macro/conduit/MacroConduitAPIMethod.php',
     'MacroCreateMemeConduitAPIMethod' => 'applications/macro/conduit/MacroCreateMemeConduitAPIMethod.php',
     'MacroQueryConduitAPIMethod' => 'applications/macro/conduit/MacroQueryConduitAPIMethod.php',
+    'ManiphestAssignEmailCommand' => 'applications/maniphest/command/ManiphestAssignEmailCommand.php',
     'ManiphestBatchEditController' => 'applications/maniphest/controller/ManiphestBatchEditController.php',
     'ManiphestBulkEditCapability' => 'applications/maniphest/capability/ManiphestBulkEditCapability.php',
+    'ManiphestClaimEmailCommand' => 'applications/maniphest/command/ManiphestClaimEmailCommand.php',
+    'ManiphestCloseEmailCommand' => 'applications/maniphest/command/ManiphestCloseEmailCommand.php',
     'ManiphestConduitAPIMethod' => 'applications/maniphest/conduit/ManiphestConduitAPIMethod.php',
     'ManiphestConfiguredCustomField' => 'applications/maniphest/field/ManiphestConfiguredCustomField.php',
     'ManiphestConstants' => 'applications/maniphest/constants/ManiphestConstants.php',
@@ -1022,6 +1025,7 @@ phutil_register_library_map(array(
     'ManiphestEditPriorityCapability' => 'applications/maniphest/capability/ManiphestEditPriorityCapability.php',
     'ManiphestEditProjectsCapability' => 'applications/maniphest/capability/ManiphestEditProjectsCapability.php',
     'ManiphestEditStatusCapability' => 'applications/maniphest/capability/ManiphestEditStatusCapability.php',
+    'ManiphestEmailCommand' => 'applications/maniphest/command/ManiphestEmailCommand.php',
     'ManiphestExcelDefaultFormat' => 'applications/maniphest/export/ManiphestExcelDefaultFormat.php',
     'ManiphestExcelFormat' => 'applications/maniphest/export/ManiphestExcelFormat.php',
     'ManiphestExportController' => 'applications/maniphest/controller/ManiphestExportController.php',
@@ -4241,8 +4245,11 @@ phutil_register_library_map(array(
     'MacroConduitAPIMethod' => 'ConduitAPIMethod',
     'MacroCreateMemeConduitAPIMethod' => 'MacroConduitAPIMethod',
     'MacroQueryConduitAPIMethod' => 'MacroConduitAPIMethod',
+    'ManiphestAssignEmailCommand' => 'ManiphestEmailCommand',
     'ManiphestBatchEditController' => 'ManiphestController',
     'ManiphestBulkEditCapability' => 'PhabricatorPolicyCapability',
+    'ManiphestClaimEmailCommand' => 'ManiphestEmailCommand',
+    'ManiphestCloseEmailCommand' => 'ManiphestEmailCommand',
     'ManiphestConduitAPIMethod' => 'ConduitAPIMethod',
     'ManiphestConfiguredCustomField' => array(
       'ManiphestCustomField',
@@ -4265,6 +4272,7 @@ phutil_register_library_map(array(
     'ManiphestEditPriorityCapability' => 'PhabricatorPolicyCapability',
     'ManiphestEditProjectsCapability' => 'PhabricatorPolicyCapability',
     'ManiphestEditStatusCapability' => 'PhabricatorPolicyCapability',
+    'ManiphestEmailCommand' => 'MetaMTAEmailTransactionCommand',
     'ManiphestExcelDefaultFormat' => 'ManiphestExcelFormat',
     'ManiphestExportController' => 'ManiphestController',
     'ManiphestGetTaskTransactionsConduitAPIMethod' => 'ManiphestConduitAPIMethod',
@@ -4275,7 +4283,7 @@ phutil_register_library_map(array(
     'ManiphestQueryConduitAPIMethod' => 'ManiphestConduitAPIMethod',
     'ManiphestQueryStatusesConduitAPIMethod' => 'ManiphestConduitAPIMethod',
     'ManiphestRemarkupRule' => 'PhabricatorObjectRemarkupRule',
-    'ManiphestReplyHandler' => 'PhabricatorMailReplyHandler',
+    'ManiphestReplyHandler' => 'PhabricatorApplicationTransactionReplyHandler',
     'ManiphestReportController' => 'ManiphestController',
     'ManiphestSchemaSpec' => 'PhabricatorConfigSchemaSpec',
     'ManiphestSearchIndexer' => 'PhabricatorSearchDocumentIndexer',

src/applications/maniphest/command/ManiphestAssignEmailCommand.php

@@ -0,0 +1,42 @@
+<?php
+
+final class ManiphestAssignEmailCommand
+  extends ManiphestEmailCommand {
+
+  public function getCommand() {
+    return 'assign';
+  }
+
+  public function buildTransactions(
+    PhabricatorUser $viewer,
+    PhabricatorApplicationTransactionInterface $object,
+    PhabricatorMetaMTAReceivedMail $mail,
+    $command,
+    array $argv) {
+    $xactions = array();
+
+
+    $assign_to = head($argv);
+    if ($assign_to) {
+      $assign_user = id(new PhabricatorPeopleQuery())
+        ->setViewer($viewer)
+        ->withUsernames(array($assign_to))
+        ->executeOne();
+      if ($assign_user) {
+        $assign_phid = $assign_user->getPHID();
+      }
+    }
+
+    // Treat bad "!assign" like "!claim".
+    if (!$assign_phid) {
+      $assign_phid = $viewer->getPHID();
+    }
+
+    $xactions[] = $object->getApplicationTransactionTemplate()
+      ->setTransactionType(ManiphestTransaction::TYPE_OWNER)
+      ->setNewValue($assign_phid);
+
+    return $xactions;
+  }
+
+}

src/applications/maniphest/command/ManiphestClaimEmailCommand.php

@@ -0,0 +1,25 @@
+<?php
+
+final class ManiphestClaimEmailCommand
+  extends ManiphestEmailCommand {
+
+  public function getCommand() {
+    return 'claim';
+  }
+
+  public function buildTransactions(
+    PhabricatorUser $viewer,
+    PhabricatorApplicationTransactionInterface $object,
+    PhabricatorMetaMTAReceivedMail $mail,
+    $command,
+    array $argv) {
+    $xactions = array();
+
+    $xactions[] = $object->getApplicationTransactionTemplate()
+      ->setTransactionType(ManiphestTransaction::TYPE_OWNER)
+      ->setNewValue($viewer->getPHID());
+
+    return $xactions;
+  }
+
+}

src/applications/maniphest/command/ManiphestCloseEmailCommand.php

@@ -0,0 +1,25 @@
+<?php
+
+final class ManiphestCloseEmailCommand
+  extends ManiphestEmailCommand {
+
+  public function getCommand() {
+    return 'close';
+  }
+
+  public function buildTransactions(
+    PhabricatorUser $viewer,
+    PhabricatorApplicationTransactionInterface $object,
+    PhabricatorMetaMTAReceivedMail $mail,
+    $command,
+    array $argv) {
+    $xactions = array();
+
+    $xactions[] = $object->getApplicationTransactionTemplate()
+      ->setTransactionType(ManiphestTransaction::TYPE_STATUS)
+      ->setNewValue(ManiphestTaskStatus::getDefaultClosedStatus());
+
+    return $xactions;
+  }
+
+}

src/applications/maniphest/command/ManiphestEmailCommand.php

@@ -0,0 +1,11 @@
+<?php
+
+abstract class ManiphestEmailCommand
+  extends MetaMTAEmailTransactionCommand {
+
+  public function isCommandSupportedForObject(
+    PhabricatorApplicationTransactionInterface $object) {
+    return ($object instanceof ManiphestTask);
+  }
+
+}

src/applications/maniphest/mail/ManiphestReplyHandler.php

@@ -1,6 +1,7 @@
 <?php
 
-final class ManiphestReplyHandler extends PhabricatorMailReplyHandler {
+final class ManiphestReplyHandler
+  extends PhabricatorApplicationTransactionReplyHandler {
 
   public function validateMailReceiver($mail_receiver) {
     if (!($mail_receiver instanceof ManiphestTask)) {
@@ -8,146 +9,31 @@ final class ManiphestReplyHandler extends PhabricatorMailReplyHandler {
     }
   }
 
-  public function getPrivateReplyHandlerEmailAddress(
+  public function getObjectPrefix() {
-    PhabricatorObjectHandle $handle) {
+    return 'T';
-    return $this->getDefaultPrivateReplyHandlerEmailAddress($handle, 'T');
   }
 
-  public function getPublicReplyHandlerEmailAddress() {
+  protected function didReceiveMail(
-    return $this->getDefaultPublicReplyHandlerEmailAddress('T');
+    PhabricatorMetaMTAReceivedMail $mail,
-  }
+    $body) {
 
-  protected function receiveEmail(PhabricatorMetaMTAReceivedMail $mail) {
+    $object = $this->getMailReceiver();
-    // NOTE: We'll drop in here on both the "reply to a task" and "create a
+    $is_new = !$object->getID();
-    // new task" workflows! Make sure you test both if you make changes!
-    $task = $this->getMailReceiver();
-    $viewer = $this->getActor();
-
-    $is_new_task = !$task->getID();
-
-    $body_data = $mail->parseBody();
-    $body = $body_data['body'];
-    $body = $this->enhanceBodyWithAttachments($body, $mail->getAttachments());
-
-    $content_source = PhabricatorContentSource::newForSource(
-      PhabricatorContentSource::SOURCE_EMAIL,
-      array(
-        'id' => $mail->getID(),
-      ));
-
-    $template = new ManiphestTransaction();
 
     $xactions = array();
-    if ($is_new_task) {
+
-      $xactions[] = id(clone $template)
+    if ($is_new) {
+      $xactions[] = $object->getApplicationTransactionTemplate()
         ->setTransactionType(ManiphestTransaction::TYPE_TITLE)
         ->setNewValue(nonempty($mail->getSubject(), pht('Untitled Task')));
 
-      $xactions[] = id(clone $template)
+      $xactions[] = $object->getApplicationTransactionTemplate()
         ->setTransactionType(ManiphestTransaction::TYPE_DESCRIPTION)
         ->setNewValue($body);
-    } else {
-      $xactions[] = id(clone $template)
-        ->setTransactionType(PhabricatorTransactions::TYPE_COMMENT)
-        ->attachComment(
-          id(new ManiphestTransactionComment())
-            ->setContent($body));
     }
 
-    $commands = $body_data['commands'];
+    return $xactions;
-    foreach ($commands as $command_argv) {
-      $command = head($command_argv);
-      $args = array_slice($command_argv, 1);
-      switch ($command) {
-        case 'close':
-          $xactions[] = id(clone $template)
-            ->setTransactionType(ManiphestTransaction::TYPE_STATUS)
-            ->setNewValue(ManiphestTaskStatus::getDefaultClosedStatus());
-          break;
-        case 'claim':
-          $xactions[] = id(clone $template)
-            ->setTransactionType(ManiphestTransaction::TYPE_OWNER)
-            ->setNewValue($viewer->getPHID());
-          break;
-        case 'assign':
-          $assign_to = head($args);
-          if ($assign_to) {
-            $assign_user = id(new PhabricatorPeopleQuery())
-              ->setViewer($viewer)
-              ->withUsernames(array($assign_to))
-              ->executeOne();
-            if ($assign_user) {
-              $assign_phid = $assign_user->getPHID();
-            }
-          }
-
-          // Treat bad "!assign" like "!claim".
-          if (!$assign_phid) {
-            $assign_phid = $viewer->getPHID();
-          }
-
-          $xactions[] = id(clone $template)
-            ->setTransactionType(ManiphestTransaction::TYPE_OWNER)
-            ->setNewValue($assign_phid);
-          break;
-        case 'unsubscribe':
-          $xactions[] = id(clone $template)
-            ->setTransactionType(PhabricatorTransactions::TYPE_SUBSCRIBERS)
-            ->setNewValue(
-              array(
-                '-' => array($viewer->getPHID()),
-              ));
-          break;
-      }
-    }
-
-    $ccs = $mail->loadCCPHIDs();
-    if ($ccs) {
-      $xactions[] = id(clone $template)
-        ->setTransactionType(PhabricatorTransactions::TYPE_SUBSCRIBERS)
-        ->setNewValue(
-          array(
-            '+' => array($viewer->getPHID()),
-          ));
-    }
-
-    $event = new PhabricatorEvent(
-      PhabricatorEventType::TYPE_MANIPHEST_WILLEDITTASK,
-      array(
-        'task'          => $task,
-        'mail'          => $mail,
-        'new'           => $is_new_task,
-        'transactions'  => $xactions,
-      ));
-    $event->setUser($viewer);
-    PhutilEventEngine::dispatchEvent($event);
-
-    $task = $event->getValue('task');
-    $xactions = $event->getValue('transactions');
-
-    $editor = id(new ManiphestTransactionEditor())
-      ->setActor($viewer)
-      ->setParentMessageID($mail->getMessageID())
-      ->setExcludeMailRecipientPHIDs($this->getExcludeMailRecipientPHIDs())
-      ->setContinueOnNoEffect(true)
-      ->setContinueOnMissingFields(true)
-      ->setContentSource($content_source);
-
-    if ($this->getApplicationEmail()) {
-      $editor->setApplicationEmail($this->getApplicationEmail());
-    }
-
-    $editor->applyTransactions($task, $xactions);
-
-    $event = new PhabricatorEvent(
-      PhabricatorEventType::TYPE_MANIPHEST_DIDEDITTASK,
-      array(
-        'task'          => $task,
-        'new'           => $is_new_task,
-        'transactions'  => $xactions,
-      ));
-    $event->setUser($viewer);
-    PhutilEventEngine::dispatchEvent($event);
   }
 
+
 }

src/applications/transactions/replyhandler/PhabricatorApplicationTransactionReplyHandler.php

@@ -43,15 +43,25 @@ abstract class PhabricatorApplicationTransactionReplyHandler
     return $this->getMailReceiver()->getApplicationTransactionTemplate();
   }
 
+  protected function didReceiveMail(
+    PhabricatorMetaMTAReceivedMail $mail,
+    $body) {
+    return array();
+  }
+
+  protected function shouldCreateCommentFromMailBody() {
+    return (bool)$this->getMailReceiver()->getID();
+  }
+
   final protected function receiveEmail(PhabricatorMetaMTAReceivedMail $mail) {
     $viewer = $this->getActor();
     $object = $this->getMailReceiver();
 
     $body_data = $mail->parseBody();
+    $body = $body_data['body'];
+    $body = $this->enhanceBodyWithAttachments($body, $mail->getAttachments());
 
-    $xactions = $this->processMailCommands(
+    $xactions = $this->didReceiveMail($mail, $body);
-      $mail,
-      $body_data['commands']);
 
     // If this object is subscribable, subscribe all the users who were
     // CC'd on the message.
@@ -67,17 +77,23 @@ abstract class PhabricatorApplicationTransactionReplyHandler
       }
     }
 
-    $body = $body_data['body'];
+    $command_xactions = $this->processMailCommands(
-    $body = $this->enhanceBodyWithAttachments($body, $mail->getAttachments());
+      $mail,
+      $body_data['commands']);
+    foreach ($command_xactions as $xaction) {
+      $xactions[] = $xaction;
+    }
 
-    $comment = $this
+    if ($this->shouldCreateCommentFromMailBody()) {
-      ->newTransaction()
+      $comment = $this
-      ->getApplicationTransactionCommentObject()
+        ->newTransaction()
-      ->setContent($body);
+        ->getApplicationTransactionCommentObject()
+        ->setContent($body);
 
-    $xactions[] = $this->newTransaction()
+      $xactions[] = $this->newTransaction()
-      ->setTransactionType(PhabricatorTransactions::TYPE_COMMENT)
+        ->setTransactionType(PhabricatorTransactions::TYPE_COMMENT)
-      ->attachComment($comment);
+        ->attachComment($comment);
+    }
 
     $target = $object->getApplicationTransactionObject();