diff --git a/src/applications/maniphest/editor/ManiphestTransactionEditor.php b/src/applications/maniphest/editor/ManiphestTransactionEditor.php --- a/src/applications/maniphest/editor/ManiphestTransactionEditor.php +++ b/src/applications/maniphest/editor/ManiphestTransactionEditor.php @@ -612,12 +612,13 @@ $query = id(new ManiphestTaskQuery()) ->setViewer(PhabricatorUser::getOmnipotentUser()) - ->setOrderBy(ManiphestTaskQuery::ORDER_PRIORITY) ->withPriorities(array($priority)) ->setLimit(1); if ($is_end) { - $query->setReversePaging(true); + $query->setOrderVector(array('-priority', '-subpriority', '-id')); + } else { + $query->setOrderVector(array('priority', 'subpriority', 'id')); } $result = $query->executeOne(); @@ -675,13 +676,18 @@ // Get all of the tasks with the same subpriority as the adjacent // task, including the adjacent task itself. $shift_base = $adjacent->getSubpriority(); - $shift_all = id(new ManiphestTaskQuery()) + $query = id(new ManiphestTaskQuery()) ->setViewer(PhabricatorUser::getOmnipotentUser()) - ->setOrderBy(ManiphestTaskQuery::ORDER_PRIORITY) ->withPriorities(array($adjacent->getPriority())) - ->withSubpriorities(array($shift_base)) - ->setReversePaging(!$is_after) - ->execute(); + ->withSubpriorities(array($shift_base)); + + if (!$is_after) { + $query->setOrderVector(array('-priority', '-subpriority', '-id')); + } else { + $query->setOrderVector(array('priority', 'subpriority', 'id')); + } + + $shift_all = $query->execute(); $shift_last = last($shift_all); // Find the subpriority before or after the task at the end of the 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 @@ -21,7 +21,6 @@ private $dateCreatedBefore; private $dateModifiedAfter; private $dateModifiedBefore; - private $reversePaging; private $fullTextSearch = ''; @@ -54,14 +53,10 @@ private $needSubscriberPHIDs; private $needProjectPHIDs; - private $blockingTasks; private $blockedTasks; - private $projectPolicyCheckFailed = false; - const DEFAULT_PAGE_SIZE = 1000; - public function withAuthors(array $authors) { $this->authorPHIDs = $authors; return $this; @@ -231,6 +226,10 @@ return $this; } + protected function newResultObject() { + return new ManiphestTask(); + } + protected function willExecute() { // Make sure the user can see any projects specified in this // query FIRST. @@ -252,6 +251,67 @@ } $this->projectPHIDs = array_values($this->projectPHIDs); } + + // If we already have an order vector, use it as provided. + // TODO: This is a messy hack to make setOrderVector() stronger than + // setPriority(). + $vector = $this->getOrderVector(); + $keys = mpull(iterator_to_array($vector), 'getOrderKey'); + if (array_values($keys) !== array('id')) { + return; + } + + $parts = array(); + switch ($this->groupBy) { + case self::GROUP_NONE: + break; + case self::GROUP_PRIORITY: + $parts[] = array('priority'); + break; + case self::GROUP_OWNER: + $parts[] = array('owner'); + break; + case self::GROUP_STATUS: + $parts[] = array('status'); + break; + case self::GROUP_PROJECT: + $parts[] = array('project'); + break; + } + + if ($this->applicationSearchOrders) { + $columns = array(); + foreach ($this->applicationSearchOrders as $order) { + $part = 'custom:'.$order['key']; + if ($order['ascending']) { + $part = '-'.$part; + } + $columns[] = $part; + } + $columns[] = 'id'; + $parts[] = $columns; + } else { + switch ($this->orderBy) { + case self::ORDER_PRIORITY: + $parts[] = array('priority', 'subpriority', 'id'); + break; + case self::ORDER_CREATED: + $parts[] = array('id'); + break; + case self::ORDER_MODIFIED: + $parts[] = array('updated', 'id'); + break; + case self::ORDER_TITLE: + $parts[] = array('title', 'id'); + break; + } + } + + $parts = array_mergev($parts); + // We may have a duplicate column if we are both ordering and grouping + // by priority. + $parts = array_unique($parts); + $this->setOrderVector($parts); } protected function loadPage() { @@ -268,7 +328,6 @@ $where[] = $this->buildTaskPHIDsWhereClause($conn); $where[] = $this->buildStatusWhereClause($conn); $where[] = $this->buildStatusesWhereClause($conn); - $where[] = $this->buildPrioritiesWhereClause($conn); $where[] = $this->buildDependenciesWhereClause($conn); $where[] = $this->buildAuthorWhereClause($conn); $where[] = $this->buildOwnerWhereClause($conn); @@ -306,6 +365,20 @@ $this->dateModifiedBefore); } + if ($this->priorities) { + $where[] = qsprintf( + $conn, + 'task.priority IN (%Ld)', + $this->priorities); + } + + if ($this->subpriorities) { + $where[] = qsprintf( + $conn, + 'task.subpriority IN (%Lf)', + $this->subpriorities); + } + $where[] = $this->buildPagingClause($conn); $where = $this->formatWhereClause($where); @@ -325,13 +398,6 @@ count($this->projectPHIDs)); } - $order = $this->buildCustomOrderClause($conn); - - // TODO: Clean up this nonstandardness. - if (!$this->getLimit()) { - $this->setLimit(self::DEFAULT_PAGE_SIZE); - } - $group_column = ''; switch ($this->groupBy) { case self::GROUP_PROJECT: @@ -351,7 +417,7 @@ $where, $this->buildGroupClause($conn), $having, - $order, + $this->buildOrderClause($conn), $this->buildLimitClause($conn)); switch ($this->groupBy) { @@ -504,24 +570,6 @@ return null; } - private function buildPrioritiesWhereClause(AphrontDatabaseConnection $conn) { - if ($this->priorities) { - return qsprintf( - $conn, - 'task.priority IN (%Ld)', - $this->priorities); - } - - if ($this->subpriorities) { - return qsprintf( - $conn, - 'task.subpriority IN (%Lf)', - $this->subpriorities); - } - - return null; - } - private function buildAuthorWhereClause(AphrontDatabaseConnection $conn) { if (!$this->authorPHIDs) { return null; @@ -689,107 +737,6 @@ 'xproject.dst IS NULL'); } - private function buildCustomOrderClause(AphrontDatabaseConnection $conn) { - $reverse = ($this->getBeforeID() xor $this->getReversePaging()); - - $order = array(); - - switch ($this->groupBy) { - case self::GROUP_NONE: - break; - case self::GROUP_PRIORITY: - $order[] = 'task.priority'; - break; - case self::GROUP_OWNER: - $order[] = 'task.ownerOrdering'; - break; - case self::GROUP_STATUS: - $order[] = 'task.status'; - break; - case self::GROUP_PROJECT: - $order[] = ''; - break; - default: - throw new Exception("Unknown group query '{$this->groupBy}'!"); - } - - $app_order = $this->buildApplicationSearchOrders($conn, $reverse); - - if (!$app_order) { - switch ($this->orderBy) { - case self::ORDER_PRIORITY: - $order[] = 'task.priority'; - $order[] = 'task.subpriority'; - $order[] = 'task.dateModified'; - break; - case self::ORDER_CREATED: - $order[] = 'task.id'; - break; - case self::ORDER_MODIFIED: - $order[] = 'task.dateModified'; - break; - case self::ORDER_TITLE: - $order[] = 'task.title'; - break; - default: - throw new Exception("Unknown order query '{$this->orderBy}'!"); - } - } - - $order = array_unique($order); - - if (empty($order) && empty($app_order)) { - return null; - } - - foreach ($order as $k => $column) { - switch ($column) { - case 'subpriority': - case 'ownerOrdering': - case 'title': - if ($reverse) { - $order[$k] = "{$column} DESC"; - } else { - $order[$k] = "{$column} ASC"; - } - break; - case '': - // Put "No Project" at the end of the list. - if ($reverse) { - $order[$k] = - 'projectGroupName.indexedObjectName IS NULL DESC, '. - 'projectGroupName.indexedObjectName DESC'; - } else { - $order[$k] = - 'projectGroupName.indexedObjectName IS NULL ASC, '. - 'projectGroupName.indexedObjectName ASC'; - } - break; - default: - if ($reverse) { - $order[$k] = "{$column} ASC"; - } else { - $order[$k] = "{$column} DESC"; - } - break; - } - } - - 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); - } - private function buildJoinsClause(AphrontDatabaseConnection $conn_r) { $edge_table = PhabricatorEdgeConfig::TABLE_NAME_EDGE; @@ -954,13 +901,10 @@ switch ($this->groupBy) { case self::GROUP_NONE: - return $id; + case self::GROUP_STATUS: case self::GROUP_PRIORITY: - return $id.'.'.$result->getPriority(); case self::GROUP_OWNER: - return rtrim($id.'.'.$result->getOwnerPHID(), '.'); - case self::GROUP_STATUS: - return $id.'.'.$result->getStatus(); + return $id; case self::GROUP_PROJECT: return rtrim($id.'.'.$result->getGroupByProjectPHID(), '.'); default: @@ -968,156 +912,95 @@ } } - protected function buildPagingClause(AphrontDatabaseConnection $conn_r) { - $before_id = $this->getBeforeID(); - $after_id = $this->getAfterID(); - - if (!$before_id && !$after_id) { - return ''; - } + public function getOrderableColumns() { + return parent::getOrderableColumns() + array( + 'priority' => array( + 'table' => 'task', + 'column' => 'priority', + 'type' => 'int', + ), + 'owner' => array( + 'table' => 'task', + 'column' => 'ownerOrdering', + 'null' => 'head', + 'reverse' => true, + 'type' => 'string', + ), + 'status' => array( + 'table' => 'task', + 'column' => 'status', + 'type' => 'string', + 'reverse' => true, + ), + 'project' => array( + 'table' => 'projectGroupName', + 'column' => 'indexedObjectName', + 'type' => 'string', + 'null' => 'head', + 'reverse' => true, + ), + 'title' => array( + 'table' => 'task', + 'column' => 'title', + 'type' => 'string', + 'reverse' => true, + ), + 'subpriority' => array( + 'table' => 'task', + 'column' => 'subpriority', + 'type' => 'float', + ), + 'updated' => array( + 'table' => 'task', + 'column' => 'dateModified', + 'type' => 'int', + ), + ); + } - $cursor_id = nonempty($before_id, $after_id); - $cursor_parts = explode('.', $cursor_id, 2); + protected function getPagingValueMap($cursor, array $keys) { + $cursor_parts = explode('.', $cursor, 2); $task_id = $cursor_parts[0]; $group_id = idx($cursor_parts, 1); - $cursor = $this->loadCursorObject($task_id); - if (!$cursor) { - // We may loop if we have a cursor and don't build a paging clause; fail - // instead. - throw new PhabricatorEmptyQueryException(); - } - - $columns = array(); - - switch ($this->groupBy) { - case self::GROUP_NONE: - break; - case self::GROUP_PRIORITY: - $columns[] = array( - 'table' => 'task', - 'column' => 'priority', - 'value' => (int)$group_id, - 'type' => 'int', - ); - break; - case self::GROUP_OWNER: - $value = null; - if ($group_id) { - $paging_users = id(new PhabricatorPeopleQuery()) - ->setViewer($this->getViewer()) - ->withPHIDs(array($group_id)) - ->execute(); - if ($paging_users) { - $value = head($paging_users)->getUsername(); - } - } - $columns[] = array( - 'table' => 'task', - 'column' => 'ownerOrdering', - 'value' => $value, - 'type' => 'string', - 'null' => 'head', - 'reverse' => true, - ); - break; - case self::GROUP_STATUS: - $columns[] = array( - 'table' => 'task', - 'column' => 'status', - 'value' => $group_id, - 'type' => 'string', - ); - break; - case self::GROUP_PROJECT: - $value = null; - if ($group_id) { - $paging_projects = id(new PhabricatorProjectQuery()) - ->setViewer($this->getViewer()) - ->withPHIDs(array($group_id)) - ->execute(); - if ($paging_projects) { - $value = head($paging_projects)->getName(); - } - } + $task = $this->loadCursorObject($task_id); - $columns[] = array( - 'table' => 'projectGroupName', - 'column' => 'indexedObjectName', - 'value' => $value, - 'type' => 'string', - 'null' => 'head', - 'reverse' => true, - ); - break; - default: - throw new Exception("Unknown group query '{$this->groupBy}'!"); - } + $map = array( + 'id' => $task->getID(), + 'priority' => $task->getPriority(), + 'subpriority' => $task->getSubpriority(), + 'owner' => $task->getOwnerOrdering(), + 'status' => $task->getStatus(), + 'title' => $task->getTitle(), + 'updated' => $task->getDateModified(), + ); - $app_columns = $this->buildApplicationSearchPagination($conn_r, $cursor); - if ($app_columns) { - $columns = array_merge($columns, $app_columns); - } else { - switch ($this->orderBy) { - case self::ORDER_PRIORITY: - if ($this->groupBy != self::GROUP_PRIORITY) { - $columns[] = array( - 'table' => 'task', - 'column' => 'priority', - 'value' => (int)$cursor->getPriority(), - 'type' => 'int', - ); + foreach ($keys as $key) { + switch ($key) { + case 'project': + $value = null; + if ($group_id) { + $paging_projects = id(new PhabricatorProjectQuery()) + ->setViewer($this->getViewer()) + ->withPHIDs(array($group_id)) + ->execute(); + if ($paging_projects) { + $value = head($paging_projects)->getName(); + } } - $columns[] = array( - 'table' => 'task', - 'column' => 'subpriority', - 'value' => $cursor->getSubpriority(), - 'type' => 'float', - ); - $columns[] = array( - 'table' => 'task', - 'column' => 'dateModified', - 'value' => (int)$cursor->getDateModified(), - 'type' => 'int', - ); + $map[$key] = $value; break; - case self::ORDER_CREATED: - // This just uses the ID column, below. - break; - case self::ORDER_MODIFIED: - $columns[] = array( - 'table' => 'task', - 'column' => 'dateModified', - 'value' => (int)$cursor->getDateModified(), - 'type' => 'int', - ); - break; - case self::ORDER_TITLE: - $columns[] = array( - 'table' => 'task', - 'column' => 'title', - 'value' => $cursor->getTitle(), - 'type' => 'string', - ); - break; - default: - throw new Exception("Unknown order query '{$this->orderBy}'!"); } } - $columns[] = array( - 'table' => 'task', - 'column' => 'id', - 'value' => $cursor->getID(), - 'type' => 'int', - ); + foreach ($keys as $key) { + if ($this->isCustomFieldOrderKey($key)) { + $map += $this->getPagingValueMapForCustomFields($task); + break; + } + } - return $this->buildPagingClauseFromMultipleColumns( - $conn_r, - $columns, - array( - 'reversed' => (bool)($before_id xor $this->getReversePaging()), - )); + return $map; } protected function getPrimaryTableAlias() { @@ -1128,13 +1011,4 @@ return 'PhabricatorManiphestApplication'; } - public function setReversePaging($reverse_paging) { - $this->reversePaging = $reverse_paging; - return $this; - } - - protected function getReversePaging() { - return $this->reversePaging; - } - } 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 @@ -14,7 +14,7 @@ private $afterID; private $beforeID; private $applicationSearchConstraints = array(); - private $applicationSearchOrders = array(); + protected $applicationSearchOrders = array(); private $internalPaging; private $orderVector; @@ -161,6 +161,10 @@ return null; } + protected function newResultObject() { + return null; + } + /* -( Paging )------------------------------------------------------------- */ @@ -546,7 +550,7 @@ return array(); } - return array( + $columns = array( 'id' => array( 'table' => $this->getPrimaryTableAlias(), 'column' => 'id', @@ -555,6 +559,32 @@ 'unique' => true, ), ); + + $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; + $columns[$full_key] = array( + 'table' => 'appsearch_order_'.$digest, + 'column' => 'indexValue', + 'type' => $index->getIndexValueType(), + 'null' => 'tail', + ); + } + } + + return $columns; } @@ -961,8 +991,8 @@ foreach ($this->applicationSearchOrders as $key => $order) { $table = $order['table']; - $alias = 'appsearch_order_'.$key; $index = $order['index']; + $alias = 'appsearch_order_'.$index; $phid_column = $this->getApplicationSearchObjectPHIDColumn(); $joins[] = qsprintf( @@ -980,51 +1010,27 @@ 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) { + protected function getPagingValueMapForCustomFields( + PhabricatorCustomFieldInterface $object) { // We have to get the current field values on the cursor object. $fields = PhabricatorCustomField::getObjectFields( - $cursor, + $object, PhabricatorCustomField::ROLE_APPLICATIONSEARCH); $fields->setViewer($this->getViewer()); - $fields->readFieldsFromStorage($cursor); - - $fields = mpull($fields->getFields(), null, 'getFieldKey'); + $fields->readFieldsFromStorage($object); - $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'], - ); + $map = array(); + foreach ($fields->getFields() as $field) { + $map['custom:'.$field->getFieldKey()] = $field->getValueForStorage(); } - return $columns; + return $map; + } + + protected function isCustomFieldOrderKey($key) { + $prefix = 'custom:'; + return !strncmp($key, $prefix, strlen($prefix)); } }