diff --git a/src/applications/maniphest/query/ManiphestTaskQuery.php b/src/applications/maniphest/query/ManiphestTaskQuery.php --- a/src/applications/maniphest/query/ManiphestTaskQuery.php +++ b/src/applications/maniphest/query/ManiphestTaskQuery.php @@ -572,6 +572,8 @@ } private function buildCustomOrderClause(AphrontDatabaseConnection $conn) { + $reverse = ($this->getBeforeID() xor $this->getReversePaging()); + $order = array(); switch ($this->groupBy) { @@ -593,33 +595,35 @@ throw new Exception("Unknown group query '{$this->groupBy}'!"); } - switch ($this->orderBy) { - case self::ORDER_PRIORITY: - $order[] = 'priority'; - $order[] = 'subpriority'; - $order[] = 'dateModified'; - break; - case self::ORDER_CREATED: - $order[] = 'id'; - break; - case self::ORDER_MODIFIED: - $order[] = 'dateModified'; - break; - case self::ORDER_TITLE: - $order[] = 'title'; - break; - default: - throw new Exception("Unknown order query '{$this->orderBy}'!"); + $app_order = $this->buildApplicationSearchOrders($conn, $reverse); + + if (!$app_order) { + switch ($this->orderBy) { + case self::ORDER_PRIORITY: + $order[] = 'priority'; + $order[] = 'subpriority'; + $order[] = 'dateModified'; + break; + case self::ORDER_CREATED: + $order[] = 'id'; + break; + case self::ORDER_MODIFIED: + $order[] = 'dateModified'; + break; + case self::ORDER_TITLE: + $order[] = 'title'; + break; + default: + throw new Exception("Unknown order query '{$this->orderBy}'!"); + } } $order = array_unique($order); - if (empty($order)) { + if (empty($order) && empty($app_order)) { return null; } - $reverse = ($this->getBeforeID() xor $this->getReversePaging()); - foreach ($order as $k => $column) { switch ($column) { case 'subpriority': @@ -653,6 +657,18 @@ } } + if ($app_order) { + foreach ($app_order as $order_by) { + $order[] = $order_by; + } + + if ($reverse) { + $order[] = 'task.id ASC'; + } else { + $order[] = 'task.id DESC'; + } + } + return 'ORDER BY '.implode(', ', $order); } @@ -903,55 +919,65 @@ throw new Exception("Unknown group query '{$this->groupBy}'!"); } - switch ($this->orderBy) { - case self::ORDER_PRIORITY: - if ($this->groupBy != self::GROUP_PRIORITY) { + $app_columns = $this->buildApplicationSearchPagination($conn_r, $cursor); + if ($app_columns) { + $columns = array_merge($columns, $app_columns); + $columns[] = array( + 'name' => 'task.id', + 'value' => (int)$cursor->getID(), + 'type' => 'int', + ); + } else { + switch ($this->orderBy) { + case self::ORDER_PRIORITY: + if ($this->groupBy != self::GROUP_PRIORITY) { + $columns[] = array( + 'name' => 'task.priority', + 'value' => (int)$cursor->getPriority(), + 'type' => 'int', + ); + } $columns[] = array( - 'name' => 'task.priority', - 'value' => (int)$cursor->getPriority(), + 'name' => 'task.subpriority', + 'value' => (int)$cursor->getSubpriority(), 'type' => 'int', + 'reverse' => true, ); - } - $columns[] = array( - 'name' => 'task.subpriority', - 'value' => (int)$cursor->getSubpriority(), - 'type' => 'int', - 'reverse' => true, - ); - $columns[] = array( - 'name' => 'task.dateModified', - 'value' => (int)$cursor->getDateModified(), - 'type' => 'int', - ); - break; - case self::ORDER_CREATED: - $columns[] = array( - 'name' => 'task.id', - 'value' => (int)$cursor->getID(), - 'type' => 'int', - ); - break; - case self::ORDER_MODIFIED: - $columns[] = array( - 'name' => 'task.dateModified', - 'value' => (int)$cursor->getDateModified(), - 'type' => 'int', - ); - break; - case self::ORDER_TITLE: - $columns[] = array( - 'name' => 'task.title', - 'value' => $cursor->getTitle(), - 'type' => 'string', - ); - $columns[] = array( - 'name' => 'task.id', - 'value' => $cursor->getID(), - 'type' => 'int', - ); - break; - default: - throw new Exception("Unknown order query '{$this->orderBy}'!"); + $columns[] = array( + 'name' => 'task.dateModified', + 'value' => (int)$cursor->getDateModified(), + 'type' => 'int', + ); + break; + case self::ORDER_CREATED: + $columns[] = array( + 'name' => 'task.id', + 'value' => (int)$cursor->getID(), + 'type' => 'int', + ); + break; + case self::ORDER_MODIFIED: + $columns[] = array( + 'name' => 'task.dateModified', + 'value' => (int)$cursor->getDateModified(), + 'type' => 'int', + ); + break; + case self::ORDER_TITLE: + $columns[] = array( + 'name' => 'task.title', + 'value' => $cursor->getTitle(), + 'type' => 'string', + ); + $columns[] = array( + 'name' => 'task.id', + 'value' => $cursor->getID(), + 'type' => 'int', + ); + break; + default: + throw new Exception("Unknown order query '{$this->orderBy}'!"); + } } return $this->buildPagingClauseFromMultipleColumns( diff --git a/src/applications/maniphest/query/ManiphestTaskSearchEngine.php b/src/applications/maniphest/query/ManiphestTaskSearchEngine.php --- a/src/applications/maniphest/query/ManiphestTaskSearchEngine.php +++ b/src/applications/maniphest/query/ManiphestTaskSearchEngine.php @@ -151,13 +151,10 @@ $query->withPriorities($priorities); } - $order = $saved->getParameter('order'); - $order = idx($this->getOrderValues(), $order); - if ($order) { - $query->setOrderBy($order); - } else { - $query->setOrderBy(head($this->getOrderValues())); - } + $this->applyOrderByToQuery( + $query, + $this->getOrderValues(), + $saved->getParameter('order')); $group = $saved->getParameter('group'); $group = idx($this->getGroupValues(), $group); @@ -306,6 +303,10 @@ $ids = $saved->getParameter('ids', array()); + $builtin_orders = $this->getOrderOptions(); + $custom_orders = $this->getCustomFieldOrderOptions(); + $all_orders = $builtin_orders + $custom_orders; + $form ->appendChild( id(new AphrontFormTokenizerControl()) @@ -385,7 +386,7 @@ ->setName('order') ->setLabel(pht('Order By')) ->setValue($saved->getParameter('order')) - ->setOptions($this->getOrderOptions())); + ->setOptions($all_orders)); } $form 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 @@ -764,6 +764,65 @@ } } + protected function applyOrderByToQuery( + PhabricatorCursorPagedPolicyAwareQuery $query, + array $standard_values, + $order) { + + if (substr($order, 0, 7) === 'custom:') { + $list = $this->getCustomFieldList(); + if (!$list) { + $query->setOrderBy(head($standard_values)); + return; + } + + foreach ($list->getFields() as $field) { + $key = $this->getKeyForCustomField($field); + + if ($key === $order) { + $index = $field->buildOrderIndex(); + + if ($index === null) { + $query->setOrderBy(head($standard_values)); + return; + } + + $query->withApplicationSearchOrder( + $field, + $index, + false); + break; + } + } + } else { + $order = idx($standard_values, $order); + if ($order) { + $query->setOrderBy($order); + } else { + $query->setOrderBy(head($standard_values)); + } + } + } + + + protected function getCustomFieldOrderOptions() { + $list = $this->getCustomFieldList(); + if (!$list) { + return; + } + + $custom_order = array(); + foreach ($list->getFields() as $field) { + if ($field->shouldAppearInApplicationSearch()) { + if ($field->buildOrderIndex() !== null) { + $key = $this->getKeyForCustomField($field); + $custom_order[$key] = $field->getFieldName(); + } + } + } + + return $custom_order; + } /** * Get a unique key identifying a field. diff --git a/src/infrastructure/customfield/field/PhabricatorCustomField.php b/src/infrastructure/customfield/field/PhabricatorCustomField.php --- a/src/infrastructure/customfield/field/PhabricatorCustomField.php +++ b/src/infrastructure/customfield/field/PhabricatorCustomField.php @@ -621,6 +621,28 @@ /** + * Return an index against which this field can be meaningfully ordered + * against to implement ApplicationSearch. + * + * This should be a single index, normally built using + * @{method:newStringIndex} and @{method:newNumericIndex}. + * + * The value of the index is not used. + * + * Return null from this method if the field can not be ordered. + * + * @return PhabricatorCustomFieldIndexStorage A single index to order by. + * @task appsearch + */ + public function buildOrderIndex() { + if ($this->proxy) { + return $this->proxy->buildOrderIndex(); + } + return null; + } + + + /** * Build a new empty storage object for storing string indexes. Normally, * this should be a concrete subclass of * @{class:PhabricatorCustomFieldStringIndexStorage}. diff --git a/src/infrastructure/customfield/standard/PhabricatorStandardCustomField.php b/src/infrastructure/customfield/standard/PhabricatorStandardCustomField.php --- a/src/infrastructure/customfield/standard/PhabricatorStandardCustomField.php +++ b/src/infrastructure/customfield/standard/PhabricatorStandardCustomField.php @@ -260,6 +260,10 @@ return array(); } + public function buildOrderIndex() { + return null; + } + public function readApplicationSearchValueFromRequest( PhabricatorApplicationSearchEngine $engine, AphrontRequest $request) { diff --git a/src/infrastructure/customfield/standard/PhabricatorStandardCustomFieldBool.php b/src/infrastructure/customfield/standard/PhabricatorStandardCustomFieldBool.php --- a/src/infrastructure/customfield/standard/PhabricatorStandardCustomFieldBool.php +++ b/src/infrastructure/customfield/standard/PhabricatorStandardCustomFieldBool.php @@ -18,6 +18,10 @@ return $indexes; } + public function buildOrderIndex() { + return $this->newNumericIndex(0); + } + public function getValueForStorage() { $value = $this->getFieldValue(); if (strlen($value)) { diff --git a/src/infrastructure/customfield/standard/PhabricatorStandardCustomFieldDate.php b/src/infrastructure/customfield/standard/PhabricatorStandardCustomFieldDate.php --- a/src/infrastructure/customfield/standard/PhabricatorStandardCustomFieldDate.php +++ b/src/infrastructure/customfield/standard/PhabricatorStandardCustomFieldDate.php @@ -18,6 +18,10 @@ return $indexes; } + public function buildOrderIndex() { + return $this->newNumericIndex(0); + } + public function getValueForStorage() { $value = $this->getFieldValue(); if (strlen($value)) { diff --git a/src/infrastructure/customfield/standard/PhabricatorStandardCustomFieldInt.php b/src/infrastructure/customfield/standard/PhabricatorStandardCustomFieldInt.php --- a/src/infrastructure/customfield/standard/PhabricatorStandardCustomFieldInt.php +++ b/src/infrastructure/customfield/standard/PhabricatorStandardCustomFieldInt.php @@ -18,6 +18,10 @@ return $indexes; } + public function buildOrderIndex() { + return $this->newNumericIndex(0); + } + public function getValueForStorage() { $value = $this->getFieldValue(); if (strlen($value)) { 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 @@ -12,6 +12,7 @@ private $afterID; private $beforeID; private $applicationSearchConstraints = array(); + private $applicationSearchOrders = array(); private $internalPaging; protected function getPagingColumn() { @@ -360,6 +361,32 @@ /** + * Order the results by an ApplicationSearch index. + * + * @param PhabricatorCustomField Field to which the index belongs. + * @param PhabricatorCustomFieldIndexStorage Table where the index is stored. + * @param bool True to sort ascending. + * @return this + * @task appsearch + */ + public function withApplicationSearchOrder( + PhabricatorCustomField $field, + PhabricatorCustomFieldIndexStorage $index, + $ascending) { + + $this->applicationSearchOrders[] = array( + 'key' => $field->getFieldKey(), + 'type' => $index->getIndexValueType(), + 'table' => $index->getTableName(), + 'index' => $index->getIndexKey(), + 'ascending' => $ascending, + ); + + return $this; + } + + + /** * Get the name of the query's primary object PHID column, for constructing * JOIN clauses. Normally (and by default) this is just `"phid"`, but if the * query construction requires a table alias it may be something like @@ -535,7 +562,72 @@ } } + foreach ($this->applicationSearchOrders as $key => $order) { + $table = $order['table']; + $alias = 'appsearch_order_'.$key; + $index = $order['index']; + $phid_column = $this->getApplicationSearchObjectPHIDColumn(); + + $joins[] = qsprintf( + $conn_r, + 'JOIN %T %T ON %T.objectPHID = %Q + AND %T.indexKey = %s', + $table, + $alias, + $alias, + $phid_column, + $alias, + $index); + } + return implode(' ', $joins); } + protected function buildApplicationSearchOrders( + AphrontDatabaseConnection $conn_r, + $reverse) { + + $orders = array(); + foreach ($this->applicationSearchOrders as $key => $order) { + $alias = 'appsearch_order_'.$key; + + if ($order['ascending'] xor $reverse) { + $orders[] = qsprintf($conn_r, '%T.indexValue ASC', $alias); + } else { + $orders[] = qsprintf($conn_r, '%T.indexValue DESC', $alias); + } + } + + return $orders; + } + + protected function buildApplicationSearchPagination( + AphrontDatabaseConnection $conn_r, + $cursor) { + + // We have to get the current field values on the cursor object. + $fields = PhabricatorCustomField::getObjectFields( + $cursor, + PhabricatorCustomField::ROLE_APPLICATIONSEARCH); + $fields->setViewer($this->getViewer()); + $fields->readFieldsFromStorage($cursor); + + $fields = mpull($fields->getFields(), null, 'getFieldKey'); + + $columns = array(); + foreach ($this->applicationSearchOrders as $key => $order) { + $alias = 'appsearch_order_'.$key; + + $field = idx($fields, $order['key']); + + $columns[] = array( + 'name' => $alias.'.indexValue', + 'value' => $field->getValueForStorage(), + 'type' => $order['type'], + ); + } + + return $columns; + } + }