Improve browsability of People query

Summary: Ref T5750. Fix this query so it works better with the browse view. In particular, make it return results before the user types a query.

Test Plan:
  - Used a people tokenizer.
  - Used global search tokenizer to search for people.
  - Ran new version of query in production to make sure it wasn't crazy.
  - Browsed through results in browse view.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T5750

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

src/applications/people/query/PhabricatorPeopleQuery.php

@@ -15,6 +15,7 @@ final class PhabricatorPeopleQuery
   private $isDisabled;
   private $isApproved;
   private $nameLike;
+  private $nameTokens;
 
   private $needPrimaryEmail;
   private $needProfile;
@@ -81,6 +82,11 @@ final class PhabricatorPeopleQuery
     return $this;
   }
 
+  public function withNameTokens(array $tokens) {
+    $this->nameTokens = array_values($tokens);
+    return $this;
+  }
+
   public function needPrimaryEmail($need) {
     $this->needPrimaryEmail = $need;
     return $this;
@@ -111,7 +117,7 @@ final class PhabricatorPeopleQuery
       $table->getTableName(),
       $this->buildJoinsClause($conn_r),
       $this->buildWhereClause($conn_r),
-      $this->buildApplicationSearchGroupClause($conn_r),
+      $this->buildGroupClause($conn_r),
       $this->buildOrderClause($conn_r),
       $this->buildLimitClause($conn_r));
 
@@ -180,6 +186,16 @@ final class PhabricatorPeopleQuery
     return $users;
   }
 
+  private function buildGroupClause(AphrontDatabaseConnection $conn) {
+    if ($this->nameTokens) {
+      return qsprintf(
+        $conn,
+        'GROUP BY user.id');
+    } else {
+      return $this->buildApplicationSearchGroupClause($conn);
+    }
+  }
+
   private function buildJoinsClause($conn_r) {
     $joins = array();
 
@@ -191,6 +207,20 @@ final class PhabricatorPeopleQuery
         $email_table->getTableName());
     }
 
+    if ($this->nameTokens) {
+      foreach ($this->nameTokens as $key => $token) {
+        $token_table = 'token_'.$key;
+        $joins[] = qsprintf(
+          $conn_r,
+          'JOIN %T %T ON %T.userID = user.id AND %T.token LIKE %>',
+          PhabricatorUser::NAMETOKEN_TABLE,
+          $token_table,
+          $token_table,
+          $token_table,
+          $token);
+      }
+    }
+
     $joins[] = $this->buildApplicationSearchJoinClause($conn_r);
 
     $joins = implode(' ', $joins);
@@ -296,4 +326,25 @@ final class PhabricatorPeopleQuery
     return 'PhabricatorPeopleApplication';
   }
 
+  public function getOrderableColumns() {
+    return parent::getOrderableColumns() + array(
+      'username' => array(
+        'table' => 'user',
+        'column' => 'username',
+        'type' => 'string',
+        'reverse' => true,
+        'unique' => true,
+      ),
+    );
+  }
+
+  public function getPagingValueMap($cursor, array $keys) {
+    $user = $this->loadCursorObject($cursor);
+    return array(
+      'id' => $user->getID(),
+      'username' => $user->getUsername(),
+    );
+  }
+
+
 }

src/applications/people/typeahead/PhabricatorPeopleDatasource.php

@@ -25,64 +25,17 @@ final class PhabricatorPeopleDatasource
 
   public function loadResults() {
     $viewer = $this->getViewer();
-    $raw_query = $this->getRawQuery();
+    $tokens = $this->getTokens();
 
-    $results = array();
+    $query = id(new PhabricatorPeopleQuery())
+      ->setOrderVector(array('username'));
 
-    $users = array();
-    if (strlen($raw_query)) {
-      // This is an arbitrary limit which is just larger than any limit we
-      // actually use in the application.
-
-      // TODO: The datasource should pass this in the query.
-      $limit = 15;
-
-      $user_table = new PhabricatorUser();
-      $conn_r = $user_table->establishConnection('r');
-      $ids = queryfx_all(
-        $conn_r,
-        'SELECT id FROM %T WHERE username LIKE %>
-          ORDER BY username ASC LIMIT %d',
-        $user_table->getTableName(),
-        $raw_query,
-        $limit);
-      $ids = ipull($ids, 'id');
-
-      if (count($ids) < $limit) {
-        // If we didn't find enough username hits, look for real name hits.
-        // We need to pull the entire pagesize so that we end up with the
-        // right number of items if this query returns many duplicate IDs
-        // that we've already selected.
-
-        $realname_ids = queryfx_all(
-          $conn_r,
-          'SELECT DISTINCT userID FROM %T WHERE token LIKE %>
-            ORDER BY token ASC LIMIT %d',
-          PhabricatorUser::NAMETOKEN_TABLE,
-          $raw_query,
-          $limit);
-        $realname_ids = ipull($realname_ids, 'userID');
-        $ids = array_merge($ids, $realname_ids);
-
-        $ids = array_unique($ids);
-        $ids = array_slice($ids, 0, $limit);
-      }
-
-      // Always add the logged-in user because some tokenizers autosort them
-      // first. They'll be filtered out on the client side if they don't
-      // match the query.
-      if ($viewer->getID()) {
-        $ids[] = $viewer->getID();
-      }
-
-      if ($ids) {
-        $users = id(new PhabricatorPeopleQuery())
-          ->setViewer($viewer)
-          ->withIDs($ids)
-          ->execute();
-      }
+    if ($tokens) {
+      $query->withNameTokens($tokens);
     }
 
+    $users = $this->executeQuery($query);
+
     if ($this->enrichResults && $users) {
       $phids = mpull($users, 'getPHID');
       $handles = id(new PhabricatorHandleQuery())
@@ -91,6 +44,7 @@ final class PhabricatorPeopleDatasource
         ->execute();
     }
 
+    $results = array();
     foreach ($users as $user) {
       $closed = null;
       if ($user->getIsDisabled()) {

src/applications/typeahead/datasource/PhabricatorTypeaheadCompositeDatasource.php

@@ -28,12 +28,14 @@ abstract class PhabricatorTypeaheadCompositeDatasource
     }
 
     $results = array_mergev($results);
-    $results = msort($results, 'getName');
+    $results = msort($results, 'getSortKey');
 
+    $count = count($results);
     if ($offset || $limit) {
       if (!$limit) {
         $limit = count($results);
       }
+
       $results = array_slice($results, $offset, $limit, $preserve_keys = true);
     }
 

src/applications/typeahead/datasource/PhabricatorTypeaheadDatasource.php

@@ -88,4 +88,18 @@ abstract class PhabricatorTypeaheadDatasource extends Phobject {
     return array_unique($tokens);
   }
 
+  public function getTokens() {
+    return self::tokenizeString($this->getRawQuery());
+  }
+
+  protected function executeQuery(
+    PhabricatorCursorPagedPolicyAwareQuery $query) {
+
+    return $query
+      ->setViewer($this->getViewer())
+      ->setOffset($this->getOffset())
+      ->setLimit($this->getLimit())
+      ->execute();
+  }
+
 }

src/applications/typeahead/storage/PhabricatorTypeaheadResult.php

@@ -85,6 +85,10 @@ final class PhabricatorTypeaheadResult {
     return $this->phid;
   }
 
+  public function getSortKey() {
+    return phutil_utf8_strtolower($this->getName());
+  }
+
   public function getWireFormat() {
     $data = array(
       $this->name,