From ca6b9f66e198eff3959ceb0ade377d7dbc40ad7e Mon Sep 17 00:00:00 2001 From: epriestley Date: Mon, 3 Feb 2014 14:29:49 -0800 Subject: [PATCH] Replace "search scope" with selectable default behavior Summary: Fixes T4365. See discussion in D8123. This implements the most conservative solution of approaches discussed in D8123. Basically: - When you search in primary search, we overwrite "query" in your default (topmost) search filter, and execute that. This doesn't implement any of the other "sticky" stuff, where the query sticks around. Maybe we'll do that eventually, but it gets messy and could be confusing. Practically, this addresses the major use case in the wild, which is to make the menu bar search mean "Open Tasks" by default. This also removes the old, obsolete "search scope" stuff. A long time ago, searching from within Maniphest would search tasks, etc., but this was pretty weird and confusing and is no longer used, and no one complained when we got rid of it. Test Plan: Dragged "Open Tasks" to my top search, searched for "asdf", got "asdf in open tasks" results. Reviewers: btrahan, chad Reviewed By: btrahan CC: bigo, aran Maniphest Tasks: T4365 Differential Revision: https://secure.phabricator.com/D8135 --- .../constants/PhabricatorSearchScope.php | 41 ----------------- .../PhabricatorSearchController.php | 45 ++++++++++++++++++- src/view/page/PhabricatorStandardPageView.php | 13 +----- .../menu/PhabricatorMainMenuSearchView.php | 18 +++----- .../page/menu/PhabricatorMainMenuView.php | 11 ----- 5 files changed, 50 insertions(+), 78 deletions(-) delete mode 100644 src/applications/search/constants/PhabricatorSearchScope.php diff --git a/src/applications/search/constants/PhabricatorSearchScope.php b/src/applications/search/constants/PhabricatorSearchScope.php deleted file mode 100644 index 8238ea1a5c..0000000000 --- a/src/applications/search/constants/PhabricatorSearchScope.php +++ /dev/null @@ -1,41 +0,0 @@ - 'All Documents', - self::SCOPE_OPEN_TASKS => 'Open Tasks', - self::SCOPE_WIKI => 'Wiki Documents', - self::SCOPE_OPEN_REVISIONS => 'Open Revisions', - self::SCOPE_COMMITS => 'Commits', - self::SCOPE_QUESTIONS => 'Ponder Questions', - ); - } - - public static function getScopePlaceholder($scope) { - switch ($scope) { - case self::SCOPE_OPEN_TASKS: - return pht('Search Open Tasks'); - case self::SCOPE_WIKI: - return pht('Search Wiki Documents'); - case self::SCOPE_OPEN_REVISIONS: - return pht('Search Open Revisions'); - case self::SCOPE_COMMITS: - return pht('Search Commits'); - default: - return pht('Search'); - } - } - -} diff --git a/src/applications/search/controller/PhabricatorSearchController.php b/src/applications/search/controller/PhabricatorSearchController.php index 82be2e14b9..aa2d87eb97 100644 --- a/src/applications/search/controller/PhabricatorSearchController.php +++ b/src/applications/search/controller/PhabricatorSearchController.php @@ -30,9 +30,52 @@ final class PhabricatorSearchController } } + $engine = new PhabricatorSearchApplicationSearchEngine(); + $engine->setViewer($viewer); + + // NOTE: This is a little weird. If we're coming from primary search, we + // load the user's first search filter and overwrite the "query" part of + // it, then send them to that result page. This is sort of odd, but lets + // users choose a default query like "Open Tasks" in a reasonable way, + // with only this piece of somewhat-sketchy code. See discussion in T4365. + + if ($request->getBool('search:primary')) { + $named_queries = $engine->loadEnabledNamedQueries(); + if ($named_queries) { + $named = head($named_queries); + + $query_key = $named->getQueryKey(); + $saved = null; + if ($engine->isBuiltinQuery($query_key)) { + $saved = $engine->buildSavedQueryFromBuiltin($query_key); + } else { + $saved = id(new PhabricatorSavedQueryQuery()) + ->setViewer($viewer) + ->withQueryKeys(array($query_key)) + ->executeOne(); + } + + if ($saved) { + $saved->setParameter('query', $request->getStr('query')); + $unguarded = AphrontWriteGuard::beginScopedUnguardedWrites(); + try { + $saved->setID(null)->save(); + } catch (AphrontQueryDuplicateKeyException $ex) { + // Ignore, this is just a repeated search. + } + unset($unguarded); + + $results_uri = $engine->getQueryResultsPageURI( + $saved->getQueryKey()).'#R'; + + return id(new AphrontRedirectResponse())->setURI($results_uri); + } + } + } + $controller = id(new PhabricatorApplicationSearchController($request)) ->setQueryKey($this->queryKey) - ->setSearchEngine(new PhabricatorSearchApplicationSearchEngine()) + ->setSearchEngine($engine) ->setUseOffsetPaging(true) ->setNavigation($this->buildSideNavView()); diff --git a/src/view/page/PhabricatorStandardPageView.php b/src/view/page/PhabricatorStandardPageView.php index d31c3567b8..f4cd10be14 100644 --- a/src/view/page/PhabricatorStandardPageView.php +++ b/src/view/page/PhabricatorStandardPageView.php @@ -13,7 +13,6 @@ final class PhabricatorStandardPageView extends PhabricatorBarePageView { private $menuContent; private $showChrome = true; private $disableConsole; - private $searchDefaultScope; private $pageObjects = array(); private $applicationMenu; @@ -58,15 +57,6 @@ final class PhabricatorStandardPageView extends PhabricatorBarePageView { return $this->showChrome; } - public function setSearchDefaultScope($search_default_scope) { - $this->searchDefaultScope = $search_default_scope; - return $this; - } - - public function getSearchDefaultScope() { - return $this->searchDefaultScope; - } - public function appendPageObjects(array $objs) { foreach ($objs as $obj) { $this->pageObjects[] = $obj; @@ -212,8 +202,7 @@ final class PhabricatorStandardPageView extends PhabricatorBarePageView { } $menu = id(new PhabricatorMainMenuView()) - ->setUser($viewer) - ->setDefaultSearchScope($this->getSearchDefaultScope()); + ->setUser($viewer); if ($this->getController()) { $menu->setController($this->getController()); diff --git a/src/view/page/menu/PhabricatorMainMenuSearchView.php b/src/view/page/menu/PhabricatorMainMenuSearchView.php index 4d2cd11045..353b148775 100644 --- a/src/view/page/menu/PhabricatorMainMenuSearchView.php +++ b/src/view/page/menu/PhabricatorMainMenuSearchView.php @@ -2,14 +2,8 @@ final class PhabricatorMainMenuSearchView extends AphrontView { - private $scope; private $id; - public function setScope($scope) { - $this->scope = $scope; - return $this; - } - public function getID() { if (!$this->id) { $this->id = celerity_generate_unique_node_id(); @@ -32,8 +26,6 @@ final class PhabricatorMainMenuSearchView extends AphrontView { 'autocomplete' => 'off', )); - $scope = $this->scope; - $target = javelin_tag( 'div', array( @@ -49,15 +41,15 @@ final class PhabricatorMainMenuSearchView extends AphrontView { 'input' => $search_id, 'src' => '/typeahead/common/mainsearch/', 'limit' => 10, - 'placeholder' => PhabricatorSearchScope::getScopePlaceholder($scope), + 'placeholder' => pht('Search'), )); - $scope_input = phutil_tag( + $primary_input = phutil_tag( 'input', array( 'type' => 'hidden', - 'name' => 'scope', - 'value' => $scope, + 'name' => 'search:primary', + 'value' => 'true', )); $form = phabricator_form( @@ -69,7 +61,7 @@ final class PhabricatorMainMenuSearchView extends AphrontView { phutil_tag_div('phabricator-main-menu-search-container', array( $input, phutil_tag('button', array(), pht('Search')), - $scope_input, + $primary_input, $target, ))); diff --git a/src/view/page/menu/PhabricatorMainMenuView.php b/src/view/page/menu/PhabricatorMainMenuView.php index 9f656e2c78..1967303675 100644 --- a/src/view/page/menu/PhabricatorMainMenuView.php +++ b/src/view/page/menu/PhabricatorMainMenuView.php @@ -2,7 +2,6 @@ final class PhabricatorMainMenuView extends AphrontView { - private $defaultSearchScope; private $controller; private $applicationMenu; @@ -24,15 +23,6 @@ final class PhabricatorMainMenuView extends AphrontView { return $this->controller; } - public function setDefaultSearchScope($default_search_scope) { - $this->defaultSearchScope = $default_search_scope; - return $this; - } - - public function getDefaultSearchScope() { - return $this->defaultSearchScope; - } - public function render() { $user = $this->user; @@ -105,7 +95,6 @@ final class PhabricatorMainMenuView extends AphrontView { if ($show_search) { $search = new PhabricatorMainMenuSearchView(); $search->setUser($user); - $search->setScope($this->getDefaultSearchScope()); $result = $search; $pref_shortcut = PhabricatorUserPreferences::PREFERENCE_SEARCH_SHORTCUT;