From 2c3dbc48ee9502f93e28beccd779b218f06579b6 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Fri, 30 Oct 2015 16:00:53 +0000
Subject: [PATCH] Move "Next Step" to a custom field in Differential

Summary:
Fixes T9672. This was never turned into a custom field, for no particular reason. Convert it into one.

This is substantially similar to the existing "Apply Patch" field, which does the same thing (only shows a command).

We might rethink or remove this eventually (e.g., in a post-"Land Revision" world) but this makes it easier, at the very least.

Test Plan:
  - Viewed a non-accepted revision (no hint).
  - Viewed an accepted revision from a raw diff source (no hint).
  - Viewed an accepted revision from Git (`arc land` hint).

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T9672

Differential Revision: https://secure.phabricator.com/D14367
---
 src/__phutil_library_map__.php                |  2 +
 .../PhabricatorDifferentialConfigOptions.php  |  2 +
 .../customfield/DifferentialNextStepField.php | 65 +++++++++++++++++++
 .../view/DifferentialRevisionDetailView.php   | 30 ---------
 4 files changed, 69 insertions(+), 30 deletions(-)
 create mode 100644 src/applications/differential/customfield/DifferentialNextStepField.php

diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
index df013da0bf..81b8d553a2 100644
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -416,6 +416,7 @@ phutil_register_library_map(array(
     'DifferentialLocalCommitsView' => 'applications/differential/view/DifferentialLocalCommitsView.php',
     'DifferentialManiphestTasksField' => 'applications/differential/customfield/DifferentialManiphestTasksField.php',
     'DifferentialModernHunk' => 'applications/differential/storage/DifferentialModernHunk.php',
+    'DifferentialNextStepField' => 'applications/differential/customfield/DifferentialNextStepField.php',
     'DifferentialParseCacheGarbageCollector' => 'applications/differential/garbagecollector/DifferentialParseCacheGarbageCollector.php',
     'DifferentialParseCommitMessageConduitAPIMethod' => 'applications/differential/conduit/DifferentialParseCommitMessageConduitAPIMethod.php',
     'DifferentialParseRenderTestCase' => 'applications/differential/__tests__/DifferentialParseRenderTestCase.php',
@@ -4160,6 +4161,7 @@ phutil_register_library_map(array(
     'DifferentialLocalCommitsView' => 'AphrontView',
     'DifferentialManiphestTasksField' => 'DifferentialCoreCustomField',
     'DifferentialModernHunk' => 'DifferentialHunk',
+    'DifferentialNextStepField' => 'DifferentialCustomField',
     'DifferentialParseCacheGarbageCollector' => 'PhabricatorGarbageCollector',
     'DifferentialParseCommitMessageConduitAPIMethod' => 'DifferentialConduitAPIMethod',
     'DifferentialParseRenderTestCase' => 'PhabricatorTestCase',
diff --git a/src/applications/differential/config/PhabricatorDifferentialConfigOptions.php b/src/applications/differential/config/PhabricatorDifferentialConfigOptions.php
index a2bb63a2a2..dd864d523f 100644
--- a/src/applications/differential/config/PhabricatorDifferentialConfigOptions.php
+++ b/src/applications/differential/config/PhabricatorDifferentialConfigOptions.php
@@ -25,6 +25,8 @@ final class PhabricatorDifferentialConfigOptions
     $custom_field_type = 'custom:PhabricatorCustomFieldConfigOptionType';
 
     $fields = array(
+      new DifferentialNextStepField(),
+
       new DifferentialTitleField(),
       new DifferentialSummaryField(),
       new DifferentialTestPlanField(),
diff --git a/src/applications/differential/customfield/DifferentialNextStepField.php b/src/applications/differential/customfield/DifferentialNextStepField.php
new file mode 100644
index 0000000000..bf0ed5972d
--- /dev/null
+++ b/src/applications/differential/customfield/DifferentialNextStepField.php
@@ -0,0 +1,65 @@
+<?php
+
+final class DifferentialNextStepField
+  extends DifferentialCustomField {
+
+  public function getFieldKey() {
+    return 'differential:next-step';
+  }
+
+  public function getFieldName() {
+    return pht('Next Step');
+  }
+
+  public function getFieldDescription() {
+    return pht('Provides a hint for the next step to take.');
+  }
+
+  public function shouldAppearInPropertyView() {
+    return true;
+  }
+
+  public function renderPropertyViewLabel() {
+    return $this->getFieldName();
+  }
+
+  public function renderPropertyViewValue(array $handles) {
+    $revision = $this->getObject();
+    $diff = $revision->getActiveDiff();
+
+    $status = $revision->getStatus();
+    if ($status != ArcanistDifferentialRevisionStatus::ACCEPTED) {
+      return null;
+    }
+
+    $local_vcs = $diff->getSourceControlSystem();
+    switch ($local_vcs) {
+      case PhabricatorRepositoryType::REPOSITORY_TYPE_MERCURIAL:
+        $bookmark = $diff->getBookmark();
+        if (strlen($bookmark)) {
+          $next_step = csprintf('arc land %R', $bookmark);
+        } else {
+          $next_step = csprintf('arc land');
+        }
+        break;
+      case PhabricatorRepositoryType::REPOSITORY_TYPE_GIT:
+        $branch = $diff->getBranch();
+        if (strlen($branch)) {
+          $next_step = csprintf('arc land %R', $branch);
+        } else {
+          $next_step = csprintf('arc land');
+        }
+        break;
+      case PhabricatorRepositoryType::REPOSITORY_TYPE_SVN:
+        $next_step = csprintf('arc commit');
+        break;
+      default:
+        return null;
+    }
+
+    $next_step = phutil_tag('tt', array(), (string)$next_step);
+
+    return $next_step;
+  }
+
+}
diff --git a/src/applications/differential/view/DifferentialRevisionDetailView.php b/src/applications/differential/view/DifferentialRevisionDetailView.php
index cf4222dbdb..493691d21a 100644
--- a/src/applications/differential/view/DifferentialRevisionDetailView.php
+++ b/src/applications/differential/view/DifferentialRevisionDetailView.php
@@ -73,36 +73,6 @@ final class DifferentialRevisionDetailView extends AphrontView {
       ->setUser($user)
       ->setObject($revision);
 
-    $status = $revision->getStatus();
-    $local_vcs = $this->getDiff()->getSourceControlSystem();
-
-    $next_step = null;
-    if ($status == ArcanistDifferentialRevisionStatus::ACCEPTED) {
-      switch ($local_vcs) {
-        case PhabricatorRepositoryType::REPOSITORY_TYPE_MERCURIAL:
-          $bookmark = $this->getDiff()->getBookmark();
-          $next_step = ($bookmark != ''
-            ? csprintf('arc land %s', $bookmark)
-            : 'arc land');
-          break;
-
-        case PhabricatorRepositoryType::REPOSITORY_TYPE_GIT:
-          $branch = $this->getDiff()->getBranch();
-          $next_step = ($branch != ''
-            ? csprintf('arc land %s', $branch)
-            : 'arc land');
-          break;
-
-        case PhabricatorRepositoryType::REPOSITORY_TYPE_SVN:
-          $next_step = 'arc commit';
-          break;
-      }
-    }
-    if ($next_step) {
-      $next_step = phutil_tag('tt', array(), $next_step);
-      $properties->addProperty(pht('Next Step'), $next_step);
-    }
-
     $properties->setHasKeyboardShortcuts(true);
     $properties->setActionList($actions);
     $this->setActionList($actions);