Provide "builtin" high-level result orders

Summary:
Ref T7803. Currently, available high-level orders are spread across Query and SearchEngine classes and implemented separately for each application.

Lift the concept of "builtin" (high-level, user-facing, named) orders (similar to "builtin" queries in ApplicationSearch) into the root Query class, and let it drive the SearchEngine implementation. This allows you to define a new order in one place and have it automatically work across the entire stack.

This will also let Conduit expose this information in a straightforward way.

Test Plan:
  - Used ApplicationSearch in Diffusion.
  - Used all result orderings.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7803

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

src/applications/owners/controller/PhabricatorOwnersListController.php

@@ -153,7 +153,7 @@ final class PhabricatorOwnersListController
     $callsigns = array('' => pht('(Any Repository)'));
     $repositories = id(new PhabricatorRepositoryQuery())
       ->setViewer($user)
-      ->setOrder(PhabricatorRepositoryQuery::ORDER_CALLSIGN)
+      ->setOrder('callsign')
       ->execute();
     foreach ($repositories as $repository) {
       $callsigns[$repository->getCallsign()] =

src/applications/ponder/query/PonderQuestionQuery.php

@@ -3,14 +3,10 @@
 final class PonderQuestionQuery
   extends PhabricatorCursorPagedPolicyAwareQuery {
 
-  const ORDER_CREATED = 'order-created';
-  const ORDER_HOTTEST = 'order-hottest';
-
   private $ids;
   private $phids;
   private $authorPHIDs;
   private $answererPHIDs;
-  private $order = self::ORDER_CREATED;
 
   private $status = 'status-any';
   const STATUS_ANY      = 'status-any';
@@ -55,11 +51,6 @@ final class PonderQuestionQuery
     return $this;
   }
 
-  public function setOrder($order) {
-    $this->order = $order;
-    return $this;
-  }
-
   private function buildWhereClause(AphrontDatabaseConnection $conn_r) {
     $where = array();
 
@@ -110,17 +101,6 @@ final class PonderQuestionQuery
     return $this->formatWhereClause($where);
   }
 
-  private function buildOrderByClause(AphrontDatabaseConnection $conn_r) {
-    switch ($this->order) {
-      case self::ORDER_HOTTEST:
-        return qsprintf($conn_r, 'ORDER BY q.heat DESC, q.id DESC');
-      case self::ORDER_CREATED:
-        return qsprintf($conn_r, 'ORDER BY q.id DESC');
-      default:
-        throw new Exception("Unknown order '{$this->order}'!");
-    }
-  }
-
   protected function loadPage() {
     $question = new PonderQuestion();
     $conn_r = $question->establishConnection('r');
@@ -131,7 +111,7 @@ final class PonderQuestionQuery
       $question->getTableName(),
       $this->buildJoinsClause($conn_r),
       $this->buildWhereClause($conn_r),
-      $this->buildOrderByClause($conn_r),
+      $this->buildOrderClause($conn_r),
       $this->buildLimitClause($conn_r));
 
     return $question->loadAllFromArray($data);

src/applications/repository/query/PhabricatorRepositoryQuery.php

@@ -23,12 +23,6 @@ final class PhabricatorRepositoryQuery
   const STATUS_ALL = 'status-all';
   private $status = self::STATUS_ALL;
 
-  const ORDER_CREATED = 'order-created';
-  const ORDER_COMMITTED = 'order-committed';
-  const ORDER_CALLSIGN = 'order-callsign';
-  const ORDER_NAME = 'order-name';
-  const ORDER_SIZE = 'order-size';
-
   const HOSTED_PHABRICATOR = 'hosted-phab';
   const HOSTED_REMOTE = 'hosted-remote';
   const HOSTED_ALL = 'hosted-all';
@@ -124,27 +118,25 @@ final class PhabricatorRepositoryQuery
     return $this;
   }
 
-  public function setOrder($order) {
+  public function getBuiltinOrders() {
-    switch ($order) {
+    return array(
-      case self::ORDER_CREATED:
+      'committed' => array(
-        $this->setOrderVector(array('id'));
+        'vector' => array('committed', 'id'),
-        break;
+        'name' => pht('Most Recent Commit'),
-      case self::ORDER_COMMITTED:
+      ),
-        $this->setOrderVector(array('committed', 'id'));
+      'name' => array(
-        break;
+        'vector' => array('name', 'id'),
-      case self::ORDER_CALLSIGN:
+        'name' => pht('Name'),
-        $this->setOrderVector(array('callsign'));
+      ),
-        break;
+      'callsign' => array(
-      case self::ORDER_NAME:
+        'vector' => array('callsign'),
-        $this->setOrderVector(array('name', 'id'));
+        'name' => pht('Callsign'),
-        break;
+      ),
-      case self::ORDER_SIZE:
+      'size' => array(
-        $this->setOrderVector(array('size', 'id'));
+        'vector' => array('size', 'id'),
-        break;
+        'name' => pht('Size'),
-      default:
+      ),
-        throw new Exception(pht('Unknown order "%s".', $order));
+    ) + parent::getBuiltinOrders();
-    }
-    return $this;
   }
 
   public function getIdentifierMap() {

src/applications/repository/query/PhabricatorRepositorySearchEngine.php

@@ -27,6 +27,7 @@ final class PhabricatorRepositorySearchEngine
 
   public function buildQueryFromSavedQuery(PhabricatorSavedQuery $saved) {
     $query = id(new PhabricatorRepositoryQuery())
+      ->setDefaultBuiltinOrder()
       ->needProjectPHIDs(true)
       ->needCommitCounts(true)
       ->needMostRecentCommits(true);
@@ -43,11 +44,8 @@ final class PhabricatorRepositorySearchEngine
     }
 
     $order = $saved->getParameter('order');
-    $order = idx($this->getOrderValues(), $order);
     if ($order) {
       $query->setOrder($order);
-    } else {
-      $query->setOrder(head($this->getOrderValues()));
     }
 
     $hosted = $saved->getParameter('hosted');
@@ -125,15 +123,12 @@ final class PhabricatorRepositorySearchEngine
         $name,
         isset($types[$key]));
     }
+    $form->appendChild($type_control);
 
-    $form
+    $this->appendOrderFieldsToForm(
-      ->appendChild($type_control)
+      $form,
-      ->appendChild(
+      $saved_query,
-        id(new AphrontFormSelectControl())
+      new PhabricatorRepositoryQuery());
-          ->setName('order')
-          ->setLabel(pht('Order'))
-          ->setValue($saved_query->getParameter('order'))
-          ->setOptions($this->getOrderOptions()));
   }
 
   protected function getURI($path) {
@@ -180,26 +175,6 @@ final class PhabricatorRepositorySearchEngine
     );
   }
 
-  private function getOrderOptions() {
-    return array(
-      'committed' => pht('Most Recent Commit'),
-      'name' => pht('Name'),
-      'callsign' => pht('Callsign'),
-      'created' => pht('Date Created'),
-      'size' => pht('Commit Count'),
-    );
-  }
-
-  private function getOrderValues() {
-    return array(
-      'committed' => PhabricatorRepositoryQuery::ORDER_COMMITTED,
-      'name' => PhabricatorRepositoryQuery::ORDER_NAME,
-      'callsign' => PhabricatorRepositoryQuery::ORDER_CALLSIGN,
-      'created' => PhabricatorRepositoryQuery::ORDER_CREATED,
-      'size' => PhabricatorRepositoryQuery::ORDER_SIZE,
-    );
-  }
-
   private function getHostedOptions() {
     return array(
       '' => pht('Hosted and Remote Repositories'),

src/applications/search/engine/PhabricatorApplicationSearchEngine.php

@@ -9,6 +9,7 @@
  * @task builtin    Builtin Queries
  * @task uri        Query URIs
  * @task dates      Date Filters
+ * @task order      Result Ordering
  * @task read       Reading Utilities
  * @task exec       Paging and Executing Queries
  * @task render     Rendering Results
@@ -577,6 +578,24 @@ abstract class PhabricatorApplicationSearchEngine {
   }
 
 
+/* -(  Result Ordering  )---------------------------------------------------- */
+
+  protected function appendOrderFieldsToForm(
+    AphrontFormView $form,
+    PhabricatorSavedQuery $saved,
+    PhabricatorCursorPagedPolicyAwareQuery $query) {
+
+    $orders = $query->getBuiltinOrders();
+    $orders = ipull($orders, 'name');
+
+    $form->appendControl(
+      id(new AphrontFormSelectControl())
+        ->setLabel(pht('Order'))
+        ->setName('order')
+        ->setOptions($orders)
+        ->setValue($saved->getParameter('order')));
+  }
+
 /* -(  Paging and Executing Queries  )--------------------------------------- */
 
 

src/infrastructure/query/policy/PhabricatorCursorPagedPolicyAwareQuery.php

@@ -17,6 +17,7 @@ abstract class PhabricatorCursorPagedPolicyAwareQuery
   protected $applicationSearchOrders = array();
   private $internalPaging;
   private $orderVector;
+  private $builtinOrder;
 
   protected function getPagingValue($result) {
     if (!is_object($result)) {
@@ -427,6 +428,134 @@ abstract class PhabricatorCursorPagedPolicyAwareQuery
 
 
   /**
+   * Select a result ordering.
+   *
+   * This is a high-level method which selects an ordering from a predefined
+   * list of builtin orders, as provided by @{method:getBuiltinOrders}. These
+   * options are user-facing and not exhaustive, but are generally convenient
+   * and meaningful.
+   *
+   * You can also use @{method:setOrderVector} to specify a low-level ordering
+   * across individual orderable columns. This offers greater control but is
+   * also more involved.
+   *
+   * @param string Key of a builtin order supported by this query.
+   * @return this
+   * @task order
+   */
+  public function setOrder($order) {
+    $orders = $this->getBuiltinOrders();
+
+    if (empty($orders[$order])) {
+      throw new Exception(
+        pht(
+          'Query "%s" does not support a builtin order "%s". Supported orders '.
+          'are: %s.',
+          get_class($this),
+          $order,
+          implode(', ', array_keys($orders))));
+    }
+
+    $this->builtinOrder = $order;
+    $this->orderVector = null;
+
+    return $this;
+  }
+
+
+  /**
+   * Select the default builtin result ordering.
+   *
+   * This sets the result order to the default order among the builtin result
+   * orders (see @{method:getBuiltinOrders}). This is often the same as the
+   * query's builtin default order vector, but some objects have different
+   * default vectors (which are internally-facing) and builtin orders (which
+   * are user-facing).
+   *
+   * For example, repositories sort by ID internally (which is efficient and
+   * consistent), but sort by most recent commit as a default builtin (which
+   * better aligns with user expectations).
+   *
+   * @return this
+   */
+  public function setDefaultBuiltinOrder() {
+    return $this->setOrder(head_key($this->getBuiltinOrders()));
+  }
+
+
+  /**
+   * Get builtin orders for this class.
+   *
+   * In application UIs, we want to be able to present users with a small
+   * selection of meaningful order options (like "Order by Title") rather than
+   * an exhaustive set of column ordering options.
+   *
+   * Meaningful user-facing orders are often really orders across multiple
+   * columns: for example, a "title" ordering is usually implemented as a
+   * "title, id" ordering under the hood.
+   *
+   * Builtin orders provide a mapping from convenient, understandable
+   * user-facing orders to implementations.
+   *
+   * A builtin order should provide these keys:
+   *
+   *   - `vector` (`list<string>`): The actual order vector to use.
+   *   - `name` (`string`): Human-readable order name.
+   *
+   * @return map<string, wild> Map from builtin order keys to specification.
+   * @task order
+   */
+  public function getBuiltinOrders() {
+    $orders = array(
+      'newest' => array(
+        'vector' => array('id'),
+        'name' => pht('Creation (Newest First)'),
+        'aliases' => array('created'),
+      ),
+      'oldest' => array(
+        'vector' => array('-id'),
+        'name' => pht('Creation (Oldest First)'),
+      ),
+    );
+
+    $object = $this->newResultObject();
+    if ($object instanceof PhabricatorCustomFieldInterface) {
+      $list = PhabricatorCustomField::getObjectFields(
+        $object,
+        PhabricatorCustomField::ROLE_APPLICATIONSEARCH);
+      foreach ($list->getFields() as $field) {
+        $index = $field->buildOrderIndex();
+        if (!$index) {
+          continue;
+        }
+
+        $key = $field->getFieldKey();
+        $digest = $field->getFieldIndex();
+
+        $full_key = 'custom:'.$key;
+        $orders[$full_key] = array(
+          'vector' => array($full_key, 'id'),
+          'name' => $field->getFieldName(),
+        );
+      }
+    }
+
+    return $orders;
+  }
+
+
+  /**
+   * Set a low-level column ordering.
+   *
+   * This is a low-level method which offers granular control over column
+   * ordering. In most cases, applications can more easily use
+   * @{method:setOrder} to choose a high-level builtin order.
+   *
+   * To set an order vector, specify a list of order keys as provided by
+   * @{method:getOrderableColumns}.
+   *
+   * @param PhabricatorQueryOrderVector|list<string> List of order keys.
+   * @return this
    * @task order
    */
   public function setOrderVector($vector) {
@@ -485,11 +614,19 @@ abstract class PhabricatorCursorPagedPolicyAwareQuery
 
 
   /**
+   * Get the effective order vector.
+   *
+   * @return PhabricatorQueryOrderVector Effective vector.
    * @task order
    */
   protected function getOrderVector() {
     if (!$this->orderVector) {
-      $vector = $this->getDefaultOrderVector();
+      if ($this->builtinOrder !== null) {
+        $builtin_order = idx($this->getBuiltinOrders(), $this->builtinOrder);
+        $vector = $builtin_order['vector'];
+      } else {
+        $vector = $this->getDefaultOrderVector();
+      }
       $vector = PhabricatorQueryOrderVector::newFromVector($vector);
 
       // We call setOrderVector() here to apply checks to the default vector.