From afed8bb9295580c8e6a1119b8ecbb846cdcc1bf8 Mon Sep 17 00:00:00 2001 From: epriestley Date: Mon, 19 Mar 2012 19:47:34 -0700 Subject: [PATCH] Add "Group by: Project" to Maniphest Summary: Allow tasks to be grouped by project. Since this is many-to-many and we're a little deficient on indexes for doing this on the database, we pull all matching tasks and group them in PHP. This shouldn't be a huge issue for any existing installs, though, and we can add keys when we run into one. - When a task is in multiple projects, it appears under multiple headers. - When a query has a task filter, those projects are omitted from the grouping (they'd always show everything, which isn't useful). Notably, if you search for "Differential", you can now see "Bugs", "Feature Requests", etc. Test Plan: Selected "Group by: Project". Reviewers: btrahan, Josereyes Reviewed By: btrahan CC: aran, epriestley Maniphest Tasks: T923 Differential Revision: https://secure.phabricator.com/D1953 --- .../tasklist/ManiphestTaskListController.php | 37 ++++++++- .../maniphest/query/ManiphestTaskQuery.php | 83 ++++++++++++++++++- src/applications/maniphest/query/__init__.php | 1 + 3 files changed, 117 insertions(+), 4 deletions(-) diff --git a/src/applications/maniphest/controller/tasklist/ManiphestTaskListController.php b/src/applications/maniphest/controller/tasklist/ManiphestTaskListController.php index b9889fcbd2..1cd0ccd3a3 100644 --- a/src/applications/maniphest/controller/tasklist/ManiphestTaskListController.php +++ b/src/applications/maniphest/controller/tasklist/ManiphestTaskListController.php @@ -378,6 +378,7 @@ final class ManiphestTaskListController extends ManiphestController { 'priority' => ManiphestTaskQuery::GROUP_PRIORITY, 'owner' => ManiphestTaskQuery::GROUP_OWNER, 'status' => ManiphestTaskQuery::GROUP_STATUS, + 'project' => ManiphestTaskQuery::GROUP_PROJECT, ); $query->setGroupBy( idx( @@ -392,6 +393,15 @@ final class ManiphestTaskListController extends ManiphestController { $data = $query->execute(); $total_row_count = $query->getRowCount(); + $project_group_phids = array(); + if ($search_query->getParameter('group') == 'project') { + foreach ($data as $task) { + foreach ($task->getProjectPHIDs() as $phid) { + $project_group_phids[] = $phid; + } + } + } + $handle_phids = mpull($data, 'getOwnerPHID'); $handle_phids = array_merge( $handle_phids, @@ -399,14 +409,14 @@ final class ManiphestTaskListController extends ManiphestController { $user_phids, $xproject_phids, $owner_phids, - $author_phids); + $author_phids, + $project_group_phids); $handles = id(new PhabricatorObjectHandleData($handle_phids)) ->loadHandles(); switch ($search_query->getParameter('group')) { case 'priority': $data = mgroup($data, 'getPriority'); - krsort($data); // If we have invalid priorities, they'll all map to "???". Merge // arrays to prevent them from overwriting each other. @@ -423,7 +433,6 @@ final class ManiphestTaskListController extends ManiphestController { break; case 'status': $data = mgroup($data, 'getStatus'); - ksort($data); $out = array(); foreach ($data as $status => $tasks) { @@ -452,6 +461,26 @@ final class ManiphestTaskListController extends ManiphestController { ksort($data); break; + case 'project': + $grouped = array(); + foreach ($data as $task) { + $phids = $task->getProjectPHIDs(); + if ($project_phids) { + // If the user is filtering on "Bugs", don't show a "Bugs" group + // with every result since that's silly (the query also does this + // on the backend). + $phids = array_diff($phids, $project_phids); + } + if ($phids) { + foreach ($phids as $phid) { + $grouped[$handles[$phid]->getName()][$task->getID()] = $task; + } + } else { + $grouped['No Project'][$task->getID()] = $task; + } + } + $data = $grouped; + break; default: $data = array( 'Tasks' => $data, @@ -527,6 +556,7 @@ final class ManiphestTaskListController extends ManiphestController { 'p' => 'priority', 's' => 'status', 'o' => 'owner', + 'j' => 'project', ); if (empty($groups[$group])) { $group = 'p'; @@ -543,6 +573,7 @@ final class ManiphestTaskListController extends ManiphestController { 'p' => 'Priority', 'o' => 'Owner', 's' => 'Status', + 'j' => 'Project', 'n' => 'None', )); diff --git a/src/applications/maniphest/query/ManiphestTaskQuery.php b/src/applications/maniphest/query/ManiphestTaskQuery.php index 861a84b194..c0edae6c0f 100644 --- a/src/applications/maniphest/query/ManiphestTaskQuery.php +++ b/src/applications/maniphest/query/ManiphestTaskQuery.php @@ -46,6 +46,7 @@ final class ManiphestTaskQuery { const GROUP_PRIORITY = 'group-priority'; const GROUP_OWNER = 'group-owner'; const GROUP_STATUS = 'group-status'; + const GROUP_PROJECT = 'group-project'; private $orderBy = 'order-modified'; const ORDER_PRIORITY = 'order-priority'; @@ -224,6 +225,11 @@ final class ManiphestTaskQuery { $offset = (int)nonempty($this->offset, 0); $limit = (int)nonempty($this->limit, self::DEFAULT_PAGE_SIZE); + if ($this->groupBy == self::GROUP_PROJECT) { + $limit = PHP_INT_MAX; + $offset = 0; + } + $data = queryfx_all( $conn, 'SELECT %Q * %Q FROM %T task %Q %Q %Q %Q %Q LIMIT %d, %d', @@ -247,7 +253,13 @@ final class ManiphestTaskQuery { $this->rowCount = null; } - return $task_dao->loadAllFromArray($data); + $tasks = $task_dao->loadAllFromArray($data); + + if ($this->groupBy == self::GROUP_PROJECT) { + $tasks = $this->applyGroupByProject($tasks); + } + + return $tasks; } private function buildTaskIDsWhereClause($conn) { @@ -420,6 +432,10 @@ final class ManiphestTaskQuery { case self::GROUP_STATUS: $order[] = 'status'; break; + case self::GROUP_PROJECT: + // NOTE: We have to load the entire result set and apply this grouping + // in the PHP process for now. + break; default: throw new Exception("Unknown group query '{$this->groupBy}'!"); } @@ -460,4 +476,69 @@ final class ManiphestTaskQuery { } + /** + * To get paging to work for "group by project", we need to do a bunch of + * server-side magic since there's currently no way to sort by project name on + * the database. + * + * TODO: Move this all to the database. + */ + private function applyGroupByProject(array $tasks) { + + $project_phids = array(); + foreach ($tasks as $task) { + foreach ($task->getProjectPHIDs() as $phid) { + $project_phids[$phid] = true; + } + } + + $handles = id(new PhabricatorObjectHandleData(array_keys($project_phids))) + ->loadHandles(); + + $max = 1; + foreach ($handles as $handle) { + $max = max($max, strlen($handle->getName())); + } + + $items = array(); + $ii = 0; + foreach ($tasks as $key => $task) { + $phids = $task->getProjectPHIDs(); + if ($this->projectPHIDs) { + $phids = array_diff($phids, $this->projectPHIDs); + } + if ($phids) { + foreach ($phids as $phid) { + $items[] = array( + 'key' => $key, + 'seq' => sprintf( + '%'.$max.'s%d', + $handles[$phid]->getName(), + $ii), + ); + } + } else { + // Sort "no project" tasks first. + $items[] = array( + 'key' => $key, + 'seq' => '', + ); + } + ++$ii; + } + + $items = isort($items, 'seq'); + $items = array_slice( + $items, + nonempty($this->offset), + nonempty($this->limit, self::DEFAULT_PAGE_SIZE)); + + $result = array(); + foreach ($items as $item) { + $result[] = $tasks[$item['key']]; + } + + return $result; + } + } diff --git a/src/applications/maniphest/query/__init__.php b/src/applications/maniphest/query/__init__.php index 86a637a392..bbd8f47297 100644 --- a/src/applications/maniphest/query/__init__.php +++ b/src/applications/maniphest/query/__init__.php @@ -10,6 +10,7 @@ phutil_require_module('phabricator', 'applications/maniphest/constants/owner'); phutil_require_module('phabricator', 'applications/maniphest/storage/subscriber'); phutil_require_module('phabricator', 'applications/maniphest/storage/task'); phutil_require_module('phabricator', 'applications/maniphest/storage/taskproject'); +phutil_require_module('phabricator', 'applications/phid/handle/data'); phutil_require_module('phabricator', 'storage/qsprintf'); phutil_require_module('phabricator', 'storage/queryfx');