Add an "Explicit-CCs" header to Differential

Summary: This header allows recipients to distinguish between CCs generated by Herald and CCs generated by humans.

Test Plan: Created a Herald rule to add a bunch of CC's to every revision. Created a revision. Added some CCs manually. Verified that only manual CCs appeared in the "Explicit" header.

Reviewers: btrahan, vrana

Reviewed By: btrahan

CC: aran, epriestley

Maniphest Tasks: T808

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

src/applications/differential/controller/subscribe/DifferentialSubscribeController.php

@@ -48,9 +48,9 @@ final class DifferentialSubscribeController extends DifferentialController {
         case 'rem':
           $button = 'Unsubscribe';
           $title = 'Unsubscribe from Revision';
-          // TODO: Once herald is in, add a notice about not getting any more
+          $prompt = 'Really unsubscribe from this revision? Herald will '.
-          // herald notifications.
+                    'not resubscribe you to a revision you unsubscribe '.
-          $prompt = 'Really unsubscribe from this revision?';
+                    'from.';
           break;
         default:
           return new Aphront400Response();

src/applications/differential/editor/revision/DifferentialRevisionEditor.php

@@ -234,6 +234,7 @@ final class DifferentialRevisionEditor {
     );
 
     $rem_ccs = array();
+    $xscript_phid = null;
     if ($diff) {
       $adapter = new HeraldDifferentialRevisionAdapter(
         $revision,
@@ -294,36 +295,37 @@ final class DifferentialRevisionEditor {
       array_keys($add['rev']),
       $this->actorPHID);
 
-/*
+    // We want to attribute new CCs to a "reasonPHID", representing the reason
-
+    // they were added. This is either a user (if some user explicitly CCs
-    // TODO: When Herald is brought over, run through this stuff to figure
+    // them, or uses "Add CCs...") or a Herald transcript PHID, indicating that
-    // out which adds are Herald's fault.
+    // they were added by a Herald rule.
-
-    // TODO: Still need to do this.
 
     if ($add['ccs'] || $rem['ccs']) {
-      foreach (array_keys($add['ccs']) as $id) {
+      $reasons = array();
-        if (empty($new['ccs'][$id])) {
+      foreach ($add['ccs'] as $phid => $ignored) {
-          $reason_phid = 'TODO';//$xscript_phid;
+        if (empty($new['ccs'][$phid])) {
+          $reasons[$phid] = $xscript_phid;
         } else {
-          $reason_phid = $this->getActorPHID();
+          $reasons[$phid] = $this->actorPHID;
         }
       }
-      foreach (array_keys($rem['ccs']) as $id) {
+      foreach ($rem['ccs'] as $phid => $ignored) {
-        if (empty($new['ccs'][$id])) {
+        if (empty($new['ccs'][$phid])) {
-          $reason_phid = $this->getActorPHID();
+          $reasons[$phid] = $this->actorPHID;
         } else {
-          $reason_phid = 'TODO';//$xscript_phid;
+          $reasons[$phid] = $xscript_phid;
         }
       }
+    } else {
+      $reasons = $this->actorPHID;
     }
-*/
+
     self::alterCCs(
       $revision,
       $this->cc,
       array_keys($rem['ccs']),
       array_keys($add['ccs']),
-      $this->actorPHID);
+      $reasons);
 
     $this->updateAuxiliaryFields();
 
@@ -614,10 +616,14 @@ final class DifferentialRevisionEditor {
     }
 
     foreach ($add_phids as $add) {
+      $reason = is_array($reason_phid)
+        ? idx($reason_phid, $add)
+        : $reason_phid;
+
       $raw[$add] = array(
         'objectPHID'  => $add,
         'sequence'    => idx($seq_map, $add, $sequence++),
-        'reasonPHID'  => $reason_phid,
+        'reasonPHID'  => $reason,
       );
     }
 

src/applications/differential/mail/base/DifferentialMail.php

@@ -116,6 +116,33 @@ abstract class DifferentialMail {
         $template->addHeader(
           'X-Differential-CCs',
           '<'.implode('>, <', $revision->getCCPHIDs()).'>');
+
+        // Determine explicit CCs (those added by humans) and put them in a
+        // header so users can differentiate between Herald CCs and human CCs.
+
+        $relation_subscribed = DifferentialRevision::RELATION_SUBSCRIBED;
+        $raw = $revision->getRawRelations($relation_subscribed);
+
+        $reason_phids = ipull($raw, 'reasonPHID');
+        $reason_handles = id(new PhabricatorObjectHandleData($reason_phids))
+          ->loadHandles();
+
+        $explicit_cc = array();
+        foreach ($raw as $relation) {
+          if (!$relation['reasonPHID']) {
+            continue;
+          }
+          $type = $reason_handles[$relation['reasonPHID']]->getType();
+          if ($type == PhabricatorPHIDConstants::PHID_TYPE_USER) {
+            $explicit_cc[] = $relation['objectPHID'];
+          }
+        }
+
+        if ($explicit_cc) {
+          $template->addHeader(
+            'X-Differential-Explicit-CCs',
+            '<'.implode('>, <', $explicit_cc).'>');
+        }
       }
     }
 

src/applications/differential/mail/base/__init__.php

@@ -6,7 +6,9 @@
 
 
 
+phutil_require_module('phabricator', 'applications/differential/storage/revision');
 phutil_require_module('phabricator', 'applications/metamta/storage/mail');
+phutil_require_module('phabricator', 'applications/phid/constants');
 phutil_require_module('phabricator', 'applications/phid/handle/data');
 phutil_require_module('phabricator', 'infrastructure/env');
 phutil_require_module('phabricator', 'infrastructure/events/constant/type');

src/applications/differential/view/revisioncomment/DifferentialRevisionCommentView.php

@@ -131,10 +131,12 @@ final class DifferentialRevisionCommentView extends AphrontView {
     $metadata = $comment->getMetadata();
     $added_reviewers = idx(
       $metadata,
-      DifferentialComment::METADATA_ADDED_REVIEWERS);
+      DifferentialComment::METADATA_ADDED_REVIEWERS,
+      array());
     $added_ccs = idx(
       $metadata,
-      DifferentialComment::METADATA_ADDED_CCS);
+      DifferentialComment::METADATA_ADDED_CCS,
+      array());
 
     $verb = DifferentialAction::getActionPastTenseVerb($comment->getAction());
     $verb = phutil_escape_html($verb);

src/docs/userguide/mail_rules.diviner

@@ -62,7 +62,10 @@ The headers Phabricator adds to mail are:
     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
-    the explicit CCs on the revision.
+    the CCs on the revision.
+  - ##X-Differential-Explicit-CCs##: this is attached to Differential mail and
+    shows the explicit CCs on the revision (those that were added by humans,
+    not by Herald).
   - ##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.)