Allow Harbormaster build plans to be disabled

Summary: Fixes T4187. Ref T1049. Allows build plans to be enabled or disabled.

Test Plan: Enabled and disabled build plans.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T4187, T1049

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

src/__phutil_library_map__.php

@@ -726,6 +726,7 @@ phutil_register_library_map(array(
     'HarbormasterPHIDTypeBuildTarget' => 'applications/harbormaster/phid/HarbormasterPHIDTypeBuildTarget.php',
     'HarbormasterPHIDTypeBuildable' => 'applications/harbormaster/phid/HarbormasterPHIDTypeBuildable.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',
     'HarbormasterPlanOrderController' => 'applications/harbormaster/controller/HarbormasterPlanOrderController.php',
@@ -3155,6 +3156,7 @@ phutil_register_library_map(array(
     'HarbormasterPHIDTypeBuildTarget' => 'PhabricatorPHIDType',
     'HarbormasterPHIDTypeBuildable' => 'PhabricatorPHIDType',
     'HarbormasterPlanController' => 'PhabricatorController',
+    'HarbormasterPlanDisableController' => 'HarbormasterPlanController',
     'HarbormasterPlanEditController' => 'HarbormasterPlanController',
     'HarbormasterPlanListController' =>
     array(

src/applications/harbormaster/application/PhabricatorApplicationHarbormaster.php

@@ -66,6 +66,7 @@ final class PhabricatorApplicationHarbormaster extends PhabricatorApplication {
             => 'HarbormasterPlanListController',
           'edit/(?:(?P<id>\d+)/)?' => 'HarbormasterPlanEditController',
           'order/(?:(?P<id>\d+)/)?' => 'HarbormasterPlanOrderController',
+          'disable/(?P<id>\d+)/' => 'HarbormasterPlanDisableController',
           '(?P<id>\d+)/' => 'HarbormasterPlanViewController',
         ),
       ),

src/applications/harbormaster/controller/HarbormasterPlanDisableController.php

@@ -0,0 +1,76 @@
+<?php
+
+final class HarbormasterPlanDisableController
+  extends HarbormasterPlanController {
+
+  private $id;
+
+  public function willProcessRequest(array $data) {
+    $this->id = $data['id'];
+  }
+
+  public function processRequest() {
+    $request = $this->getRequest();
+    $viewer = $request->getUser();
+
+    $this->requireApplicationCapability(
+      HarbormasterCapabilityManagePlans::CAPABILITY);
+
+    $plan = id(new HarbormasterBuildPlanQuery())
+      ->setViewer($viewer)
+      ->withIDs(array($this->id))
+      ->executeOne();
+    if (!$plan) {
+      return new Aphront404Response();
+    }
+
+    $plan_uri = $this->getApplicationURI('plan/'.$plan->getID().'/');
+
+    if ($request->isFormPost()) {
+
+      $type_status = HarbormasterBuildPlanTransaction::TYPE_STATUS;
+
+      $v_status = $plan->isDisabled()
+        ? HarbormasterBuildPlan::STATUS_ACTIVE
+        : HarbormasterBuildPlan::STATUS_DISABLED;
+
+      $xactions = array();
+
+      $xactions[] = id(new HarbormasterBuildPlanTransaction())
+        ->setTransactionType($type_status)
+        ->setNewValue($v_status);
+
+      $editor = id(new HarbormasterBuildPlanEditor())
+        ->setActor($viewer)
+        ->setContinueOnNoEffect(true)
+        ->setContinueOnMissingFields(true)
+        ->setContentSourceFromRequest($request);
+
+      $editor->applyTransactions($plan, $xactions);
+
+      return id(new AphrontRedirectResponse())->setURI($plan_uri);
+    }
+
+    if ($plan->isDisabled()) {
+      $title = pht('Enable Build Plan');
+      $body = pht('Enable this build plan?');
+      $button = pht('Enable Plan');
+    } else {
+      $title = pht('Disable Build Plan');
+      $body = pht(
+        'Disable this build plan? It will no longer be executed '.
+        'automatically.');
+      $button = pht('Disable Plan');
+    }
+
+    $dialog = id(new AphrontDialogView())
+      ->setUser($viewer)
+      ->setTitle($title)
+      ->appendChild($body)
+      ->addSubmitButton($button)
+      ->addCancelButton($plan_uri);
+
+    return id(new AphrontDialogResponse())->setDialog($dialog);
+  }
+
+}

src/applications/harbormaster/controller/HarbormasterPlanListController.php

@@ -39,6 +39,10 @@ final class HarbormasterPlanListController
         ->setObjectName(pht('Plan %d', $plan->getID()))
         ->setHeader($plan->getName());
 
+      if ($plan->isDisabled()) {
+        $item->setDisabled(true);
+      }
+
       $item->setHref($this->getApplicationURI("plan/{$id}/"));
 
       $list->addItem($item);

src/applications/harbormaster/controller/HarbormasterPlanViewController.php

@@ -178,6 +178,24 @@ final class HarbormasterPlanViewController
         ->setDisabled(!$can_edit)
         ->setIcon('edit'));
 
+    if ($plan->isDisabled()) {
+      $list->addAction(
+        id(new PhabricatorActionView())
+          ->setName(pht('Enable Plan'))
+          ->setHref($this->getApplicationURI("plan/disable/{$id}/"))
+          ->setWorkflow(true)
+          ->setDisabled(!$can_edit)
+          ->setIcon('enable'));
+    } else {
+      $list->addAction(
+        id(new PhabricatorActionView())
+          ->setName(pht('Disable Plan'))
+          ->setHref($this->getApplicationURI("plan/disable/{$id}/"))
+          ->setWorkflow(true)
+          ->setDisabled(!$can_edit)
+          ->setIcon('disable'));
+    }
+
     $list->addAction(
       id(new PhabricatorActionView())
         ->setName(pht('Add Build Step'))

src/applications/harbormaster/editor/HarbormasterBuildPlanEditor.php

@@ -6,6 +6,7 @@ final class HarbormasterBuildPlanEditor
   public function getTransactionTypes() {
     $types = parent::getTransactionTypes();
     $types[] = HarbormasterBuildPlanTransaction::TYPE_NAME;
+    $types[] = HarbormasterBuildPlanTransaction::TYPE_STATUS;
     $types[] = PhabricatorTransactions::TYPE_COMMENT;
     return $types;
   }
@@ -19,6 +20,8 @@ final class HarbormasterBuildPlanEditor
           return null;
         }
         return $object->getName();
+      case HarbormasterBuildPlanTransaction::TYPE_STATUS:
+        return $object->getPlanStatus();
     }
 
     return parent::getCustomTransactionOldValue($object, $xaction);
@@ -30,6 +33,8 @@ final class HarbormasterBuildPlanEditor
     switch ($xaction->getTransactionType()) {
       case HarbormasterBuildPlanTransaction::TYPE_NAME:
         return $xaction->getNewValue();
+      case HarbormasterBuildPlanTransaction::TYPE_STATUS:
+        return $xaction->getNewValue();
     }
     return parent::getCustomTransactionNewValue($object, $xaction);
   }
@@ -41,6 +46,9 @@ final class HarbormasterBuildPlanEditor
       case HarbormasterBuildPlanTransaction::TYPE_NAME:
         $object->setName($xaction->getNewValue());
         return;
+      case HarbormasterBuildPlanTransaction::TYPE_STATUS:
+        $object->setPlanStatus($xaction->getNewValue());
+        return;
     }
     return parent::applyCustomInternalTransaction($object, $xaction);
   }
@@ -50,6 +58,7 @@ final class HarbormasterBuildPlanEditor
     PhabricatorApplicationTransaction $xaction) {
     switch ($xaction->getTransactionType()) {
       case HarbormasterBuildPlanTransaction::TYPE_NAME:
+      case HarbormasterBuildPlanTransaction::TYPE_STATUS:
         return;
     }
     return parent::applyCustomExternalTransaction($object, $xaction);

src/applications/harbormaster/query/HarbormasterBuildPlanQuery.php

@@ -5,6 +5,7 @@ final class HarbormasterBuildPlanQuery
 
   private $ids;
   private $phids;
+  private $statuses;
 
   public function withIDs(array $ids) {
     $this->ids = $ids;
@@ -16,6 +17,11 @@ final class HarbormasterBuildPlanQuery
     return $this;
   }
 
+  public function withStatuses(array $statuses) {
+    $this->statuses = $statuses;
+    return $this;
+  }
+
   protected function loadPage() {
     $table = new HarbormasterBuildPlan();
     $conn_r = $table->establishConnection('r');
@@ -49,6 +55,13 @@ final class HarbormasterBuildPlanQuery
         $this->phids);
     }
 
+    if ($this->statuses) {
+      $where[] = qsprintf(
+        $conn_r,
+        'planStatus IN (%Ls)',
+        $this->statuses);
+    }
+
     $where[] = $this->buildPagingClause($conn_r);
 
     return $this->formatWhereClause($where);

src/applications/harbormaster/query/HarbormasterBuildPlanSearchEngine.php

@@ -6,12 +6,21 @@ final class HarbormasterBuildPlanSearchEngine
   public function buildSavedQueryFromRequest(AphrontRequest $request) {
     $saved = new PhabricatorSavedQuery();
 
+    $saved->setParameter(
+      'status',
+      $this->readListFromRequest($request, 'status'));
+
     return $saved;
   }
 
   public function buildQueryFromSavedQuery(PhabricatorSavedQuery $saved) {
     $query = id(new HarbormasterBuildPlanQuery());
 
+    $status = $saved->getParameter('status', array());
+    if ($status) {
+      $query->withStatuses($status);
+    }
+
     return $query;
   }
 
@@ -19,6 +28,23 @@ final class HarbormasterBuildPlanSearchEngine
     AphrontFormView $form,
     PhabricatorSavedQuery $saved_query) {
 
+    $status = $saved_query->getParameter('status', array());
+
+    $form
+      ->appendChild(
+        id(new AphrontFormCheckboxControl())
+          ->setLabel('Status')
+          ->addCheckbox(
+            'status[]',
+            HarbormasterBuildPlan::STATUS_ACTIVE,
+            pht('Active'),
+            in_array(HarbormasterBuildPlan::STATUS_ACTIVE, $status))
+          ->addCheckbox(
+            'status[]',
+            HarbormasterBuildPlan::STATUS_DISABLED,
+            pht('Disabled'),
+            in_array(HarbormasterBuildPlan::STATUS_DISABLED, $status)));
+
   }
 
   protected function getURI($path) {
@@ -27,6 +53,7 @@ final class HarbormasterBuildPlanSearchEngine
 
   public function getBuiltinQueryNames() {
     $names = array(
+      'active' => pht('Active Plans'),
       'all' => pht('All Plans'),
     );
 
@@ -39,6 +66,12 @@ final class HarbormasterBuildPlanSearchEngine
     $query->setQueryKey($query_key);
 
     switch ($query_key) {
+      case 'active':
+        return $query->setParameter(
+          'status',
+          array(
+            HarbormasterBuildPlan::STATUS_ACTIVE,
+          ));
       case 'all':
         return $query;
     }

src/applications/harbormaster/storage/HarbormasterBuildable.php

@@ -77,6 +77,12 @@ final class HarbormasterBuildable extends HarbormasterDAO
       ->withPHIDs($plan_phids)
       ->execute();
     foreach ($plans as $plan) {
+      if ($plan->isDisabled()) {
+        // TODO: This should be communicated more clearly -- maybe we should
+        // create the build but set the status to "disabled" or "derelict".
+        continue;
+      }
+
       $build = HarbormasterBuild::initializeNewBuild(
         PhabricatorUser::getOmnipotentUser());
       $build->setBuildablePHID($buildable->getPHID());

src/applications/harbormaster/storage/configuration/HarbormasterBuildPlan.php

@@ -8,11 +8,14 @@ final class HarbormasterBuildPlan extends HarbormasterDAO
   protected $name;
   protected $planStatus;
 
+  const STATUS_ACTIVE   = 'active';
+  const STATUS_DISABLED = 'disabled';
+
   private $buildSteps = self::ATTACHABLE;
 
   public static function initializeNewBuildPlan(PhabricatorUser $actor) {
     return id(new HarbormasterBuildPlan())
-      ->setPlanStatus('active'); // TODO: Figure this out.
+      ->setPlanStatus(self::STATUS_ACTIVE);
   }
 
   public function getConfiguration() {
@@ -49,6 +52,10 @@ final class HarbormasterBuildPlan extends HarbormasterDAO
       ->execute();
   }
 
+  public function isDisabled() {
+    return ($this->getPlanStatus() == self::STATUS_DISABLED);
+  }
+
 
 /* -(  PhabricatorSubscribableInterface  )----------------------------------- */
 

src/applications/harbormaster/storage/configuration/HarbormasterBuildPlanTransaction.php

@@ -4,6 +4,7 @@ final class HarbormasterBuildPlanTransaction
   extends PhabricatorApplicationTransaction {
 
   const TYPE_NAME = 'harbormaster:name';
+  const TYPE_STATUS = 'harbormaster:status';
 
   public function getApplicationName() {
     return 'harbormaster';
@@ -65,7 +66,16 @@ final class HarbormasterBuildPlanTransaction
             $old,
             $new);
         }
-        break;
+      case self::TYPE_STATUS:
+        if ($new == HarbormasterBuildPlan::STATUS_DISABLED) {
+          return pht(
+            '%s disabled this build plan.',
+            $author_handle);
+        } else {
+          return pht(
+            '%s enabled this build plan.',
+            $author_handle);
+        }
     }
 
     return parent::getTitle();

src/applications/search/engine/PhabricatorApplicationSearchEngine.php

@@ -263,10 +263,8 @@ abstract class PhabricatorApplicationSearchEngine {
     AphrontRequest $request,
     $key,
     array $allow_types = array()) {
-    $list = $request->getArr($key, null);
+
-    if ($list === null) {
+    $list = $this->readListFromRequest($request, $key);
-      $list = $request->getStrList($key);
-    }
 
     $phids = array();
     $names = array();
@@ -316,14 +314,7 @@ abstract class PhabricatorApplicationSearchEngine {
     $key,
     array $allow_types = array()) {
 
-    $list = $request->getArr($key, null);
+    $list = $this->readListFromRequest($request, $key);
-    if ($list === null) {
-      $list = $request->getStrList($key);
-    }
-
-    if (!$list) {
-      return array();
-    }
 
     $objects = id(new PhabricatorObjectQuery())
       ->setViewer($this->requireViewer())
@@ -348,6 +339,36 @@ abstract class PhabricatorApplicationSearchEngine {
     return $list;
   }
 
+
+  /**
+   * Read a list of items from the request, in either array format or string
+   * format:
+   *
+   *   list[]=item1&list[]=item2
+   *   list=item1,item2
+   *
+   * This provides flexibility when constructing URIs, especially from external
+   * sources.
+   *
+   * @param AphrontRequest  Request to read PHIDs from.
+   * @param string          Key to read in the request.
+   * @return list<string>   List of values.
+   */
+  protected function readListFromRequest(
+    AphrontRequest $request,
+    $key) {
+    $list = $request->getArr($key, null);
+    if ($list === null) {
+      $list = $request->getStrList($key);
+    }
+
+    if (!$list) {
+      return array();
+    }
+
+    return $list;
+  }
+
   protected function readBoolFromRequest(
     AphrontRequest $request,
     $key) {