From 587530d97335d0c958602adbf970af07e2124d2e Mon Sep 17 00:00:00 2001 From: epriestley Date: Mon, 27 May 2013 13:41:39 -0700 Subject: [PATCH] After saving a custom query, redirect to its results page Summary: Ref T2625. Currently, after saving a query the user is redirected to "/search/", which isn't especially useful. Instead, redirect them back into the application they came from and to the query results page. Also, query hashes may contain ".", which does not match `\w`. Use `[^/]` instead. Test Plan: Saved some custom queries. Reviewers: btrahan, blc Reviewed By: btrahan CC: aran Maniphest Tasks: T2625 Differential Revision: https://secure.phabricator.com/D6055 --- .../PhabricatorApplicationPaste.php | 2 +- .../query/PhabricatorPasteSearchEngine.php | 4 ++++ .../PhabricatorApplicationSearch.php | 2 +- .../PhabricatorSearchNameController.php | 19 +++++++++---------- .../PhabricatorApplicationSearchEngine.php | 9 +++++++++ .../search/storage/PhabricatorSavedQuery.php | 7 +++++++ 6 files changed, 31 insertions(+), 12 deletions(-) diff --git a/src/applications/paste/application/PhabricatorApplicationPaste.php b/src/applications/paste/application/PhabricatorApplicationPaste.php index 92cd6bb275..006114cfec 100644 --- a/src/applications/paste/application/PhabricatorApplicationPaste.php +++ b/src/applications/paste/application/PhabricatorApplicationPaste.php @@ -36,7 +36,7 @@ final class PhabricatorApplicationPaste extends PhabricatorApplication { 'create/' => 'PhabricatorPasteEditController', 'edit/(?P[1-9]\d*)/' => 'PhabricatorPasteEditController', 'filter/(?P\w+)/' => 'PhabricatorPasteListController', - 'query/(?P\w+)/'=> 'PhabricatorPasteListController', + 'query/(?P[^/]+)/'=> 'PhabricatorPasteListController', 'savedqueries/' => 'PhabricatorPasteQueriesController', ), ); diff --git a/src/applications/paste/query/PhabricatorPasteSearchEngine.php b/src/applications/paste/query/PhabricatorPasteSearchEngine.php index 51c18e81f8..553a7d67df 100644 --- a/src/applications/paste/query/PhabricatorPasteSearchEngine.php +++ b/src/applications/paste/query/PhabricatorPasteSearchEngine.php @@ -105,4 +105,8 @@ final class PhabricatorPasteSearchEngine return $this; } + public function getQueryResultsPageURI(PhabricatorSavedQuery $query) { + return '/paste/query/'.$query->getQueryKey().'/'; + } + } diff --git a/src/applications/search/application/PhabricatorApplicationSearch.php b/src/applications/search/application/PhabricatorApplicationSearch.php index e0e0ef4951..86945fbcdf 100644 --- a/src/applications/search/application/PhabricatorApplicationSearch.php +++ b/src/applications/search/application/PhabricatorApplicationSearch.php @@ -34,7 +34,7 @@ final class PhabricatorApplicationSearch extends PhabricatorApplication { 'index/(?P[^/]+)/' => 'PhabricatorSearchIndexController', 'hovercard/(?Pretrieve|test)/' => 'PhabricatorSearchHovercardController', - 'name/(?P\w+)/' => 'PhabricatorSearchNameController', + 'name/(?P[^/]+)/' => 'PhabricatorSearchNameController', ), ); } diff --git a/src/applications/search/controller/PhabricatorSearchNameController.php b/src/applications/search/controller/PhabricatorSearchNameController.php index b24e0e8fc9..0a3f986549 100644 --- a/src/applications/search/controller/PhabricatorSearchNameController.php +++ b/src/applications/search/controller/PhabricatorSearchNameController.php @@ -16,18 +16,17 @@ final class PhabricatorSearchNameController $request = $this->getRequest(); $user = $request->getUser(); - if ($this->queryKey) { - $saved_query = id(new PhabricatorSavedQueryQuery()) - ->setViewer($user) - ->withQueryKeys(array($this->queryKey)) - ->executeOne(); - if (!$saved_query) { - return new Aphront404Response(); - } - } else { + $saved_query = id(new PhabricatorSavedQueryQuery()) + ->setViewer($user) + ->withQueryKeys(array($this->queryKey)) + ->executeOne(); + + if (!$saved_query) { return new Aphront404Response(); } + $engine = $saved_query->newEngine(); + if ($request->isFormPost()) { $named_query = id(new PhabricatorNamedQuery()) ->setUserPHID($user->getPHID()) @@ -42,7 +41,7 @@ final class PhabricatorSearchNameController } return id(new AphrontRedirectResponse()) - ->setURI('/search/'); + ->setURI($engine->getQueryResultsPageURI($saved_query)); } $form = id(new AphrontFormView()) diff --git a/src/applications/search/engine/PhabricatorApplicationSearchEngine.php b/src/applications/search/engine/PhabricatorApplicationSearchEngine.php index d1dedbb2de..b837042392 100644 --- a/src/applications/search/engine/PhabricatorApplicationSearchEngine.php +++ b/src/applications/search/engine/PhabricatorApplicationSearchEngine.php @@ -34,4 +34,13 @@ abstract class PhabricatorApplicationSearchEngine { */ abstract public function buildSearchForm(PhabricatorSavedQuery $query); + + /** + * Return an application URI corresponding to the results page of a query. + * Normally, this is something like `/application/query/QUERYKEY/`. + * + * @param PhabricatorSavedQuery The query to build a URI for. + * @return string URI where the query can be executed. + */ + abstract public function getQueryResultsPageURI(PhabricatorSavedQuery $query); } diff --git a/src/applications/search/storage/PhabricatorSavedQuery.php b/src/applications/search/storage/PhabricatorSavedQuery.php index 460dc504d2..45da12fe46 100644 --- a/src/applications/search/storage/PhabricatorSavedQuery.php +++ b/src/applications/search/storage/PhabricatorSavedQuery.php @@ -31,12 +31,19 @@ final class PhabricatorSavedQuery extends PhabricatorSearchDAO throw new Exception(pht("Engine class is null.")); } + // Instantiate the engine to make sure it's valid. + $this->newEngine(); + $serial = $this->getEngineClassName().serialize($this->parameters); $this->queryKey = PhabricatorHash::digestForIndex($serial); return parent::save(); } + public function newEngine() { + return newv($this->getEngineClassName(), array()); + } + /* -( PhabricatorPolicyInterface )----------------------------------------- */