From a4b934cad2b520fb2de57a5f08eeffa6aff28ffe Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Tue, 31 Oct 2017 11:45:38 -0700
Subject: [PATCH] Clean up Differential draft mail behaviors

Summary:
Ref T2543. Fixes two relatively minor things:

  - When builds finish in Harbormaster, send mail "From" the author.
  - Set the `firstBroadcast` flag so that initial mail picks up earlier history (notably, the "reviewers" line).

For now, I'm not setting `firstBroadcast` on explicit "Request Review" (but maybe we should), and not trying to deal with weird cases where you leave a bunch of comments on a draft. Those might be fine as-is or may get tweaked later.

Test Plan: Created a revision with Harbormaster builds, ran builds, saw initial email come "From" the right user with more metadata.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T2543

Differential Revision: https://secure.phabricator.com/D18748
---
 .../editor/DifferentialTransactionEditor.php   | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/src/applications/differential/editor/DifferentialTransactionEditor.php b/src/applications/differential/editor/DifferentialTransactionEditor.php
index 70c3272a83..79a582a43f 100644
--- a/src/applications/differential/editor/DifferentialTransactionEditor.php
+++ b/src/applications/differential/editor/DifferentialTransactionEditor.php
@@ -9,6 +9,7 @@ final class DifferentialTransactionEditor
   private $didExpandInlineState = false;
   private $hasReviewTransaction = false;
   private $affectedPaths;
+  private $firstBroadcast = false;
 
   public function getEditorApplicationClass() {
     return 'PhabricatorDifferentialApplication';
@@ -27,7 +28,7 @@ final class DifferentialTransactionEditor
   }
 
   public function isFirstBroadcast() {
-    return $this->getIsNewObject();
+    return $this->firstBroadcast;
   }
 
   public function getDiffUpdateTransaction(array $xactions) {
@@ -1449,11 +1450,13 @@ final class DifferentialTransactionEditor
   protected function getCustomWorkerState() {
     return array(
       'changedPriorToCommitURI' => $this->changedPriorToCommitURI,
+      'firstBroadcast' => $this->firstBroadcast,
     );
   }
 
   protected function loadCustomWorkerState(array $state) {
     $this->changedPriorToCommitURI = idx($state, 'changedPriorToCommitURI');
+    $this->firstBroadcast = idx($state, 'firstBroadcast');
     return $this;
   }
 
@@ -1566,6 +1569,19 @@ final class DifferentialTransactionEditor
         // natural and more useful.
         $author_phid = $object->getAuthorPHID();
 
+        // Additionally, we change the acting PHID for the transaction set
+        // to the author if it isn't already a user so that mail comes from
+        // the natural author.
+        $acting_phid = $this->getActingAsPHID();
+        $user_type = PhabricatorPeopleUserPHIDType::TYPECONST;
+        if (phid_get_type($acting_phid) != $user_type) {
+          $this->setActingAsPHID($author_phid);
+        }
+
+        // Mark this as the first broadcast we're sending about the revision
+        // so mail can generate specially.
+        $this->firstBroadcast = true;
+
         $xaction = $object->getApplicationTransactionTemplate()
           ->setAuthorPHID($author_phid)
           ->setTransactionType(