From 7ab025eef559bbc1d78d767da66b3988ba424bf6 Mon Sep 17 00:00:00 2001 From: epriestley Date: Thu, 23 Apr 2015 03:58:04 -0700 Subject: [PATCH] Use a function typeahead for "projects" in Maniphest Summary: Ref T4100. Collapse the five inputs into one. Test Plan: - Searched for a bunch of stuff. - Used "Group By: Project", which is a bit of a special case and possibly tricky. - Created an old query with all the fields, then updated; verified it was preserved/transformed correctly. {F379971} Reviewers: btrahan Reviewed By: btrahan Subscribers: epriestley Maniphest Tasks: T4100 Differential Revision: https://secure.phabricator.com/D12525 --- .../maniphest/query/ManiphestTaskQuery.php | 39 ++--- .../query/ManiphestTaskSearchEngine.php | 133 ++++++------------ ...abricatorProjectLogicalOrNotDatasource.php | 4 +- ...habricatorProjectLogicalUserDatasource.php | 2 + 4 files changed, 68 insertions(+), 110 deletions(-) diff --git a/src/applications/maniphest/query/ManiphestTaskQuery.php b/src/applications/maniphest/query/ManiphestTaskQuery.php index 68a7dee3a3..ebf0b891a1 100644 --- a/src/applications/maniphest/query/ManiphestTaskQuery.php +++ b/src/applications/maniphest/query/ManiphestTaskQuery.php @@ -403,23 +403,13 @@ final class ManiphestTaskQuery extends PhabricatorCursorPagedPolicyAwareQuery { $this->subpriorityMax); } - $where[] = $this->buildPagingClause($conn); + $where[] = $this->buildWhereClauseParts($conn); $where = $this->formatWhereClause($where); - $having = ''; $count = ''; - if (count($this->projectPHIDs) > 1) { - // 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. - $count = ', COUNT(project.dst) projectCount'; - $having = qsprintf( - $conn, - 'HAVING projectCount = %d', - count($this->projectPHIDs)); } $group_column = ''; @@ -433,14 +423,15 @@ final class ManiphestTaskQuery extends PhabricatorCursorPagedPolicyAwareQuery { $rows = queryfx_all( $conn, - 'SELECT task.* %Q %Q FROM %T task %Q %Q %Q %Q %Q %Q', + '%Q %Q %Q FROM %T task %Q %Q %Q %Q %Q %Q', + $this->buildSelectClause($conn), $count, $group_column, $task_dao->getTableName(), - $this->buildJoinsClause($conn), + $this->buildJoinClause($conn), $where, $this->buildGroupClause($conn), - $having, + $this->buildHavingClause($conn), $this->buildOrderClause($conn), $this->buildLimitClause($conn)); @@ -761,7 +752,7 @@ final class ManiphestTaskQuery extends PhabricatorCursorPagedPolicyAwareQuery { 'xproject.dst IS NULL'); } - private function buildJoinsClause(AphrontDatabaseConnection $conn_r) { + protected function buildJoinClauseParts(AphrontDatabaseConnection $conn_r) { $edge_table = PhabricatorEdgeConfig::TABLE_NAME_EDGE; $joins = array(); @@ -856,9 +847,9 @@ final class ManiphestTaskQuery extends PhabricatorCursorPagedPolicyAwareQuery { break; } - $joins[] = $this->buildApplicationSearchJoinClause($conn_r); + $joins[] = parent::buildJoinClauseParts($conn_r); - return implode(' ', $joins); + return $joins; } protected function buildGroupClause(AphrontDatabaseConnection $conn_r) { @@ -866,7 +857,7 @@ final class ManiphestTaskQuery extends PhabricatorCursorPagedPolicyAwareQuery { (count($this->anyProjectPHIDs) > 1) || $this->shouldJoinBlockingTasks() || $this->shouldJoinBlockedTasks() || - ($this->getApplicationSearchMayJoinMultipleRows()); + ($this->shouldGroupQueryResultRows()); $joined_project_name = ($this->groupBy == self::GROUP_PROJECT); @@ -920,6 +911,18 @@ final class ManiphestTaskQuery extends PhabricatorCursorPagedPolicyAwareQuery { return array_mergev($phids); } + // TODO: Remove this when moving fully to edge logic. + protected function buildHavingClauseParts(AphrontDatabaseConnection $conn) { + $having = parent::buildHavingClauseParts($conn); + if (count($this->projectPHIDs) > 1) { + $having[] = qsprintf( + $conn, + 'projectCount = %d', + count($this->projectPHIDs)); + } + return $having; + } + protected function getResultCursor($result) { $id = $result->getID(); diff --git a/src/applications/maniphest/query/ManiphestTaskSearchEngine.php b/src/applications/maniphest/query/ManiphestTaskSearchEngine.php index d47297d456..6edf7badf6 100644 --- a/src/applications/maniphest/query/ManiphestTaskSearchEngine.php +++ b/src/applications/maniphest/query/ManiphestTaskSearchEngine.php @@ -89,24 +89,8 @@ final class ManiphestTaskSearchEngine $saved->setParameter('fulltext', $request->getStr('fulltext')); $saved->setParameter( - 'allProjectPHIDs', - $this->readPHIDsFromRequest($request, 'allProjects')); - - $saved->setParameter( - 'withNoProject', - $request->getBool('withNoProject')); - - $saved->setParameter( - 'anyProjectPHIDs', - $this->readPHIDsFromRequest($request, 'anyProjects')); - - $saved->setParameter( - 'excludeProjectPHIDs', - $this->readPHIDsFromRequest($request, 'excludeProjects')); - - $saved->setParameter( - 'userProjectPHIDs', - $this->readUsersFromRequest($request, 'userProjects')); + 'projects', + $this->readProjectsFromRequest($request, 'projects')); $saved->setParameter('createdStart', $request->getStr('createdStart')); $saved->setParameter('createdEnd', $request->getStr('createdEnd')); @@ -183,30 +167,9 @@ final class ManiphestTaskSearchEngine $query->withFullTextSearch($fulltext); } - $with_no_project = $saved->getParameter('withNoProject'); - if ($with_no_project) { - $query->withAllProjects(array(ManiphestTaskOwner::PROJECT_NO_PROJECT)); - } else { - $project_phids = $saved->getParameter('allProjectPHIDs'); - if ($project_phids) { - $query->withAllProjects($project_phids); - } - } - - $any_project_phids = $saved->getParameter('anyProjectPHIDs'); - if ($any_project_phids) { - $query->withAnyProjects($any_project_phids); - } - - $exclude_project_phids = $saved->getParameter('excludeProjectPHIDs'); - if ($exclude_project_phids) { - $query->withoutProjects($exclude_project_phids); - } - - $user_project_phids = $saved->getParameter('userProjectPHIDs'); - if ($user_project_phids) { - $query->withAnyUserProjects($user_project_phids); - } + $projects = $this->readProjectTokens($saved); + $adjusted = id(clone $saved)->setParameter('projects', $projects); + $this->setQueryProjects($query, $adjusted); $start = $this->parseDateTime($saved->getParameter('createdStart')); $end = $this->parseDateTime($saved->getParameter('createdEnd')); @@ -242,21 +205,9 @@ final class ManiphestTaskSearchEngine $assigned_phids = $this->readAssignedPHIDs($saved); $author_phids = $saved->getParameter('authorPHIDs', array()); - $all_project_phids = $saved->getParameter( - 'allProjectPHIDs', - array()); - $any_project_phids = $saved->getParameter( - 'anyProjectPHIDs', - array()); - $exclude_project_phids = $saved->getParameter( - 'excludeProjectPHIDs', - array()); - $user_project_phids = $saved->getParameter( - 'userProjectPHIDs', - array()); - $subscriber_phids = $saved->getParameter('subscriberPHIDs', array()); + $projects = $this->readProjectTokens($saved); - $with_no_projects = $saved->getParameter('withNoProject'); + $subscriber_phids = $saved->getParameter('subscriberPHIDs', array()); $statuses = $saved->getParameter('statuses', array()); $statuses = array_fuse($statuses); @@ -317,41 +268,10 @@ final class ManiphestTaskSearchEngine ->setValue($assigned_phids)) ->appendControl( id(new AphrontFormTokenizerControl()) - ->setDatasource(new PhabricatorProjectDatasource()) - ->setName('allProjects') - ->setLabel(pht('In All Projects')) - ->setValue($all_project_phids)); - - if (!$this->getIsBoardView()) { - $form - ->appendChild( - id(new AphrontFormCheckboxControl()) - ->addCheckbox( - 'withNoProject', - 1, - pht('Show only tasks with no projects.'), - $with_no_projects)); - } - - $form - ->appendControl( - id(new AphrontFormTokenizerControl()) - ->setDatasource(new PhabricatorProjectDatasource()) - ->setName('anyProjects') - ->setLabel(pht('In Any Project')) - ->setValue($any_project_phids)) - ->appendControl( - id(new AphrontFormTokenizerControl()) - ->setDatasource(new PhabricatorProjectDatasource()) - ->setName('excludeProjects') - ->setLabel(pht('Not In Projects')) - ->setValue($exclude_project_phids)) - ->appendControl( - id(new AphrontFormTokenizerControl()) - ->setDatasource(new PhabricatorPeopleDatasource()) - ->setName('userProjects') - ->setLabel(pht('In Users\' Projects')) - ->setValue($user_project_phids)) + ->setDatasource(new PhabricatorProjectLogicalDatasource()) + ->setName('projects') + ->setLabel(pht('Projects')) + ->setValue($projects)) ->appendControl( id(new AphrontFormTokenizerControl()) ->setDatasource(new PhabricatorPeopleDatasource()) @@ -566,4 +486,35 @@ final class ManiphestTaskSearchEngine return $assigned_phids; } + private function readProjectTokens(PhabricatorSavedQuery $saved) { + $projects = $saved->getParameter('projects', array()); + + $all = $saved->getParameter('allProjectPHIDs', array()); + foreach ($all as $phid) { + $projects[] = $phid; + } + + $any = $saved->getParameter('anyProjectPHIDs', array()); + foreach ($any as $phid) { + $projects[] = 'any('.$phid.')'; + } + + $not = $saved->getParameter('excludeProjectPHIDs', array()); + foreach ($not as $phid) { + $projects[] = 'not('.$phid.')'; + } + + $users = $saved->getParameter('userProjectPHIDs', array()); + foreach ($users as $phid) { + $projects[] = 'projects('.$phid.')'; + } + + $no = $saved->getParameter('withNoProject'); + if ($no) { + $projects[] = 'null()'; + } + + return $projects; + } + } diff --git a/src/applications/project/typeahead/PhabricatorProjectLogicalOrNotDatasource.php b/src/applications/project/typeahead/PhabricatorProjectLogicalOrNotDatasource.php index 2efb1b4eb6..ad392a677a 100644 --- a/src/applications/project/typeahead/PhabricatorProjectLogicalOrNotDatasource.php +++ b/src/applications/project/typeahead/PhabricatorProjectLogicalOrNotDatasource.php @@ -85,7 +85,8 @@ final class PhabricatorProjectLogicalOrNotDatasource foreach ($results as $result) { $result ->setTokenType(PhabricatorTypeaheadTokenView::TYPE_FUNCTION) - ->setIcon('fa-asterisk'); + ->setIcon('fa-asterisk') + ->setColor(null); if ($return_any) { $return[] = id(clone $result) @@ -134,6 +135,7 @@ final class PhabricatorProjectLogicalOrNotDatasource $tokens = $this->renderTokens($phids); foreach ($tokens as $token) { + $token->setColor(null); if ($token->isInvalid()) { if ($function == 'any') { $token->setValue(pht('In Any: Invalid Project')); diff --git a/src/applications/project/typeahead/PhabricatorProjectLogicalUserDatasource.php b/src/applications/project/typeahead/PhabricatorProjectLogicalUserDatasource.php index 8d1542d9ca..d2cb58a7ee 100644 --- a/src/applications/project/typeahead/PhabricatorProjectLogicalUserDatasource.php +++ b/src/applications/project/typeahead/PhabricatorProjectLogicalUserDatasource.php @@ -42,6 +42,7 @@ final class PhabricatorProjectLogicalUserDatasource protected function didLoadResults(array $results) { foreach ($results as $result) { $result + ->setColor(null) ->setTokenType(PhabricatorTypeaheadTokenView::TYPE_FUNCTION) ->setIcon('fa-asterisk') ->setPHID('projects('.$result->getPHID().')') @@ -85,6 +86,7 @@ final class PhabricatorProjectLogicalUserDatasource $tokens = $this->renderTokens($phids); foreach ($tokens as $token) { + $token->setColor(null); if ($token->isInvalid()) { $token ->setValue(pht("User's Projects: Invalid User"));