From 75a04551524f87c9b1b8dc09343a5ee3ce3e98c7 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Wed, 29 Aug 2018 14:11:35 -0700
Subject: [PATCH] Add "Revision test plan" as a Herald field; remove test plan
 from the "Revision summary" field

Summary:
See PHI844. Ref T13189.

Add "Revision test plan" as an available field for Herald. This is a little niche -- and a little odd because it sticks around even if you fully disable test plans -- but probably broadly reasonable.

The existing "Revision summary" field counterintuitively included the test plan. Separate this out since it's now a separate field and the behavior was weird historic nonsense. I'll note this in the changelog.

Test Plan: Wrote a rule using both fields, verified they generated the expected values.

Reviewers: amckinley

Maniphest Tasks: T13189

Differential Revision: https://secure.phabricator.com/D19623
---
 src/__phutil_library_map__.php                |  2 ++
 ...DifferentialRevisionSummaryHeraldField.php |  5 +----
 ...ifferentialRevisionTestPlanHeraldField.php | 20 +++++++++++++++++++
 3 files changed, 23 insertions(+), 4 deletions(-)
 create mode 100644 src/applications/differential/herald/DifferentialRevisionTestPlanHeraldField.php

diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
index a86027bf13..3abc10ef49 100644
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -644,6 +644,7 @@ phutil_register_library_map(array(
     'DifferentialRevisionStatusTransaction' => 'applications/differential/xaction/DifferentialRevisionStatusTransaction.php',
     'DifferentialRevisionSummaryHeraldField' => 'applications/differential/herald/DifferentialRevisionSummaryHeraldField.php',
     'DifferentialRevisionSummaryTransaction' => 'applications/differential/xaction/DifferentialRevisionSummaryTransaction.php',
+    'DifferentialRevisionTestPlanHeraldField' => 'applications/differential/herald/DifferentialRevisionTestPlanHeraldField.php',
     'DifferentialRevisionTestPlanTransaction' => 'applications/differential/xaction/DifferentialRevisionTestPlanTransaction.php',
     'DifferentialRevisionTitleHeraldField' => 'applications/differential/herald/DifferentialRevisionTitleHeraldField.php',
     'DifferentialRevisionTitleTransaction' => 'applications/differential/xaction/DifferentialRevisionTitleTransaction.php',
@@ -6004,6 +6005,7 @@ phutil_register_library_map(array(
     'DifferentialRevisionStatusTransaction' => 'DifferentialRevisionTransactionType',
     'DifferentialRevisionSummaryHeraldField' => 'DifferentialRevisionHeraldField',
     'DifferentialRevisionSummaryTransaction' => 'DifferentialRevisionTransactionType',
+    'DifferentialRevisionTestPlanHeraldField' => 'DifferentialRevisionHeraldField',
     'DifferentialRevisionTestPlanTransaction' => 'DifferentialRevisionTransactionType',
     'DifferentialRevisionTitleHeraldField' => 'DifferentialRevisionHeraldField',
     'DifferentialRevisionTitleTransaction' => 'DifferentialRevisionTransactionType',
diff --git a/src/applications/differential/herald/DifferentialRevisionSummaryHeraldField.php b/src/applications/differential/herald/DifferentialRevisionSummaryHeraldField.php
index eafc9bc990..587468d549 100644
--- a/src/applications/differential/herald/DifferentialRevisionSummaryHeraldField.php
+++ b/src/applications/differential/herald/DifferentialRevisionSummaryHeraldField.php
@@ -10,10 +10,7 @@ final class DifferentialRevisionSummaryHeraldField
   }
 
   public function getHeraldFieldValue($object) {
-    // NOTE: For historical reasons, this field includes the test plan. We
-    // could maybe try to fix this some day, but it probably aligns reasonably
-    // well with user expectation without harming anything.
-    return $object->getSummary()."\n\n".$object->getTestPlan();
+    return $object->getSummary();
   }
 
   protected function getHeraldFieldStandardType() {
diff --git a/src/applications/differential/herald/DifferentialRevisionTestPlanHeraldField.php b/src/applications/differential/herald/DifferentialRevisionTestPlanHeraldField.php
new file mode 100644
index 0000000000..bb2e6d0910
--- /dev/null
+++ b/src/applications/differential/herald/DifferentialRevisionTestPlanHeraldField.php
@@ -0,0 +1,20 @@
+<?php
+
+final class DifferentialRevisionTestPlanHeraldField
+  extends DifferentialRevisionHeraldField {
+
+  const FIELDCONST = 'differential.revision.test-plan';
+
+  public function getHeraldFieldName() {
+    return pht('Revision test plan');
+  }
+
+  public function getHeraldFieldValue($object) {
+    return $object->getTestPlan();
+  }
+
+  protected function getHeraldFieldStandardType() {
+    return self::STANDARD_TEXT;
+  }
+
+}