Update Buildable search in Harbormaster

Summary:
Fixes T10011.

  - Modernize searching for buildables.
  - Prepare for `harbormaster.buildable.search`.
  - Allow users to query by status (see T10011).
  - Collapse the four weird "commit / diff / revision / repository" fields into two slightly less weird fields with more UI hinting?

Test Plan: {F1131918}

Reviewers: chad

Reviewed By: chad

Subscribers: Luke081515.2

Maniphest Tasks: T10011

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

src/applications/harbormaster/phid/HarbormasterBuildablePHIDType.php

@@ -29,7 +29,7 @@ final class HarbormasterBuildablePHIDType extends PhabricatorPHIDType {
     array $handles,
     array $objects) {
 
-    $viewer = $this->getViewer();
+    $viewer = $query->getViewer();
 
     $target_phids = array();
     foreach ($objects as $phid => $object) {

src/applications/harbormaster/query/HarbormasterBuildableQuery.php

@@ -7,6 +7,7 @@ final class HarbormasterBuildableQuery
   private $phids;
   private $buildablePHIDs;
   private $containerPHIDs;
+  private $statuses;
   private $manualBuildables;
 
   private $needContainerObjects;
@@ -43,6 +44,11 @@ final class HarbormasterBuildableQuery
     return $this;
   }
 
+  public function withStatuses(array $statuses) {
+    $this->statuses = $statuses;
+    return $this;
+  }
+
   public function needBuilds($need) {
     $this->needBuilds = $need;
     return $this;
@@ -154,6 +160,13 @@ final class HarbormasterBuildableQuery
         $this->containerPHIDs);
     }
 
+    if ($this->statuses !== null) {
+      $where[] = qsprintf(
+        $conn,
+        'buildableStatus in (%Ls)',
+        $this->statuses);
+    }
+
     if ($this->manualBuildables !== null) {
       $where[] = qsprintf(
         $conn,

src/applications/harbormaster/query/HarbormasterBuildableSearchEngine.php

@@ -11,146 +11,87 @@ final class HarbormasterBuildableSearchEngine
     return 'PhabricatorHarbormasterApplication';
   }
 
-  public function buildSavedQueryFromRequest(AphrontRequest $request) {
+  public function newQuery() {
-    $saved = new PhabricatorSavedQuery();
+    return new HarbormasterBuildableQuery();
-
-    $revisions = $this->readPHIDsFromRequest(
-      $request,
-      'revisions',
-      array(
-        DifferentialRevisionPHIDType::TYPECONST,
-      ));
-
-    $repositories = $this->readPHIDsFromRequest(
-      $request,
-      'repositories',
-      array(
-        PhabricatorRepositoryRepositoryPHIDType::TYPECONST,
-      ));
-
-    $container_phids = array_merge($revisions, $repositories);
-    $saved->setParameter('containerPHIDs', $container_phids);
-
-    $commits = $this->readPHIDsFromRequest(
-      $request,
-      'commits',
-      array(
-        PhabricatorRepositoryCommitPHIDType::TYPECONST,
-      ));
-
-    $diffs = $this->readListFromRequest($request, 'diffs');
-    if ($diffs) {
-      $diffs = id(new DifferentialDiffQuery())
-        ->setViewer($this->requireViewer())
-        ->withIDs($diffs)
-        ->execute();
-      $diffs = mpull($diffs, 'getPHID', 'getPHID');
-    }
-
-    $buildable_phids = array_merge($commits, $diffs);
-    $saved->setParameter('buildablePHIDs', $buildable_phids);
-
-    $saved->setParameter(
-      'manual',
-      $this->readBoolFromRequest($request, 'manual'));
-
-    return $saved;
   }
 
-  public function buildQueryFromSavedQuery(PhabricatorSavedQuery $saved) {
+  protected function buildCustomSearchFields() {
-    $query = id(new HarbormasterBuildableQuery());
+    return array(
-
+      id(new PhabricatorSearchStringListField())
-    $container_phids = $saved->getParameter('containerPHIDs', array());
+        ->setKey('objectPHIDs')
-    if ($container_phids) {
+        ->setAliases(array('objects'))
-      $query->withContainerPHIDs($container_phids);
+        ->setLabel(pht('Objects'))
-    }
+        ->setPlaceholder(pht('rXabcdef, PHID-DIFF-1234, ...'))
-
+        ->setDescription(pht('Search for builds of particular objects.')),
-    $buildable_phids = $saved->getParameter('buildablePHIDs', array());
+      id(new PhabricatorSearchStringListField())
-
+        ->setKey('containerPHIDs')
-    if ($buildable_phids) {
+        ->setAliases(array('containers'))
-      $query->withBuildablePHIDs($buildable_phids);
+        ->setLabel(pht('Containers'))
-    }
+        ->setPlaceholder(pht('rXYZ, R123, D456, ...'))
-
+        ->setDescription(
-    $manual = $saved->getParameter('manual');
+          pht('Search for builds by containing revision or repository.')),
-    if ($manual !== null) {
+      id(new PhabricatorSearchCheckboxesField())
-      $query->withManualBuildables($manual);
+        ->setKey('statuses')
-    }
+        ->setLabel(pht('Statuses'))
-
+        ->setOptions(HarbormasterBuildable::getBuildStatusMap())
-    return $query;
+        ->setDescription(pht('Search for builds by buildable status.')),
+      id(new PhabricatorSearchThreeStateField())
+        ->setLabel(pht('Manual'))
+        ->setKey('manual')
+        ->setDescription(
+          pht('Search for only manual or automatic buildables.'))
+        ->setOptions(
+          pht('(Show All)'),
+          pht('Show Only Manual Builds'),
+          pht('Show Only Automated Builds')),
+    );
   }
 
-  public function buildSearchForm(
+  private function resolvePHIDs(array $names) {
-    AphrontFormView $form,
+    $viewer = $this->requireViewer();
-    PhabricatorSavedQuery $saved_query) {
 
-    $container_phids = $saved_query->getParameter('containerPHIDs', array());
+    $objects = id(new PhabricatorObjectQuery())
-    $buildable_phids = $saved_query->getParameter('buildablePHIDs', array());
+      ->setViewer($viewer)
+      ->withNames($names)
+      ->execute();
 
-    $all_phids = array_merge($container_phids, $buildable_phids);
+    // TODO: Instead of using string lists, we should ideally be using some
+    // kind of smart field with resolver logic that can help users type the
+    // right stuff. For now, just return a bogus value here so nothing matches
+    // but the form doesn't explode.
+    if (!$objects) {
+      return array('-');
+    }
 
-    $revision_names = array();
+    return mpull($objects, 'getPHID');
-    $diff_names = array();
+  }
-    $repository_names = array();
-    $commit_names = array();
 
-    if ($all_phids) {
+  protected function buildQueryFromParameters(array $map) {
-      $objects = id(new PhabricatorObjectQuery())
+    $query = $this->newQuery();
-        ->setViewer($this->requireViewer())
-        ->withPHIDs($all_phids)
-        ->execute();
 
-      foreach ($all_phids as $phid) {
+    if ($map['objectPHIDs']) {
-        $object = idx($objects, $phid);
+      $phids = $this->resolvePHIDs($map['objectPHIDs']);
-        if (!$object) {
+      if ($phids) {
-          continue;
+        $query->withBuildablePHIDs($phids);
-        }
-
-        if ($object instanceof DifferentialRevision) {
-          $revision_names[] = 'D'.$object->getID();
-        } else if ($object instanceof DifferentialDiff) {
-          $diff_names[] = $object->getID();
-        } else if ($object instanceof PhabricatorRepository) {
-          $repository_names[] = $object->getMonogram();
-        } else if ($object instanceof PhabricatorRepositoryCommit) {
-          $repository = $object->getRepository();
-          $commit_names[] = $repository->formatCommitName(
-            $object->getCommitIdentifier());
-        }
       }
     }
 
-    $form
+    if ($map['containerPHIDs']) {
-      ->appendChild(
+      $phids = $this->resolvePHIDs($map['containerPHIDs']);
-        id(new AphrontFormTextControl())
+      if ($phids) {
-          ->setLabel(pht('Differential Revisions'))
+        $query->withContainerPHIDs($phids);
-          ->setName('revisions')
+      }
-          ->setValue(implode(', ', $revision_names)))
+    }
-      ->appendChild(
+
-        id(new AphrontFormTextControl())
+    if ($map['statuses']) {
-          ->setLabel(pht('Differential Diffs'))
+      $query->withStatuses($map['statuses']);
-          ->setName('diffs')
+    }
-          ->setValue(implode(', ', $diff_names)))
+
-      ->appendChild(
+    if ($map['manual'] !== null) {
-        id(new AphrontFormTextControl())
+      $query->withManualBuildables($map['manual']);
-          ->setLabel(pht('Repositories'))
+    }
-          ->setName('repositories')
+
-          ->setValue(implode(', ', $repository_names)))
+    return $query;
-      ->appendChild(
-        id(new AphrontFormTextControl())
-          ->setLabel(pht('Commits'))
-          ->setName('commits')
-          ->setValue(implode(', ', $commit_names)))
-      ->appendChild(
-        id(new AphrontFormSelectControl())
-          ->setLabel(pht('Origin'))
-          ->setName('manual')
-          ->setValue($this->getBoolFromQuery($saved_query, 'manual'))
-          ->setOptions(
-            array(
-              '' => pht('(All Origins)'),
-              'true' => pht('Manual Buildables'),
-              'false' => pht('Automatic Buildables'),
-            )));
   }
 
   protected function getURI($path) {

src/applications/harbormaster/storage/HarbormasterBuildable.php

@@ -20,16 +20,16 @@ final class HarbormasterBuildable extends HarbormasterDAO
   const STATUS_FAILED = 'failed';
 
   public static function getBuildableStatusName($status) {
-    switch ($status) {
+    $map = self::getBuildStatusMap();
-      case self::STATUS_BUILDING:
+    return idx($map, $status, pht('Unknown ("%s")', $status));
-        return pht('Building');
+  }
-      case self::STATUS_PASSED:
+
-        return pht('Passed');
+  public static function getBuildStatusMap() {
-      case self::STATUS_FAILED:
+    return array(
-        return pht('Failed');
+      self::STATUS_BUILDING => pht('Building'),
-      default:
+      self::STATUS_PASSED => pht('Passed'),
-        return pht('Unknown');
+      self::STATUS_FAILED => pht('Failed'),
-    }
+    );
   }
 
   public static function getBuildableStatusIcon($status) {

src/applications/search/field/PhabricatorSearchStringListField.php

@@ -3,6 +3,17 @@
 final class PhabricatorSearchStringListField
   extends PhabricatorSearchField {
 
+  private $placeholder;
+
+  public function setPlaceholder($placeholder) {
+    $this->placeholder = $placeholder;
+    return $this;
+  }
+
+  public function getPlaceholder() {
+    return $this->placeholder;
+  }
+
   protected function getDefaultValue() {
     return array();
   }
@@ -12,7 +23,14 @@ final class PhabricatorSearchStringListField
   }
 
   protected function newControl() {
-    return new AphrontFormTextControl();
+    $control = new AphrontFormTextControl();
+
+    $placeholder = $this->getPlaceholder();
+    if ($placeholder !== null) {
+      $control->setPlaceholder($placeholder);
+    }
+
+    return $control;
   }
 
   protected function getValueForControl() {