From 59cea9bfc3d3bcb12c7562cb98a38a5d42c035ef Mon Sep 17 00:00:00 2001 From: epriestley Date: Fri, 31 May 2013 10:51:20 -0700 Subject: [PATCH] Implement ApplicationSearch in People Summary: Ref T2625. Fixes T2812. Implement ApplicationSearch in People. {F44788} Test Plan: Made People queries. Used Conduit. Used `@mentions`. Reviewers: btrahan, chad Reviewed By: chad CC: aran Maniphest Tasks: T2625, T2812 Differential Revision: https://secure.phabricator.com/D6092 --- src/__phutil_library_map__.php | 13 +- .../PhabricatorMailReplyHandler.php | 7 +- .../PhabricatorApplicationPeople.php | 4 +- .../conduit/ConduitAPI_user_query_Method.php | 2 + .../PhabricatorPeopleController.php | 37 +++-- .../PhabricatorPeopleListController.php | 82 +++++----- .../{ => query}/PhabricatorPeopleQuery.php | 149 ++++++++++++++---- .../query/PhabricatorPeopleSearchEngine.php | 137 ++++++++++++++++ .../PhabricatorRemarkupRuleMention.php | 1 + .../people/storage/PhabricatorUser.php | 29 +++- ...PhabricatorApplicationSearchController.php | 2 +- .../PhabricatorApplicationSearchEngine.php | 6 +- 12 files changed, 368 insertions(+), 101 deletions(-) rename src/applications/people/{ => query}/PhabricatorPeopleQuery.php (51%) create mode 100644 src/applications/people/query/PhabricatorPeopleSearchEngine.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 19a8917d7b..3893778b5f 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -1255,7 +1255,8 @@ phutil_register_library_map(array( 'PhabricatorPeopleListController' => 'applications/people/controller/PhabricatorPeopleListController.php', 'PhabricatorPeopleLogsController' => 'applications/people/controller/PhabricatorPeopleLogsController.php', 'PhabricatorPeopleProfileController' => 'applications/people/controller/PhabricatorPeopleProfileController.php', - 'PhabricatorPeopleQuery' => 'applications/people/PhabricatorPeopleQuery.php', + 'PhabricatorPeopleQuery' => 'applications/people/query/PhabricatorPeopleQuery.php', + 'PhabricatorPeopleSearchEngine' => 'applications/people/query/PhabricatorPeopleSearchEngine.php', 'PhabricatorPeopleTestDataGenerator' => 'applications/people/lipsum/PhabricatorPeopleTestDataGenerator.php', 'PhabricatorPhabricatorOAuthConfigOptions' => 'applications/config/option/PhabricatorPhabricatorOAuthConfigOptions.php', 'PhabricatorPhameConfigOptions' => 'applications/phame/config/PhabricatorPhameConfigOptions.php', @@ -3052,10 +3053,15 @@ phutil_register_library_map(array( 'PhabricatorPeopleEditController' => 'PhabricatorPeopleController', 'PhabricatorPeopleHovercardEventListener' => 'PhutilEventListener', 'PhabricatorPeopleLdapController' => 'PhabricatorPeopleController', - 'PhabricatorPeopleListController' => 'PhabricatorPeopleController', + 'PhabricatorPeopleListController' => + array( + 0 => 'PhabricatorPeopleController', + 1 => 'PhabricatorApplicationSearchResultsControllerInterface', + ), 'PhabricatorPeopleLogsController' => 'PhabricatorPeopleController', 'PhabricatorPeopleProfileController' => 'PhabricatorPeopleController', - 'PhabricatorPeopleQuery' => 'PhabricatorOffsetPagedQuery', + 'PhabricatorPeopleQuery' => 'PhabricatorPolicyAwareCursorPagedQuery', + 'PhabricatorPeopleSearchEngine' => 'PhabricatorApplicationSearchEngine', 'PhabricatorPeopleTestDataGenerator' => 'PhabricatorTestDataGenerator', 'PhabricatorPhabricatorOAuthConfigOptions' => 'PhabricatorApplicationConfigOptions', 'PhabricatorPhameConfigOptions' => 'PhabricatorApplicationConfigOptions', @@ -3307,6 +3313,7 @@ phutil_register_library_map(array( array( 0 => 'PhabricatorUserDAO', 1 => 'PhutilPerson', + 2 => 'PhabricatorPolicyInterface', ), 'PhabricatorUserDAO' => 'PhabricatorLiskDAO', 'PhabricatorUserEditor' => 'PhabricatorEditor', diff --git a/src/applications/metamta/replyhandler/PhabricatorMailReplyHandler.php b/src/applications/metamta/replyhandler/PhabricatorMailReplyHandler.php index 7bae7e51a6..5d50bffd97 100644 --- a/src/applications/metamta/replyhandler/PhabricatorMailReplyHandler.php +++ b/src/applications/metamta/replyhandler/PhabricatorMailReplyHandler.php @@ -285,9 +285,10 @@ EOBODY; return null; } - $user = head(id(new PhabricatorPeopleQuery()) - ->withPhids(array($handle->getPHID())) - ->execute()); + $user = id(new PhabricatorPeopleQuery()) + ->setViewer(PhabricatorUser::getOmnipotentUser()) + ->withPHIDs(array($handle->getPHID())) + ->executeOne(); $receiver = $this->getMailReceiver(); $receiver_id = $receiver->getID(); diff --git a/src/applications/people/application/PhabricatorApplicationPeople.php b/src/applications/people/application/PhabricatorApplicationPeople.php index 25071b53d8..dda4bb15fd 100644 --- a/src/applications/people/application/PhabricatorApplicationPeople.php +++ b/src/applications/people/application/PhabricatorApplicationPeople.php @@ -23,7 +23,7 @@ final class PhabricatorApplicationPeople extends PhabricatorApplication { } public function getApplicationGroup() { - return self::GROUP_ADMIN; + return self::GROUP_ORGANIZATION; } public function canUninstall() { @@ -39,7 +39,7 @@ final class PhabricatorApplicationPeople extends PhabricatorApplication { public function getRoutes() { return array( '/people/' => array( - '' => 'PhabricatorPeopleListController', + '(query/(?P[^/]+)/)?' => 'PhabricatorPeopleListController', 'logs/' => 'PhabricatorPeopleLogsController', 'edit/(?:(?P[1-9]\d*)/(?:(?P\w+)/)?)?' => 'PhabricatorPeopleEditController', diff --git a/src/applications/people/conduit/ConduitAPI_user_query_Method.php b/src/applications/people/conduit/ConduitAPI_user_query_Method.php index 35b6fc94fc..33a9b00a3c 100644 --- a/src/applications/people/conduit/ConduitAPI_user_query_Method.php +++ b/src/applications/people/conduit/ConduitAPI_user_query_Method.php @@ -44,6 +44,8 @@ final class ConduitAPI_user_query_Method $limit = $request->getValue('limit', 100); $query = new PhabricatorPeopleQuery(); + $query->setViewer($request->getUser()); + if ($usernames) { $query->withUsernames($usernames); } diff --git a/src/applications/people/controller/PhabricatorPeopleController.php b/src/applications/people/controller/PhabricatorPeopleController.php index 9894b5f70e..b2cd19bba6 100644 --- a/src/applications/people/controller/PhabricatorPeopleController.php +++ b/src/applications/people/controller/PhabricatorPeopleController.php @@ -10,18 +10,21 @@ abstract class PhabricatorPeopleController extends PhabricatorController { $nav = new AphrontSideNavFilterView(); $nav->setBaseURI(new PhutilURI($this->getApplicationURI())); - $nav->addLabel(pht('User Administration')); - $nav->addFilter('edit', pht('Create New User')); - if (PhabricatorEnv::getEnvConfig('ldap.auth-enabled') === true) { - $nav->addFilter('ldap', pht('Import from LDAP')); + $viewer = $this->getRequest()->getUser(); + + id(new PhabricatorPeopleSearchEngine()) + ->setViewer($viewer) + ->addNavigationItems($nav->getMenu()); + + if ($viewer->getIsAdmin()) { + $nav->addLabel(pht('User Administration')); + if (PhabricatorEnv::getEnvConfig('ldap.auth-enabled') === true) { + $nav->addFilter('ldap', pht('Import from LDAP')); + } + + $nav->addFilter('logs', pht('Activity Logs')); } - $nav->addFilter('people', - pht('User Directory'), - $this->getApplicationURI()); - - $nav->addFilter('logs', pht('Activity Logs')); - return $nav; } @@ -32,11 +35,15 @@ abstract class PhabricatorPeopleController extends PhabricatorController { public function buildApplicationCrumbs() { $crumbs = parent::buildApplicationCrumbs(); - $crumbs->addAction( - id(new PhabricatorMenuItemView()) - ->setName(pht('Create New User')) - ->setHref($this->getApplicationURI('edit')) - ->setIcon('create')); + $viewer = $this->getRequest()->getUser(); + + if ($viewer->getIsAdmin()) { + $crumbs->addAction( + id(new PhabricatorMenuItemView()) + ->setName(pht('Create New User')) + ->setHref($this->getApplicationURI('edit')) + ->setIcon('create')); + } return $crumbs; } diff --git a/src/applications/people/controller/PhabricatorPeopleListController.php b/src/applications/people/controller/PhabricatorPeopleListController.php index 05d79b6487..6c48b0e91f 100644 --- a/src/applications/people/controller/PhabricatorPeopleListController.php +++ b/src/applications/people/controller/PhabricatorPeopleListController.php @@ -1,29 +1,37 @@ key = idx($data, 'key', 'all'); + } public function processRequest() { + $request = $this->getRequest(); + $controller = id(new PhabricatorApplicationSearchController($request)) + ->setQueryKey($this->key) + ->setSearchEngine(new PhabricatorPeopleSearchEngine()) + ->setNavigation($this->buildSideNavView()); + + return $this->delegateToController($controller); + } + + public function renderResultsList(array $users) { + assert_instances_of($users, 'PhabricatorUser'); + $request = $this->getRequest(); $viewer = $request->getUser(); - $is_admin = $viewer->getIsAdmin(); - - $user = new PhabricatorUser(); - - $count = queryfx_one( - $user->establishConnection('r'), - 'SELECT COUNT(*) N FROM %T', - $user->getTableName()); - $count = idx($count, 'N', 0); - - $pager = new AphrontPagerView(); - $pager->setOffset($request->getInt('page', 0)); - $pager->setCount($count); - $pager->setURI($request->getRequestURI(), 'page'); - - $users = id(new PhabricatorPeopleQuery()) - ->needPrimaryEmail(true) - ->executeWithOffsetPager($pager); $list = new PhabricatorObjectItemListView(); @@ -40,7 +48,7 @@ final class PhabricatorPeopleListController $item = new PhabricatorObjectItemView(); $item->setHeader($user->getFullName()) - ->setHref('/people/edit/'.$user->getID().'/') + ->setHref('/p/'.$user->getUsername().'/') ->addAttribute(hsprintf('%s %s', phabricator_date($user->getDateCreated(), $viewer), phabricator_time($user->getDateCreated(), $viewer))) @@ -58,31 +66,17 @@ final class PhabricatorPeopleListController $item->addIcon('computer', pht('System Agent')); } + if ($viewer->getIsAdmin()) { + $uid = $user->getID(); + $item->addAction( + id(new PhabricatorMenuItemView()) + ->setIcon('edit') + ->setHref($this->getApplicationURI('edit/'.$uid.'/'))); + } + $list->addItem($item); } - $header = new PhabricatorHeaderView(); - $header->setHeader(pht('People (%d)', number_format($count))); - - $crumbs = $this->buildApplicationCrumbs(); - $crumbs->addCrumb( - id(new PhabricatorCrumbView()) - ->setName(pht('User Directory')) - ->setHref('/people/')); - - $nav = $this->buildSideNavView(); - $nav->selectFilter('people'); - $nav->appendChild($header); - $nav->appendChild($list); - $nav->appendChild($pager); - $nav->setCrumbs($crumbs); - - return $this->buildApplicationPage( - $nav, - array( - 'title' => pht('People'), - 'device' => true, - 'dust' => true, - )); + return $list; } } diff --git a/src/applications/people/PhabricatorPeopleQuery.php b/src/applications/people/query/PhabricatorPeopleQuery.php similarity index 51% rename from src/applications/people/PhabricatorPeopleQuery.php rename to src/applications/people/query/PhabricatorPeopleQuery.php index 0da8fb731d..dc719a0e77 100644 --- a/src/applications/people/PhabricatorPeopleQuery.php +++ b/src/applications/people/query/PhabricatorPeopleQuery.php @@ -1,37 +1,79 @@ ids = $ids; return $this; } - public function withPhids(array $phids) { + + public function withPHIDs(array $phids) { $this->phids = $phids; return $this; } + public function withEmails(array $emails) { $this->emails = $emails; return $this; } + public function withRealnames(array $realnames) { $this->realnames = $realnames; return $this; } + public function withUsernames(array $usernames) { $this->usernames = $usernames; return $this; } + public function withDateCreatedBefore($date_created_before) { + $this->dateCreatedBefore = $date_created_before; + return $this; + } + + public function withDateCreatedAfter($date_created_after) { + $this->dateCreatedAfter = $date_created_after; + return $this; + } + + public function withIsAdmin($admin) { + $this->isAdmin = $admin; + return $this; + } + + public function withIsSystemAgent($system_agent) { + $this->isSystemAgent = $system_agent; + return $this; + } + + public function withIsDisabled($disabled) { + $this->isDisabled = $disabled; + return $this; + } + + public function withNameLike($like) { + $this->nameLike = $like; + return $this; + } + public function needPrimaryEmail($need) { $this->needPrimaryEmail = $need; return $this; @@ -47,21 +89,18 @@ final class PhabricatorPeopleQuery extends PhabricatorOffsetPagedQuery { return $this; } - public function execute() { + public function loadPage() { $table = new PhabricatorUser(); $conn_r = $table->establishConnection('r'); - $joins_clause = $this->buildJoinsClause($conn_r); - $where_clause = $this->buildWhereClause($conn_r); - $limit_clause = $this->buildLimitClause($conn_r); - $data = queryfx_all( $conn_r, - 'SELECT * FROM %T user %Q %Q %Q', + 'SELECT * FROM %T user %Q %Q %Q %Q', $table->getTableName(), - $joins_clause, - $where_clause, - $limit_clause); + $this->buildJoinsClause($conn_r), + $this->buildWhereClause($conn_r), + $this->buildOrderClause($conn_r), + $this->buildLimitClause($conn_r)); if ($this->needPrimaryEmail) { $table->putInSet(new LiskDAOSet()); @@ -88,9 +127,10 @@ final class PhabricatorPeopleQuery extends PhabricatorOffsetPagedQuery { } if ($this->needProfileImage) { - // Change this once we migrate this to CursorPagedPolicyAwareQuery - $files = id(new PhabricatorFile()) - ->loadAllWhere('phid IN (%Ls)', mpull($users, 'getProfileImagePHID')); + $files = id(new PhabricatorFileQuery()) + ->setViewer($this->getViewer()) + ->withPHIDs(mpull($users, 'getProfileImagePHID')) + ->execute(); $files = mpull($files, null, 'getPHID'); foreach ($users as $user) { $image_phid = $user->getProfileImagePHID(); @@ -125,29 +165,78 @@ final class PhabricatorPeopleQuery extends PhabricatorOffsetPagedQuery { $where = array(); if ($this->usernames) { - $where[] = qsprintf($conn_r, - 'user.userName IN (%Ls)', - $this->usernames); + $where[] = qsprintf( + $conn_r, + 'user.userName IN (%Ls)', + $this->usernames); } + if ($this->emails) { - $where[] = qsprintf($conn_r, - 'email.address IN (%Ls)', - $this->emails); + $where[] = qsprintf( + $conn_r, + 'email.address IN (%Ls)', + $this->emails); } + if ($this->realnames) { - $where[] = qsprintf($conn_r, - 'user.realName IN (%Ls)', - $this->realnames); + $where[] = qsprintf( + $conn_r, + 'user.realName IN (%Ls)', + $this->realnames); } + if ($this->phids) { - $where[] = qsprintf($conn_r, - 'user.phid IN (%Ls)', - $this->phids); + $where[] = qsprintf( + $conn_r, + 'user.phid IN (%Ls)', + $this->phids); } + if ($this->ids) { - $where[] = qsprintf($conn_r, - 'user.id IN (%Ld)', - $this->ids); + $where[] = qsprintf( + $conn_r, + 'user.id IN (%Ld)', + $this->ids); + } + + if ($this->dateCreatedAfter) { + $where[] = qsprintf( + $conn_r, + 'user.dateCreated >= %d', + $this->dateCreatedAfter); + } + + if ($this->dateCreatedBefore) { + $where[] = qsprintf( + $conn_r, + 'user.dateCreated <= %d', + $this->dateCreatedBefore); + } + + if ($this->isAdmin) { + $where[] = qsprintf( + $conn_r, + 'user.isAdmin = 1'); + } + + if ($this->isDisabled) { + $where[] = qsprintf( + $conn_r, + 'user.isDisabled = 1'); + } + + if ($this->isSystemAgent) { + $where[] = qsprintf( + $conn_r, + 'user.isSystemAgent = 1'); + } + + if (strlen($this->nameLike)) { + $where[] = qsprintf( + $conn_r, + 'user.username LIKE %~ OR user.realname LIKE %~', + $this->nameLike, + $this->nameLike); } return $this->formatWhereClause($where); diff --git a/src/applications/people/query/PhabricatorPeopleSearchEngine.php b/src/applications/people/query/PhabricatorPeopleSearchEngine.php new file mode 100644 index 0000000000..0a52a437ec --- /dev/null +++ b/src/applications/people/query/PhabricatorPeopleSearchEngine.php @@ -0,0 +1,137 @@ +setParameter('usernames', $request->getStrList('usernames')); + $saved->setParameter('nameLike', $request->getStr('nameLike')); + $saved->setParameter('isAdmin', $request->getStr('isAdmin')); + $saved->setParameter('isDisabled', $request->getStr('isDisabled')); + $saved->setParameter('isSystemAgent', $request->getStr('isSystemAgent')); + $saved->setParameter('createdStart', $request->getStr('createdStart')); + $saved->setParameter('createdEnd', $request->getStr('createdEnd')); + + return $saved; + } + + public function buildQueryFromSavedQuery(PhabricatorSavedQuery $saved) { + $query = id(new PhabricatorPeopleQuery()) + ->needPrimaryEmail(true); + + $usernames = $saved->getParameter('usernames', array()); + if ($usernames) { + $query->withUsernames($usernames); + } + + $like = $saved->getParameter('nameLike'); + if ($like) { + $query->withNameLike($like); + } + + $is_admin = $saved->getParameter('isAdmin'); + $is_disabled = $saved->getParameter('isDisabled'); + $is_system_agent = $saved->getParameter('isSystemAgent'); + + if ($is_admin) { + $query->withIsAdmin(true); + } + + if ($is_disabled) { + $query->withIsDisabled(true); + } + + if ($is_system_agent) { + $query->withIsSystemAgent(true); + } + + $start = $this->parseDateTime($saved->getParameter('createdStart')); + $end = $this->parseDateTime($saved->getParameter('createdEnd')); + + if ($start) { + $query->withDateCreatedAfter($start); + } + + if ($end) { + $query->withDateCreatedBefore($end); + } + return $query; + } + + public function buildSearchForm( + AphrontFormView $form, + PhabricatorSavedQuery $saved) { + + $usernames = $saved->getParameter('usernames', array()); + $like = $saved->getParameter('nameLike'); + + $is_admin = $saved->getParameter('isAdmin'); + $is_disabled = $saved->getParameter('isDisabled'); + $is_system_agent = $saved->getParameter('isSystemAgent'); + + $form + ->appendChild( + id(new AphrontFormTextControl()) + ->setName('usernames') + ->setLabel(pht('Usernames')) + ->setValue(implode(', ', $usernames))) + ->appendChild( + id(new AphrontFormTextControl()) + ->setName('nameLike') + ->setLabel(pht('Name Contains')) + ->setValue($like)) + ->appendChild( + id(new AphrontFormCheckboxControl()) + ->setLabel('Role') + ->addCheckbox( + 'isAdmin', + 1, + pht('Show only Administrators.'), + $is_admin) + ->addCheckbox( + 'isDisabled', + 1, + pht('Show only disabled users.'), + $is_disabled) + ->addCheckbox( + 'isSystemAgent', + 1, + pht('Show only System Agents.'), + $is_system_agent)); + + $this->buildDateRange( + $form, + $saved, + 'createdStart', + pht('Joined After'), + 'createdEnd', + pht('Joined Before')); + } + + protected function getURI($path) { + return '/people/'.$path; + } + + public function getBuiltinQueryNames() { + $names = array( + 'all' => pht('All'), + ); + + return $names; + } + + public function buildSavedQueryFromBuiltin($query_key) { + $query = $this->newSavedQuery(); + $query->setQueryKey($query_key); + + switch ($query_key) { + case 'all': + return $query; + } + + return parent::buildSavedQueryFromBuiltin($query_key); + } + +} diff --git a/src/applications/people/remarkup/PhabricatorRemarkupRuleMention.php b/src/applications/people/remarkup/PhabricatorRemarkupRuleMention.php index 0fc226815e..31d2f4a2cf 100644 --- a/src/applications/people/remarkup/PhabricatorRemarkupRuleMention.php +++ b/src/applications/people/remarkup/PhabricatorRemarkupRuleMention.php @@ -71,6 +71,7 @@ final class PhabricatorRemarkupRuleMention $usernames = array_keys($metadata); $users = id(new PhabricatorPeopleQuery()) + ->setViewer($this->getEngine()->getConfig('viewer')) ->withUsernames($usernames) ->execute(); diff --git a/src/applications/people/storage/PhabricatorUser.php b/src/applications/people/storage/PhabricatorUser.php index b69c3211fe..62bd8ecb37 100644 --- a/src/applications/people/storage/PhabricatorUser.php +++ b/src/applications/people/storage/PhabricatorUser.php @@ -1,6 +1,8 @@ getPHID() && ($viewer->getPHID() === $this->getPHID()); + } + + } diff --git a/src/applications/search/controller/PhabricatorApplicationSearchController.php b/src/applications/search/controller/PhabricatorApplicationSearchController.php index 96be9b7514..d059f50640 100644 --- a/src/applications/search/controller/PhabricatorApplicationSearchController.php +++ b/src/applications/search/controller/PhabricatorApplicationSearchController.php @@ -142,7 +142,7 @@ final class PhabricatorApplicationSearchController $submit = id(new AphrontFormSubmitControl()) ->setValue(pht('Execute Query')); - if ($run_query && !$named_query) { + if ($run_query && !$named_query && $user->isLoggedIn()) { $submit->addCancelButton( '/search/edit/'.$saved_query->getQueryKey().'/', pht('Save Custom Query...')); diff --git a/src/applications/search/engine/PhabricatorApplicationSearchEngine.php b/src/applications/search/engine/PhabricatorApplicationSearchEngine.php index 9ad79215d7..b1c8adc32a 100644 --- a/src/applications/search/engine/PhabricatorApplicationSearchEngine.php +++ b/src/applications/search/engine/PhabricatorApplicationSearchEngine.php @@ -125,8 +125,10 @@ abstract class PhabricatorApplicationSearchEngine { $menu->newLink($query->getQueryName(), $uri, 'query/'.$key); } - $manage_uri = $this->getQueryManagementURI(); - $menu->newLink(pht('Edit Queries...'), $manage_uri, 'query/edit'); + if ($viewer->isLoggedIn()) { + $manage_uri = $this->getQueryManagementURI(); + $menu->newLink(pht('Edit Queries...'), $manage_uri, 'query/edit'); + } $menu->newLabel(pht('Search')); $advanced_uri = $this->getQueryResultsPageURI('advanced');