Implement explicit build step ordering in Harbormaster

Summary: This implements support for explicitly marking the sequence of build steps.  Users can now drag and re-order build steps in plans, and artifact dependencies are re-calculated so that if you move "Run Command" before "Lease Host", the "Run Command" step has it's artifact setting cleared and thus the step becomes invalid.

Test Plan: Re-ordered build steps and observed dependencies being correctly recalculated.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley

CC: Korvin, epriestley, aran

Maniphest Tasks: T1049

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

resources/sql/patches/20131205.buildsteporder.sql

@@ -0,0 +1,2 @@
+ALTER TABLE {$NAMESPACE}_harbormaster.harbormaster_buildstep
+ADD COLUMN sequence INT UNSIGNED NOT NULL;

resources/sql/patches/20131205.buildstepordermig.php

@@ -0,0 +1,41 @@
+<?php
+
+$table = new HarbormasterBuildPlan();
+$conn_w = $table->establishConnection('w');
+$viewer = PhabricatorUser::getOmnipotentUser();
+
+// Since HarbormasterBuildStepQuery has been updated to handle the
+// correct order, we can't use the built in database access.
+
+foreach (new LiskMigrationIterator($table) as $plan) {
+  $planname = $plan->getName();
+  echo "Migrating steps in {$planname}...\n";
+
+  $rows = queryfx_all(
+    $conn_w,
+    "SELECT id, sequence FROM harbormaster_buildstep ".
+    "WHERE buildPlanPHID = %s ".
+    "ORDER BY id ASC",
+    $plan->getPHID());
+
+  $sequence = 1;
+  foreach ($rows as $row) {
+    $id = $row['id'];
+    $existing = $row['sequence'];
+    if ($existing != 0) {
+      echo "  - {$id} (already migrated)...\n";
+      continue;
+    }
+    echo "  - {$id} to position {$sequence}...\n";
+    queryfx(
+      $conn_w,
+      "UPDATE harbormaster_buildstep ".
+      "SET sequence = %d ".
+      "WHERE id = %d",
+      $sequence,
+      $id);
+    $sequence++;
+  }
+}
+
+echo "Done.\n";

src/__phutil_library_map__.php

@@ -723,6 +723,7 @@ phutil_register_library_map(array(
     'HarbormasterPlanController' => 'applications/harbormaster/controller/HarbormasterPlanController.php',
     'HarbormasterPlanEditController' => 'applications/harbormaster/controller/HarbormasterPlanEditController.php',
     'HarbormasterPlanListController' => 'applications/harbormaster/controller/HarbormasterPlanListController.php',
+    'HarbormasterPlanOrderController' => 'applications/harbormaster/controller/HarbormasterPlanOrderController.php',
     'HarbormasterPlanViewController' => 'applications/harbormaster/controller/HarbormasterPlanViewController.php',
     'HarbormasterRemarkupRule' => 'applications/harbormaster/remarkup/HarbormasterRemarkupRule.php',
     'HarbormasterScratchTable' => 'applications/harbormaster/storage/HarbormasterScratchTable.php',
@@ -3111,6 +3112,7 @@ phutil_register_library_map(array(
       0 => 'HarbormasterPlanController',
       1 => 'PhabricatorApplicationSearchResultsControllerInterface',
     ),
+    'HarbormasterPlanOrderController' => 'HarbormasterController',
     'HarbormasterPlanViewController' => 'HarbormasterPlanController',
     'HarbormasterRemarkupRule' => 'PhabricatorRemarkupRuleObject',
     'HarbormasterScratchTable' => 'HarbormasterDAO',

src/applications/harbormaster/application/PhabricatorApplicationHarbormaster.php

@@ -65,6 +65,7 @@ final class PhabricatorApplicationHarbormaster extends PhabricatorApplication {
           '(?:query/(?P<queryKey>[^/]+)/)?'
             => 'HarbormasterPlanListController',
           'edit/(?:(?P<id>\d+)/)?' => 'HarbormasterPlanEditController',
+          'order/(?:(?P<id>\d+)/)?' => 'HarbormasterPlanOrderController',
           '(?P<id>\d+)/' => 'HarbormasterPlanViewController',
         ),
       ),

src/applications/harbormaster/controller/HarbormasterPlanOrderController.php

@@ -0,0 +1,84 @@
+<?php
+
+/**
+ * @group search
+ */
+final class HarbormasterPlanOrderController extends HarbormasterController {
+
+  private $id;
+
+  public function willProcessRequest(array $data) {
+    $this->id = idx($data, 'id');
+  }
+
+  public function processRequest() {
+    $request = $this->getRequest();
+    $user = $request->getUser();
+
+    $request->validateCSRF();
+
+    $this->requireApplicationCapability(
+      HarbormasterCapabilityManagePlans::CAPABILITY);
+
+    $plan = id(new HarbormasterBuildPlanQuery())
+      ->setViewer($user)
+      ->withIDs(array($this->id))
+      ->executeOne();
+    if (!$plan) {
+      return new Aphront404Response();
+    }
+
+    // Load all steps.
+    $order = $request->getStrList('order');
+    $steps = id(new HarbormasterBuildStepQuery())
+      ->setViewer($user)
+      ->withIDs($order)
+      ->execute();
+    $steps = array_select_keys($steps, $order);
+    $reordered_steps = array();
+
+    // Apply sequences.
+    $sequence = 1;
+    foreach ($steps as $step) {
+      $step->setSequence($sequence++);
+      $step->save();
+
+      $reordered_steps[] = $step;
+    }
+
+    // We must ensure that steps with artifacts become invalid if they are
+    // placed before the steps that produce them.
+    foreach ($reordered_steps as $step) {
+      $implementation = $step->getStepImplementation();
+      $settings = $implementation->getSettings();
+      foreach ($implementation->getSettingDefinitions() as $name => $opt) {
+        switch ($opt['type']) {
+          case BuildStepImplementation::SETTING_TYPE_ARTIFACT:
+            $value = $settings[$name];
+            $filter = $opt['artifact_type'];
+            $available_artifacts =
+              BuildStepImplementation::getAvailableArtifacts(
+                $plan,
+                $reordered_steps,
+                $step,
+                $filter);
+            $artifact_found = false;
+            foreach ($available_artifacts as $key => $type) {
+              if ($key === $value) {
+                $artifact_found = true;
+              }
+            }
+            if (!$artifact_found) {
+              $step->setDetail($name, null);
+            }
+            break;
+        }
+        $step->save();
+      }
+    }
+
+    // Force the page to re-render.
+    return id(new AphrontRedirectResponse());
+  }
+
+}

src/applications/harbormaster/controller/HarbormasterPlanViewController.php

@@ -74,6 +74,8 @@ final class HarbormasterPlanViewController
     $request = $this->getRequest();
     $viewer = $request->getUser();
 
+    $list_id = celerity_generate_unique_node_id();
+
     $steps = id(new HarbormasterBuildStepQuery())
       ->setViewer($viewer)
       ->withBuildPlanPHIDs(array($plan->getPHID()))
@@ -84,7 +86,14 @@ final class HarbormasterPlanViewController
 
     $i = 1;
     $step_list = id(new PHUIObjectItemListView())
-      ->setUser($viewer);
+      ->setUser($viewer)
+      ->setID($list_id);
+    Javelin::initBehavior(
+      'harbormaster-reorder-steps',
+      array(
+        'listID' => $list_id,
+        'orderURI' => '/harbormaster/plan/order/'.$plan->getID().'/',
+      ));
     foreach ($steps as $step) {
       $implementation = null;
       try {
@@ -136,6 +145,12 @@ final class HarbormasterPlanViewController
               ->setName(pht("Delete"))
               ->setHref(
                 $this->getApplicationURI("step/delete/".$step->getID()."/")));
+        $item->setGrippable(true);
+        $item->addSigil('build-step');
+        $item->setMetadata(
+          array(
+            'stepID' => $step->getID(),
+          ));
       }
 
       $step_list->addItem($item);

src/applications/harbormaster/controller/HarbormasterStepAddController.php

@@ -36,10 +36,13 @@ final class HarbormasterStepAddController
         return $this->createDialog($implementations);
       }
 
+      $steps = $plan->loadOrderedBuildSteps();
+
       $step = new HarbormasterBuildStep();
       $step->setBuildPlanPHID($plan->getPHID());
       $step->setClassName($class);
       $step->setDetails(array());
+      $step->setSequence(count($steps) + 1);
       $step->save();
 
       $edit_uri = $this->getApplicationURI("step/edit/".$step->getID()."/");

src/applications/harbormaster/controller/HarbormasterStepEditController.php

@@ -94,7 +94,7 @@ final class HarbormasterStepEditController
         case BuildStepImplementation::SETTING_TYPE_ARTIFACT:
           $filter = $opt['artifact_type'];
           $available_artifacts =
-            BuildStepImplementation::getAvailableArtifacts(
+            BuildStepImplementation::loadAvailableArtifacts(
               $plan,
               $step,
               $filter);

src/applications/harbormaster/query/HarbormasterBuildStepQuery.php

@@ -23,7 +23,7 @@ final class HarbormasterBuildStepQuery
   }
 
   public function getPagingColumn() {
-    return 'id';
+    return 'sequence';
   }
 
   public function getReversePaging() {

src/applications/harbormaster/step/BuildStepImplementation.php

@@ -117,13 +117,29 @@ abstract class BuildStepImplementation {
   /**
    * Returns a list of all artifacts made available by previous build steps.
    */
-  public static function getAvailableArtifacts(
+  public static function loadAvailableArtifacts(
     HarbormasterBuildPlan $build_plan,
     HarbormasterBuildStep $current_build_step,
     $artifact_type) {
 
     $build_steps = $build_plan->loadOrderedBuildSteps();
 
+    return self::getAvailableArtifacts(
+      $build_plan,
+      $build_steps,
+      $current_build_step,
+      $artifact_type);
+  }
+
+  /**
+   * Returns a list of all artifacts made available by previous build steps.
+   */
+  public static function getAvailableArtifacts(
+    HarbormasterBuildPlan $build_plan,
+    array $build_steps,
+    HarbormasterBuildStep $current_build_step,
+    $artifact_type) {
+
     $previous_implementations = array();
     foreach ($build_steps as $build_step) {
       if ($build_step->getPHID() === $current_build_step->getPHID()) {
@@ -136,7 +152,7 @@ abstract class BuildStepImplementation {
     $artifacts = array();
     foreach ($artifact_arrays as $array) {
       foreach ($array as $name => $type) {
-        if ($type !== $artifact_type) {
+        if ($type !== $artifact_type && $artifact_type !== null) {
           continue;
         }
         $artifacts[$name] = $type;

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

@@ -6,6 +6,7 @@ final class HarbormasterBuildStep extends HarbormasterDAO
   protected $buildPlanPHID;
   protected $className;
   protected $details = array();
+  protected $sequence;
 
   private $buildPlan = self::ATTACHABLE;
 
@@ -61,6 +62,7 @@ final class HarbormasterBuildStep extends HarbormasterDAO
     return $implementation;
   }
 
+
 /* -(  PhabricatorPolicyInterface  )----------------------------------------- */
 
 

src/infrastructure/storage/patch/PhabricatorBuiltinPatchList.php

@@ -1804,6 +1804,14 @@ final class PhabricatorBuiltinPatchList extends PhabricatorSQLPatchList {
         'type' => 'sql',
         'name' => $this->getPatchPath('20131204.pushlog.sql'),
       ),
+      '20131205.buildsteporder.sql' => array(
+        'type' => 'sql',
+        'name' => $this->getPatchPath('20131205.buildsteporder.sql'),
+      ),
+      '20131205.buildstepordermig.php' => array(
+        'type' => 'php',
+        'name' => $this->getPatchPath('20131205.buildstepordermig.php'),
+      ),
     );
   }
 }

webroot/rsrc/js/application/harbormaster/behavior-reorder-steps.js

@@ -0,0 +1,42 @@
+/**
+ * @provides javelin-behavior-harbormaster-reorder-steps
+ * @requires javelin-behavior
+ *           javelin-stratcom
+ *           javelin-workflow
+ *           javelin-dom
+ *           phabricator-draggable-list
+ */
+
+JX.behavior('harbormaster-reorder-steps', function(config) {
+
+  var root = JX.$(config.listID);
+
+  var list = new JX.DraggableList('build-step', root)
+    .setFindItemsHandler(function() {
+      return JX.DOM.scry(root, 'li', 'build-step');
+    });
+
+  list.listen('didDrop', function(node, after) {
+    var nodes = list.findItems();
+    var order = [];
+    var key;
+    for (var ii = 0; ii < nodes.length; ii++) {
+      key = JX.Stratcom.getData(nodes[ii]).stepID;
+      if (key) {
+        order.push(key);
+      }
+    }
+
+    list.lock();
+    JX.DOM.alterClass(node, 'drag-sending', true);
+
+    new JX.Workflow(config.orderURI, {order: order.join()})
+      .setHandler(function(e) {
+        JX.DOM.alterClass(node, 'drag-sending', false);
+        list.unlock();
+      })
+      .start();
+  });
+
+});
+