Page MenuHomePhabricator

D12379.diff
No OneTemporary

D12379.diff

diff --git a/src/applications/owners/controller/PhabricatorOwnersListController.php b/src/applications/owners/controller/PhabricatorOwnersListController.php
--- a/src/applications/owners/controller/PhabricatorOwnersListController.php
+++ b/src/applications/owners/controller/PhabricatorOwnersListController.php
@@ -153,7 +153,7 @@
$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()] =
diff --git a/src/applications/ponder/query/PonderQuestionQuery.php b/src/applications/ponder/query/PonderQuestionQuery.php
--- a/src/applications/ponder/query/PonderQuestionQuery.php
+++ b/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 @@
return $this;
}
- public function setOrder($order) {
- $this->order = $order;
- return $this;
- }
-
private function buildWhereClause(AphrontDatabaseConnection $conn_r) {
$where = array();
@@ -110,17 +101,6 @@
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 @@
$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);
diff --git a/src/applications/repository/query/PhabricatorRepositoryQuery.php b/src/applications/repository/query/PhabricatorRepositoryQuery.php
--- a/src/applications/repository/query/PhabricatorRepositoryQuery.php
+++ b/src/applications/repository/query/PhabricatorRepositoryQuery.php
@@ -23,12 +23,6 @@
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 @@
return $this;
}
- public function setOrder($order) {
- switch ($order) {
- case self::ORDER_CREATED:
- $this->setOrderVector(array('id'));
- break;
- case self::ORDER_COMMITTED:
- $this->setOrderVector(array('committed', 'id'));
- break;
- case self::ORDER_CALLSIGN:
- $this->setOrderVector(array('callsign'));
- break;
- case self::ORDER_NAME:
- $this->setOrderVector(array('name', 'id'));
- break;
- case self::ORDER_SIZE:
- $this->setOrderVector(array('size', 'id'));
- break;
- default:
- throw new Exception(pht('Unknown order "%s".', $order));
- }
- return $this;
+ public function getBuiltinOrders() {
+ return array(
+ 'committed' => array(
+ 'vector' => array('committed', 'id'),
+ 'name' => pht('Most Recent Commit'),
+ ),
+ 'name' => array(
+ 'vector' => array('name', 'id'),
+ 'name' => pht('Name'),
+ ),
+ 'callsign' => array(
+ 'vector' => array('callsign'),
+ 'name' => pht('Callsign'),
+ ),
+ 'size' => array(
+ 'vector' => array('size', 'id'),
+ 'name' => pht('Size'),
+ ),
+ ) + parent::getBuiltinOrders();
}
public function getIdentifierMap() {
diff --git a/src/applications/repository/query/PhabricatorRepositorySearchEngine.php b/src/applications/repository/query/PhabricatorRepositorySearchEngine.php
--- a/src/applications/repository/query/PhabricatorRepositorySearchEngine.php
+++ b/src/applications/repository/query/PhabricatorRepositorySearchEngine.php
@@ -27,6 +27,7 @@
public function buildQueryFromSavedQuery(PhabricatorSavedQuery $saved) {
$query = id(new PhabricatorRepositoryQuery())
+ ->setDefaultBuiltinOrder()
->needProjectPHIDs(true)
->needCommitCounts(true)
->needMostRecentCommits(true);
@@ -43,11 +44,8 @@
}
$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 @@
$name,
isset($types[$key]));
}
+ $form->appendChild($type_control);
- $form
- ->appendChild($type_control)
- ->appendChild(
- id(new AphrontFormSelectControl())
- ->setName('order')
- ->setLabel(pht('Order'))
- ->setValue($saved_query->getParameter('order'))
- ->setOptions($this->getOrderOptions()));
+ $this->appendOrderFieldsToForm(
+ $form,
+ $saved_query,
+ new PhabricatorRepositoryQuery());
}
protected function getURI($path) {
@@ -180,26 +175,6 @@
);
}
- 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'),
diff --git a/src/applications/search/engine/PhabricatorApplicationSearchEngine.php b/src/applications/search/engine/PhabricatorApplicationSearchEngine.php
--- a/src/applications/search/engine/PhabricatorApplicationSearchEngine.php
+++ b/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 @@
}
+/* -( 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 )--------------------------------------- */
diff --git a/src/infrastructure/query/policy/PhabricatorCursorPagedPolicyAwareQuery.php b/src/infrastructure/query/policy/PhabricatorCursorPagedPolicyAwareQuery.php
--- a/src/infrastructure/query/policy/PhabricatorCursorPagedPolicyAwareQuery.php
+++ b/src/infrastructure/query/policy/PhabricatorCursorPagedPolicyAwareQuery.php
@@ -17,6 +17,7 @@
protected $applicationSearchOrders = array();
private $internalPaging;
private $orderVector;
+ private $builtinOrder;
protected function getPagingValue($result) {
if (!is_object($result)) {
@@ -427,6 +428,134 @@
/**
+ * 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 @@
/**
+ * 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.

File Metadata

Mime Type
text/plain
Expires
May 12 2024, 3:46 AM (5 w, 10 h ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6288399
Default Alt Text
D12379.diff (13 KB)

Event Timeline