Improve search result listing

Summary:
Make it prettier, paginate, add user pictures, show document types, clean some
stuff up a little. Plenty of room for improvement but this should make it a lot
more useful.

Test Plan:
Here's what the new one looks like:
https://secure.phabricator.com/file/view/PHID-FILE-edce2b83c2e3a121c2b7/

Reviewed By: jungejason
Reviewers: tomo, jungejason, aran, tuomaspelkonen, mroch
Commenters: tomo
CC: aran, tomo, jungejason, epriestley
Differential Revision: 545

src/__celerity_resource_map__.php

@@ -1064,6 +1064,15 @@ celerity_register_resource_map(array(
     ),
     'disk' => '/rsrc/css/core/remarkup.css',
   ),
+  'phabricator-search-results-css' =>
+  array(
+    'uri' => '/res/9a9eeaf2/rsrc/css/application/search/search-results.css',
+    'type' => 'css',
+    'requires' =>
+    array(
+    ),
+    'disk' => '/rsrc/css/application/search/search-results.css',
+  ),
   'phabricator-shaped-request' =>
   array(
     'uri' => '/res/d7ba774e/rsrc/js/application/core/ShapedRequest.js',

src/__phutil_library_map__.php

@@ -497,6 +497,7 @@ phutil_register_library_map(array(
     'PhabricatorSearchMySQLExecutor' => 'applications/search/execute/mysql',
     'PhabricatorSearchQuery' => 'applications/search/storage/query',
     'PhabricatorSearchRelationship' => 'applications/search/constants/relationship',
+    'PhabricatorSearchResultView' => 'applications/search/view/searchresult',
     'PhabricatorSearchSelectController' => 'applications/search/controller/select',
     'PhabricatorSearchUserIndexer' => 'applications/search/index/indexer/user',
     'PhabricatorSetup' => 'infrastructure/setup',
@@ -960,6 +961,7 @@ phutil_register_library_map(array(
     'PhabricatorSearchManiphestIndexer' => 'PhabricatorSearchDocumentIndexer',
     'PhabricatorSearchMySQLExecutor' => 'PhabricatorSearchExecutor',
     'PhabricatorSearchQuery' => 'PhabricatorSearchDAO',
+    'PhabricatorSearchResultView' => 'AphrontView',
     'PhabricatorSearchSelectController' => 'PhabricatorSearchController',
     'PhabricatorSearchUserIndexer' => 'PhabricatorSearchDocumentIndexer',
     'PhabricatorStandardPageView' => 'AphrontPageView',

src/applications/phid/handle/PhabricatorObjectHandle.php

@@ -112,6 +112,17 @@ class PhabricatorObjectHandle {
     return $this->alternateID;
   }
 
+  public function getTypeName() {
+    static $map = array(
+      PhabricatorPHIDConstants::PHID_TYPE_USER => 'User',
+      PhabricatorPHIDConstants::PHID_TYPE_TASK => 'Task',
+      PhabricatorPHIDConstants::PHID_TYPE_DREV => 'Revision',
+      PhabricatorPHIDConstants::PHID_TYPE_CMIT => 'Commit',
+    );
+
+    return idx($map, $this->getType());
+  }
+
   public function renderLink() {
 
     switch ($this->getType()) {

src/applications/phid/handle/__init__.php

@@ -9,6 +9,7 @@
 phutil_require_module('phabricator', 'applications/phid/constants');
 
 phutil_require_module('phutil', 'markup');
+phutil_require_module('phutil', 'utils');
 
 
 phutil_require_source('PhabricatorObjectHandle.php');

src/applications/phid/handle/data/PhabricatorObjectHandleData.php

@@ -24,6 +24,55 @@ class PhabricatorObjectHandleData {
     $this->phids = $phids;
   }
 
+  public function loadObjects() {
+    $types = array();
+    foreach ($this->phids as $phid) {
+      $type = $this->lookupType($phid);
+      $types[$type][] = $phid;
+    }
+
+    $objects = array_fill_keys($this->phids, null);
+    foreach ($types as $type => $phids) {
+      switch ($type) {
+        case PhabricatorPHIDConstants::PHID_TYPE_USER:
+          $user_dao = newv('PhabricatorUser', array());
+          $users = $user_dao->loadAllWhere(
+            'phid in (%Ls)',
+            $phids);
+          foreach ($users as $user) {
+            $objects[$user->getPHID()] = $user;
+          }
+          break;
+        case PhabricatorPHIDConstants::PHID_TYPE_CMIT:
+          $commit_dao = newv('PhabricatorRepositoryCommit', array());
+          $commits = $commit_dao->loadAllWhere(
+            'phid IN (%Ls)',
+            $phids);
+          $commit_data = array();
+          if ($commits) {
+            $data_dao = newv('PhabricatorRepositoryCommitData', array());
+            $commit_data = $data_dao->loadAllWhere(
+              'id IN (%Ld)',
+              mpull($commits, 'getID'));
+            $commit_data = mpull($commit_data, null, 'getCommitID');
+          }
+          foreach ($commits as $commit) {
+            $data = idx($commit_data, $commit->getID());
+            if ($data) {
+              $commit->attachCommitData($data);
+              $objects[$commit->getPHID()] = $commit;
+            } else {
+             // If we couldn't load the commit data, just act as though we
+             // couldn't load the object at all so we don't load half an object.
+            }
+          }
+          break;
+      }
+    }
+
+    return $objects;
+  }
+
   public function loadHandles() {
 
     $types = array();
@@ -157,8 +206,17 @@ class PhabricatorObjectHandleData {
             } else {
               $commit = $commits[$phid];
               $callsign = $callsigns[$repository_ids[$phid]];
+              $repository = $repositories[$repository_ids[$phid]];
               $commit_identifier = $commit->getCommitIdentifier();
-              $handle->setName('Commit '.'r'.$callsign.$commit_identifier);
+
+              $vcs = $repository->getVersionControlSystem();
+              if ($vcs == PhabricatorRepositoryType::REPOSITORY_TYPE_GIT) {
+                $short_identifier = substr($commit_identifier, 0, 16);
+              } else {
+                $short_identifier = $commit_identifier;
+              }
+
+              $handle->setName('r'.$callsign.$short_identifier);
               $handle->setURI('/r'.$callsign.$commit_identifier);
               $handle->setFullName('r'.$callsign.$commit_identifier);
               $handle->setTimestamp($commit->getEpoch());

src/applications/phid/handle/data/__init__.php

@@ -9,6 +9,7 @@
 phutil_require_module('phabricator', 'applications/files/uri');
 phutil_require_module('phabricator', 'applications/phid/constants');
 phutil_require_module('phabricator', 'applications/phid/handle');
+phutil_require_module('phabricator', 'applications/repository/constants/repositorytype');
 phutil_require_module('phabricator', 'applications/repository/storage/repository');
 phutil_require_module('phabricator', 'infrastructure/env');
 

src/applications/repository/storage/commit/PhabricatorRepositoryCommit.php

@@ -23,6 +23,8 @@ class PhabricatorRepositoryCommit extends PhabricatorRepositoryDAO {
   protected $commitIdentifier;
   protected $epoch;
 
+  private $commitData;
+
   public function getConfiguration() {
     return array(
       self::CONFIG_AUX_PHID   => true,
@@ -35,4 +37,16 @@ class PhabricatorRepositoryCommit extends PhabricatorRepositoryDAO {
       PhabricatorPHIDConstants::PHID_TYPE_CMIT);
   }
 
+  public function attachCommitData(PhabricatorRepositoryCommitData $data) {
+    $this->commitData = $data;
+    return $this;
+  }
+
+  public function getCommitData() {
+    if (!$this->commitData) {
+      throw new Exception("Attach commit data with attachCommitData() first!");
+    }
+    return $this->commitData;
+  }
+
 }

src/applications/search/controller/search/PhabricatorSearchController.php

@@ -72,6 +72,7 @@ class PhabricatorSearchController extends PhabricatorSearchBaseController {
       PhabricatorPHIDConstants::PHID_TYPE_DREV => 'Differential Revisions',
       PhabricatorPHIDConstants::PHID_TYPE_CMIT => 'Repository Commits',
       PhabricatorPHIDConstants::PHID_TYPE_TASK => 'Maniphest Tasks',
+      PhabricatorPHIDConstants::PHID_TYPE_USER => 'Phabricator Users',
     ) + $more;
 
     $status_options = array(
@@ -152,34 +153,56 @@ class PhabricatorSearchController extends PhabricatorSearchBaseController {
     $search_panel->setHeader('Search Phabricator');
     $search_panel->appendChild($search_form);
 
+    require_celerity_resource('phabricator-search-results-css');
+
     if ($query->getID()) {
+
+      $limit = 20;
+
+      $pager = new AphrontPagerView();
+      $pager->setURI($request->getRequestURI(), 'page');
+      $pager->setPageSize($limit);
+      $pager->setOffset($request->getInt('page'));
+
+      $query->setParameter('limit', $limit + 1);
+      $query->setParameter('offset', $pager->getOffset());
+
       $executor = new PhabricatorSearchMySQLExecutor();
       $results = $executor->executeSearch($query);
       $results = ipull($results, 'phid');
-      $handles = id(new PhabricatorObjectHandleData($results))
+
-        ->loadHandles();
+      $results = $pager->sliceResults($results);
+
+      if ($results) {
+
+        $loader = new PhabricatorObjectHandleData($results);
+        $handles = $loader->loadHandles();
+        $objects = $loader->loadObjects();
         $results = array();
-      foreach ($handles as $handle) {
+        foreach ($handles as $phid => $handle) {
-        $results[] =
+          $view = new PhabricatorSearchResultView();
-          '<h1 style="font-size: 14px; font-weight: normal; margin: 4px 0 8px;">'.
+          $view->setHandle($handle);
-            phutil_render_tag(
+          $view->setQuery($query);
-              'a',
+          $view->setObject($objects[$phid]);
-              array(
+          $results[] = $view->render();
-                'href' => $handle->getURI(),
-              ),
-              $this->emboldenQuery($handle->getName(), $query->getQuery())).
-          '</h1>';
         }
         $results =
-        '<div style="padding: 1em 3em 2em;">'.
+          '<div class="phabricator-search-result-list">'.
             implode("\n", $results).
+            '<div class="search-results-pager">'.
+              $pager->render().
+            '</div>'.
           '</div>';
+      } else {
+        $results =
+          '<div class="phabricator-search-result-list">'.
+            '<p class="phabricator-search-no-results">No search results.</p>'.
+          '</div>';
+      }
     } else {
       $results = null;
     }
 
-    $results = print_r($results, true);
-
     return $this->buildStandardPageResponse(
       array(
         $search_panel,
@@ -190,18 +213,5 @@ class PhabricatorSearchController extends PhabricatorSearchBaseController {
       ));
   }
 
-  private function emboldenQuery($str, $query) {
-    $query = preg_split("/\s+/", $query);
-    $query = array_filter($query);
-    $str = phutil_escape_html($str);
-    foreach ($query as $word) {
-      $word = phutil_escape_html($word);
-      $str = preg_replace(
-        '/(?:^|\b)('.preg_quote($word, '/').')(?:\b|$)/i',
-        '<strong>\1</strong>',
-        $str);
-    }
-    return $str;
-  }
 
 }

src/applications/search/controller/search/__init__.php

@@ -13,7 +13,10 @@ phutil_require_module('phabricator', 'applications/phid/handle/data');
 phutil_require_module('phabricator', 'applications/search/controller/base');
 phutil_require_module('phabricator', 'applications/search/execute/mysql');
 phutil_require_module('phabricator', 'applications/search/storage/query');
+phutil_require_module('phabricator', 'applications/search/view/searchresult');
+phutil_require_module('phabricator', 'infrastructure/celerity/api');
 phutil_require_module('phabricator', 'infrastructure/env');
+phutil_require_module('phabricator', 'view/control/pager');
 phutil_require_module('phabricator', 'view/form/base');
 phutil_require_module('phabricator', 'view/form/control/select');
 phutil_require_module('phabricator', 'view/form/control/submit');
@@ -21,7 +24,6 @@ phutil_require_module('phabricator', 'view/form/control/text');
 phutil_require_module('phabricator', 'view/form/control/tokenizer');
 phutil_require_module('phabricator', 'view/layout/panel');
 
-phutil_require_module('phutil', 'markup');
 phutil_require_module('phutil', 'utils');
 
 

src/applications/search/execute/mysql/PhabricatorSearchMySQLExecutor.php

@@ -110,12 +110,15 @@ class PhabricatorSearchMySQLExecutor extends PhabricatorSearchExecutor {
         'MATCH(corpus) AGAINST (%s)',
         $q);
 
-//      if ($query->getParameter('order') == AdjutantQuery::ORDER_RELEVANCE) {
+      // When searching for a string, promote user listings above other
+      // listings.
       $order = qsprintf(
         $conn_r,
-          'ORDER BY MAX(MATCH(corpus) AGAINST (%s)) DESC',
+        'ORDER BY
+          IF(documentType = %s, 0, 1) ASC,
+          MAX(MATCH(corpus) AGAINST (%s)) DESC',
+        'USER',
         $q);
-//      }
 
       $field = $query->getParameter('field');
       if ($field/* && $field != AdjutantQuery::FIELD_ALL*/) {
@@ -201,6 +204,9 @@ class PhabricatorSearchMySQLExecutor extends PhabricatorSearchExecutor {
       $where = '';
     }
 
+    $offset = (int)$query->getParameter('offset', 0);
+    $limit  = (int)$query->getParameter('limit', 25);
+
     $hits = queryfx_all(
       $conn_r,
       'SELECT
@@ -213,11 +219,13 @@ class PhabricatorSearchMySQLExecutor extends PhabricatorSearchExecutor {
           %Q
         GROUP BY document.phid
           %Q
-        LIMIT 50',
+        LIMIT %d, %d',
       $t_doc,
       $join,
       $where,
-      $order);
+      $order,
+      $offset,
+      $limit);
 
     return $hits;
   }

src/applications/search/view/searchresult/PhabricatorSearchResultView.php

@@ -0,0 +1,131 @@
+<?php
+
+/*
+ * Copyright 2011 Facebook, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+final class PhabricatorSearchResultView extends AphrontView {
+
+  private $handle;
+  private $query;
+  private $object;
+
+  public function setHandle(PhabricatorObjectHandle $handle) {
+    $this->handle = $handle;
+    return $this;
+  }
+
+  public function setQuery(PhabricatorSearchQuery $query) {
+    $this->query = $query;
+    return $this;
+  }
+
+  public function setObject($object) {
+    $this->object = $object;
+    return $this;
+  }
+
+  public function render() {
+    $handle = $this->handle;
+
+    $type_name = nonempty($handle->getTypeName(), 'Document');
+
+    require_celerity_resource('phabricator-search-results-css');
+
+    $link = phutil_render_tag(
+      'a',
+      array(
+        'href' => $handle->getURI(),
+      ),
+      PhabricatorEnv::getProductionURI($handle->getURI()));
+
+    switch ($handle->getType()) {
+      case PhabricatorPHIDConstants::PHID_TYPE_USER:
+        if ($this->object) {
+          $img_phid = $this->object->getProfileImagePHID();
+          $img = PhabricatorFileURI::getViewURIForPHID($img_phid);
+        }
+        break;
+      default:
+        $img = null;
+        break;
+    }
+
+    if ($img) {
+      $img = phutil_render_tag(
+        'div',
+        array(
+          'class' => 'result-image',
+          'style' => "background-image: url('{$img}');",
+        ),
+        '');
+    }
+
+    switch ($handle->getType()) {
+      case PhabricatorPHIDConstants::PHID_TYPE_CMIT:
+        $object_name = $handle->getName();
+        if ($this->object) {
+          $data = $this->object->getCommitData();
+          $summary = $data->getSummary();
+          if (strlen($summary)) {
+            $object_name = $handle->getName().': '.$data->getSummary();
+          }
+        }
+        break;
+      default:
+        $object_name = $handle->getFullName();
+        break;
+    }
+
+    return
+      '<div class="phabricator-search-result">'.
+        $img.
+        '<div class="result-desc">'.
+          phutil_render_tag(
+            'a',
+            array(
+              'class' => 'result-name',
+              'href' => $handle->getURI(),
+            ),
+            $this->emboldenQuery($object_name)).
+          '<div class="result-type">'.$type_name.' &middot; '.$link.'</div>'.
+        '</div>'.
+        '<div style="clear: both;"></div>'.
+      '</div>';
+  }
+
+  private function emboldenQuery($str) {
+    if (!$this->query) {
+      return phutil_escape_html($str);
+    }
+
+    $query = $this->query->getQuery();
+
+    $query = preg_split("/\s+/", $query);
+    $query = array_filter($query);
+    $str = phutil_escape_html($str);
+    foreach ($query as $word) {
+      $word = phutil_escape_html($word);
+      $str = preg_replace(
+        '/(?:^|\b)('.preg_quote($word, '/').')(?:\b|$)/i',
+        '<strong>\1</strong>',
+        $str);
+    }
+    return $str;
+  }
+
+
+
+}

src/applications/search/view/searchresult/__init__.php

@@ -0,0 +1,19 @@
+<?php
+/**
+ * This file is automatically generated. Lint this module to rebuild it.
+ * @generated
+ */
+
+
+
+phutil_require_module('phabricator', 'applications/files/uri');
+phutil_require_module('phabricator', 'applications/phid/constants');
+phutil_require_module('phabricator', 'infrastructure/celerity/api');
+phutil_require_module('phabricator', 'infrastructure/env');
+phutil_require_module('phabricator', 'view/base');
+
+phutil_require_module('phutil', 'markup');
+phutil_require_module('phutil', 'utils');
+
+
+phutil_require_source('PhabricatorSearchResultView.php');

webroot/rsrc/css/application/search/search-results.css

@@ -0,0 +1,41 @@
+/**
+ * @provides phabricator-search-results-css
+ */
+
+.phabricator-search-result-list {
+  padding: 1em 3em 2em;
+}
+
+.phabricator-search-result {
+  margin: 4px 0 16px;
+}
+
+.phabricator-search-result .result-image {
+  position: relative;
+  float: left;
+  width: 50px;
+  height: 50px;
+}
+
+.phabricator-search-result .result-desc {
+  margin-left: 58px;
+}
+
+.phabricator-search-result .result-type {
+  color: #888888;
+  font-size: 11px;
+  margin-top: 4px;
+}
+
+.phabricator-search-result .result-name {
+  font-size: 16px;
+}
+
+.search-results-pager .aphront-pager-view {
+  text-align: center;
+}
+
+.phabricator-search-no-results {
+  color: #888888;
+  font-size: 18px;
+}