From 7f0fb63c445027cd5c9148bd9d8f7d4263b68802 Mon Sep 17 00:00:00 2001 From: epriestley Date: Thu, 17 Jul 2014 15:49:00 -0700 Subject: [PATCH] Modernize "owner" typeahead datasource Summary: Ref T4420. This one is users plus "upforgrabs". I renamed that to "none" and gave it a special visual style to make it more discoverable. Future diffs will improve this. Test Plan: - Used it in global search. - Used it in batch editor. Reviewers: btrahan Reviewed By: btrahan Subscribers: epriestley Maniphest Tasks: T4420 Differential Revision: https://secure.phabricator.com/D9891 --- src/__phutil_library_map__.php | 6 +- .../ManiphestBatchEditController.php | 6 +- ...abricatorSearchApplicationSearchEngine.php | 2 +- .../PhabricatorApplicationTypeahead.php | 2 - ...torTypeaheadCommonDatasourceController.php | 179 ------------------ .../PhabricatorTypeaheadNoOwnerDatasource.php | 28 +++ .../PhabricatorTypeaheadOwnerDatasource.php | 17 ++ .../control/AphrontFormTokenizerControl.php | 34 +--- 8 files changed, 60 insertions(+), 214 deletions(-) delete mode 100644 src/applications/typeahead/controller/PhabricatorTypeaheadCommonDatasourceController.php create mode 100644 src/applications/typeahead/datasource/PhabricatorTypeaheadNoOwnerDatasource.php create mode 100644 src/applications/typeahead/datasource/PhabricatorTypeaheadOwnerDatasource.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 1010f952da..86ff4fc8cc 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -2333,12 +2333,13 @@ phutil_register_library_map(array( 'PhabricatorTranslationsConfigOptions' => 'applications/config/option/PhabricatorTranslationsConfigOptions.php', 'PhabricatorTrivialTestCase' => 'infrastructure/testing/__tests__/PhabricatorTrivialTestCase.php', 'PhabricatorTwoColumnExample' => 'applications/uiexample/examples/PhabricatorTwoColumnExample.php', - 'PhabricatorTypeaheadCommonDatasourceController' => 'applications/typeahead/controller/PhabricatorTypeaheadCommonDatasourceController.php', 'PhabricatorTypeaheadCompositeDatasource' => 'applications/typeahead/datasource/PhabricatorTypeaheadCompositeDatasource.php', 'PhabricatorTypeaheadDatasource' => 'applications/typeahead/datasource/PhabricatorTypeaheadDatasource.php', 'PhabricatorTypeaheadDatasourceController' => 'applications/typeahead/controller/PhabricatorTypeaheadDatasourceController.php', 'PhabricatorTypeaheadModularDatasourceController' => 'applications/typeahead/controller/PhabricatorTypeaheadModularDatasourceController.php', 'PhabricatorTypeaheadMonogramDatasource' => 'applications/typeahead/datasource/PhabricatorTypeaheadMonogramDatasource.php', + 'PhabricatorTypeaheadNoOwnerDatasource' => 'applications/typeahead/datasource/PhabricatorTypeaheadNoOwnerDatasource.php', + 'PhabricatorTypeaheadOwnerDatasource' => 'applications/typeahead/datasource/PhabricatorTypeaheadOwnerDatasource.php', 'PhabricatorTypeaheadResult' => 'applications/typeahead/storage/PhabricatorTypeaheadResult.php', 'PhabricatorTypeaheadRuntimeCompositeDatasource' => 'applications/typeahead/datasource/PhabricatorTypeaheadRuntimeCompositeDatasource.php', 'PhabricatorUIConfigOptions' => 'applications/config/option/PhabricatorUIConfigOptions.php', @@ -5173,12 +5174,13 @@ phutil_register_library_map(array( 'PhabricatorTranslationsConfigOptions' => 'PhabricatorApplicationConfigOptions', 'PhabricatorTrivialTestCase' => 'PhabricatorTestCase', 'PhabricatorTwoColumnExample' => 'PhabricatorUIExample', - 'PhabricatorTypeaheadCommonDatasourceController' => 'PhabricatorTypeaheadDatasourceController', 'PhabricatorTypeaheadCompositeDatasource' => 'PhabricatorTypeaheadDatasource', 'PhabricatorTypeaheadDatasource' => 'Phobject', 'PhabricatorTypeaheadDatasourceController' => 'PhabricatorController', 'PhabricatorTypeaheadModularDatasourceController' => 'PhabricatorTypeaheadDatasourceController', 'PhabricatorTypeaheadMonogramDatasource' => 'PhabricatorTypeaheadDatasource', + 'PhabricatorTypeaheadNoOwnerDatasource' => 'PhabricatorTypeaheadDatasource', + 'PhabricatorTypeaheadOwnerDatasource' => 'PhabricatorTypeaheadCompositeDatasource', 'PhabricatorTypeaheadRuntimeCompositeDatasource' => 'PhabricatorTypeaheadCompositeDatasource', 'PhabricatorUIConfigOptions' => 'PhabricatorApplicationConfigOptions', 'PhabricatorUIExampleRenderController' => 'PhabricatorController', diff --git a/src/applications/maniphest/controller/ManiphestBatchEditController.php b/src/applications/maniphest/controller/ManiphestBatchEditController.php index 59f361931a..0488000b3a 100644 --- a/src/applications/maniphest/controller/ManiphestBatchEditController.php +++ b/src/applications/maniphest/controller/ManiphestBatchEditController.php @@ -63,6 +63,7 @@ final class ManiphestBatchEditController extends ManiphestController { $projects_source = new PhabricatorProjectDatasource(); $mailable_source = new PhabricatorMetaMTAMailableDatasource(); + $owner_source = new PhabricatorTypeaheadOwnerDatasource(); require_celerity_resource('maniphest-batch-editor'); Javelin::initBehavior( @@ -76,9 +77,8 @@ final class ManiphestBatchEditController extends ManiphestController { 'placeholder' => $projects_source->getPlaceholderText(), ), 'owner' => array( - 'src' => '/typeahead/common/searchowner/', - 'placeholder' => pht( - 'Type a user name or "upforgrabs" to unassign...'), + 'src' => $owner_source->getDatasourceURI(), + 'placeholder' => $owner_source->getPlaceholderText(), 'limit' => 1, ), 'cc' => array( diff --git a/src/applications/search/query/PhabricatorSearchApplicationSearchEngine.php b/src/applications/search/query/PhabricatorSearchApplicationSearchEngine.php index 76c7b4013f..57aa9c4db4 100644 --- a/src/applications/search/query/PhabricatorSearchApplicationSearchEngine.php +++ b/src/applications/search/query/PhabricatorSearchApplicationSearchEngine.php @@ -142,7 +142,7 @@ final class PhabricatorSearchApplicationSearchEngine id(new AphrontFormTokenizerControl()) ->setName('ownerPHIDs') ->setLabel('Owners') - ->setDatasource('/typeahead/common/searchowner/') + ->setDatasource(new PhabricatorTypeaheadOwnerDatasource()) ->setValue($owner_handles)) ->appendChild( id(new AphrontFormCheckboxControl()) diff --git a/src/applications/typeahead/application/PhabricatorApplicationTypeahead.php b/src/applications/typeahead/application/PhabricatorApplicationTypeahead.php index 5a8a0b69f2..0e5c0afd4f 100644 --- a/src/applications/typeahead/application/PhabricatorApplicationTypeahead.php +++ b/src/applications/typeahead/application/PhabricatorApplicationTypeahead.php @@ -5,8 +5,6 @@ final class PhabricatorApplicationTypeahead extends PhabricatorApplication { public function getRoutes() { return array( '/typeahead/' => array( - 'common/(?P\w+)/' - => 'PhabricatorTypeaheadCommonDatasourceController', 'class/(?:(?P\w+)/)?' => 'PhabricatorTypeaheadModularDatasourceController', ), diff --git a/src/applications/typeahead/controller/PhabricatorTypeaheadCommonDatasourceController.php b/src/applications/typeahead/controller/PhabricatorTypeaheadCommonDatasourceController.php deleted file mode 100644 index dc54c9b7d6..0000000000 --- a/src/applications/typeahead/controller/PhabricatorTypeaheadCommonDatasourceController.php +++ /dev/null @@ -1,179 +0,0 @@ -type = $data['type']; - } - - public function processRequest() { - $request = $this->getRequest(); - $viewer = $request->getUser(); - $query = $request->getStr('q'); - $raw_query = $request->getStr('raw'); - - $need_users = false; - $need_upforgrabs = false; - $need_rich_data = false; - switch ($this->type) { - case 'searchowner': - $need_users = true; - $need_upforgrabs = true; - break; - } - - $results = array(); - - if ($need_upforgrabs) { - $results[] = id(new PhabricatorTypeaheadResult()) - ->setName('upforgrabs (Up For Grabs)') - ->setPHID(ManiphestTaskOwner::OWNER_UP_FOR_GRABS); - } - - if ($need_users) { - $columns = array( - 'isSystemAgent', - 'isAdmin', - 'isDisabled', - 'userName', - 'realName', - 'phid'); - - if ($query) { - // This is an arbitrary limit which is just larger than any limit we - // actually use in the application. - - // TODO: The datasource should pass this in the query. - $limit = 15; - - $user_table = new PhabricatorUser(); - $conn_r = $user_table->establishConnection('r'); - $ids = queryfx_all( - $conn_r, - 'SELECT id FROM %T WHERE username LIKE %> - ORDER BY username ASC LIMIT %d', - $user_table->getTableName(), - $query, - $limit); - $ids = ipull($ids, 'id'); - - if (count($ids) < $limit) { - // If we didn't find enough username hits, look for real name hits. - // We need to pull the entire pagesize so that we end up with the - // right number of items if this query returns many duplicate IDs - // that we've already selected. - - $realname_ids = queryfx_all( - $conn_r, - 'SELECT DISTINCT userID FROM %T WHERE token LIKE %> - ORDER BY token ASC LIMIT %d', - PhabricatorUser::NAMETOKEN_TABLE, - $query, - $limit); - $realname_ids = ipull($realname_ids, 'userID'); - $ids = array_merge($ids, $realname_ids); - - $ids = array_unique($ids); - $ids = array_slice($ids, 0, $limit); - } - - // Always add the logged-in user because some tokenizers autosort them - // first. They'll be filtered out on the client side if they don't - // match the query. - $ids[] = $request->getUser()->getID(); - - if ($ids) { - $users = id(new PhabricatorUser())->loadColumnsWhere( - $columns, - 'id IN (%Ld)', - $ids); - } else { - $users = array(); - } - } else { - $users = id(new PhabricatorUser())->loadColumns($columns); - } - - if ($need_rich_data) { - $phids = mpull($users, 'getPHID'); - $handles = $this->loadViewerHandles($phids); - } - - foreach ($users as $user) { - $closed = null; - if ($user->getIsDisabled()) { - $closed = pht('Disabled'); - } else if ($user->getIsSystemAgent()) { - $closed = pht('Bot/Script'); - } - - $result = id(new PhabricatorTypeaheadResult()) - ->setName($user->getFullName()) - ->setURI('/p/'.$user->getUsername()) - ->setPHID($user->getPHID()) - ->setPriorityString($user->getUsername()) - ->setIcon('fa-user bluegrey') - ->setPriorityType('user') - ->setClosed($closed); - - if ($need_rich_data) { - $display_type = 'User'; - if ($user->getIsAdmin()) { - $display_type = 'Administrator'; - } - $result->setDisplayType($display_type); - $result->setImageURI($handles[$user->getPHID()]->getImageURI()); - } - $results[] = $result; - } - } - - $content = mpull($results, 'getWireFormat'); - - if ($request->isAjax()) { - return id(new AphrontAjaxResponse())->setContent($content); - } - - // If there's a non-Ajax request to this endpoint, show results in a tabular - // format to make it easier to debug typeahead output. - - $rows = array(); - foreach ($results as $result) { - $wire = $result->getWireFormat(); - $rows[] = $wire; - } - - $table = new AphrontTableView($rows); - $table->setHeaders( - array( - 'Name', - 'URI', - 'PHID', - 'Priority', - 'Display Name', - 'Display Type', - 'Image URI', - 'Priority Type', - 'Sprite Class', - )); - - $panel = new AphrontPanelView(); - $panel->setHeader('Typeahead Results'); - $panel->appendChild($table); - - return $this->buildStandardPageResponse( - $panel, - array( - 'title' => pht('Typeahead Results'), - 'device' => true - )); - } - -} diff --git a/src/applications/typeahead/datasource/PhabricatorTypeaheadNoOwnerDatasource.php b/src/applications/typeahead/datasource/PhabricatorTypeaheadNoOwnerDatasource.php new file mode 100644 index 0000000000..76dd3148d8 --- /dev/null +++ b/src/applications/typeahead/datasource/PhabricatorTypeaheadNoOwnerDatasource.php @@ -0,0 +1,28 @@ +getViewer(); + $raw_query = $this->getRawQuery(); + + $results = array(); + + $results[] = id(new PhabricatorTypeaheadResult()) + ->setName(pht('None')) + ->setIcon('fa-ban orange') + ->setPHID(ManiphestTaskOwner::OWNER_UP_FOR_GRABS); + + return $results; + } + +} diff --git a/src/applications/typeahead/datasource/PhabricatorTypeaheadOwnerDatasource.php b/src/applications/typeahead/datasource/PhabricatorTypeaheadOwnerDatasource.php new file mode 100644 index 0000000000..a2aaf4f991 --- /dev/null +++ b/src/applications/typeahead/datasource/PhabricatorTypeaheadOwnerDatasource.php @@ -0,0 +1,17 @@ +datasource = $datasource; return $this; } @@ -43,8 +43,11 @@ final class AphrontFormTokenizerControl extends AphrontFormControl { $id = celerity_generate_unique_node_id(); } + $placeholder = null; if (!strlen($this->placeholder)) { - $placeholder = $this->getDefaultPlaceholder(); + if ($this->datasource) { + $placeholder = $this->datasource->getPlaceholderText(); + } } else { $placeholder = $this->placeholder; } @@ -59,10 +62,9 @@ final class AphrontFormTokenizerControl extends AphrontFormControl { $username = $this->user->getUsername(); } - if ($this->datasource instanceof PhabricatorTypeaheadDatasource) { + $datasource_uri = null; + if ($this->datasource) { $datasource_uri = $this->datasource->getDatasourceURI(); - } else { - $datasource_uri = $this->datasource; } if (!$this->disableBehavior) { @@ -80,26 +82,4 @@ final class AphrontFormTokenizerControl extends AphrontFormControl { return $template->render(); } - private function getDefaultPlaceholder() { - $datasource = $this->datasource; - - if ($datasource instanceof PhabricatorTypeaheadDatasource) { - return $datasource->getPlaceholderText(); - } - - $matches = null; - if (!preg_match('@^/typeahead/common/(.*)/$@', $datasource, $matches)) { - return null; - } - - $request = $matches[1]; - - $map = array( - 'searchowner' => pht('Type a user name...'), - ); - - return idx($map, $request); - } - - }