diff --git a/CHANGELOG b/CHANGELOG index 09b5bdd0a9..171beb5130 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -2,6 +2,11 @@ This is not a complete list of changes, just of API or workflow changes that may break existing installs. Newer changes are listed at the top. If you pull new changes and things stop working, check here first! +June 29 2011 - Maniphest project indexes + Old Maniphest tasks will not appear in project filter views until you run + "scripts/search/reindex_maniphest.php" to build indexes. New tasks will have + their indexes built automatically. + May 31 2011 - Javelin submodule moved The externals/javelin submodule location has moved. If you have an older checkout of Phabricator, you may need to edit .git/config to point at diff --git a/resources/sql/patches/051.projectfilter.sql b/resources/sql/patches/051.projectfilter.sql new file mode 100644 index 0000000000..42554d01a7 --- /dev/null +++ b/resources/sql/patches/051.projectfilter.sql @@ -0,0 +1,6 @@ +CREATE TABLE phabricator_maniphest.maniphest_taskproject ( + taskPHID varchar(64) BINARY NOT NULL, + projectPHID varchar(64) BINARY NOT NULL, + PRIMARY KEY (projectPHID, taskPHID), + UNIQUE KEY (taskPHID, projectPHID) +); diff --git a/scripts/search/reindex_maniphest.php b/scripts/search/reindex_maniphest.php new file mode 100755 index 0000000000..62550d56d2 --- /dev/null +++ b/scripts/search/reindex_maniphest.php @@ -0,0 +1,32 @@ +#!/usr/bin/env php +loadAll(); +echo "Updating relationships for ".count($tasks)." tasks"; +foreach ($tasks as $task) { + ManiphestTaskProject::updateTaskProjects($task); + echo '.'; +} +echo "\nDone.\n"; + diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 271fcc39f5..d050187ca1 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -272,6 +272,8 @@ phutil_register_library_map(array( 'ManiphestTaskListView' => 'applications/maniphest/view/tasklist', 'ManiphestTaskOwner' => 'applications/maniphest/constants/owner', 'ManiphestTaskPriority' => 'applications/maniphest/constants/priority', + 'ManiphestTaskProject' => 'applications/maniphest/storage/taskproject', + 'ManiphestTaskQuery' => 'applications/maniphest/query', 'ManiphestTaskStatus' => 'applications/maniphest/constants/status', 'ManiphestTaskSummaryView' => 'applications/maniphest/view/tasksummary', 'ManiphestTransaction' => 'applications/maniphest/storage/transaction', @@ -765,6 +767,7 @@ phutil_register_library_map(array( 'ManiphestTaskEditController' => 'ManiphestController', 'ManiphestTaskListController' => 'ManiphestController', 'ManiphestTaskListView' => 'AphrontView', + 'ManiphestTaskProject' => 'ManiphestDAO', 'ManiphestTaskSummaryView' => 'AphrontView', 'ManiphestTransaction' => 'ManiphestDAO', 'ManiphestTransactionDetailView' => 'AphrontView', diff --git a/src/applications/maniphest/controller/tasklist/ManiphestTaskListController.php b/src/applications/maniphest/controller/tasklist/ManiphestTaskListController.php index 72250e2d1a..0c0dee7d1f 100644 --- a/src/applications/maniphest/controller/tasklist/ManiphestTaskListController.php +++ b/src/applications/maniphest/controller/tasklist/ManiphestTaskListController.php @@ -33,23 +33,30 @@ class ManiphestTaskListController extends ManiphestController { $uri = $request->getRequestURI(); if ($request->isFormPost()) { - $phid_arr = $request->getArr('view_user'); - $view_target = head($phid_arr); - return id(new AphrontRedirectResponse()) - ->setURI($request->getRequestURI()->alter('phid', $view_target)); - } + // Redirect to GET so URIs can be copy/pasted. + $user_phids = $request->getArr('set_users'); + $proj_phids = $request->getArr('set_projects'); + $user_phids = implode(',', $user_phids); + $proj_phids = implode(',', $proj_phids); + $user_phids = nonempty($user_phids, null); + $proj_phids = nonempty($proj_phids, null); + + $uri = $request->getRequestURI() + ->alter('users', $user_phids) + ->alter('projects', $proj_phids); + + return id(new AphrontRedirectResponse())->setURI($uri); + } $views = array( 'User Tasks', 'action' => 'Assigned', 'created' => 'Created', 'triage' => 'Need Triage', -// 'touched' => 'Touched', '
', 'All Tasks', 'alltriage' => 'Need Triage', - 'unassigned' => 'Unassigned', 'all' => 'All Tasks', ); @@ -77,7 +84,7 @@ class ManiphestTaskListController extends ManiphestController { phutil_render_tag( 'a', array( - 'href' => $uri, + 'href' => $uri->alter('page', null), 'class' => ($this->view == $view) ? 'aphront-side-nav-selected' : null, @@ -90,13 +97,26 @@ class ManiphestTaskListController extends ManiphestController { list($grouping, $group_links) = $this->renderGroupLinks(); list($order, $order_links) = $this->renderOrderLinks(); - $view_phid = nonempty($request->getStr('phid'), $user->getPHID()); + $user_phids = $request->getStr('users'); + if (strlen($user_phids)) { + $user_phids = explode(',', $user_phids); + } else { + $user_phids = array($user->getPHID()); + } + + $project_phids = $request->getStr('projects'); + if (strlen($project_phids)) { + $project_phids = explode(',', $project_phids); + } else { + $project_phids = array(); + } $page = $request->getInt('page'); $page_size = self::DEFAULT_PAGE_SIZE; list($tasks, $handles, $total_count) = $this->loadTasks( - $view_phid, + $user_phids, + $project_phids, array( 'status' => $status_map, 'group' => $grouping, @@ -105,24 +125,34 @@ class ManiphestTaskListController extends ManiphestController { 'limit' => $page_size, )); - $form = id(new AphrontFormView()) - ->setUser($user); + ->setUser($user) + ->setAction($request->getRequestURI()); if (isset($has_filter[$this->view])) { + $tokens = array(); + foreach ($user_phids as $phid) { + $tokens[$phid] = $handles[$phid]->getFullName(); + } $form->appendChild( id(new AphrontFormTokenizerControl()) - ->setLimit(1) ->setDatasource('/typeahead/common/searchowner/') - ->setName('view_user') - ->setLabel('View User') - ->setCaption('Use "upforgrabs" to find unassigned tasks.') - ->setValue( - array( - $view_phid => $handles[$view_phid]->getFullName(), - ))); + ->setName('set_users') + ->setLabel('Users') + ->setValue($tokens)); } + $tokens = array(); + foreach ($project_phids as $phid) { + $tokens[$phid] = $handles[$phid]->getFullName(); + } + $form->appendChild( + id(new AphrontFormTokenizerControl()) + ->setDatasource('/typeahead/common/projects/') + ->setName('set_projects') + ->setLabel('Projects') + ->setValue($tokens)); + $form ->appendChild( id(new AphrontFormToggleButtonsControl()) @@ -137,6 +167,10 @@ class ManiphestTaskListController extends ManiphestController { ->setLabel('Order') ->setValue($order_links)); + $form->appendChild( + id(new AphrontFormSubmitControl()) + ->setValue('Filter Tasks')); + $filter = new AphrontListFilterView(); $filter->addButton( phutil_render_tag( @@ -209,141 +243,71 @@ class ManiphestTaskListController extends ManiphestController { )); } - private function loadTasks($view_phid, array $dict) { - $phids = array($view_phid); + private function loadTasks( + array $user_phids, + array $project_phids, + array $dict) { - $include_upforgrabs = false; - foreach ($phids as $key => $phid) { - if ($phid == ManiphestTaskOwner::OWNER_UP_FOR_GRABS) { - unset($phids[$key]); - $include_upforgrabs = true; - } - } - - $task = new ManiphestTask(); - - $argv = array(); + $query = new ManiphestTaskQuery(); + $query->withProjects($project_phids); $status = $dict['status']; if (!empty($status['open']) && !empty($status['closed'])) { - $status_clause = '1 = 1'; + $query->withStatus(ManiphestTaskQuery::STATUS_ANY); } else if (!empty($status['open'])) { - $status_clause = 'status = %d'; - $argv[] = 0; + $query->withStatus(ManiphestTaskQuery::STATUS_OPEN); } else { - $status_clause = 'status > %d'; - $argv[] = 0; + $query->withStatus(ManiphestTaskQuery::STATUS_CLOSED); } - $extra_clause = '1 = 1'; switch ($this->view) { case 'action': - $parts = array(); - if ($phids) { - $parts[] = 'ownerPHID in (%Ls)'; - $argv[] = $phids; - } - if ($include_upforgrabs) { - $parts[] = 'ownerPHID IS NULL'; - } - $extra_clause = '('.implode(' OR ', $parts).')'; + $query->withOwners($user_phids); break; case 'created': - $parts = array(); - if ($phids) { - $parts[] = 'authorPHID in (%Ls)'; - $argv[] = $phids; - } - if ($include_upforgrabs) { - // This should be impossible since every task is supposed to have a - // valid author, but we might as well run the query. - $parts[] = 'authorPHID IS NULL'; - } - $extra_clause = '('.implode(' OR ', $parts).')'; + $query->withAuthors($user_phids); break; case 'triage': - $parts = array(); - if ($phids) { - $parts[] = 'ownerPHID in (%Ls)'; - $argv[] = $phids; - } - if ($include_upforgrabs) { - $parts[] = 'ownerPHID IS NULL'; - } - $extra_clause = '('.implode(' OR ', $parts).') AND priority = %d'; - $argv[] = ManiphestTaskPriority::PRIORITY_TRIAGE; + $query->withOwners($user_phids); + $query->withPriority(ManiphestTaskPriority::PRIORITY_TRIAGE); break; case 'alltriage': - $extra_clause = 'priority = %d'; - $argv[] = ManiphestTaskPriority::PRIORITY_TRIAGE; - break; - case 'unassigned': - $extra_clause = 'ownerPHID is NULL'; + $query->withPriority(ManiphestTaskPriority::PRIORITY_TRIAGE); break; case 'all': break; } - $order = array(); - switch ($dict['group']) { - case 'priority': - $order[] = 'priority'; - break; - case 'owner': - $order[] = 'ownerOrdering'; - break; - case 'status': - $order[] = 'status'; - break; - } + $order_map = array( + 'priority' => ManiphestTaskQuery::ORDER_PRIORITY, + 'created' => ManiphestTaskQuery::ORDER_CREATED, + ); + $query->setOrderBy( + idx( + $order_map, + $dict['order'], + ManiphestTaskQuery::ORDER_MODIFIED)); - switch ($dict['order']) { - case 'priority': - $order[] = 'priority'; - $order[] = 'dateModified'; - break; - case 'created': - $order[] = 'id'; - break; - default: - $order[] = 'dateModified'; - break; - } + $group_map = array( + 'priority' => ManiphestTaskQuery::GROUP_PRIORITY, + 'owner' => ManiphestTaskQuery::GROUP_OWNER, + 'status' => ManiphestTaskQuery::GROUP_STATUS, + ); + $query->setGroupBy( + idx( + $group_map, + $dict['group'], + ManiphestTaskQuery::GROUP_NONE)); - $order = array_unique($order); + $query->setCalculateRows(true); + $query->setLimit($dict['limit']); + $query->setOffset($dict['offset']); - foreach ($order as $k => $column) { - switch ($column) { - case 'ownerOrdering': - $order[$k] = "{$column} ASC"; - break; - default: - $order[$k] = "{$column} DESC"; - break; - } - } - - $order = implode(', ', $order); - - $offset = (int)idx($dict, 'offset', 0); - $limit = (int)idx($dict, 'limit', self::DEFAULT_PAGE_SIZE); - - $sql = "SELECT SQL_CALC_FOUND_ROWS * FROM %T WHERE ". - "({$status_clause}) AND ({$extra_clause}) ORDER BY {$order} ". - "LIMIT {$offset}, {$limit}"; - - array_unshift($argv, $task->getTableName()); - - $conn = $task->establishConnection('r'); - $data = vqueryfx_all($conn, $sql, $argv); - - $total_row_count = queryfx_one($conn, 'SELECT FOUND_ROWS() N'); - $total_row_count = $total_row_count['N']; - - $data = $task->loadAllFromArray($data); + $data = $query->execute(); + $total_row_count = $query->getRowCount(); $handle_phids = mpull($data, 'getOwnerPHID'); - $handle_phids[] = $view_phid; + $handle_phids = array_merge($handle_phids, $project_phids, $user_phids); $handles = id(new PhabricatorObjectHandleData($handle_phids)) ->loadHandles(); diff --git a/src/applications/maniphest/controller/tasklist/__init__.php b/src/applications/maniphest/controller/tasklist/__init__.php index 16e6296085..28c7ec5a4c 100644 --- a/src/applications/maniphest/controller/tasklist/__init__.php +++ b/src/applications/maniphest/controller/tasklist/__init__.php @@ -7,17 +7,16 @@ phutil_require_module('phabricator', 'aphront/response/redirect'); -phutil_require_module('phabricator', 'applications/maniphest/constants/owner'); phutil_require_module('phabricator', 'applications/maniphest/constants/priority'); phutil_require_module('phabricator', 'applications/maniphest/constants/status'); phutil_require_module('phabricator', 'applications/maniphest/controller/base'); -phutil_require_module('phabricator', 'applications/maniphest/storage/task'); +phutil_require_module('phabricator', 'applications/maniphest/query'); phutil_require_module('phabricator', 'applications/maniphest/view/tasklist'); phutil_require_module('phabricator', 'applications/phid/handle/data'); phutil_require_module('phabricator', 'infrastructure/celerity/api'); -phutil_require_module('phabricator', 'storage/queryfx'); phutil_require_module('phabricator', 'view/control/pager'); phutil_require_module('phabricator', 'view/form/base'); +phutil_require_module('phabricator', 'view/form/control/submit'); phutil_require_module('phabricator', 'view/form/control/togglebuttons'); phutil_require_module('phabricator', 'view/form/control/tokenizer'); phutil_require_module('phabricator', 'view/layout/listfilter'); diff --git a/src/applications/maniphest/query/ManiphestTaskQuery.php b/src/applications/maniphest/query/ManiphestTaskQuery.php new file mode 100644 index 0000000000..d7f874f1eb --- /dev/null +++ b/src/applications/maniphest/query/ManiphestTaskQuery.php @@ -0,0 +1,351 @@ +authorPHIDs = $authors; + return $this; + } + + public function withOwners(array $owners) { + $this->includeUnowned = false; + foreach ($owners as $k => $phid) { + if ($phid == ManiphestTaskOwner::OWNER_UP_FOR_GRABS) { + $this->includeUnowned = true; + unset($owners[$k]); + break; + } + } + $this->ownerPHIDs = $owners; + return $this; + } + + public function withProjects(array $projects) { + $this->projectPHIDs = $projects; + return $this; + } + + public function withStatus($status) { + $this->status = $status; + return $this; + } + + public function withPriority($priority) { + $this->priority = $priority; + return $this; + } + + public function setGroupBy($group) { + $this->groupBy = $group; + return $this; + } + + public function setOrderBy($order) { + $this->orderBy = $order; + return $this; + } + + public function setLimit($limit) { + $this->limit = $limit; + return $this; + } + + public function setOffset($offset) { + $this->offset = $offset; + return $this; + } + + public function setCalculateRows($calculate_rows) { + $this->calculateRows = $calculate_rows; + return $this; + } + + public function getRowCount() { + if ($this->rowCount === null) { + throw new Exception( + "You must execute a query with setCalculateRows() before you can ". + "retrieve a row count."); + } + return $this->rowCount; + } + + public function execute() { + + $task_dao = new ManiphestTask(); + $conn = $task_dao->establishConnection('r'); + + if ($this->calculateRows) { + $calc = 'SQL_CALC_FOUND_ROWS'; + } else { + $calc = ''; + } + + $where = array(); + $where[] = $this->buildStatusWhereClause($conn); + $where[] = $this->buildPriorityWhereClause($conn); + $where[] = $this->buildAuthorWhereClause($conn); + $where[] = $this->buildOwnerWhereClause($conn); + $where[] = $this->buildProjectWhereClause($conn); + + $where = array_filter($where); + if ($where) { + $where = 'WHERE ('.implode(') AND (', $where).')'; + } else { + $where = ''; + } + + $join = array(); + $join[] = $this->buildProjectJoinClause($conn); + + $join = array_filter($join); + if ($join) { + $join = implode(' ', $join); + } else { + $join = ''; + } + + $having = ''; + $count = ''; + $group = ''; + if (count($this->projectPHIDs) > 1) { + + // If we're searching for more than one project: + // - We'll get multiple rows for tasks when they join the project table + // multiple times. We use GROUP BY to make them distinct again. + // - We want to treat the query as an intersection query, not a union + // query. We sum the project count and require it be the same as the + // number of projects we're searching for. + + $group = 'GROUP BY task.id'; + $count = ', COUNT(1) projectCount'; + $having = qsprintf( + $conn, + 'HAVING projectCount = %d', + count($this->projectPHIDs)); + } + + $order = $this->buildOrderClause($conn); + + $offset = (int)nonempty($this->offset, 0); + $limit = (int)nonempty($this->limit, self::DEFAULT_PAGE_SIZE); + + $data = queryfx_all( + $conn, + 'SELECT %Q * %Q FROM %T task %Q %Q %Q %Q %Q LIMIT %d, %d', + $calc, + $count, + $task_dao->getTableName(), + $join, + $where, + $group, + $having, + $order, + $offset, + $limit); + + if ($this->calculateRows) { + $count = queryfx_one( + $conn, + 'SELECT FOUND_ROWS() N'); + $this->rowCount = $count['N']; + } else { + $this->rowCount = null; + } + + return $task_dao->loadAllFromArray($data); + } + + private function buildStatusWhereClause($conn) { + switch ($this->status) { + case self::STATUS_ANY: + return null; + case self::STATUS_OPEN: + return 'status = 0'; + case self::STATUS_CLOSED: + return 'status > 0'; + default: + throw new Exception("Unknown status query '{$this->status}'!"); + } + } + + private function buildPriorityWhereClause($conn) { + if ($this->priority === null) { + return null; + } + + return qsprintf( + $conn, + 'priority = %d', + $this->priority); + } + + private function buildAuthorWhereClause($conn) { + if (!$this->authorPHIDs) { + return null; + } + + return qsprintf( + $conn, + 'authorPHID in (%Ls)', + $this->authorPHIDs); + } + + private function buildOwnerWhereClause($conn) { + if (!$this->ownerPHIDs) { + if ($this->includeUnowned === null) { + return null; + } else if ($this->includeUnowned) { + return qsprintf( + $conn, + 'ownerPHID IS NULL'); + } else { + return qsprintf( + $conn, + 'ownerPHID IS NOT NULL'); + } + } + + if ($this->includeUnowned) { + return qsprintf( + $conn, + 'ownerPHID IN (%Ls) OR ownerPHID IS NULL', + $this->ownerPHIDs); + } else { + return qsprintf( + $conn, + 'ownerPHID IN (%Ls)', + $this->ownerPHIDs); + } + } + + private function buildProjectWhereClause($conn) { + if (!$this->projectPHIDs) { + return null; + } + + return qsprintf( + $conn, + 'project.projectPHID IN (%Ls)', + $this->projectPHIDs); + } + + private function buildProjectJoinClause($conn) { + if (!$this->projectPHIDs) { + return null; + } + + $project_dao = new ManiphestTaskProject(); + return qsprintf( + $conn, + 'JOIN %T project ON project.taskPHID = task.phid', + $project_dao->getTableName()); + } + + private function buildOrderClause($conn) { + $order = array(); + + switch ($this->groupBy) { + case self::GROUP_NONE: + break; + case self::GROUP_PRIORITY: + $order[] = 'priority'; + break; + case self::GROUP_OWNER: + $order[] = 'ownerOrdering'; + break; + case self::GROUP_STATUS: + $order[] = 'status'; + break; + default: + throw new Exception("Unknown group query '{$this->groupBy}'!"); + } + + switch ($this->orderBy) { + case self::ORDER_PRIORITY: + $order[] = 'priority'; + $order[] = 'dateModified'; + break; + case self::ORDER_CREATED: + $order[] = 'id'; + break; + case self::ORDER_MODIFIED: + $order[] = 'dateModified'; + break; + default: + throw new Exception("Unknown order query '{$this->orderBy}'!"); + } + + $order = array_unique($order); + + if (empty($order)) { + return null; + } + + foreach ($order as $k => $column) { + switch ($column) { + case 'ownerOrdering': + $order[$k] = "task.{$column} ASC"; + break; + default: + $order[$k] = "task.{$column} DESC"; + break; + } + } + + return 'ORDER BY '.implode(', ', $order); + } + + +} diff --git a/src/applications/maniphest/query/__init__.php b/src/applications/maniphest/query/__init__.php new file mode 100644 index 0000000000..19b4f9fc8c --- /dev/null +++ b/src/applications/maniphest/query/__init__.php @@ -0,0 +1,18 @@ +ccPHIDs, array()); } + public function setProjectPHIDs(array $phids) { + $this->projectPHIDs = $phids; + $this->projectsNeedUpdate = true; + return $this; + } + public function save() { if (!$this->mailKey) { $this->mailKey = sha1(Filesystem::readRandomBytes(20)); } - return parent::save(); + + $result = parent::save(); + + if ($this->projectsNeedUpdate) { + // If we've changed the project PHIDs for this task, update the link + // table. + ManiphestTaskProject::updateTaskProjects($this); + $this->projectsNeedUpdate = false; + } + + return $result; } } diff --git a/src/applications/maniphest/storage/task/__init__.php b/src/applications/maniphest/storage/task/__init__.php index 22a5346f8c..14ba412113 100644 --- a/src/applications/maniphest/storage/task/__init__.php +++ b/src/applications/maniphest/storage/task/__init__.php @@ -7,6 +7,7 @@ phutil_require_module('phabricator', 'applications/maniphest/storage/base'); +phutil_require_module('phabricator', 'applications/maniphest/storage/taskproject'); phutil_require_module('phabricator', 'applications/phid/constants'); phutil_require_module('phabricator', 'applications/phid/storage/phid'); diff --git a/src/applications/maniphest/storage/taskproject/ManiphestTaskProject.php b/src/applications/maniphest/storage/taskproject/ManiphestTaskProject.php new file mode 100644 index 0000000000..687694934e --- /dev/null +++ b/src/applications/maniphest/storage/taskproject/ManiphestTaskProject.php @@ -0,0 +1,67 @@ + Project table, which denormalizes the + * relationship between tasks and projects into a link table so it can be + * efficiently queried. This table is not authoritative; the projectPHIDs field + * of ManiphestTask is. The rows in this table are regenerated when transactions + * are applied to tasks which affected their associated projects. + * + * @group maniphest + */ +final class ManiphestTaskProject extends ManiphestDAO { + + protected $taskPHID; + protected $projectPHID; + + public function getConfiguration() { + return array( + self::CONFIG_IDS => self::IDS_MANUAL, + self::CONFIG_TIMESTAMPS => false, + ); + } + + public static function updateTaskProjects(ManiphestTask $task) { + $dao = new ManiphestTaskProject(); + $conn = $dao->establishConnection('w'); + + $sql = array(); + foreach ($task->getProjectPHIDs() as $project_phid) { + $sql[] = qsprintf( + $conn, + '(%s, %s)', + $task->getPHID(), + $project_phid); + } + + queryfx( + $conn, + 'DELETE FROM %T WHERE taskPHID = %s', + $dao->getTableName(), + $task->getPHID()); + if ($sql) { + queryfx( + $conn, + 'INSERT INTO %T (taskPHID, projectPHID) VALUES %Q', + $dao->getTableName(), + implode(', ', $sql)); + } + } + +} diff --git a/src/applications/maniphest/storage/taskproject/__init__.php b/src/applications/maniphest/storage/taskproject/__init__.php new file mode 100644 index 0000000000..8a3c5b26af --- /dev/null +++ b/src/applications/maniphest/storage/taskproject/__init__.php @@ -0,0 +1,14 @@ +