From 135c8799e67ca47413fe53096413e01917e2cb34 Mon Sep 17 00:00:00 2001 From: epriestley Date: Thu, 23 Apr 2015 03:07:24 -0700 Subject: [PATCH] Refine global search UI for tokenizer functions Summary: Ref T4100. - Removes the "with unowned" checkbox in favor of the "no owners" function. - Support functions in "Authors" and "Owners". Test Plan: - Ran various global search and Maniphest queries. {F379931} Reviewers: btrahan Reviewed By: btrahan Subscribers: epriestley Maniphest Tasks: T4100 Differential Revision: https://secure.phabricator.com/D12523 --- src/__phutil_library_map__.php | 8 +-- .../ManiphestBatchEditController.php | 4 +- .../maniphest/query/ManiphestTaskQuery.php | 2 +- .../query/ManiphestTaskSearchEngine.php | 6 +- .../typeahead/ManiphestAssigneeDatasource.php | 2 +- .../PhabricatorPeopleNoOwnerDatasource.php} | 4 +- .../PhabricatorPeopleOwnerDatasource.php} | 4 +- .../PhabricatorProjectMembersDatasource.php | 3 + ...abricatorSearchApplicationSearchEngine.php | 62 +++++++++++++------ 9 files changed, 61 insertions(+), 34 deletions(-) rename src/applications/{maniphest/typeahead/ManiphestNoOwnerDatasource.php => people/typeahead/PhabricatorPeopleNoOwnerDatasource.php} (95%) rename src/applications/{maniphest/typeahead/ManiphestOwnerDatasource.php => people/typeahead/PhabricatorPeopleOwnerDatasource.php} (82%) diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 2098a28b73..9df0472f56 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -1033,8 +1033,6 @@ phutil_register_library_map(array( 'ManiphestInfoConduitAPIMethod' => 'applications/maniphest/conduit/ManiphestInfoConduitAPIMethod.php', 'ManiphestNameIndex' => 'applications/maniphest/storage/ManiphestNameIndex.php', 'ManiphestNameIndexEventListener' => 'applications/maniphest/event/ManiphestNameIndexEventListener.php', - 'ManiphestNoOwnerDatasource' => 'applications/maniphest/typeahead/ManiphestNoOwnerDatasource.php', - 'ManiphestOwnerDatasource' => 'applications/maniphest/typeahead/ManiphestOwnerDatasource.php', 'ManiphestPriorityEmailCommand' => 'applications/maniphest/command/ManiphestPriorityEmailCommand.php', 'ManiphestQueryConduitAPIMethod' => 'applications/maniphest/conduit/ManiphestQueryConduitAPIMethod.php', 'ManiphestQueryStatusesConduitAPIMethod' => 'applications/maniphest/conduit/ManiphestQueryStatusesConduitAPIMethod.php', @@ -2207,6 +2205,8 @@ phutil_register_library_map(array( 'PhabricatorPeopleLogSearchEngine' => 'applications/people/query/PhabricatorPeopleLogSearchEngine.php', 'PhabricatorPeopleLogsController' => 'applications/people/controller/PhabricatorPeopleLogsController.php', 'PhabricatorPeopleNewController' => 'applications/people/controller/PhabricatorPeopleNewController.php', + 'PhabricatorPeopleNoOwnerDatasource' => 'applications/people/typeahead/PhabricatorPeopleNoOwnerDatasource.php', + 'PhabricatorPeopleOwnerDatasource' => 'applications/people/typeahead/PhabricatorPeopleOwnerDatasource.php', 'PhabricatorPeopleProfileController' => 'applications/people/controller/PhabricatorPeopleProfileController.php', 'PhabricatorPeopleProfileEditController' => 'applications/people/controller/PhabricatorPeopleProfileEditController.php', 'PhabricatorPeopleProfilePictureController' => 'applications/people/controller/PhabricatorPeopleProfilePictureController.php', @@ -4309,8 +4309,6 @@ phutil_register_library_map(array( 'ManiphestInfoConduitAPIMethod' => 'ManiphestConduitAPIMethod', 'ManiphestNameIndex' => 'ManiphestDAO', 'ManiphestNameIndexEventListener' => 'PhabricatorEventListener', - 'ManiphestNoOwnerDatasource' => 'PhabricatorTypeaheadDatasource', - 'ManiphestOwnerDatasource' => 'PhabricatorTypeaheadCompositeDatasource', 'ManiphestPriorityEmailCommand' => 'ManiphestEmailCommand', 'ManiphestQueryConduitAPIMethod' => 'ManiphestConduitAPIMethod', 'ManiphestQueryStatusesConduitAPIMethod' => 'ManiphestConduitAPIMethod', @@ -5575,6 +5573,8 @@ phutil_register_library_map(array( 'PhabricatorPeopleLogSearchEngine' => 'PhabricatorApplicationSearchEngine', 'PhabricatorPeopleLogsController' => 'PhabricatorPeopleController', 'PhabricatorPeopleNewController' => 'PhabricatorPeopleController', + 'PhabricatorPeopleNoOwnerDatasource' => 'PhabricatorTypeaheadDatasource', + 'PhabricatorPeopleOwnerDatasource' => 'PhabricatorTypeaheadCompositeDatasource', 'PhabricatorPeopleProfileController' => 'PhabricatorPeopleController', 'PhabricatorPeopleProfileEditController' => 'PhabricatorPeopleController', 'PhabricatorPeopleProfilePictureController' => 'PhabricatorPeopleController', diff --git a/src/applications/maniphest/controller/ManiphestBatchEditController.php b/src/applications/maniphest/controller/ManiphestBatchEditController.php index 944025d48d..75900acd53 100644 --- a/src/applications/maniphest/controller/ManiphestBatchEditController.php +++ b/src/applications/maniphest/controller/ManiphestBatchEditController.php @@ -265,8 +265,8 @@ final class ManiphestBatchEditController extends ManiphestController { continue 2; } $value = head($value); - $no_owner = ManiphestNoOwnerDatasource::FUNCTION_TOKEN; - if ($value === ManiphestNoOwnerDatasource::FUNCTION_TOKEN) { + $no_owner = PhabricatorPeopleNoOwnerDatasource::FUNCTION_TOKEN; + if ($value === $no_owner) { $value = null; } break; diff --git a/src/applications/maniphest/query/ManiphestTaskQuery.php b/src/applications/maniphest/query/ManiphestTaskQuery.php index 0046bd5163..68a7dee3a3 100644 --- a/src/applications/maniphest/query/ManiphestTaskQuery.php +++ b/src/applications/maniphest/query/ManiphestTaskQuery.php @@ -75,7 +75,7 @@ final class ManiphestTaskQuery extends PhabricatorCursorPagedPolicyAwareQuery { } public function withOwners(array $owners) { - $no_owner = ManiphestNoOwnerDatasource::FUNCTION_TOKEN; + $no_owner = PhabricatorPeopleNoOwnerDatasource::FUNCTION_TOKEN; $this->includeUnowned = false; foreach ($owners as $k => $phid) { diff --git a/src/applications/maniphest/query/ManiphestTaskSearchEngine.php b/src/applications/maniphest/query/ManiphestTaskSearchEngine.php index 1ba226c1f2..d47297d456 100644 --- a/src/applications/maniphest/query/ManiphestTaskSearchEngine.php +++ b/src/applications/maniphest/query/ManiphestTaskSearchEngine.php @@ -137,7 +137,7 @@ final class ManiphestTaskSearchEngine $query->withSubscribers($subscriber_phids); } - $datasource = id(new ManiphestOwnerDatasource()) + $datasource = id(new PhabricatorPeopleOwnerDatasource()) ->setViewer($this->requireViewer()); $assigned_phids = $this->readAssignedPHIDs($saved); @@ -311,7 +311,7 @@ final class ManiphestTaskSearchEngine $form ->appendControl( id(new AphrontFormTokenizerControl()) - ->setDatasource(new ManiphestOwnerDatasource()) + ->setDatasource(new PhabricatorPeopleOwnerDatasource()) ->setName('assigned') ->setLabel(pht('Assigned To')) ->setValue($assigned_phids)) @@ -560,7 +560,7 @@ final class ManiphestTaskSearchEngine // typeaheads, and is retained for compatibility. We could remove it by // migrating old saved queries. if ($saved->getParameter('withUnassigned')) { - $assigned_phids[] = ManiphestNoOwnerDatasource::FUNCTION_TOKEN; + $assigned_phids[] = PhabricatorPeopleNoOwnerDatasource::FUNCTION_TOKEN; } return $assigned_phids; diff --git a/src/applications/maniphest/typeahead/ManiphestAssigneeDatasource.php b/src/applications/maniphest/typeahead/ManiphestAssigneeDatasource.php index 0cbf81998d..f2d4c8b117 100644 --- a/src/applications/maniphest/typeahead/ManiphestAssigneeDatasource.php +++ b/src/applications/maniphest/typeahead/ManiphestAssigneeDatasource.php @@ -14,7 +14,7 @@ final class ManiphestAssigneeDatasource public function getComponentDatasources() { return array( new PhabricatorPeopleDatasource(), - new ManiphestNoOwnerDatasource(), + new PhabricatorPeopleNoOwnerDatasource(), ); } diff --git a/src/applications/maniphest/typeahead/ManiphestNoOwnerDatasource.php b/src/applications/people/typeahead/PhabricatorPeopleNoOwnerDatasource.php similarity index 95% rename from src/applications/maniphest/typeahead/ManiphestNoOwnerDatasource.php rename to src/applications/people/typeahead/PhabricatorPeopleNoOwnerDatasource.php index fa37024225..90c887ac06 100644 --- a/src/applications/maniphest/typeahead/ManiphestNoOwnerDatasource.php +++ b/src/applications/people/typeahead/PhabricatorPeopleNoOwnerDatasource.php @@ -1,6 +1,6 @@ renderTokens($phids); foreach ($tokens as $token) { + // Remove any project color on this token. + $token->setColor(null); + if ($token->isInvalid()) { $token ->setValue(pht('Members: Invalid Project')); diff --git a/src/applications/search/query/PhabricatorSearchApplicationSearchEngine.php b/src/applications/search/query/PhabricatorSearchApplicationSearchEngine.php index d362c21a83..ad3c5c749b 100644 --- a/src/applications/search/query/PhabricatorSearchApplicationSearchEngine.php +++ b/src/applications/search/query/PhabricatorSearchApplicationSearchEngine.php @@ -30,10 +30,6 @@ final class PhabricatorSearchApplicationSearchEngine 'ownerPHIDs', $this->readUsersFromRequest($request, 'ownerPHIDs')); - $saved->setParameter( - 'withUnowned', - $this->readBoolFromRequest($request, 'withUnowned')); - $saved->setParameter( 'subscriberPHIDs', $this->readPHIDsFromRequest($request, 'subscriberPHIDs')); @@ -46,8 +42,34 @@ final class PhabricatorSearchApplicationSearchEngine } public function buildQueryFromSavedQuery(PhabricatorSavedQuery $saved) { - $query = id(new PhabricatorSearchDocumentQuery()) - ->withSavedQuery($saved); + $query = new PhabricatorSearchDocumentQuery(); + + // Convert the saved query into a resolved form (without typeahead + // functions) which the fulltext search engines can execute. + $config = clone $saved; + $viewer = $this->requireViewer(); + + $datasource = id(new PhabricatorPeopleOwnerDatasource()) + ->setViewer($viewer); + $owner_phids = $this->readOwnerPHIDs($config); + $owner_phids = $datasource->evaluateTokens($owner_phids); + foreach ($owner_phids as $key => $phid) { + if ($phid == PhabricatorPeopleNoOwnerDatasource::FUNCTION_TOKEN) { + $config->setParameter('withUnowned', true); + unset($owner_phids[$key]); + } + } + $config->setParameter('ownerPHIDs', $owner_phids); + + + $datasource = id(new PhabricatorTypeaheadUserParameterizedDatasource()) + ->setViewer($viewer); + $author_phids = $config->getParameter('authorPHIDs', array()); + $author_phids = $datasource->evaluateTokens($author_phids); + $config->setParameter('authorPHIDs', $author_phids); + + $query->withSavedQuery($config); + return $query; } @@ -62,12 +84,10 @@ final class PhabricatorSearchApplicationSearchEngine $project_value = null; $author_phids = $saved->getParameter('authorPHIDs', array()); - $owner_phids = $saved->getParameter('ownerPHIDs', array()); + $owner_phids = $this->readOwnerPHIDs($saved); $subscriber_phids = $saved->getParameter('subscriberPHIDs', array()); $project_phids = $saved->getParameter('projectPHIDs', array()); - $with_unowned = $saved->getParameter('withUnowned', array()); - $status_values = $saved->getParameter('statuses', array()); $status_values = array_fuse($status_values); @@ -114,26 +134,19 @@ final class PhabricatorSearchApplicationSearchEngine id(new AphrontFormTokenizerControl()) ->setName('authorPHIDs') ->setLabel('Authors') - ->setDatasource(new PhabricatorPeopleDatasource()) + ->setDatasource(new PhabricatorTypeaheadUserParameterizedDatasource()) ->setValue($author_phids)) ->appendControl( id(new AphrontFormTokenizerControl()) ->setName('ownerPHIDs') ->setLabel('Owners') - ->setDatasource(new ManiphestOwnerDatasource()) + ->setDatasource(new PhabricatorPeopleOwnerDatasource()) ->setValue($owner_phids)) - ->appendChild( - id(new AphrontFormCheckboxControl()) - ->addCheckbox( - 'withUnowned', - 1, - pht('Show only unowned documents.'), - $with_unowned)) ->appendControl( id(new AphrontFormTokenizerControl()) ->setName('subscriberPHIDs') ->setLabel('Subscribers') - ->setDatasource(new PhabricatorPeopleDatasource()) + ->setDatasource(new PhabricatorMetaMTAMailableDatasource()) ->setValue($subscriber_phids)) ->appendControl( id(new AphrontFormTokenizerControl()) @@ -242,4 +255,15 @@ final class PhabricatorSearchApplicationSearchEngine return $results; } + private function readOwnerPHIDs(PhabricatorSavedQuery $saved) { + $owner_phids = $saved->getParameter('ownerPHIDs', array()); + + // This was an old checkbox from before typeahead functions. + if ($saved->getParameter('withUnowned')) { + $owner_phids[] = PhabricatorPeopleNoOwnerDatasource::FUNCTION_TOKEN; + } + + return $owner_phids; + } + }