diff --git a/src/applications/differential/controller/subscribe/DifferentialSubscribeController.php b/src/applications/differential/controller/subscribe/DifferentialSubscribeController.php index bbd22df5ac..de72ff86fc 100644 --- a/src/applications/differential/controller/subscribe/DifferentialSubscribeController.php +++ b/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 - // herald notifications. - $prompt = 'Really unsubscribe from this revision?'; + $prompt = 'Really unsubscribe from this revision? Herald will '. + 'not resubscribe you to a revision you unsubscribe '. + 'from.'; break; default: return new Aphront400Response(); diff --git a/src/applications/differential/editor/revision/DifferentialRevisionEditor.php b/src/applications/differential/editor/revision/DifferentialRevisionEditor.php index fc8ffaf617..681a321eaa 100644 --- a/src/applications/differential/editor/revision/DifferentialRevisionEditor.php +++ b/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); -/* - - // TODO: When Herald is brought over, run through this stuff to figure - // out which adds are Herald's fault. - - // TODO: Still need to do this. + // 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 + // them, or uses "Add CCs...") or a Herald transcript PHID, indicating that + // they were added by a Herald rule. if ($add['ccs'] || $rem['ccs']) { - foreach (array_keys($add['ccs']) as $id) { - if (empty($new['ccs'][$id])) { - $reason_phid = 'TODO';//$xscript_phid; + $reasons = array(); + foreach ($add['ccs'] as $phid => $ignored) { + 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) { - if (empty($new['ccs'][$id])) { - $reason_phid = $this->getActorPHID(); + foreach ($rem['ccs'] as $phid => $ignored) { + if (empty($new['ccs'][$phid])) { + $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, ); } diff --git a/src/applications/differential/mail/base/DifferentialMail.php b/src/applications/differential/mail/base/DifferentialMail.php index 67b1b963a7..9358446b3a 100644 --- a/src/applications/differential/mail/base/DifferentialMail.php +++ b/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).'>'); + } } } diff --git a/src/applications/differential/mail/base/__init__.php b/src/applications/differential/mail/base/__init__.php index 7c9230ba90..b1b70e951e 100644 --- a/src/applications/differential/mail/base/__init__.php +++ b/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'); diff --git a/src/applications/differential/view/revisioncomment/DifferentialRevisionCommentView.php b/src/applications/differential/view/revisioncomment/DifferentialRevisionCommentView.php index a08df37894..fdec2a3b1d 100644 --- a/src/applications/differential/view/revisioncomment/DifferentialRevisionCommentView.php +++ b/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); diff --git a/src/docs/userguide/mail_rules.diviner b/src/docs/userguide/mail_rules.diviner index 07bb07bc51..27079b93d7 100644 --- a/src/docs/userguide/mail_rules.diviner +++ b/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.)