Make reply-to fully work in Maniphest and Differential for open source

Phabricator

Summary:
Hook up the last pieces. This shouldn't impact the Facebook install, EXCEPT that
I removed "!accept" and added "!rethink" (plan changes). If you want to continue
supporting !accept, you should override the method in your subclass if you don't
already.

Test Plan:
Used the Mail Receiver test console to send mail to tasks and revisions.

Reviewed By: aran
Reviewers: jungejason, tuomaspelkonen, aran
CC: aran
Differential Revision: 289

src/applications/differential/mail/base/DifferentialMail.php

@@ -156,7 +156,7 @@ EOTEXT;
     return $body;
   }
 
-  protected function getReplyHandler() {
+  public function getReplyHandler() {
     if ($this->replyHandler) {
       return $this->replyHandler;
     }
@@ -164,14 +164,26 @@ EOTEXT;
     $handler_class = PhabricatorEnv::getEnvConfig(
       'metamta.differential.reply-handler');
 
-    $reply_handler = newv($handler_class, array());
+    $reply_handler = self::newReplyHandlerForRevision($this->getRevision());
-    $reply_handler->setMailReceiver($this->getRevision());
 
     $this->replyHandler = $reply_handler;
 
     return $this->replyHandler;
   }
 
+  public static function newReplyHandlerForRevision(
+    DifferentialRevision $revision) {
+
+    $handler_class = PhabricatorEnv::getEnvConfig(
+      'metamta.differential.reply-handler');
+
+    $reply_handler = newv($handler_class, array());
+    $reply_handler->setMailReceiver($revision);
+
+    return $reply_handler;
+  }
+
+
   protected function formatText($text) {
     $text = explode("\n", $text);
     foreach ($text as &$line) {

src/applications/differential/replyhandler/DifferentialReplyHandler.php

@@ -84,14 +84,18 @@ class DifferentialReplyHandler extends PhabricatorMailReplyHandler {
   public function getSupportedCommands() {
     return array(
       DifferentialAction::ACTION_COMMENT,
-      DifferentialAction::ACTION_ACCEPT,
       DifferentialAction::ACTION_REJECT,
       DifferentialAction::ACTION_ABANDON,
       DifferentialAction::ACTION_RECLAIM,
       DifferentialAction::ACTION_RESIGN,
+      DifferentialAction::ACTION_RETHINK,
     );
   }
 
+  public function receiveEmail(PhabricatorMetaMTAReceivedMail $mail) {
+    $this->handleAction($mail->getCleanTextBody());
+  }
+
   public function handleAction($body) {
     // all commands start with a bang and separated from the body by a newline
     // to make sure that actual feedback text couldn't trigger an action.

src/applications/maniphest/editor/transaction/ManiphestTransactionEditor.php

@@ -212,7 +212,7 @@ class ManiphestTransactionEditor {
     }
   }
 
-  private function buildReplyHandler(ManiphestTask $task) {
+  public function buildReplyHandler(ManiphestTask $task) {
     $handler_class = PhabricatorEnv::getEnvConfig(
       'metamta.maniphest.reply-handler');
 

src/applications/maniphest/replyhandler/ManiphestReplyHandler.php

@@ -36,12 +36,66 @@ class ManiphestReplyHandler extends PhabricatorMailReplyHandler {
 
   public function getReplyHandlerInstructions() {
     if ($this->supportsReplies()) {
-      return "Reply to comment or attach files, or !close, !claim, ".
+      return "Reply to comment or attach files, or !close, !claim, or ".
-             "!unsubscribe, or !assign <user|upforgrabs>. ".
+             "!unsubscribe.";
-             "TODO: None of this works yet!";
     } else {
       return null;
     }
   }
 
+  public function receiveEmail(PhabricatorMetaMTAReceivedMail $mail) {
+
+    $task = $this->getMailReceiver();
+    $user = $this->getActor();
+
+    $body = $mail->getCleanTextBody();
+    $body = trim($body);
+
+    $lines = explode("\n", trim($body));
+    $first_line = head($lines);
+
+    $command = null;
+    $matches = null;
+    if (preg_match('/^!(\w+)/', $first_line, $matches)) {
+      $lines = array_slice($lines, 1);
+      $body = implode("\n", $lines);
+      $body = trim($body);
+
+      $command = $matches[1];
+    }
+
+    $ttype = ManiphestTransactionType::TYPE_NONE;
+    $new_value = null;
+    switch ($command) {
+      case 'close':
+        $ttype = ManiphestTransactionType::TYPE_STATUS;
+        $new_value = ManiphestTaskStatus::STATUS_CLOSED_RESOLVED;
+        break;
+      case 'claim':
+        $ttype = ManiphestTransactionType::TYPE_OWNER;
+        $new_value = $user->getPHID();
+        break;
+      case 'unsubscribe':
+        $ttype = ManiphestTransactionType::TYPE_CCS;
+        $ccs = $task->getCCPHIDs();
+        foreach ($ccs as $k => $phid) {
+          if ($phid == $user->getPHID()) {
+            unset($ccs[$k]);
+          }
+        }
+        $new_value = array_values($ccs);
+        break;
+    }
+
+    $xaction = new ManiphestTransaction();
+    $xaction->setAuthorPHID($user->getPHID());
+    $xaction->setTransactionType($ttype);
+    $xaction->setNewValue($new_value);
+    $xaction->setComments($body);
+
+    $editor = new ManiphestTransactionEditor();
+    $editor->applyTransactions($task, array($xaction));
+
+  }
+
 }

src/applications/maniphest/replyhandler/__init__.php

@@ -6,8 +6,14 @@
 
 
 
+phutil_require_module('phabricator', 'applications/maniphest/constants/status');
+phutil_require_module('phabricator', 'applications/maniphest/constants/transactiontype');
+phutil_require_module('phabricator', 'applications/maniphest/editor/transaction');
+phutil_require_module('phabricator', 'applications/maniphest/storage/transaction');
 phutil_require_module('phabricator', 'applications/metamta/replyhandler/base');
 phutil_require_module('phabricator', 'infrastructure/env');
 
+phutil_require_module('phutil', 'utils');
+
 
 phutil_require_source('ManiphestReplyHandler.php');

src/applications/metamta/controller/receive/PhabricatorMetaMTAReceiveController.php

@@ -32,8 +32,8 @@ class PhabricatorMetaMTAReceiveController
       }
 
       $hash = PhabricatorMetaMTAReceivedMail::computeMailHash(
-        $receiver,
+        $receiver->getMailKey(),
-        $user);
+        $user->getPHID());
 
       $received = new PhabricatorMetaMTAReceivedMail();
       $received->setHeaders(

src/applications/metamta/replyhandler/base/PhabricatorMailReplyHandler.php

@@ -45,6 +45,7 @@ abstract class PhabricatorMailReplyHandler {
     PhabricatorObjectHandle $handle);
   abstract public function getReplyHandlerDomain();
   abstract public function getReplyHandlerInstructions();
+  abstract public function receiveEmail(PhabricatorMetaMTAReceivedMail $mail);
 
   public function supportsPrivateReplies() {
     return (bool)$this->getReplyHandlerDomain();

src/applications/metamta/storage/receivedmail/PhabricatorMetaMTAReceivedMail.php

@@ -78,42 +78,22 @@ class PhabricatorMetaMTAReceivedMail extends PhabricatorMetaMTADAO {
       return $this->setMessage("Invalid mail hash!")->save();
     }
 
-    // TODO: Move this into the application logic instead.
     if ($receiver instanceof ManiphestTask) {
-      $this->processManiphestMail($receiver, $user);
+      $editor = new ManiphestTransactionEditor();
+      $handler = $editor->buildReplyHandler($receiver);
     } else if ($receiver instanceof DifferentialRevision) {
-      $this->processDifferentialMail($receiver, $user);
+      $handler = DifferentialMail::newReplyHandlerForRevision($receiver);
     }
 
+    $handler->setActor($user);
+    $handler->receiveEmail($this);
+
     $this->setMessage('OK');
 
     return $this->save();
   }
 
-  private function processManiphestMail(
+  public function getCleanTextBody() {
-    ManiphestTask $task,
-    PhabricatorUser $user) {
-
-    // TODO: implement this
-
-  }
-
-  private function processDifferentialMail(
-    DifferentialRevision $revision,
-    PhabricatorUser $user) {
-
-    // TODO: Support actions
-
-    $editor = new DifferentialCommentEditor(
-      $revision,
-      $user->getPHID(),
-      DifferentialAction::ACTION_COMMENT);
-    $editor->setMessage($this->getCleanTextBody());
-    $editor->save();
-
-  }
-
-  private function getCleanTextBody() {
     $body = idx($this->bodies, 'text');
 
     // TODO: Detect quoted content and exclude it.

src/applications/metamta/storage/receivedmail/__init__.php

@@ -6,8 +6,8 @@
 
 
 
-phutil_require_module('phabricator', 'applications/differential/constants/action');
+phutil_require_module('phabricator', 'applications/differential/mail/base');
-phutil_require_module('phabricator', 'applications/differential/editor/comment');
+phutil_require_module('phabricator', 'applications/maniphest/editor/transaction');
 phutil_require_module('phabricator', 'applications/metamta/storage/base');
 phutil_require_module('phabricator', 'applications/people/storage/user');
 phutil_require_module('phabricator', 'infrastructure/env');