From a69f217f980c5de2c2faf82de8c8823120dc0f60 Mon Sep 17 00:00:00 2001 From: epriestley Date: Mon, 16 May 2011 12:31:18 -0700 Subject: [PATCH] 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 --- .../mail/base/DifferentialMail.php | 18 +++++- .../replyhandler/DifferentialReplyHandler.php | 6 +- .../ManiphestTransactionEditor.php | 2 +- .../replyhandler/ManiphestReplyHandler.php | 60 ++++++++++++++++++- .../maniphest/replyhandler/__init__.php | 6 ++ .../PhabricatorMetaMTAReceiveController.php | 4 +- .../base/PhabricatorMailReplyHandler.php | 1 + .../PhabricatorMetaMTAReceivedMail.php | 34 +++-------- .../metamta/storage/receivedmail/__init__.php | 4 +- 9 files changed, 96 insertions(+), 39 deletions(-) diff --git a/src/applications/differential/mail/base/DifferentialMail.php b/src/applications/differential/mail/base/DifferentialMail.php index a517440392..0fb82a5211 100644 --- a/src/applications/differential/mail/base/DifferentialMail.php +++ b/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->setMailReceiver($this->getRevision()); + $reply_handler = self::newReplyHandlerForRevision($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) { diff --git a/src/applications/differential/replyhandler/DifferentialReplyHandler.php b/src/applications/differential/replyhandler/DifferentialReplyHandler.php index e5eda3e040..37e1bc60d6 100644 --- a/src/applications/differential/replyhandler/DifferentialReplyHandler.php +++ b/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. diff --git a/src/applications/maniphest/editor/transaction/ManiphestTransactionEditor.php b/src/applications/maniphest/editor/transaction/ManiphestTransactionEditor.php index 8b34b0880a..de6af55571 100644 --- a/src/applications/maniphest/editor/transaction/ManiphestTransactionEditor.php +++ b/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'); diff --git a/src/applications/maniphest/replyhandler/ManiphestReplyHandler.php b/src/applications/maniphest/replyhandler/ManiphestReplyHandler.php index 5b06ef951d..d7d541c4f8 100644 --- a/src/applications/maniphest/replyhandler/ManiphestReplyHandler.php +++ b/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, ". - "!unsubscribe, or !assign . ". - "TODO: None of this works yet!"; + return "Reply to comment or attach files, or !close, !claim, or ". + "!unsubscribe."; } 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)); + + } + } diff --git a/src/applications/maniphest/replyhandler/__init__.php b/src/applications/maniphest/replyhandler/__init__.php index a77e07150e..73f2b10c1e 100644 --- a/src/applications/maniphest/replyhandler/__init__.php +++ b/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'); diff --git a/src/applications/metamta/controller/receive/PhabricatorMetaMTAReceiveController.php b/src/applications/metamta/controller/receive/PhabricatorMetaMTAReceiveController.php index 900d67c2b7..7fef2f7907 100644 --- a/src/applications/metamta/controller/receive/PhabricatorMetaMTAReceiveController.php +++ b/src/applications/metamta/controller/receive/PhabricatorMetaMTAReceiveController.php @@ -32,8 +32,8 @@ class PhabricatorMetaMTAReceiveController } $hash = PhabricatorMetaMTAReceivedMail::computeMailHash( - $receiver, - $user); + $receiver->getMailKey(), + $user->getPHID()); $received = new PhabricatorMetaMTAReceivedMail(); $received->setHeaders( diff --git a/src/applications/metamta/replyhandler/base/PhabricatorMailReplyHandler.php b/src/applications/metamta/replyhandler/base/PhabricatorMailReplyHandler.php index 14f083db81..f8ab23fa3a 100644 --- a/src/applications/metamta/replyhandler/base/PhabricatorMailReplyHandler.php +++ b/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(); diff --git a/src/applications/metamta/storage/receivedmail/PhabricatorMetaMTAReceivedMail.php b/src/applications/metamta/storage/receivedmail/PhabricatorMetaMTAReceivedMail.php index 1a25ab49c4..1f61c56615 100644 --- a/src/applications/metamta/storage/receivedmail/PhabricatorMetaMTAReceivedMail.php +++ b/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( - 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() { + public function getCleanTextBody() { $body = idx($this->bodies, 'text'); // TODO: Detect quoted content and exclude it. diff --git a/src/applications/metamta/storage/receivedmail/__init__.php b/src/applications/metamta/storage/receivedmail/__init__.php index b48e2acdbe..72c98b9068 100644 --- a/src/applications/metamta/storage/receivedmail/__init__.php +++ b/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/editor/comment'); +phutil_require_module('phabricator', 'applications/differential/mail/base'); +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');