From 02f40fd7ef48a24eabdb54c47d7ce88302fc512e Mon Sep 17 00:00:00 2001 From: epriestley Date: Mon, 16 Jul 2012 19:02:06 -0700 Subject: [PATCH] Allow noncritical blocks in email bodies to be disabled via config Summary: See T931. LLVM users would also prefer simpler mail, and have similarly harsh opinions about the current state of affairs. Allow installs to disable the hint blocks if they don't want them. Test Plan: Sent myself mail with these settings on/off, got mail with/without the blocks. Reviewers: btrahan, vrana Reviewed By: vrana CC: aran Maniphest Tasks: T931 Differential Revision: https://secure.phabricator.com/D2968 --- conf/default.conf.php | 14 +++++ .../editor/PhabricatorAuditCommentEditor.php | 25 +++----- .../PhabricatorMailReplyHandler.php | 14 +++-- .../view/PhabricatorMetaMTAMailBody.php | 15 ++++- .../PhabricatorMetaMTAMailBodyTestCase.php | 63 ++++++++++++++++--- 5 files changed, 99 insertions(+), 32 deletions(-) diff --git a/conf/default.conf.php b/conf/default.conf.php index df9cf35c69..426d00f774 100644 --- a/conf/default.conf.php +++ b/conf/default.conf.php @@ -466,6 +466,20 @@ return array( // address will be stored in an 'From Email' field on the task. 'metamta.maniphest.default-public-author' => null, + // You can disable the Herald hints in email if users prefer smaller messages. + // These are the links under the headers "MANAGE HERALD RULES" and + // "WHY DID I GET THIS EMAIL?". If you set this to true, they will not appear + // in any mail. Users can still navigate to the links via the web interface. + 'metamta.herald.show-hints' => true, + + // You can disable the hints under "REPLY HANDLER ACTIONS" if users prefer + // smaller messages. The actions themselves will still work properly. + 'metamta.reply.show-hints' => true, + + // You can disable the "To:" and "Cc:" footers in mail if users prefer + // smaller messages. + 'metamta.recipients.show-hints' => true, + // If this option is enabled, Phabricator will add a "Precedence: bulk" // header to transactional mail (e.g., Differential, Maniphest and Herald // notifications). This may improve the behavior of some auto-responder diff --git a/src/applications/audit/editor/PhabricatorAuditCommentEditor.php b/src/applications/audit/editor/PhabricatorAuditCommentEditor.php index 53f76bec3f..8a352a3a72 100644 --- a/src/applications/audit/editor/PhabricatorAuditCommentEditor.php +++ b/src/applications/audit/editor/PhabricatorAuditCommentEditor.php @@ -486,12 +486,9 @@ final class PhabricatorAuditCommentEditor { $verb = PhabricatorAuditActionConstants::getActionPastTenseVerb( $comment->getAction()); - $body = array(); - $body[] = "{$name} {$verb} commit {$cname}."; - - if ($comment->getContent()) { - $body[] = $comment->getContent(); - } + $body = new PhabricatorMetaMTAMailBody(); + $body->addRawSection("{$name} {$verb} commit {$cname}."); + $body->addRawSection($comment->getContent()); if ($inline_comments) { $block = array(); @@ -518,18 +515,16 @@ final class PhabricatorAuditCommentEditor { $content = $inline->getContent(); $block[] = "{$path}:{$range} {$content}"; } - $body[] = "INLINE COMMENTS\n ".implode("\n ", $block); + + $body->addTextSection(pht('INLINE COMMENTS'), implode("\n", $block)); } - $body[] = "COMMIT\n ".PhabricatorEnv::getProductionURI($handle->getURI()); + $body->addTextSection( + pht('COMMIT'), + PhabricatorEnv::getProductionURI($handle->getURI())); + $body->addReplySection($reply_handler->getReplyHandlerInstructions()); - - $reply_instructions = $reply_handler->getReplyHandlerInstructions(); - if ($reply_instructions) { - $body[] = "REPLY HANDLER ACTIONS\n ".$reply_instructions; - } - - return implode("\n\n", $body)."\n"; + return $body->render(); } } diff --git a/src/applications/metamta/replyhandler/PhabricatorMailReplyHandler.php b/src/applications/metamta/replyhandler/PhabricatorMailReplyHandler.php index 8c78a513f9..18a2640f3c 100644 --- a/src/applications/metamta/replyhandler/PhabricatorMailReplyHandler.php +++ b/src/applications/metamta/replyhandler/PhabricatorMailReplyHandler.php @@ -78,12 +78,16 @@ abstract class PhabricatorMailReplyHandler { assert_instances_of($cc_handles, 'PhabricatorObjectHandle'); $body = ''; - if ($to_handles) { - $body .= "To: ".implode(', ', mpull($to_handles, 'getName'))."\n"; - } - if ($cc_handles) { - $body .= "Cc: ".implode(', ', mpull($cc_handles, 'getName'))."\n"; + + if (PhabricatorEnv::getEnvConfig('metamta.recipients.show-hints')) { + if ($to_handles) { + $body .= "To: ".implode(', ', mpull($to_handles, 'getName'))."\n"; + } + if ($cc_handles) { + $body .= "Cc: ".implode(', ', mpull($cc_handles, 'getName'))."\n"; + } } + return $body; } diff --git a/src/applications/metamta/view/PhabricatorMetaMTAMailBody.php b/src/applications/metamta/view/PhabricatorMetaMTAMailBody.php index 3bf292670f..556a0d0448 100644 --- a/src/applications/metamta/view/PhabricatorMetaMTAMailBody.php +++ b/src/applications/metamta/view/PhabricatorMetaMTAMailBody.php @@ -72,12 +72,17 @@ final class PhabricatorMetaMTAMailBody { * @task compose */ public function addHeraldSection($rules_uri, $xscript_uri) { + if (!PhabricatorEnv::getEnvConfig('metamta.herald.show-hints')) { + return $this; + } + $this->addTextSection( pht('MANAGE HERALD RULES'), PhabricatorEnv::getProductionURI($rules_uri)); $this->addTextSection( pht('WHY DID I GET THIS EMAIL?'), PhabricatorEnv::getProductionURI($xscript_uri)); + return $this; } @@ -90,9 +95,15 @@ final class PhabricatorMetaMTAMailBody { * @task compose */ public function addReplySection($instructions) { - if (strlen($instructions)) { - $this->addTextSection(pht('REPLY HANDLER ACTIONS'), $instructions); + if (!PhabricatorEnv::getEnvConfig('metamta.reply.show-hints')) { + return $this; } + if (!strlen($instructions)) { + return $this; + } + + $this->addTextSection(pht('REPLY HANDLER ACTIONS'), $instructions); + return $this; } diff --git a/src/applications/metamta/view/__tests__/PhabricatorMetaMTAMailBodyTestCase.php b/src/applications/metamta/view/__tests__/PhabricatorMetaMTAMailBodyTestCase.php index abbb2dbe93..ca8d293f5f 100644 --- a/src/applications/metamta/view/__tests__/PhabricatorMetaMTAMailBodyTestCase.php +++ b/src/applications/metamta/view/__tests__/PhabricatorMetaMTAMailBodyTestCase.php @@ -21,17 +21,8 @@ */ final class PhabricatorMetaMTAMailBodyTestCase extends PhabricatorTestCase { + public function testBodyRender() { - - $env = PhabricatorEnv::beginScopedEnv(); - $env->overrideEnvConfig('phabricator.base-uri', 'http://test.com/'); - - $body = new PhabricatorMetaMTAMailBody(); - $body->addRawSection("salmon"); - $body->addTextSection("HEADER", "bass\ntrout\n"); - $body->addHeraldSection("/rules/", "/xscript/"); - $body->addReplySection("pike"); - $expect = <<assertEmail($expect, true, true); + } + + + public function testBodyRenderNoHerald() { + $expect = <<assertEmail($expect, false, true); + } + + + public function testBodyRenderNoReply() { + $expect = <<assertEmail($expect, true, false); + } + + private function assertEmail($expect, $herald_hints, $reply_hints) { + $env = PhabricatorEnv::beginScopedEnv(); + $env->overrideEnvConfig('phabricator.base-uri', 'http://test.com/'); + $env->overrideEnvConfig('metamta.herald.show-hints', $herald_hints); + $env->overrideEnvConfig('metamta.reply.show-hints', $reply_hints); + + $body = new PhabricatorMetaMTAMailBody(); + $body->addRawSection("salmon"); + $body->addTextSection("HEADER", "bass\ntrout\n"); + $body->addHeraldSection("/rules/", "/xscript/"); + $body->addReplySection("pike"); + $this->assertEqual($expect, $body->render()); }