Restore "Branch" and "changes since last update" fields to Differential mail

Summary: Ref T418. Fixes T4642. The "changes since last update" and "branch" fields got dropped; restore them in a general, field-driven way.

Test Plan:
  - Created a revision, got relevant sections in mail.
  - Commented on a revision, got relevant sections in mail.
  - Updated a revision, got relevant sections in mail.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: spicyj, epriestley

Maniphest Tasks: T418, T4642

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

src/__phutil_library_map__.php

@@ -327,6 +327,7 @@ phutil_register_library_map(array(
     'DifferentialBranchField' => 'applications/differential/customfield/DifferentialBranchField.php',
     'DifferentialCapabilityDefaultView' => 'applications/differential/capability/DifferentialCapabilityDefaultView.php',
     'DifferentialChangeType' => 'applications/differential/constants/DifferentialChangeType.php',
+    'DifferentialChangesSinceLastUpdateField' => 'applications/differential/customfield/DifferentialChangesSinceLastUpdateField.php',
     'DifferentialChangeset' => 'applications/differential/storage/DifferentialChangeset.php',
     'DifferentialChangesetDetailView' => 'applications/differential/view/DifferentialChangesetDetailView.php',
     'DifferentialChangesetFileTreeSideNavBuilder' => 'applications/differential/view/DifferentialChangesetFileTreeSideNavBuilder.php',
@@ -2893,6 +2894,7 @@ phutil_register_library_map(array(
     'DifferentialBlameRevisionField' => 'DifferentialStoredCustomField',
     'DifferentialBranchField' => 'DifferentialCustomField',
     'DifferentialCapabilityDefaultView' => 'PhabricatorPolicyCapability',
+    'DifferentialChangesSinceLastUpdateField' => 'DifferentialCustomField',
     'DifferentialChangeset' => 'DifferentialDAO',
     'DifferentialChangesetDetailView' => 'AphrontView',
     'DifferentialChangesetHTMLRenderer' => 'DifferentialChangesetRenderer',

src/applications/differential/config/PhabricatorDifferentialConfigOptions.php

@@ -37,6 +37,9 @@ final class PhabricatorDifferentialConfigOptions
       new DifferentialJIRAIssuesField(),
       new DifferentialAsanaRepresentationField(),
 
+      new DifferentialChangesSinceLastUpdateField(),
+      new DifferentialBranchField(),
+
       new DifferentialBlameRevisionField(),
       new DifferentialPathField(),
       new DifferentialHostField(),

src/applications/differential/customfield/DifferentialBranchField.php

@@ -50,5 +50,30 @@ final class DifferentialBranchField
     );
   }
 
+  public function shouldAppearInTransactionMail() {
+    return true;
+  }
+
+  public function updateTransactionMailBody(
+    PhabricatorMetaMTAMailBody $body,
+    PhabricatorApplicationTransactionEditor $editor,
+    array $xactions) {
+
+    $status_accepted = ArcanistDifferentialRevisionStatus::ACCEPTED;
+
+    // Show the "BRANCH" section only if there's a new diff or the revision
+    // is "Accepted".
+    if ((!$editor->getDiffUpdateTransaction($xactions)) &&
+        ($this->getObject()->getStatus() != $status_accepted)) {
+      return;
+    }
+
+    $branch = $this->getBranchDescription($this->getObject()->getActiveDiff());
+    if ($branch === null) {
+      return;
+    }
+
+    $body->addTextSection(pht('BRANCH'), $branch);
+  }
 
 }

src/applications/differential/customfield/DifferentialChangesSinceLastUpdateField.php

@@ -0,0 +1,60 @@
+<?php
+
+final class DifferentialChangesSinceLastUpdateField
+  extends DifferentialCustomField {
+
+  public function getFieldKey() {
+    return 'differential:changes-since-last-update';
+  }
+
+  public function getFieldName() {
+    return pht('Changes Since Last Update');
+  }
+
+  public function getFieldDescription() {
+    return pht('Links to changes since the last update in email.');
+  }
+
+  public function shouldAppearInTransactionMail() {
+    return true;
+  }
+
+  public function updateTransactionMailBody(
+    PhabricatorMetaMTAMailBody $body,
+    PhabricatorApplicationTransactionEditor $editor,
+    array $xactions) {
+
+    if ($editor->getIsNewObject()) {
+      return;
+    }
+
+    if ($editor->getIsCloseByCommit()) {
+      return;
+    }
+
+    $xaction = $editor->getDiffUpdateTransaction($xactions);
+    if (!$xaction) {
+      return;
+    }
+
+    $original = id(new DifferentialDiffQuery())
+      ->setViewer($this->getViewer())
+      ->withPHIDs(array($xaction->getOldValue()))
+      ->executeOne();
+    if (!$original) {
+      return;
+    }
+
+    $revision = $this->getObject();
+    $current = $revision->getActiveDiff();
+
+    $old_id = $original->getID();
+    $new_id = $current->getID();
+
+    $uri = '/'.$revision->getMonogram().'?vs='.$old_id.'&id='.$new_id;
+    $uri = PhabricatorEnv::getProductionURI($uri);
+
+    $body->addTextSection(pht('CHANGES SINCE LAST UPDATE'), $uri);
+  }
+
+}

src/applications/differential/customfield/DifferentialSummaryField.php

@@ -147,4 +147,25 @@ final class DifferentialSummaryField
     return true;
   }
 
+  public function shouldAppearInTransactionMail() {
+    return true;
+  }
+
+  public function updateTransactionMailBody(
+    PhabricatorMetaMTAMailBody $body,
+    PhabricatorApplicationTransactionEditor $editor,
+    array $xactions) {
+
+    if (!$editor->getIsNewObject()) {
+      return;
+    }
+
+    $summary = $this->getValue();
+    if (!strlen(trim($summary))) {
+      return;
+    }
+
+    $body->addTextSection(pht('REVISION SUMMARY'), $summary);
+  }
+
 }

src/applications/differential/customfield/DifferentialTestPlanField.php

@@ -177,5 +177,26 @@ final class DifferentialTestPlanField
     }
   }
 
+  public function shouldAppearInTransactionMail() {
+    return true;
+  }
+
+  public function updateTransactionMailBody(
+    PhabricatorMetaMTAMailBody $body,
+    PhabricatorApplicationTransactionEditor $editor,
+    array $xactions) {
+
+    if (!$editor->getIsNewObject()) {
+      return;
+    }
+
+    $test_plan = $this->getValue();
+    if (!strlen(trim($test_plan))) {
+      return;
+    }
+
+    $body->addTextSection(pht('TEST PLAN'), $test_plan);
+  }
+
 
 }

src/applications/differential/editor/DifferentialTransactionEditor.php

@@ -7,6 +7,18 @@ final class DifferentialTransactionEditor
   private $changedPriorToCommitURI;
   private $isCloseByCommit;
 
+  public function getDiffUpdateTransaction(array $xactions) {
+    $type_update = DifferentialTransaction::TYPE_UPDATE;
+
+    foreach ($xactions as $xaction) {
+      if ($xaction->getTransactionType() == $type_update) {
+        return $xaction;
+      }
+    }
+
+    return null;
+  }
+
   public function setIsCloseByCommit($is_close_by_commit) {
     $this->isCloseByCommit = $is_close_by_commit;
     return $this;
@@ -189,6 +201,7 @@ final class DifferentialTransactionEditor
         $object->setLineCount($diff->getLineCount());
         $object->setRepositoryPHID($diff->getRepositoryPHID());
         $object->setArcanistProjectPHID($diff->getArcanistProjectPHID());
+        $object->attachActiveDiff($diff);
 
         // TODO: Update the `diffPHID` once we add that.
         return;
@@ -1096,22 +1109,6 @@ final class DifferentialTransactionEditor
 
     $body = parent::buildMailBody($object, $xactions);
 
-    if ($this->getIsNewObject()) {
-      $summary = $object->getSummary();
-      if (strlen(trim($summary))) {
-        $body->addTextSection(
-          pht('REVISION SUMMARY'),
-          $summary);
-      }
-
-      $test_plan = $object->getTestPlan();
-      if (strlen(trim($test_plan))) {
-        $body->addTextSection(
-          pht('TEST PLAN'),
-          $test_plan);
-      }
-    }
-
     $type_inline = DifferentialTransaction::TYPE_INLINE;
 
     $inlines = array();

src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php

@@ -103,7 +103,7 @@ abstract class PhabricatorApplicationTransactionEditor
     return $this->parentMessageID;
   }
 
-  protected function getIsNewObject() {
+  public function getIsNewObject() {
     return $this->isNewObject;
   }
 
@@ -1888,6 +1888,21 @@ abstract class PhabricatorApplicationTransactionEditor
       $body->addRawSection($comment);
     }
 
+    if ($object instanceof PhabricatorCustomFieldInterface) {
+      $field_list = PhabricatorCustomField::getObjectFields(
+        $object,
+        PhabricatorCustomField::ROLE_TRANSACTIONMAIL);
+      $field_list->setViewer($this->getActor());
+      $field_list->readFieldsFromStorage($object);
+
+      foreach ($field_list->getFields() as $field) {
+        $field->updateTransactionMailBody(
+          $body,
+          $this,
+          $xactions);
+      }
+    }
+
     return $body;
   }
 

src/infrastructure/customfield/field/PhabricatorCustomField.php

@@ -11,6 +11,7 @@
  * @task list         Integration with List views
  * @task appsearch    Integration with ApplicationSearch
  * @task appxaction   Integration with ApplicationTransactions
+ * @task xactionmail  Integration with Transaction Mail
  * @task globalsearch Integration with Global Search
  */
 abstract class PhabricatorCustomField {
@@ -20,6 +21,7 @@ abstract class PhabricatorCustomField {
   private $proxy;
 
   const ROLE_APPLICATIONTRANSACTIONS  = 'ApplicationTransactions';
+  const ROLE_TRANSACTIONMAIL          = 'ApplicationTransactions.mail';
   const ROLE_APPLICATIONSEARCH        = 'ApplicationSearch';
   const ROLE_STORAGE                  = 'storage';
   const ROLE_DEFAULT                  = 'default';
@@ -264,6 +266,8 @@ abstract class PhabricatorCustomField {
         return $this->shouldAppearInGlobalSearch();
       case self::ROLE_CONDUIT:
         return $this->shouldAppearInConduitDictionary();
+      case self::ROLE_TRANSACTIONMAIL:
+        return $this->shouldAppearInTransactionMail();
       case self::ROLE_DEFAULT:
         return true;
       default:
@@ -1023,6 +1027,33 @@ abstract class PhabricatorCustomField {
   }
 
 
+/* -(  Transaction Mail  )--------------------------------------------------- */
+
+
+  /**
+   * @task xactionmail
+   */
+  public function shouldAppearInTransactionMail() {
+    if ($this->proxy) {
+      return $this->proxy->shouldAppearInTransactionMail();
+    }
+    return false;
+  }
+
+
+  /**
+   * @task xactionmail
+   */
+  public function updateTransactionMailBody(
+    PhabricatorMetaMTAMailBody $body,
+    PhabricatorApplicationTransactionEditor $editor,
+    array $xactions) {
+    if ($this->proxy) {
+      return $this->proxy->updateTransactionMailBody($body, $editor, $xactions);
+    }
+    return;
+  }
+
 
 /* -(  Edit View  )---------------------------------------------------------- */