Send several X-Differential-CC headers

Summary: People want to create filters checking if they are in CC which is almost impossible with multiplexing in Outlook.

Test Plan: Sent e-mail with multiple CCs, verified headers.

Reviewers: epriestley, nh

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D2829

src/applications/differential/mail/DifferentialMail.php

@@ -110,15 +110,26 @@ abstract class DifferentialMail {
           'X-Differential-Author',
           '<'.$revision->getAuthorPHID().'>');
       }
-      if ($revision->getReviewers()) {
+
+      $reviewer_phids = $revision->getReviewers();
+      if ($reviewer_phids) {
+        // Add several headers to support e-mail clients which are not able to
+        // create rules using regular expressions or wildcards (namely Outlook).
+        $template->addPHIDHeaders('X-Differential-Reviewer', $reviewer_phids);
+
+        // Add it also as a list to allow matching of the first reviewer and
+        // also for backwards compatibility.
         $template->addHeader(
           'X-Differential-Reviewers',
-          '<'.implode('>, <', $revision->getReviewers()).'>');
+          '<'.implode('>, <', $reviewer_phids).'>');
       }
-      if ($revision->getCCPHIDs()) {
+
+      $cc_phids = $revision->getCCPHIDs();
+      if ($cc_phids) {
+        $template->addPHIDHeaders('X-Differential-CC', $cc_phids);
         $template->addHeader(
           'X-Differential-CCs',
-          '<'.implode('>, <', $revision->getCCPHIDs()).'>');
+          '<'.implode('>, <', $cc_phids).'>');
 
         // Determine explicit CCs (those added by humans) and put them in a
         // header so users can differentiate between Herald CCs and human CCs.
@@ -142,6 +153,7 @@ abstract class DifferentialMail {
         }
 
         if ($explicit_cc) {
+          $template->addPHIDHeaders('X-Differential-Explicit-CC', $explicit_cc);
           $template->addHeader(
             'X-Differential-Explicit-CCs',
             '<'.implode('>, <', $explicit_cc).'>');

src/applications/metamta/replyhandler/PhabricatorMailReplyHandler.php

@@ -118,24 +118,24 @@ abstract class PhabricatorMailReplyHandler {
       }
     }
 
+    $tos = mpull($to_handles, null, 'getPHID');
+    $ccs = mpull($cc_handles, null, 'getPHID');
+
     // Merge all the recipients together. TODO: We could keep the CCs as real
     // CCs and send to a "noreply@domain.com" type address, but keep it simple
     // for now.
-    $recipients = mpull($to_handles, null, 'getPHID') +
-                  mpull($cc_handles, null, 'getPHID');
+    $recipients = $tos + $ccs;
 
     // When multiplexing mail, explicitly include To/Cc information in the
     // message body and headers.
-    $add_headers = array();
+
+    $mail_template = clone $mail_template;
+
+    $mail_template->addPHIDHeaders('X-Phabricator-To', array_keys($tos));
+    $mail_template->addPHIDHeaders('X-Phabricator-Cc', array_keys($ccs));
 
     $body = $mail_template->getBody();
     $body .= "\n";
-    if ($to_handles) {
-      $add_headers['X-Phabricator-To'] = $this->formatPHIDList($to_handles);
-    }
-    if ($cc_handles) {
-      $add_headers['X-Phabricator-Cc'] = $this->formatPHIDList($cc_handles);
-    }
     $body .= $this->getRecipientsSummary($to_handles, $cc_handles);
 
     foreach ($recipients as $recipient) {
@@ -143,9 +143,6 @@ abstract class PhabricatorMailReplyHandler {
       $mail->addTos(array($recipient->getPHID()));
 
       $mail->setBody($body);
-      foreach ($add_headers as $header => $value) {
-        $mail->addHeader($header, $value);
-      }
 
       $reply_to = null;
       if (!$reply_to && $this->supportsPrivateReplies()) {
@@ -166,15 +163,6 @@ abstract class PhabricatorMailReplyHandler {
     return $result;
   }
 
-  protected function formatPHIDList(array $handles) {
-    assert_instances_of($handles, 'PhabricatorObjectHandle');
-    $list = array();
-    foreach ($handles as $handle) {
-      $list[] = '<'.$handle->getPHID().'>';
-    }
-    return implode(', ', $list);
-  }
-
   protected function getDefaultPublicReplyHandlerEmailAddress($prefix) {
 
     $receiver = $this->getMailReceiver();

src/applications/metamta/storage/PhabricatorMetaMTAMail.php

@@ -141,8 +141,15 @@ final class PhabricatorMetaMTAMail extends PhabricatorMetaMTADAO {
     return $return;
   }
 
+  public function addPHIDHeaders($name, array $phids) {
+    foreach ($phids as $phid) {
+      $this->addHeader($name, '<'.$phid.'>');
+    }
+    return $this;
+  }
+
   public function addHeader($name, $value) {
-    $this->parameters['headers'][$name] = $value;
+    $this->parameters['headers'][] = array($name, $value);
     return $this;
   }
 
@@ -408,7 +415,9 @@ final class PhabricatorMetaMTAMail extends PhabricatorMetaMTADAO {
             }
             break;
           case 'headers':
-            foreach ($value as $header_key => $header_value) {
+            foreach ($value as $pair) {
+              list($header_key, $header_value) = $pair;
+
               // NOTE: If we have \n in a header, SES rejects the email.
               $header_value = str_replace("\n", " ", $header_value);
 

src/docs/userguide/mail_rules.diviner

@@ -34,7 +34,7 @@ to write more sophisticated mail rules.
 Phabricator sends a variety of mail headers that can be useful in crafting rules
 to route and manage mail.
 
-Many of these headers contain lists. A list containing two items, ##1## and
+Headers in plural contain lists. A list containing two items, ##1## and
 ##15## will generally be formatted like this:
 
   X-Header: <1>, <15>
@@ -43,13 +43,17 @@ The intent is to allow you to write a rule which matches against "<1>". If you
 just match against "1", you'll incorrectly match "15", but matching "<1>" will
 correctly match only "<1>".
 
+Some other headers use a single value but can be presented multiple times.
+It is to support e-mail clients which are not able to create rules using regular
+expressions or wildcards (namely Outlook).
+
 The headers Phabricator adds to mail are:
 
   - ##X-Phabricator-Sent-This-Message##: this is attached to all mail
     Phabricator sends. You can use it to differentiate between email from
     Phabricator and replies/forwards of Phabricator mail from human beings.
   - ##X-Phabricator-To##: this is attached to all mail Phabricator sends.
-    It this shows the PHIDs of the original "To" line, before any mutation
+    It shows the PHIDs of the original "To" line, before any mutation
     by the mailer configuration.
   - ##X-Phabricator-Cc##: this is attached to all mail Phabricator sends.
     It shows the PHIDs of the original "Cc" line, before any mutation by the
@@ -57,15 +61,18 @@ The headers Phabricator adds to mail are:
   - ##X-Differential-Author##: this is attached to Differential mail and shows
     the revision's author. You can use it to filter mail about your revisions
     (or other users' revisions).
-  - ##X-Differential-Reviewers##: this is attached to Differential mail and
+  - ##X-Differential-Reviewer##: this is attached to Differential mail and
     shows the reviewers. You can use it to filter mail about revisions you
     are reviewing, versus revisions you are explicitly CC'd on or CC'd as
     a result of Herald rules.
-  - ##X-Differential-CCs##: this is attached to Differential mail and shows
+  - ##X-Differential-Reviewers##: list version of the previous.
+  - ##X-Differential-CC##: this is attached to Differential mail and shows
     the CCs on the revision.
-  - ##X-Differential-Explicit-CCs##: this is attached to Differential mail and
+  - ##X-Differential-CCs##: list version of the previous.
+  - ##X-Differential-Explicit-CC##: this is attached to Differential mail and
     shows the explicit CCs on the revision (those that were added by humans,
     not by Herald).
+  - ##X-Differential-Explicit-CCs##: list version of the previous.
   - ##X-Phabricator-Mail-Tags##: this is attached to some mail and has
     a list of descriptors about the mail. (This is fairly new and subject
     to some change.)