From c458f09dccb34287a517fb4c5799817863594e20 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Fri, 16 Dec 2016 09:38:58 -0800
Subject: [PATCH] Simplify "Test Plan" custom field

Summary: Ref T11114. This leaves mail integration and UI integration, but strips all the editing (now handled by EditEngine) and commit message stuff (now handled by CommitMessageField).

Test Plan: Viewed and edited test plans and test plan transactions.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T11114

Differential Revision: https://secure.phabricator.com/D17084
---
 .../customfield/DifferentialTestPlanField.php | 110 ------------------
 .../storage/DifferentialTransaction.php       |   2 +
 2 files changed, 2 insertions(+), 110 deletions(-)

diff --git a/src/applications/differential/customfield/DifferentialTestPlanField.php b/src/applications/differential/customfield/DifferentialTestPlanField.php
index 6a1029dba4..c774e81a08 100644
--- a/src/applications/differential/customfield/DifferentialTestPlanField.php
+++ b/src/applications/differential/customfield/DifferentialTestPlanField.php
@@ -7,10 +7,6 @@ final class DifferentialTestPlanField
     return 'differential:test-plan';
   }
 
-  public function getFieldKeyForConduit() {
-    return 'testPlan';
-  }
-
   public function getFieldName() {
     return pht('Test Plan');
   }
@@ -27,83 +23,10 @@ final class DifferentialTestPlanField
     return $revision->getTestPlan();
   }
 
-  protected function writeValueToRevision(
-    DifferentialRevision $revision,
-    $value) {
-    $revision->setTestPlan($value);
-  }
-
-  protected function isCoreFieldRequired() {
-    return PhabricatorEnv::getEnvConfig('differential.require-test-plan-field');
-  }
-
   public function canDisableField() {
     return true;
   }
 
-  protected function getCoreFieldRequiredErrorString() {
-    return pht(
-      'You must provide a test plan. Describe the actions you performed '.
-      'to verify the behavior of this change.');
-  }
-
-  public function readValueFromRequest(AphrontRequest $request) {
-    $this->setValue($request->getStr($this->getFieldKey()));
-  }
-
-  public function renderEditControl(array $handles) {
-    return id(new PhabricatorRemarkupControl())
-      ->setUser($this->getViewer())
-      ->setName($this->getFieldKey())
-      ->setValue($this->getValue())
-      ->setError($this->getFieldError())
-      ->setLabel($this->getFieldName());
-  }
-
-  public function getApplicationTransactionTitle(
-    PhabricatorApplicationTransaction $xaction) {
-    $author_phid = $xaction->getAuthorPHID();
-    $old = $xaction->getOldValue();
-    $new = $xaction->getNewValue();
-
-    return pht(
-      '%s updated the test plan for this revision.',
-      $xaction->renderHandleLink($author_phid));
-  }
-
-  public function getApplicationTransactionTitleForFeed(
-    PhabricatorApplicationTransaction $xaction) {
-
-    $object_phid = $xaction->getObjectPHID();
-    $author_phid = $xaction->getAuthorPHID();
-    $old = $xaction->getOldValue();
-    $new = $xaction->getNewValue();
-
-    return pht(
-      '%s updated the test plan for %s.',
-      $xaction->renderHandleLink($author_phid),
-      $xaction->renderHandleLink($object_phid));
-  }
-
-  public function getApplicationTransactionHasChangeDetails(
-    PhabricatorApplicationTransaction $xaction) {
-    return true;
-  }
-
-  public function getApplicationTransactionChangeDetails(
-    PhabricatorApplicationTransaction $xaction,
-    PhabricatorUser $viewer) {
-    return $xaction->renderTextCorpusChangeDetails(
-      $viewer,
-      $xaction->getOldValue(),
-      $xaction->getNewValue());
-  }
-
-  public function shouldHideInApplicationTransactions(
-    PhabricatorApplicationTransaction $xaction) {
-    return ($xaction->getOldValue() === null);
-  }
-
   public function shouldAppearInGlobalSearch() {
     return true;
   }
@@ -139,39 +62,6 @@ final class DifferentialTestPlanField
     return new PHUIRemarkupView($this->getViewer(), $this->getValue());
   }
 
-  public function getApplicationTransactionRemarkupBlocks(
-    PhabricatorApplicationTransaction $xaction) {
-    return array($xaction->getNewValue());
-  }
-
-  public function shouldAppearInCommitMessage() {
-    return true;
-  }
-
-  public function shouldAppearInCommitMessageTemplate() {
-    return true;
-  }
-
-  public function shouldOverwriteWhenCommitMessageIsEdited() {
-    return true;
-  }
-
-  public function getCommitMessageLabels() {
-    return array(
-      'Test Plan',
-      'Testplan',
-      'Tested',
-      'Tests',
-    );
-  }
-
-  public function validateCommitMessageValue($value) {
-    if (!strlen($value) && $this->isCoreFieldRequired()) {
-      throw new DifferentialFieldValidationException(
-        $this->getCoreFieldRequiredErrorString());
-    }
-  }
-
   public function shouldAppearInTransactionMail() {
     return true;
   }
diff --git a/src/applications/differential/storage/DifferentialTransaction.php b/src/applications/differential/storage/DifferentialTransaction.php
index 0ea2731354..fdf4121e1f 100644
--- a/src/applications/differential/storage/DifferentialTransaction.php
+++ b/src/applications/differential/storage/DifferentialTransaction.php
@@ -32,6 +32,8 @@ final class DifferentialTransaction
       switch ($this->getMetadataValue('customfield:key')) {
         case 'differential:title':
           return new DifferentialRevisionTitleTransaction();
+        case 'differential:test-plan':
+          return new DifferentialRevisionTestPlanTransaction();
       }
     }