From fd66ab55791ea288632d0ef8cc3a5bfc22b29e1f Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Fri, 26 Jan 2018 06:47:15 -0800
Subject: [PATCH] Always use the same set of transactions to generate mail and
 mail tags

Summary:
See PHI307. Currently, when reviews undraft, we retroactively add in older activity to the mail ("alice created this revision...").

However, we don't add that activity to the mail tags, so the relevant tags (like "revision created") are dropped forever.

Instead, use the same set of transactions for both mail body and mail tag construction.

This should be obsoleted in the relatively near future by T10448, but it's a better/more correct behavior in general and we probably can't get rid of tags completely for a while.

Test Plan:
Applied patch, created a revision with builds, saw it auto-undraft after builds finished. Used `bin/mail list-outbound` and `bin/mail show-outbound` to see the mail. Verified that it included retroactive text ("created this revision") AND retroactive tags.

Note that the tag for "A new revision is created" is `DifferentialTransaction::MAILTAG_REVIEW_REQUEST` with literal value `differential-review-request`.

Reviewers: amckinley

Reviewed By: amckinley

Differential Revision: https://secure.phabricator.com/D18941
---
 .../editor/DifferentialTransactionEditor.php   | 16 +++++++++++-----
 ...PhabricatorApplicationTransactionEditor.php | 18 +++++++++++++-----
 2 files changed, 24 insertions(+), 10 deletions(-)

diff --git a/src/applications/differential/editor/DifferentialTransactionEditor.php b/src/applications/differential/editor/DifferentialTransactionEditor.php
index b5eaac4b5e..ecd1e6c95c 100644
--- a/src/applications/differential/editor/DifferentialTransactionEditor.php
+++ b/src/applications/differential/editor/DifferentialTransactionEditor.php
@@ -700,20 +700,26 @@ final class DifferentialTransactionEditor
       ->addHeader('Thread-Topic', $thread_topic);
   }
 
-  protected function buildMailBody(
+  protected function getTransactionsForMail(
     PhabricatorLiskDAO $object,
     array $xactions) {
-
-    $viewer = $this->requireActor();
-
     // If this is the first time we're sending mail about this revision, we
     // generate mail for all prior transactions, not just whatever is being
     // applied now. This gets the "added reviewers" lines and other relevant
     // information into the mail.
     if ($this->isFirstBroadcast()) {
-      $xactions = $this->loadUnbroadcastTransactions($object);
+      return $this->loadUnbroadcastTransactions($object);
     }
 
+    return $xactions;
+  }
+
+  protected function buildMailBody(
+    PhabricatorLiskDAO $object,
+    array $xactions) {
+
+    $viewer = $this->requireActor();
+
     $body = new PhabricatorMetaMTAMailBody();
     $body->setViewer($this->requireActor());
 
diff --git a/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php b/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php
index 829e3ab43d..7dbd41f1e4 100644
--- a/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php
+++ b/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php
@@ -2546,6 +2546,12 @@ abstract class PhabricatorApplicationTransactionEditor
     return $messages;
   }
 
+  protected function getTransactionsForMail(
+    PhabricatorLiskDAO $object,
+    array $xactions) {
+    return $xactions;
+  }
+
   private function buildMailForTarget(
     PhabricatorLiskDAO $object,
     array $xactions,
@@ -2566,17 +2572,19 @@ abstract class PhabricatorApplicationTransactionEditor
       return null;
     }
 
-    $mail = $this->buildMailTemplate($object);
-    $body = $this->buildMailBody($object, $xactions);
+    $mail_xactions = $this->getTransactionsForMail($object, $xactions);
 
-    $mail_tags = $this->getMailTags($object, $xactions);
-    $action = $this->getMailAction($object, $xactions);
+    $mail = $this->buildMailTemplate($object);
+    $body = $this->buildMailBody($object, $mail_xactions);
+
+    $mail_tags = $this->getMailTags($object, $mail_xactions);
+    $action = $this->getMailAction($object, $mail_xactions);
 
     if (PhabricatorEnv::getEnvConfig('metamta.email-preferences')) {
       $this->addEmailPreferenceSectionToMailBody(
         $body,
         $object,
-        $xactions);
+        $mail_xactions);
     }
 
     $mail