Allow Almanac interfaces to be browsed

Summary:
Fixes T10205. Ref T10246. Previously, the issue was that the result set was not ordered, so "More Results" would not have been able to work in a reasonable way if there were more than 100 matching interfaces.

You would have seen 100 interfaces more or less at random, then clicked "more" and gotten 100 more random interfaces.

Now, you would see 100 "a" interfaces, then click more to get the next 100 alphabetical interfaces (say, "b" and "c" interfaces).

Test Plan:
  - Clicked browse when binding an interface.
  - Got a browse dialog.
  - Artificially set query limit to 1, paged through "local" interfaces in an ordered, consistent way.

{F1121313}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10205, T10246

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

src/applications/almanac/query/AlmanacInterfaceQuery.php

@@ -34,19 +34,12 @@ final class AlmanacInterfaceQuery
     return $this;
   }
 
+  public function newResultObject() {
+    return new AlmanacInterface();
+  }
+
   protected function loadPage() {
-    $table = new AlmanacInterface();
-    $conn_r = $table->establishConnection('r');
-
-    $data = queryfx_all(
-      $conn_r,
-      'SELECT * FROM %T %Q %Q %Q',
-      $table->getTableName(),
-      $this->buildWhereClause($conn_r),
-      $this->buildOrderClause($conn_r),
-      $this->buildLimitClause($conn_r));
-
-    return $table->loadAllFromArray($data);
+    return $this->loadStandardPage($this->newResultObject());
   }
 
   protected function willFilterPage(array $interfaces) {
@@ -83,34 +76,34 @@ final class AlmanacInterfaceQuery
     return $interfaces;
   }
 
-  protected function buildWhereClause(AphrontDatabaseConnection $conn_r) {
-    $where = array();
+  protected function buildWhereClauseParts(AphrontDatabaseConnection $conn) {
+    $where = parent::buildWhereClauseParts($conn);
 
     if ($this->ids !== null) {
       $where[] = qsprintf(
-        $conn_r,
-        'id IN (%Ld)',
+        $conn,
+        'interface.id IN (%Ld)',
         $this->ids);
     }
 
     if ($this->phids !== null) {
       $where[] = qsprintf(
-        $conn_r,
-        'phid IN (%Ls)',
+        $conn,
+        'interface.phid IN (%Ls)',
         $this->phids);
     }
 
     if ($this->networkPHIDs !== null) {
       $where[] = qsprintf(
-        $conn_r,
-        'networkPHID IN (%Ls)',
+        $conn,
+        'interface.networkPHID IN (%Ls)',
         $this->networkPHIDs);
     }
 
     if ($this->devicePHIDs !== null) {
       $where[] = qsprintf(
-        $conn_r,
-        'devicePHID IN (%Ls)',
+        $conn,
+        'interface.devicePHID IN (%Ls)',
         $this->devicePHIDs);
     }
 
@@ -118,8 +111,10 @@ final class AlmanacInterfaceQuery
       $parts = array();
       foreach ($this->addresses as $address) {
         $parts[] = qsprintf(
-          $conn_r,
-          '(networkPHID = %s AND address = %s AND port = %d)',
+          $conn,
+          '(interface.networkPHID = %s '.
+            'AND interface.address = %s '.
+            'AND interface.port = %d)',
           $address->getNetworkPHID(),
           $address->getAddress(),
           $address->getPort());
@@ -127,13 +122,77 @@ final class AlmanacInterfaceQuery
       $where[] = implode(' OR ', $parts);
     }
 
-    $where[] = $this->buildPagingClause($conn_r);
+    return $where;
+  }
 
-    return $this->formatWhereClause($where);
+  protected function buildJoinClauseParts(AphrontDatabaseConnection $conn) {
+    $joins = parent::buildJoinClauseParts($conn);
+
+    if ($this->shouldJoinDeviceTable()) {
+      $joins[] = qsprintf(
+        $conn,
+        'JOIN %T device ON device.phid = interface.devicePHID',
+        id(new AlmanacDevice())->getTableName());
+    }
+
+    return $joins;
+  }
+
+  protected function shouldGroupQueryResultRows() {
+    if ($this->shouldJoinDeviceTable()) {
+      return true;
+    }
+
+    return parent::shouldGroupQueryResultRows();
+  }
+
+  private function shouldJoinDeviceTable() {
+    $vector = $this->getOrderVector();
+
+    if ($vector->containsKey('name')) {
+      return true;
+    }
+
+    return false;
+  }
+
+  protected function getPrimaryTableAlias() {
+    return 'interface';
   }
 
   public function getQueryApplicationClass() {
     return 'PhabricatorAlmanacApplication';
   }
 
+  public function getBuiltinOrders() {
+    return array(
+      'name' => array(
+        'vector' => array('name', 'id'),
+        'name' => pht('Device Name'),
+      ),
+    ) + parent::getBuiltinOrders();
+  }
+
+  public function getOrderableColumns() {
+    return parent::getOrderableColumns() + array(
+      'name' => array(
+        'table' => 'device',
+        'column' => 'name',
+        'type' => 'string',
+        'reverse' => true,
+      ),
+    );
+  }
+
+  protected function getPagingValueMap($cursor, array $keys) {
+    $interface = $this->loadCursorObject($cursor);
+
+    $map = array(
+      'id' => $interface->getID(),
+      'name' => $interface->getDevice()->getName(),
+    );
+
+    return $map;
+  }
+
 }

src/applications/almanac/typeahead/AlmanacInterfaceDatasource.php

@@ -3,12 +3,6 @@
 final class AlmanacInterfaceDatasource
   extends PhabricatorTypeaheadDatasource {
 
-  public function isBrowsable() {
-    // TODO: We should make this browsable, but need to make the result set
-    // orderable by device name.
-    return false;
-  }
-
   public function getBrowseTitle() {
     return pht('Browse Interfaces');
   }
@@ -31,10 +25,12 @@ final class AlmanacInterfaceDatasource
       ->execute();
 
     if ($devices) {
-      $interfaces = id(new AlmanacInterfaceQuery())
+      $interface_query = id(new AlmanacInterfaceQuery())
         ->setViewer($viewer)
         ->withDevicePHIDs(mpull($devices, 'getPHID'))
-        ->execute();
+        ->setOrder('name');
+
+      $interfaces = $this->executeQuery($interface_query);
     } else {
       $interfaces = array();
     }

src/applications/typeahead/controller/PhabricatorTypeaheadModularDatasourceController.php

@@ -107,6 +107,8 @@ final class PhabricatorTypeaheadModularDatasourceController
           if (($offset + (2 * $limit)) < $hard_limit) {
             $next_uri = id(new PhutilURI($request->getRequestURI()))
               ->setQueryParam('offset', $offset + $limit)
+              ->setQueryParam('q', $query)
+              ->setQueryParam('raw', $raw_query)
               ->setQueryParam('format', 'html');
 
             $next_link = javelin_tag(