From 9434df9d7cdd6f44d33d81a91c75dba692a4ddb4 Mon Sep 17 00:00:00 2001 From: epriestley Date: Fri, 4 Oct 2013 20:41:50 -0700 Subject: [PATCH] Accommodate project reviewers in Differential search Summary: Ref T1279. Two changes to the search/query for Differential: - "Reviewers" now accepts users and projects. - "Responsible Users" now includes revisions where a project you are a member of is a reviewer. Test Plan: - Searched for project reviewers. - Verified that the dashboard now shows reviews which I'm only part of via project membership. Reviewers: btrahan Reviewed By: btrahan CC: aran Maniphest Tasks: T1279 Differential Revision: https://secure.phabricator.com/D7231 --- .../query/DifferentialRevisionQuery.php | 15 +++++++++++++-- .../query/DifferentialRevisionSearchEngine.php | 9 +++++++-- .../engine/PhabricatorApplicationSearchEngine.php | 13 +++++++++++-- ...ricatorTypeaheadCommonDatasourceController.php | 5 +++++ .../form/control/AphrontFormTokenizerControl.php | 1 + 5 files changed, 37 insertions(+), 6 deletions(-) diff --git a/src/applications/differential/query/DifferentialRevisionQuery.php b/src/applications/differential/query/DifferentialRevisionQuery.php index d455ddb19f..8dfcd60648 100644 --- a/src/applications/differential/query/DifferentialRevisionQuery.php +++ b/src/applications/differential/query/DifferentialRevisionQuery.php @@ -495,15 +495,26 @@ final class DifferentialRevisionQuery if ($this->responsibles) { $basic_authors = $this->authors; $basic_reviewers = $this->reviewers; + + $authority_projects = id(new PhabricatorProjectQuery()) + ->setViewer($this->getViewer()) + ->withMemberPHIDs($this->responsibles) + ->execute(); + $authority_phids = mpull($authority_projects, 'getPHID'); + try { // Build the query where the responsible users are authors. $this->authors = array_merge($basic_authors, $this->responsibles); $this->reviewers = $basic_reviewers; $selects[] = $this->buildSelectStatement($conn_r); - // Build the query where the responsible users are reviewers. + // Build the query where the responsible users are reviewers, or + // projects they are members of are reviewers. $this->authors = $basic_authors; - $this->reviewers = array_merge($basic_reviewers, $this->responsibles); + $this->reviewers = array_merge( + $basic_reviewers, + $this->responsibles, + $authority_phids); $selects[] = $this->buildSelectStatement($conn_r); // Put everything back like it was. diff --git a/src/applications/differential/query/DifferentialRevisionSearchEngine.php b/src/applications/differential/query/DifferentialRevisionSearchEngine.php index 377d2b600d..b9e2969cce 100644 --- a/src/applications/differential/query/DifferentialRevisionSearchEngine.php +++ b/src/applications/differential/query/DifferentialRevisionSearchEngine.php @@ -23,7 +23,12 @@ final class DifferentialRevisionSearchEngine $saved->setParameter( 'reviewerPHIDs', - $this->readUsersFromRequest($request, 'reviewers')); + $this->readUsersFromRequest( + $request, + 'reviewers', + array( + PhabricatorProjectPHIDTypeProject::TYPECONST, + ))); $saved->setParameter( 'subscriberPHIDs', @@ -140,7 +145,7 @@ final class DifferentialRevisionSearchEngine id(new AphrontFormTokenizerControl()) ->setLabel(pht('Reviewers')) ->setName('reviewers') - ->setDatasource('/typeahead/common/accounts/') + ->setDatasource('/typeahead/common/accountsorprojects/') ->setValue(array_select_keys($handles, $reviewer_phids))) ->appendChild( id(new AphrontFormTokenizerControl()) diff --git a/src/applications/search/engine/PhabricatorApplicationSearchEngine.php b/src/applications/search/engine/PhabricatorApplicationSearchEngine.php index 70cc7536ec..f960631a13 100644 --- a/src/applications/search/engine/PhabricatorApplicationSearchEngine.php +++ b/src/applications/search/engine/PhabricatorApplicationSearchEngine.php @@ -254,11 +254,15 @@ abstract class PhabricatorApplicationSearchEngine { * * @param AphrontRequest Request to read user PHIDs from. * @param string Key to read in the request. + * @param list Other permitted PHID types. * @return list List of user PHIDs. * * @task read */ - protected function readUsersFromRequest(AphrontRequest $request, $key) { + protected function readUsersFromRequest( + AphrontRequest $request, + $key, + array $allow_types = array()) { $list = $request->getArr($key, null); if ($list === null) { $list = $request->getStrList($key); @@ -266,9 +270,14 @@ abstract class PhabricatorApplicationSearchEngine { $phids = array(); $names = array(); + $allow_types = array_fuse($allow_types); $user_type = PhabricatorPHIDConstants::PHID_TYPE_USER; foreach ($list as $item) { - if (phid_get_type($item) == $user_type) { + $type = phid_get_type($item); + phlog($type); + if ($type == $user_type) { + $phids[] = $item; + } else if (isset($allow_types[$type])) { $phids[] = $item; } else { $names[] = $item; diff --git a/src/applications/typeahead/controller/PhabricatorTypeaheadCommonDatasourceController.php b/src/applications/typeahead/controller/PhabricatorTypeaheadCommonDatasourceController.php index 87881dc2e5..fb0c5a66d2 100644 --- a/src/applications/typeahead/controller/PhabricatorTypeaheadCommonDatasourceController.php +++ b/src/applications/typeahead/controller/PhabricatorTypeaheadCommonDatasourceController.php @@ -85,6 +85,11 @@ final class PhabricatorTypeaheadCommonDatasourceController $need_users = true; $need_all_users = true; break; + case 'accountsorprojects': + $need_users = true; + $need_all_users = true; + $need_projs = true; + break; case 'arcanistprojects': $need_arcanist_projects = true; break; diff --git a/src/view/form/control/AphrontFormTokenizerControl.php b/src/view/form/control/AphrontFormTokenizerControl.php index f8670c9dbe..c9a56f96c4 100644 --- a/src/view/form/control/AphrontFormTokenizerControl.php +++ b/src/view/form/control/AphrontFormTokenizerControl.php @@ -103,6 +103,7 @@ final class AphrontFormTokenizerControl extends AphrontFormControl { 'repositories' => pht('Type a repository name...'), 'packages' => pht('Type a package name...'), 'arcanistproject' => pht('Type an arc project name...'), + 'accountsorprojects' => pht('Type a user or project name...'), ); return idx($map, $request);