From d7a7bca85c6b76cfc041a0c175efc57051dccb60 Mon Sep 17 00:00:00 2001 From: epriestley Date: Mon, 27 Feb 2012 12:57:57 -0800 Subject: [PATCH] Enable email for audits Summary: When users submit an audit, send email to relevant parties informing them. Allow email to be replied to. Just basic support so far; no "!raise" stuff and no threading with the Herald commit notification. Test Plan: Made comments, got email. Replied to email, got comments. Reviewers: btrahan, jungejason Reviewed By: btrahan CC: aran, epriestley Maniphest Tasks: T904 Differential Revision: https://secure.phabricator.com/D1698 --- conf/default.conf.php | 11 ++ src/__phutil_library_map__.php | 2 + .../comment/PhabricatorAuditCommentEditor.php | 104 +++++++++++++++++- .../audit/editor/comment/__init__.php | 4 + .../PhabricatorAuditReplyHandler.php | 68 ++++++++++++ .../audit/replyhandler/__init__.php | 18 +++ .../base/PhabricatorMailReplyHandler.php | 5 +- .../PhabricatorMetaMTAReceivedMail.php | 10 +- .../metamta/storage/receivedmail/__init__.php | 1 + .../commit/PhabricatorRepositoryCommit.php | 8 ++ 10 files changed, 227 insertions(+), 4 deletions(-) create mode 100644 src/applications/audit/replyhandler/PhabricatorAuditReplyHandler.php create mode 100644 src/applications/audit/replyhandler/__init__.php diff --git a/conf/default.conf.php b/conf/default.conf.php index f604a7a9c4..892dc6987d 100644 --- a/conf/default.conf.php +++ b/conf/default.conf.php @@ -235,6 +235,17 @@ return array( // adapter. 'metamta.differential.attach-patches' => false, + // Prefix prepended to mail sent by Diffusion. + 'metamta.diffusion.subject-prefix' => '[Diffusion]', + + // See 'metamta.maniphest.reply-handler-domain'. This does the same thing, + // but allows email replies via Diffusion. + 'metamta.diffusion.reply-handler-domain' => null, + + // See 'metamta.maniphest.reply-handler'. This does the same thing, but + // affects Diffusion. + 'metamta.diffusion.reply-handler' => 'PhabricatorAuditReplyHandler', + // By default, Phabricator generates unique reply-to addresses and sends a // separate email to each recipient when you enable reply handling. This is // more secure than using "From" to establish user identity, but can mean diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 4e1aa1c113..6f3d9b5033 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -450,6 +450,7 @@ phutil_register_library_map(array( 'PhabricatorAuditListController' => 'applications/audit/controller/list', 'PhabricatorAuditListView' => 'applications/audit/view/list', 'PhabricatorAuditQuery' => 'applications/audit/query/audit', + 'PhabricatorAuditReplyHandler' => 'applications/audit/replyhandler', 'PhabricatorAuditStatusConstants' => 'applications/audit/constants/status', 'PhabricatorAuthController' => 'applications/auth/controller/base', 'PhabricatorCalendarBrowseController' => 'applications/calendar/controller/browse', @@ -1240,6 +1241,7 @@ phutil_register_library_map(array( 'PhabricatorAuditEditController' => 'PhabricatorAuditController', 'PhabricatorAuditListController' => 'PhabricatorAuditController', 'PhabricatorAuditListView' => 'AphrontView', + 'PhabricatorAuditReplyHandler' => 'PhabricatorMailReplyHandler', 'PhabricatorAuthController' => 'PhabricatorController', 'PhabricatorCalendarBrowseController' => 'PhabricatorCalendarController', 'PhabricatorCalendarController' => 'PhabricatorController', diff --git a/src/applications/audit/editor/comment/PhabricatorAuditCommentEditor.php b/src/applications/audit/editor/comment/PhabricatorAuditCommentEditor.php index a274b2221a..79bf2b3623 100644 --- a/src/applications/audit/editor/comment/PhabricatorAuditCommentEditor.php +++ b/src/applications/audit/editor/comment/PhabricatorAuditCommentEditor.php @@ -36,6 +36,10 @@ final class PhabricatorAuditCommentEditor { $commit = $this->commit; $user = $this->user; + $other_comments = id(new PhabricatorAuditComment())->loadAllWhere( + 'targetPHID = %s', + $commit->getPHID()); + $comment ->setActorPHID($user->getPHID()) ->setTargetPHID($commit->getPHID()) @@ -89,7 +93,7 @@ final class PhabricatorAuditCommentEditor { $this->publishFeedStory($comment, array_keys($audit_phids)); PhabricatorSearchCommitIndexer::indexCommit($commit); - // TODO: Email. + $this->sendMail($comment, $other_comments); } @@ -156,4 +160,102 @@ final class PhabricatorAuditCommentEditor { ->publish(); } + private function sendMail( + PhabricatorAuditComment $comment, + array $other_comments) { + $commit = $this->commit; + + $data = $commit->loadCommitData(); + $summary = $data->getSummary(); + + $commit_phid = $commit->getPHID(); + $phids = array($commit_phid); + $handles = id(new PhabricatorObjectHandleData($phids))->loadHandles(); + $handle = $handles[$commit_phid]; + + $name = $handle->getName(); + + $map = array( + PhabricatorAuditActionConstants::CONCERN => 'Raised Concern', + PhabricatorAuditActionConstants::ACCEPT => 'Accepted', + ); + $verb = idx($map, $comment->getAction(), 'Commented On'); + + $prefix = PhabricatorEnv::getEnvConfig('metamta.diffusion.subject-prefix'); + $subject = "{$prefix} [{$verb}] {$name}: {$summary}"; + $thread_id = 'getPHID().'>'; + $is_new = !count($other_comments); + $body = $this->renderMailBody( + $comment, + "{$name}: {$summary}", + $handle); + + $email_to = array(); + + $author_phid = $data->getCommitDetail('authorPHID'); + if ($author_phid) { + $email_to[] = $author_phid; + } + + $email_cc = array(); + foreach ($other_comments as $comment) { + $email_cc[] = $comment->getActorPHID(); + } + + $phids = array_merge($email_to, $email_cc); + $handles = id(new PhabricatorObjectHandleData($phids))->loadHandles(); + + $template = id(new PhabricatorMetaMTAMail()) + ->setSubject($subject) + ->setFrom($comment->getActorPHID()) + ->addHeader('Thread-Topic', 'Diffusion Audit '.$commit->getPHID()) + ->setThreadID($thread_id, $is_new) + ->setRelatedPHID($commit->getPHID()) + ->setIsBulk(true) + ->setBody($body); + + $reply_handler = self::newReplyHandlerForCommit($commit); + + $mails = $reply_handler->multiplexMail( + $template, + array_select_keys($handles, $email_to), + array_select_keys($handles, $email_cc)); + + foreach ($mails as $mail) { + $mail->saveAndSend(); + } + } + + public static function newReplyHandlerForCommit($commit) { + $handler_class = PhabricatorEnv::getEnvConfig( + 'metamta.diffusion.reply-handler'); + $reply_handler = newv($handler_class, array()); + $reply_handler->setMailReceiver($commit); + return $reply_handler; + } + + private function renderMailBody( + PhabricatorAuditComment $comment, + $cname, + PhabricatorObjectHandle $handle) { + + $commit = $this->commit; + $user = $this->user; + $name = $user->getUsername(); + + $verb = PhabricatorAuditActionConstants::getActionPastTenseVerb( + $comment->getAction()); + + $body = array(); + $body[] = "{$name} {$verb} commit {$cname}."; + + if ($comment->getContent()) { + $body[] = $comment->getContent(); + } + + $body[] = "COMMIT\n ".PhabricatorEnv::getProductionURI($handle->getURI()); + + return implode("\n\n", $body)."\n"; + } + } diff --git a/src/applications/audit/editor/comment/__init__.php b/src/applications/audit/editor/comment/__init__.php index 4fa8bbf9cf..08dfaf2fc3 100644 --- a/src/applications/audit/editor/comment/__init__.php +++ b/src/applications/audit/editor/comment/__init__.php @@ -10,11 +10,15 @@ phutil_require_module('phabricator', 'applications/audit/constants/action'); phutil_require_module('phabricator', 'applications/feed/constants/story'); phutil_require_module('phabricator', 'applications/feed/publisher'); phutil_require_module('phabricator', 'applications/audit/constants/status'); +phutil_require_module('phabricator', 'applications/audit/storage/auditcomment'); +phutil_require_module('phabricator', 'applications/metamta/storage/mail'); phutil_require_module('phabricator', 'applications/owners/storage/owner'); phutil_require_module('phabricator', 'applications/owners/storage/package'); phutil_require_module('phabricator', 'applications/owners/storage/packagecommitrelationship'); +phutil_require_module('phabricator', 'applications/phid/handle/data'); phutil_require_module('phabricator', 'applications/project/query/project'); phutil_require_module('phabricator', 'applications/search/index/indexer/repository'); +phutil_require_module('phabricator', 'infrastructure/env'); phutil_require_module('phutil', 'utils'); diff --git a/src/applications/audit/replyhandler/PhabricatorAuditReplyHandler.php b/src/applications/audit/replyhandler/PhabricatorAuditReplyHandler.php new file mode 100644 index 0000000000..203b6c5b7c --- /dev/null +++ b/src/applications/audit/replyhandler/PhabricatorAuditReplyHandler.php @@ -0,0 +1,68 @@ +getDefaultPrivateReplyHandlerEmailAddress($handle, 'C'); + } + + public function getPublicReplyHandlerEmailAddress() { + return $this->getDefaultPublicReplyHandlerEmailAddress('C'); + } + + public function getReplyHandlerDomain() { + return PhabricatorEnv::getEnvConfig( + 'metamta.diffusion.reply-handler-domain'); + } + + public function getReplyHandlerInstructions() { + if ($this->supportsReplies()) { + return "Reply to comment."; + } else { + return null; + } + } + + public function receiveEmail(PhabricatorMetaMTAReceivedMail $mail) { + $commit = $this->getMailReceiver(); + $actor = $this->getActor(); + + // TODO: Support !raise, !accept, etc. + // TODO: Content sources. + + $comment = id(new PhabricatorAuditComment()) + ->setAction(PhabricatorAuditActionConstants::COMMENT) + ->setContent($mail->getCleanTextBody()); + + $editor = new PhabricatorAuditCommentEditor($commit); + $editor->setUser($actor); + $editor->addComment($comment); + } + +} diff --git a/src/applications/audit/replyhandler/__init__.php b/src/applications/audit/replyhandler/__init__.php new file mode 100644 index 0000000000..f584318422 --- /dev/null +++ b/src/applications/audit/replyhandler/__init__.php @@ -0,0 +1,18 @@ +getReplyHandlerDomain()) { + return false; + } return (bool)$this->getPublicReplyHandlerEmailAddress(); } diff --git a/src/applications/metamta/storage/receivedmail/PhabricatorMetaMTAReceivedMail.php b/src/applications/metamta/storage/receivedmail/PhabricatorMetaMTAReceivedMail.php index 7f83330bd9..5cbb63b97e 100644 --- a/src/applications/metamta/storage/receivedmail/PhabricatorMetaMTAReceivedMail.php +++ b/src/applications/metamta/storage/receivedmail/PhabricatorMetaMTAReceivedMail.php @@ -1,7 +1,7 @@ buildReplyHandler($receiver); } else if ($receiver instanceof DifferentialRevision) { $handler = DifferentialMail::newReplyHandlerForRevision($receiver); + } else if ($receiver instanceof PhabricatorRepositoryCommit) { + $handler = PhabricatorAuditCommentEditor::newReplyHandlerForCommit( + $receiver); } $handler->setActor($user); @@ -222,6 +225,9 @@ class PhabricatorMetaMTAReceivedMail extends PhabricatorMetaMTADAO { case 'D': $class_obj = newv('DifferentialRevision', array()); break; + case 'C': + $class_obj = newv('PhabricatorRepositoryCommit', array()); + break; default: return null; } diff --git a/src/applications/metamta/storage/receivedmail/__init__.php b/src/applications/metamta/storage/receivedmail/__init__.php index 37d4e56863..b5100b8dea 100644 --- a/src/applications/metamta/storage/receivedmail/__init__.php +++ b/src/applications/metamta/storage/receivedmail/__init__.php @@ -6,6 +6,7 @@ +phutil_require_module('phabricator', 'applications/audit/editor/comment'); phutil_require_module('phabricator', 'applications/differential/mail/base'); phutil_require_module('phabricator', 'applications/maniphest/constants/priority'); phutil_require_module('phabricator', 'applications/maniphest/editor/transaction'); diff --git a/src/applications/repository/storage/commit/PhabricatorRepositoryCommit.php b/src/applications/repository/storage/commit/PhabricatorRepositoryCommit.php index 9984f64a89..fa55ab5c05 100644 --- a/src/applications/repository/storage/commit/PhabricatorRepositoryCommit.php +++ b/src/applications/repository/storage/commit/PhabricatorRepositoryCommit.php @@ -23,6 +23,9 @@ class PhabricatorRepositoryCommit extends PhabricatorRepositoryDAO { protected $commitIdentifier; protected $epoch; + // TODO: Add this! + // protected $mailKey; + private $commitData; public function getConfiguration() { @@ -58,6 +61,11 @@ class PhabricatorRepositoryCommit extends PhabricatorRepositoryDAO { return $this->commitData; } + public function getMailKey() { + // TODO: Fix properly! + return $this->phid; + } + public function delete() { $data = $this->loadCommitData(); $this->openTransaction();