Make owners typeahead mostly reasonable

Summary: Ref T8320. Fixes T8427. This is still a little funky because Owners has weird name rules, but should fix the bugs (unselectable packages) in T8427.

Test Plan: Browsed Owners typaheads, used various search functions.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T8320, T8427

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

src/applications/owners/query/PhabricatorOwnersPackageQuery.php

@@ -7,6 +7,7 @@ final class PhabricatorOwnersPackageQuery
   private $phids;
   private $ownerPHIDs;
   private $repositoryPHIDs;
+  private $namePrefix;
 
   /**
    * Owners are direct owners, and members of owning projects.
@@ -31,62 +32,59 @@ final class PhabricatorOwnersPackageQuery
     return $this;
   }
 
-  protected function loadPage() {
-    $table = new PhabricatorOwnersPackage();
-    $conn_r = $table->establishConnection('r');
-
-    $data = queryfx_all(
-      $conn_r,
-      'SELECT p.* FROM %T p %Q %Q %Q %Q',
-      $table->getTableName(),
-      $this->buildJoinClause($conn_r),
-      $this->buildWhereClause($conn_r),
-      $this->buildOrderClause($conn_r),
-      $this->buildLimitClause($conn_r));
-
-    return $table->loadAllFromArray($data);
+  public function withNamePrefix($prefix) {
+    $this->namePrefix = $prefix;
+    return $this;
   }
 
-  protected function buildJoinClause(AphrontDatabaseConnection $conn_r) {
-    $joins = array();
+  public function newResultObject() {
+    return new PhabricatorOwnersPackage();
+  }
+
+  protected function loadPage() {
+    return $this->loadStandardPage(new PhabricatorOwnersPackage());
+  }
+
+  protected function buildJoinClauseParts(AphrontDatabaseConnection $conn) {
+    $joins = parent::buildJoinClauseParts($conn);
 
     if ($this->ownerPHIDs !== null) {
       $joins[] = qsprintf(
-        $conn_r,
+        $conn,
         'JOIN %T o ON o.packageID = p.id',
         id(new PhabricatorOwnersOwner())->getTableName());
     }
 
     if ($this->repositoryPHIDs !== null) {
       $joins[] = qsprintf(
-        $conn_r,
+        $conn,
         'JOIN %T rpath ON rpath.packageID = p.id',
         id(new PhabricatorOwnersPath())->getTableName());
     }
 
-    return implode(' ', $joins);
+    return $joins;
   }
 
-  protected function buildWhereClause(AphrontDatabaseConnection $conn_r) {
-    $where = array();
+  protected function buildWhereClauseParts(AphrontDatabaseConnection $conn) {
+    $where = parent::buildWhereClauseParts($conn);
 
     if ($this->phids !== null) {
       $where[] = qsprintf(
-        $conn_r,
+        $conn,
         'p.phid IN (%Ls)',
         $this->phids);
     }
 
     if ($this->ids !== null) {
       $where[] = qsprintf(
-        $conn_r,
+        $conn,
         'p.id IN (%Ld)',
         $this->ids);
     }
 
     if ($this->repositoryPHIDs !== null) {
       $where[] = qsprintf(
-        $conn_r,
+        $conn,
         'rpath.repositoryPHID IN (%Ls)',
         $this->repositoryPHIDs);
     }
@@ -94,26 +92,79 @@ final class PhabricatorOwnersPackageQuery
     if ($this->ownerPHIDs !== null) {
       $base_phids = $this->ownerPHIDs;
 
-      $query = new PhabricatorProjectQuery();
-      $query->setViewer($this->getViewer());
-      $query->withMemberPHIDs($base_phids);
-      $projects = $query->execute();
+      $projects = id(new PhabricatorProjectQuery())
+        ->setViewer($this->getViewer())
+        ->withMemberPHIDs($base_phids)
+        ->execute();
       $project_phids = mpull($projects, 'getPHID');
 
       $all_phids = array_merge($base_phids, $project_phids);
 
       $where[] = qsprintf(
-        $conn_r,
+        $conn,
         'o.userPHID IN (%Ls)',
         $all_phids);
     }
 
-    $where[] = $this->buildPagingClause($conn_r);
-    return $this->formatWhereClause($where);
+    if (strlen($this->namePrefix)) {
+      // NOTE: This is a hacky mess, but this column is currently case
+      // sensitive and unique.
+      $where[] = qsprintf(
+        $conn,
+        'LOWER(p.name) LIKE %>',
+        phutil_utf8_strtolower($this->namePrefix));
+    }
+
+    return $where;
+  }
+
+  protected function shouldGroupQueryResultRows() {
+    if ($this->repositoryPHIDs) {
+      return true;
+    }
+
+    if ($this->ownerPHIDs) {
+      return true;
+    }
+
+    return parent::shouldGroupQueryResultRows();
+  }
+
+  public function getBuiltinOrders() {
+    return array(
+      'name' => array(
+        'vector' => array('name'),
+        'name' => pht('Name'),
+      ),
+    ) + parent::getBuiltinOrders();
+  }
+
+  public function getOrderableColumns() {
+    return parent::getOrderableColumns() + array(
+      'name' => array(
+        'table' => $this->getPrimaryTableAlias(),
+        'column' => 'name',
+        'type' => 'string',
+        'unique' => true,
+        'reverse' => true,
+      ),
+    );
+  }
+
+  protected function getPagingValueMap($cursor, array $keys) {
+    $package = $this->loadCursorObject($cursor);
+    return array(
+      'id' => $package->getID(),
+      'name' => $package->getName(),
+    );
   }
 
   public function getQueryApplicationClass() {
     return 'PhabricatorOwnersApplication';
   }
 
+  protected function getPrimaryTableAlias() {
+    return 'p';
+  }
+
 }

src/applications/owners/query/PhabricatorOwnersPackageSearchEngine.php

@@ -11,68 +11,39 @@ final class PhabricatorOwnersPackageSearchEngine
     return 'PhabricatorOwnersApplication';
   }
 
-  public function buildSavedQueryFromRequest(AphrontRequest $request) {
-    $saved = new PhabricatorSavedQuery();
-
-    $saved->setParameter(
-      'ownerPHIDs',
-      $this->readUsersFromRequest(
-        $request,
-        'owners',
-        array(
-          PhabricatorProjectProjectPHIDType::TYPECONST,
-        )));
-
-    $saved->setParameter(
-      'repositoryPHIDs',
-      $this->readPHIDsFromRequest(
-        $request,
-        'repositories',
-        array(
-          PhabricatorRepositoryRepositoryPHIDType::TYPECONST,
-        )));
-
-    return $saved;
+  public function newQuery() {
+    return new PhabricatorOwnersPackageQuery();
   }
 
-  public function buildQueryFromSavedQuery(PhabricatorSavedQuery $saved) {
-    $query = id(new PhabricatorOwnersPackageQuery());
+  protected function buildCustomSearchFields() {
+    return array(
+      id(new PhabricatorSearchDatasourceField())
+        ->setLabel(pht('Owners'))
+        ->setKey('ownerPHIDs')
+        ->setAliases(array('owner', 'owners'))
+        ->setDatasource(new PhabricatorProjectOrUserDatasource()),
+      id(new PhabricatorSearchDatasourceField())
+        ->setLabel(pht('Repositories'))
+        ->setKey('repositoryPHIDs')
+        ->setAliases(array('repository', 'repositories'))
+        ->setDatasource(new DiffusionRepositoryDatasource()),
+    );
+  }
 
-    $owner_phids = $saved->getParameter('ownerPHIDs', array());
-    if ($owner_phids) {
-      $query->withOwnerPHIDs($owner_phids);
+  protected function buildQueryFromParameters(array $map) {
+    $query = $this->newQuery();
+
+    if ($map['ownerPHIDs']) {
+      $query->withOwnerPHIDs($map['ownerPHIDs']);
     }
 
-    $repository_phids = $saved->getParameter('repositoryPHIDs', array());
-    if ($repository_phids) {
-      $query->withRepositoryPHIDs($repository_phids);
+    if ($map['repositoryPHIDs']) {
+      $query->withRepositoryPHIDs($map['repositoryPHIDs']);
     }
 
     return $query;
   }
 
-  public function buildSearchForm(
-    AphrontFormView $form,
-    PhabricatorSavedQuery $saved) {
-
-    $owner_phids = $saved->getParameter('ownerPHIDs', array());
-    $repository_phids = $saved->getParameter('repositoryPHIDs', array());
-
-    $form
-      ->appendControl(
-        id(new AphrontFormTokenizerControl())
-          ->setDatasource(new PhabricatorProjectOrUserDatasource())
-          ->setName('owners')
-          ->setLabel(pht('Owners'))
-          ->setValue($owner_phids))
-      ->appendControl(
-        id(new AphrontFormTokenizerControl())
-          ->setDatasource(new DiffusionRepositoryDatasource())
-          ->setName('repositories')
-          ->setLabel(pht('Repositories'))
-          ->setValue($repository_phids));
-  }
-
   protected function getURI($path) {
     return '/owners/'.$path;
   }

src/applications/owners/typeahead/PhabricatorOwnersPackageDatasource.php

@@ -3,11 +3,6 @@
 final class PhabricatorOwnersPackageDatasource
   extends PhabricatorTypeaheadDatasource {
 
-  public function isBrowsable() {
-    // TODO: Make this browsable.
-    return false;
-  }
-
   public function getBrowseTitle() {
     return pht('Browse Packages');
   }
@@ -26,10 +21,11 @@ final class PhabricatorOwnersPackageDatasource
 
     $results = array();
 
-    $packages = id(new PhabricatorOwnersPackageQuery())
-      ->setViewer($viewer)
-      ->execute();
+    $query = id(new PhabricatorOwnersPackageQuery())
+      ->withNamePrefix($raw_query)
+      ->setOrder('name');
 
+    $packages = $this->executeQuery($query);
     foreach ($packages as $package) {
       $results[] = id(new PhabricatorTypeaheadResult())
         ->setName($package->getName())
@@ -37,7 +33,7 @@ final class PhabricatorOwnersPackageDatasource
         ->setPHID($package->getPHID());
     }
 
-    return $results;
+    return $this->filterResultsAgainstTokens($results);
   }
 
 }