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

@@ -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

src/applications/audit/editor/PhabricatorAuditCommentEditor.php

@@ -486,12 +486,9 @@ final class PhabricatorAuditCommentEditor {
     $verb = PhabricatorAuditActionConstants::getActionPastTenseVerb(
       $comment->getAction());
 
-    $body = array();
+    $body = new PhabricatorMetaMTAMailBody();
-    $body[] = "{$name} {$verb} commit {$cname}.";
+    $body->addRawSection("{$name} {$verb} commit {$cname}.");
-
+    $body->addRawSection($comment->getContent());
-    if ($comment->getContent()) {
-      $body[] = $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());
 
-
+    return $body->render();
-    $reply_instructions = $reply_handler->getReplyHandlerInstructions();
-    if ($reply_instructions) {
-      $body[] = "REPLY HANDLER ACTIONS\n  ".$reply_instructions;
-    }
-
-    return implode("\n\n", $body)."\n";
   }
 
 }

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 (PhabricatorEnv::getEnvConfig('metamta.recipients.show-hints')) {
-    }
+      if ($to_handles) {
-    if ($cc_handles) {
+        $body .= "To: ".implode(', ', mpull($to_handles, 'getName'))."\n";
-      $body .= "Cc: ".implode(', ', mpull($cc_handles, 'getName'))."\n";
+      }
+      if ($cc_handles) {
+        $body .= "Cc: ".implode(', ', mpull($cc_handles, 'getName'))."\n";
+      }
     }
+
     return $body;
   }
 

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)) {
+    if (!PhabricatorEnv::getEnvConfig('metamta.reply.show-hints')) {
-      $this->addTextSection(pht('REPLY HANDLER ACTIONS'), $instructions);
+      return $this;
     }
+    if (!strlen($instructions)) {
+      return $this;
+    }
+
+    $this->addTextSection(pht('REPLY HANDLER ACTIONS'), $instructions);
+
     return $this;
   }
 

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 = <<<EOTEXT
 salmon
 
@@ -50,6 +41,58 @@ REPLY HANDLER ACTIONS
 
 EOTEXT;
 
+    $this->assertEmail($expect, true, true);
+  }
+
+
+  public function testBodyRenderNoHerald() {
+    $expect = <<<EOTEXT
+salmon
+
+HEADER
+  bass
+  trout
+
+REPLY HANDLER ACTIONS
+  pike
+
+EOTEXT;
+
+    $this->assertEmail($expect, false, true);
+  }
+
+
+  public function testBodyRenderNoReply() {
+    $expect = <<<EOTEXT
+salmon
+
+HEADER
+  bass
+  trout
+
+MANAGE HERALD RULES
+  http://test.com/rules/
+
+WHY DID I GET THIS EMAIL?
+  http://test.com/xscript/
+
+EOTEXT;
+
+    $this->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());
   }