Lump Harbormaster build steps into groups

Summary:
Ref T8089. We have a lot of broken/confusing/prototype build steps that I want to hide from users when we unprototype Harbormaster.

The dialog is also just kind of unwieldy.

Organize this UI a little better and put all the sketchy junk in a "prototypes" group that you can't see unless prototypes are enabled.

This doesn't break anything (the old steps will still work fine), but should reduce user confusion.

Test Plan:
Old UI:

{F691439}

New UI (prototypes off):

{F691440}

New UI (prototypes on):

{F691441}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T8089

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

src/__phutil_library_map__.php

@@ -951,6 +951,7 @@ phutil_register_library_map(array(
     'HarbormasterBuildStepCoreCustomField' => 'applications/harbormaster/customfield/HarbormasterBuildStepCoreCustomField.php',
     'HarbormasterBuildStepCustomField' => 'applications/harbormaster/customfield/HarbormasterBuildStepCustomField.php',
     'HarbormasterBuildStepEditor' => 'applications/harbormaster/editor/HarbormasterBuildStepEditor.php',
+    'HarbormasterBuildStepGroup' => 'applications/harbormaster/stepgroup/HarbormasterBuildStepGroup.php',
     'HarbormasterBuildStepImplementation' => 'applications/harbormaster/step/HarbormasterBuildStepImplementation.php',
     'HarbormasterBuildStepImplementationTestCase' => 'applications/harbormaster/step/__tests__/HarbormasterBuildStepImplementationTestCase.php',
     'HarbormasterBuildStepPHIDType' => 'applications/harbormaster/phid/HarbormasterBuildStepPHIDType.php',
@@ -978,10 +979,12 @@ phutil_register_library_map(array(
     'HarbormasterBuildableTransactionEditor' => 'applications/harbormaster/editor/HarbormasterBuildableTransactionEditor.php',
     'HarbormasterBuildableTransactionQuery' => 'applications/harbormaster/query/HarbormasterBuildableTransactionQuery.php',
     'HarbormasterBuildableViewController' => 'applications/harbormaster/controller/HarbormasterBuildableViewController.php',
+    'HarbormasterBuiltinBuildStepGroup' => 'applications/harbormaster/stepgroup/HarbormasterBuiltinBuildStepGroup.php',
     'HarbormasterCommandBuildStepImplementation' => 'applications/harbormaster/step/HarbormasterCommandBuildStepImplementation.php',
     'HarbormasterConduitAPIMethod' => 'applications/harbormaster/conduit/HarbormasterConduitAPIMethod.php',
     'HarbormasterController' => 'applications/harbormaster/controller/HarbormasterController.php',
     'HarbormasterDAO' => 'applications/harbormaster/storage/HarbormasterDAO.php',
+    'HarbormasterExternalBuildStepGroup' => 'applications/harbormaster/stepgroup/HarbormasterExternalBuildStepGroup.php',
     'HarbormasterHTTPRequestBuildStepImplementation' => 'applications/harbormaster/step/HarbormasterHTTPRequestBuildStepImplementation.php',
     'HarbormasterLeaseHostBuildStepImplementation' => 'applications/harbormaster/step/HarbormasterLeaseHostBuildStepImplementation.php',
     'HarbormasterLintMessagesController' => 'applications/harbormaster/controller/HarbormasterLintMessagesController.php',
@@ -992,12 +995,14 @@ phutil_register_library_map(array(
     'HarbormasterManagementWorkflow' => 'applications/harbormaster/management/HarbormasterManagementWorkflow.php',
     'HarbormasterMessageType' => 'applications/harbormaster/engine/HarbormasterMessageType.php',
     'HarbormasterObject' => 'applications/harbormaster/storage/HarbormasterObject.php',
+    'HarbormasterOtherBuildStepGroup' => 'applications/harbormaster/stepgroup/HarbormasterOtherBuildStepGroup.php',
     'HarbormasterPlanController' => 'applications/harbormaster/controller/HarbormasterPlanController.php',
     'HarbormasterPlanDisableController' => 'applications/harbormaster/controller/HarbormasterPlanDisableController.php',
     'HarbormasterPlanEditController' => 'applications/harbormaster/controller/HarbormasterPlanEditController.php',
     'HarbormasterPlanListController' => 'applications/harbormaster/controller/HarbormasterPlanListController.php',
     'HarbormasterPlanRunController' => 'applications/harbormaster/controller/HarbormasterPlanRunController.php',
     'HarbormasterPlanViewController' => 'applications/harbormaster/controller/HarbormasterPlanViewController.php',
+    'HarbormasterPrototypeBuildStepGroup' => 'applications/harbormaster/stepgroup/HarbormasterPrototypeBuildStepGroup.php',
     'HarbormasterPublishFragmentBuildStepImplementation' => 'applications/harbormaster/step/HarbormasterPublishFragmentBuildStepImplementation.php',
     'HarbormasterQueryAutotargetsConduitAPIMethod' => 'applications/harbormaster/conduit/HarbormasterQueryAutotargetsConduitAPIMethod.php',
     'HarbormasterQueryBuildablesConduitAPIMethod' => 'applications/harbormaster/conduit/HarbormasterQueryBuildablesConduitAPIMethod.php',
@@ -1013,6 +1018,7 @@ phutil_register_library_map(array(
     'HarbormasterStepEditController' => 'applications/harbormaster/controller/HarbormasterStepEditController.php',
     'HarbormasterTargetEngine' => 'applications/harbormaster/engine/HarbormasterTargetEngine.php',
     'HarbormasterTargetWorker' => 'applications/harbormaster/worker/HarbormasterTargetWorker.php',
+    'HarbormasterTestBuildStepGroup' => 'applications/harbormaster/stepgroup/HarbormasterTestBuildStepGroup.php',
     'HarbormasterThrowExceptionBuildStep' => 'applications/harbormaster/step/HarbormasterThrowExceptionBuildStep.php',
     'HarbormasterUIEventListener' => 'applications/harbormaster/event/HarbormasterUIEventListener.php',
     'HarbormasterUnitMessagesController' => 'applications/harbormaster/controller/HarbormasterUnitMessagesController.php',
@@ -4660,6 +4666,7 @@ phutil_register_library_map(array(
     ),
     'HarbormasterBuildStepCustomField' => 'PhabricatorCustomField',
     'HarbormasterBuildStepEditor' => 'PhabricatorApplicationTransactionEditor',
+    'HarbormasterBuildStepGroup' => 'Phobject',
     'HarbormasterBuildStepImplementation' => 'Phobject',
     'HarbormasterBuildStepImplementationTestCase' => 'PhabricatorTestCase',
     'HarbormasterBuildStepPHIDType' => 'PhabricatorPHIDType',
@@ -4693,10 +4700,12 @@ phutil_register_library_map(array(
     'HarbormasterBuildableTransactionEditor' => 'PhabricatorApplicationTransactionEditor',
     'HarbormasterBuildableTransactionQuery' => 'PhabricatorApplicationTransactionQuery',
     'HarbormasterBuildableViewController' => 'HarbormasterController',
+    'HarbormasterBuiltinBuildStepGroup' => 'HarbormasterBuildStepGroup',
     'HarbormasterCommandBuildStepImplementation' => 'HarbormasterBuildStepImplementation',
     'HarbormasterConduitAPIMethod' => 'ConduitAPIMethod',
     'HarbormasterController' => 'PhabricatorController',
     'HarbormasterDAO' => 'PhabricatorLiskDAO',
+    'HarbormasterExternalBuildStepGroup' => 'HarbormasterBuildStepGroup',
     'HarbormasterHTTPRequestBuildStepImplementation' => 'HarbormasterBuildStepImplementation',
     'HarbormasterLeaseHostBuildStepImplementation' => 'HarbormasterBuildStepImplementation',
     'HarbormasterLintMessagesController' => 'HarbormasterController',
@@ -4707,12 +4716,14 @@ phutil_register_library_map(array(
     'HarbormasterManagementWorkflow' => 'PhabricatorManagementWorkflow',
     'HarbormasterMessageType' => 'Phobject',
     'HarbormasterObject' => 'HarbormasterDAO',
+    'HarbormasterOtherBuildStepGroup' => 'HarbormasterBuildStepGroup',
     'HarbormasterPlanController' => 'HarbormasterController',
     'HarbormasterPlanDisableController' => 'HarbormasterPlanController',
     'HarbormasterPlanEditController' => 'HarbormasterPlanController',
     'HarbormasterPlanListController' => 'HarbormasterPlanController',
     'HarbormasterPlanRunController' => 'HarbormasterController',
     'HarbormasterPlanViewController' => 'HarbormasterPlanController',
+    'HarbormasterPrototypeBuildStepGroup' => 'HarbormasterBuildStepGroup',
     'HarbormasterPublishFragmentBuildStepImplementation' => 'HarbormasterBuildStepImplementation',
     'HarbormasterQueryAutotargetsConduitAPIMethod' => 'HarbormasterConduitAPIMethod',
     'HarbormasterQueryBuildablesConduitAPIMethod' => 'HarbormasterConduitAPIMethod',
@@ -4728,6 +4739,7 @@ phutil_register_library_map(array(
     'HarbormasterStepEditController' => 'HarbormasterController',
     'HarbormasterTargetEngine' => 'Phobject',
     'HarbormasterTargetWorker' => 'HarbormasterWorker',
+    'HarbormasterTestBuildStepGroup' => 'HarbormasterBuildStepGroup',
     'HarbormasterThrowExceptionBuildStep' => 'HarbormasterBuildStepImplementation',
     'HarbormasterUIEventListener' => 'PhabricatorEventListener',
     'HarbormasterUnitMessagesController' => 'HarbormasterController',

src/applications/harbormaster/controller/HarbormasterPlanViewController.php

@@ -235,7 +235,7 @@ final class HarbormasterPlanViewController extends HarbormasterPlanController {
             id(new PHUIIconView())
               ->setIconFont('fa-plus'))
           ->setDisabled(!$can_edit)
-          ->setWorkflow(true));
+          ->setWorkflow(!$can_edit));
 
     $step_box = id(new PHUIObjectBoxView())
       ->setHeader($header)

src/applications/harbormaster/controller/HarbormasterStepAddController.php

@@ -23,48 +23,83 @@ final class HarbormasterStepAddController extends HarbormasterController {
 
     $plan_id = $plan->getID();
     $cancel_uri = $this->getApplicationURI("plan/{$plan_id}/");
+    $plan_title = pht('Plan %d', $plan_id);
 
     $all = HarbormasterBuildStepImplementation::getImplementations();
-    foreach ($all as $key => $impl) {
+    $all = msort($all, 'getName');
+
+    $all_groups = HarbormasterBuildStepGroup::getAllGroups();
+    foreach ($all as $impl) {
+      $group_key = $impl->getBuildStepGroupKey();
+      if (empty($all_groups[$group_key])) {
+        throw new Exception(
+          pht(
+            'Build step "%s" has step group key "%s", but no step group '.
+            'with that key exists.',
+            get_class($impl),
+            $group_key));
+      }
+    }
+
+    $groups = mgroup($all, 'getBuildStepGroupKey');
+    $lists = array();
+
+    $enabled_groups = HarbormasterBuildStepGroup::getAllEnabledGroups();
+    foreach ($enabled_groups as $group) {
+      $list = id(new PHUIObjectItemListView())
+        ->setHeader($group->getGroupName())
+        ->setNoDataString(
+          pht(
+            'This group has no available build steps.'));
+
+      $steps = idx($groups, $group->getGroupKey(), array());
+
+      foreach ($steps as $key => $impl) {
         if ($impl->shouldRequireAutotargeting()) {
-        unset($all[$key]);
+          unset($steps[$key]);
+          continue;
         }
       }
 
-    $errors = array();
+      if (!$steps && !$group->shouldShowIfEmpty()) {
-    if ($request->isFormPost()) {
+        continue;
-      $class = $request->getStr('class');
-      if (empty($all[$class])) {
-        $errors[] = pht('Choose the type of build step you want to add.');
       }
-      if (!$errors) {
+
+      foreach ($steps as $key => $impl) {
+        $class = get_class($impl);
+
         $new_uri = $this->getApplicationURI("step/new/{$plan_id}/{$class}/");
-        return id(new AphrontRedirectResponse())->setURI($new_uri);
+
-      }
+        $item = id(new PHUIObjectItemView())
+          ->setHeader($impl->getName())
+          ->setHref($new_uri)
+          ->addAttribute($impl->getGenericDescription());
+
+        $list->addItem($item);
       }
 
-    $control = id(new AphrontFormRadioButtonControl())
+      $lists[] = $list;
-      ->setName('class');
-
-    foreach ($all as $class => $implementation) {
-      $control->addButton(
-        $class,
-        $implementation->getName(),
-        $implementation->getGenericDescription());
     }
 
-    if ($errors) {
+    $crumbs = $this->buildApplicationCrumbs()
-      $errors = id(new PHUIInfoView())
+      ->addTextCrumb($plan_title, $cancel_uri)
-        ->setErrors($errors);
+      ->addTextCrumb(pht('Add Build Step'));
-    }
 
-    return $this->newDialog()
+    $box = id(new PHUIObjectBoxView())
-      ->setTitle(pht('Add New Step'))
+      ->setHeaderText(pht('Add Build Step'))
-      ->addSubmitButton(pht('Add Build Step'))
+      ->appendChild($lists);
-      ->addCancelButton($cancel_uri)
+
-      ->appendChild($errors)
+    return $this->buildApplicationPage(
-      ->appendParagraph(pht('Choose a type of build step to add:'))
+      array(
-      ->appendChild($control);
+        $crumbs,
+        $box,
+      ),
+      array(
+        'title' => array(
+          $plan_title,
+          pht('Add Build Step'),
+        ),
+      ));
   }
 
 }

src/applications/harbormaster/step/HarbormasterArcLintBuildStepImplementation.php

@@ -25,6 +25,10 @@ final class HarbormasterArcLintBuildStepImplementation
     return pht('Automatic `arc lint` step.');
   }
 
+  public function getBuildStepGroupKey() {
+    return HarbormasterBuiltinBuildStepGroup::GROUPKEY;
+  }
+
   public function execute(
     HarbormasterBuild $build,
     HarbormasterBuildTarget $build_target) {

src/applications/harbormaster/step/HarbormasterArcUnitBuildStepImplementation.php

@@ -25,6 +25,10 @@ final class HarbormasterArcUnitBuildStepImplementation
     return pht('Automatic `arc unit` step.');
   }
 
+  public function getBuildStepGroupKey() {
+    return HarbormasterBuiltinBuildStepGroup::GROUPKEY;
+  }
+
   public function execute(
     HarbormasterBuild $build,
     HarbormasterBuildTarget $build_target) {

src/applications/harbormaster/step/HarbormasterBuildStepImplementation.php

@@ -41,6 +41,10 @@ abstract class HarbormasterBuildStepImplementation extends Phobject {
    */
   abstract public function getName();
 
+  public function getBuildStepGroupKey() {
+    return HarbormasterOtherBuildStepGroup::GROUPKEY;
+  }
+
   /**
    * The generic description of the implementation.
    */

src/applications/harbormaster/step/HarbormasterCommandBuildStepImplementation.php

@@ -13,6 +13,10 @@ final class HarbormasterCommandBuildStepImplementation
     return pht('Run a command on Drydock host.');
   }
 
+  public function getBuildStepGroupKey() {
+    return HarbormasterPrototypeBuildStepGroup::GROUPKEY;
+  }
+
   public function getDescription() {
     return pht(
       'Run command %s on host %s.',

src/applications/harbormaster/step/HarbormasterHTTPRequestBuildStepImplementation.php

@@ -11,6 +11,10 @@ final class HarbormasterHTTPRequestBuildStepImplementation
     return pht('Make an HTTP request.');
   }
 
+  public function getBuildStepGroupKey() {
+    return HarbormasterExternalBuildStepGroup::GROUPKEY;
+  }
+
   public function getDescription() {
     $domain = null;
     $uri = $this->getSetting('uri');

src/applications/harbormaster/step/HarbormasterLeaseHostBuildStepImplementation.php

@@ -11,6 +11,10 @@ final class HarbormasterLeaseHostBuildStepImplementation
     return pht('Obtain a lease on a Drydock host for performing builds.');
   }
 
+  public function getBuildStepGroupKey() {
+    return HarbormasterPrototypeBuildStepGroup::GROUPKEY;
+  }
+
   public function execute(
     HarbormasterBuild $build,
     HarbormasterBuildTarget $build_target) {

src/applications/harbormaster/step/HarbormasterPublishFragmentBuildStepImplementation.php

@@ -11,6 +11,11 @@ final class HarbormasterPublishFragmentBuildStepImplementation
     return pht('Publish a fragment based on a file artifact.');
   }
 
+
+  public function getBuildStepGroupKey() {
+    return HarbormasterPrototypeBuildStepGroup::GROUPKEY;
+  }
+
   public function getDescription() {
     return pht(
       'Publish file artifact %s as fragment %s.',

src/applications/harbormaster/step/HarbormasterSleepBuildStepImplementation.php

@@ -11,6 +11,11 @@ final class HarbormasterSleepBuildStepImplementation
     return pht('Sleep for a specified number of seconds.');
   }
 
+
+  public function getBuildStepGroupKey() {
+    return HarbormasterTestBuildStepGroup::GROUPKEY;
+  }
+
   public function getDescription() {
     return pht(
       'Sleep for %s seconds.',

src/applications/harbormaster/step/HarbormasterThrowExceptionBuildStep.php

@@ -11,6 +11,10 @@ final class HarbormasterThrowExceptionBuildStep
     return pht('Throw an exception.');
   }
 
+  public function getBuildStepGroupKey() {
+    return HarbormasterTestBuildStepGroup::GROUPKEY;
+  }
+
   public function execute(
     HarbormasterBuild $build,
     HarbormasterBuildTarget $build_target) {

src/applications/harbormaster/step/HarbormasterUploadArtifactBuildStepImplementation.php

@@ -11,6 +11,10 @@ final class HarbormasterUploadArtifactBuildStepImplementation
     return pht('Upload a file from a host to Phabricator.');
   }
 
+  public function getBuildStepGroupKey() {
+    return HarbormasterPrototypeBuildStepGroup::GROUPKEY;
+  }
+
   public function getDescription() {
     return pht(
       'Upload %s from %s.',

src/applications/harbormaster/step/HarbormasterWaitForPreviousBuildStepImplementation.php

@@ -13,6 +13,10 @@ final class HarbormasterWaitForPreviousBuildStepImplementation
       'before continuing.');
   }
 
+  public function getBuildStepGroupKey() {
+    return HarbormasterPrototypeBuildStepGroup::GROUPKEY;
+  }
+
   public function execute(
     HarbormasterBuild $build,
     HarbormasterBuildTarget $build_target) {

src/applications/harbormaster/stepgroup/HarbormasterBuildStepGroup.php

@@ -0,0 +1,52 @@
+<?php
+
+abstract class HarbormasterBuildStepGroup extends Phobject {
+
+  abstract public function getGroupName();
+  abstract public function getGroupOrder();
+
+  public function isEnabled() {
+    return true;
+  }
+
+  public function shouldShowIfEmpty() {
+    return true;
+  }
+
+  final public function getGroupKey() {
+    $class = new ReflectionClass($this);
+
+    $const = $class->getConstant('GROUPKEY');
+    if ($const === false) {
+      throw new Exception(
+        pht(
+          '"%s" class "%s" must define a "%s" property.',
+          __CLASS__,
+          get_class($this),
+          'GROUPKEY'));
+    }
+
+    return $const;
+  }
+
+  final public static function getAllGroups() {
+    return id(new PhutilClassMapQuery())
+      ->setAncestorClass(__CLASS__)
+      ->setUniqueMethod('getGroupKey')
+      ->setSortMethod('getGroupOrder')
+      ->execute();
+  }
+
+  final public static function getAllEnabledGroups() {
+    $groups = self::getAllGroups();
+
+    foreach ($groups as $key => $group) {
+      if (!$group->isEnabled()) {
+        unset($groups[$key]);
+      }
+    }
+
+    return $groups;
+  }
+
+}

src/applications/harbormaster/stepgroup/HarbormasterBuiltinBuildStepGroup.php

@@ -0,0 +1,20 @@
+<?php
+
+final class HarbormasterBuiltinBuildStepGroup
+  extends HarbormasterBuildStepGroup {
+
+  const GROUPKEY = 'harbormaster.builtin';
+
+  public function getGroupName() {
+    return pht('Builtins');
+  }
+
+  public function getGroupOrder() {
+    return 0;
+  }
+
+  public function shouldShowIfEmpty() {
+    return false;
+  }
+
+}

src/applications/harbormaster/stepgroup/HarbormasterExternalBuildStepGroup.php

@@ -0,0 +1,16 @@
+<?php
+
+final class HarbormasterExternalBuildStepGroup
+  extends HarbormasterBuildStepGroup {
+
+  const GROUPKEY = 'harbormaster.external';
+
+  public function getGroupName() {
+    return pht('Interacting with External Build Sytems');
+  }
+
+  public function getGroupOrder() {
+    return 4000;
+  }
+
+}

src/applications/harbormaster/stepgroup/HarbormasterOtherBuildStepGroup.php

@@ -0,0 +1,20 @@
+<?php
+
+final class HarbormasterOtherBuildStepGroup
+  extends HarbormasterBuildStepGroup {
+
+  const GROUPKEY = 'harbormaster.other';
+
+  public function getGroupName() {
+    return pht('Other Build Steps');
+  }
+
+  public function getGroupOrder() {
+    return 9000;
+  }
+
+  public function shouldShowIfEmpty() {
+    return false;
+  }
+
+}

src/applications/harbormaster/stepgroup/HarbormasterPrototypeBuildStepGroup.php

@@ -0,0 +1,24 @@
+<?php
+
+final class HarbormasterPrototypeBuildStepGroup
+  extends HarbormasterBuildStepGroup {
+
+  const GROUPKEY = 'harbormaster.prototype';
+
+  public function getGroupName() {
+    return pht('Prototypes');
+  }
+
+  public function getGroupOrder() {
+    return 8000;
+  }
+
+  public function isEnabled() {
+    return PhabricatorEnv::getEnvConfig('phabricator.show-prototypes');
+  }
+
+  public function shouldShowIfEmpty() {
+    return false;
+  }
+
+}

src/applications/harbormaster/stepgroup/HarbormasterTestBuildStepGroup.php

@@ -0,0 +1,16 @@
+<?php
+
+final class HarbormasterTestBuildStepGroup
+  extends HarbormasterBuildStepGroup {
+
+  const GROUPKEY = 'harbormaster.test';
+
+  public function getGroupName() {
+    return pht('Testing Utilities');
+  }
+
+  public function getGroupOrder() {
+    return 7000;
+  }
+
+}