From 758586abda2d121be29b4c27635364e91aaf8ef4 Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 5 Jun 2013 05:28:25 -0700 Subject: [PATCH] Allow builtin named queries to be disabled Summary: Applications come with builtin queries, but users might want to get rid of them. Allow users to disable named queries if they prefer. This has one funky behavior, which is that the first time you disable a named query it goes to the top of your list. That will be fixed in the next diff, which will make them reorderable. Test Plan: Added/edited/removed named queries, disabled/enabled builtin named queries. Reviewers: chad Reviewed By: chad CC: aran Differential Revision: https://secure.phabricator.com/D6128 --- .../sql/patches/20130602.namedqueries.sql | 8 ++ .../PhabricatorApplicationSearch.php | 3 +- ...PhabricatorApplicationSearchController.php | 39 ++++++---- .../PhabricatorSearchDeleteController.php | 76 +++++++++++++++---- .../PhabricatorApplicationSearchEngine.php | 47 ++++++++++-- .../search/storage/PhabricatorNamedQuery.php | 21 ++--- .../patch/PhabricatorBuiltinPatchList.php | 4 + 7 files changed, 144 insertions(+), 54 deletions(-) create mode 100644 resources/sql/patches/20130602.namedqueries.sql diff --git a/resources/sql/patches/20130602.namedqueries.sql b/resources/sql/patches/20130602.namedqueries.sql new file mode 100644 index 0000000000..e29decc79e --- /dev/null +++ b/resources/sql/patches/20130602.namedqueries.sql @@ -0,0 +1,8 @@ +ALTER TABLE {$NAMESPACE}_search.search_namedquery + ADD isBuiltin BOOL NOT NULL DEFAULT 0; + +ALTER TABLE {$NAMESPACE}_search.search_namedquery + ADD isDisabled BOOL NOT NULL DEFAULT 0; + +ALTER TABLE {$NAMESPACE}_search.search_namedquery + ADD sequence INT UNSIGNED NOT NULL DEFAULT 0; diff --git a/src/applications/search/application/PhabricatorApplicationSearch.php b/src/applications/search/application/PhabricatorApplicationSearch.php index 7e6d027ebe..c3723fe53d 100644 --- a/src/applications/search/application/PhabricatorApplicationSearch.php +++ b/src/applications/search/application/PhabricatorApplicationSearch.php @@ -35,7 +35,8 @@ final class PhabricatorApplicationSearch extends PhabricatorApplication { 'hovercard/(?Pretrieve|test)/' => 'PhabricatorSearchHovercardController', 'edit/(?P[^/]+)/' => 'PhabricatorSearchEditController', - 'delete/(?P[^/]+)/' => 'PhabricatorSearchDeleteController', + 'delete/(?P[^/]+)/(?P[^/]+)/' + => 'PhabricatorSearchDeleteController', ), ); } diff --git a/src/applications/search/controller/PhabricatorApplicationSearchController.php b/src/applications/search/controller/PhabricatorApplicationSearchController.php index d059f50640..a4d718a0e8 100644 --- a/src/applications/search/controller/PhabricatorApplicationSearchController.php +++ b/src/applications/search/controller/PhabricatorApplicationSearchController.php @@ -226,40 +226,47 @@ final class PhabricatorApplicationSearchController $engine = $this->getSearchEngine(); $nav = $this->getNavigation(); - $named_queries = id(new PhabricatorNamedQueryQuery()) - ->setViewer($user) - ->withUserPHIDs(array($user->getPHID())) - ->withEngineClassNames(array(get_class($engine))) - ->execute(); - - $named_queries += $engine->getBuiltinQueries(); + $named_queries = $engine->loadAllNamedQueries(); $list = new PhabricatorObjectItemListView(); $list->setUser($user); foreach ($named_queries as $named_query) { + $class = get_class($engine); + $key = $named_query->getQueryKey(); + $date_created = phabricator_datetime( $named_query->getDateCreated(), $user); $item = id(new PhabricatorObjectItemView()) ->setHeader($named_query->getQueryName()) - ->setHref($engine->getQueryResultsPageURI($named_query->getQueryKey())); + ->setHref($engine->getQueryResultsPageURI($key)); + + if ($named_query->getIsBuiltin() && $named_query->getIsDisabled()) { + $icon = 'new'; + } else { + $icon = 'delete'; + } + + $item->addAction( + id(new PhabricatorMenuItemView()) + ->setIcon($icon) + ->setHref('/search/delete/'.$key.'/'.$class.'/') + ->setWorkflow(true)); if ($named_query->getIsBuiltin()) { - $item->addIcon('lock-grey', pht('Builtin')); + if ($named_query->getIsDisabled()) { + $item->addIcon('delete-grey', pht('Disabled')); + } else { + $item->addIcon('lock-grey', pht('Builtin')); + } $item->setBarColor('grey'); } else { - $item->addIcon('none', $date_created); - $item->addAction( - id(new PhabricatorMenuItemView()) - ->setIcon('delete') - ->setHref('/search/delete/'.$named_query->getQueryKey().'/') - ->setWorkflow(true)); $item->addAction( id(new PhabricatorMenuItemView()) ->setIcon('edit') - ->setHref('/search/edit/'.$named_query->getQueryKey().'/')); + ->setHref('/search/edit/'.$key.'/')); } $list->addItem($item); diff --git a/src/applications/search/controller/PhabricatorSearchDeleteController.php b/src/applications/search/controller/PhabricatorSearchDeleteController.php index 3745d06d75..eff5c50202 100644 --- a/src/applications/search/controller/PhabricatorSearchDeleteController.php +++ b/src/applications/search/controller/PhabricatorSearchDeleteController.php @@ -7,52 +7,96 @@ final class PhabricatorSearchDeleteController extends PhabricatorSearchBaseController { private $queryKey; + private $engineClass; public function willProcessRequest(array $data) { $this->queryKey = idx($data, 'queryKey'); + $this->engineClass = idx($data, 'engine'); } public function processRequest() { $request = $this->getRequest(); $user = $request->getUser(); - $saved_query = id(new PhabricatorSavedQueryQuery()) - ->setViewer($user) - ->withQueryKeys(array($this->queryKey)) - ->executeOne(); + $key = $this->queryKey; - if (!$saved_query) { - return new Aphront404Response(); + $base_class = 'PhabricatorApplicationSearchEngine'; + if (!is_subclass_of($this->engineClass, $base_class)) { + return new Aphront400Response(); } - $engine = $saved_query->newEngine(); + $engine = newv($this->engineClass, array()); + $engine->setViewer($user); $named_query = id(new PhabricatorNamedQueryQuery()) ->setViewer($user) - ->withQueryKeys(array($saved_query->getQueryKey())) + ->withEngineClassNames(array($this->engineClass)) + ->withQueryKeys(array($key)) ->withUserPHIDs(array($user->getPHID())) ->executeOne(); + + if (!$named_query && $engine->isBuiltinQuery($key)) { + $named_query = id(new PhabricatorNamedQuery()) + ->setUserPHID($user->getPHID()) + ->setQueryName('(BUILTIN)') + ->setQueryKey($key) + ->setEngineClassName($this->engineClass) + ->setIsBuiltin(true); + } + if (!$named_query) { return new Aphront404Response(); } + $builtin = null; + if ($engine->isBuiltinQuery($key)) { + $builtin = $engine->getBuiltinQuery($key); + } + $return_uri = $engine->getQueryManagementURI(); if ($request->isDialogFormPost()) { - $named_query->delete(); + if ($named_query->getIsBuiltin()) { + $named_query->setIsDisabled((int)(!$named_query->getIsDisabled())); + $named_query->save(); + } else { + $named_query->delete(); + } + return id(new AphrontRedirectResponse())->setURI($return_uri); } + if ($named_query->getIsBuiltin()) { + if ($named_query->getIsDisabled()) { + $title = pht('Enable Query?'); + $desc = pht( + 'Enable the built-in query "%s"? It will appear in your menu again.', + $builtin->getQueryName()); + $button = pht('Enable Query'); + } else { + $title = pht('Disable Query?'); + $desc = pht( + 'This built-in query can not be deleted, but you can disable it so '. + 'it does not appear in your query menu. You can enable it again '. + 'later. Disable built-in query "%s"?', + $builtin->getQueryName()); + $button = pht('Disable Query'); + } + } else { + $title = pht('Really Delete Query?'); + $desc = pht( + 'Really delete the query "%s"? You can not undo this. Remember '. + 'all the great times you had filtering results together?', + $named_query->getQueryName()); + $button = pht('Delete Query'); + } + $dialog = id(new AphrontDialogView()) ->setUser($user) - ->setTitle(pht("Really Delete Query?")) - ->appendChild( - pht( - 'Really delete the query "%s"? You can not undo this. Remember '. - 'all the great times you had filtering results together?', - $named_query->getQueryName())) + ->setTitle($title) + ->appendChild($desc) ->addCancelButton($return_uri) - ->addSubmitButton(pht('Delete Query')); + ->addSubmitButton($button); return id(new AphrontDialogResponse())->setDialog($dialog); } diff --git a/src/applications/search/engine/PhabricatorApplicationSearchEngine.php b/src/applications/search/engine/PhabricatorApplicationSearchEngine.php index ed69cb4fa1..830a36833d 100644 --- a/src/applications/search/engine/PhabricatorApplicationSearchEngine.php +++ b/src/applications/search/engine/PhabricatorApplicationSearchEngine.php @@ -111,13 +111,7 @@ abstract class PhabricatorApplicationSearchEngine { $menu->newLabel(pht('Queries')); - $named_queries = id(new PhabricatorNamedQueryQuery()) - ->setViewer($viewer) - ->withUserPHIDs(array($viewer->getPHID())) - ->withEngineClassNames(array(get_class($this))) - ->execute(); - - $named_queries = $named_queries + $this->getBuiltinQueries($viewer); + $named_queries = $this->loadEnabledNamedQueries(); foreach ($named_queries as $query) { $key = $query->getQueryKey(); @@ -137,6 +131,45 @@ abstract class PhabricatorApplicationSearchEngine { return $this; } + public function loadAllNamedQueries() { + $viewer = $this->requireViewer(); + + $named_queries = id(new PhabricatorNamedQueryQuery()) + ->setViewer($viewer) + ->withUserPHIDs(array($viewer->getPHID())) + ->withEngineClassNames(array(get_class($this))) + ->execute(); + $named_queries = mpull($named_queries, null, 'getQueryKey'); + + $builtin = $this->getBuiltinQueries($viewer); + $builtin = mpull($builtin, null, 'getQueryKey'); + + foreach ($named_queries as $key => $named_query) { + if ($named_query->getIsBuiltin()) { + if (isset($builtin[$key])) { + $named_queries[$key]->setQueryName($builtin[$key]->getQueryName()); + unset($builtin[$key]); + } else { + unset($named_queries[$key]); + } + } + + unset($builtin[$key]); + } + + return $named_queries + $builtin; + } + + public function loadEnabledNamedQueries() { + $named_queries = $this->loadAllNamedQueries(); + foreach ($named_queries as $key => $named_query) { + if ($named_query->getIsBuiltin() && $named_query->getIsDisabled()) { + unset($named_queries[$key]); + } + } + return $named_queries; + } + /* -( Builtin Queries )---------------------------------------------------- */ diff --git a/src/applications/search/storage/PhabricatorNamedQuery.php b/src/applications/search/storage/PhabricatorNamedQuery.php index 0d420c9d08..8bc11b6cce 100644 --- a/src/applications/search/storage/PhabricatorNamedQuery.php +++ b/src/applications/search/storage/PhabricatorNamedQuery.php @@ -6,21 +6,14 @@ final class PhabricatorNamedQuery extends PhabricatorSearchDAO implements PhabricatorPolicyInterface { - protected $queryKey = ""; - protected $queryName = ""; - protected $userPHID = ""; - protected $engineClassName = ""; + protected $queryKey; + protected $queryName; + protected $userPHID; + protected $engineClassName; - private $isBuiltin; - - public function setIsBuiltin($is_builtin) { - $this->isBuiltin = $is_builtin; - return $this; - } - - public function getIsBuiltin() { - return $this->isBuiltin; - } + protected $isBuiltin = 0; + protected $isDisabled = 0; + protected $sequence = 0; /* -( PhabricatorPolicyInterface )----------------------------------------- */ diff --git a/src/infrastructure/storage/patch/PhabricatorBuiltinPatchList.php b/src/infrastructure/storage/patch/PhabricatorBuiltinPatchList.php index bb4567bef8..f6e12ac0ca 100644 --- a/src/infrastructure/storage/patch/PhabricatorBuiltinPatchList.php +++ b/src/infrastructure/storage/patch/PhabricatorBuiltinPatchList.php @@ -1346,6 +1346,10 @@ final class PhabricatorBuiltinPatchList extends PhabricatorSQLPatchList { 'type' => 'sql', 'name' => $this->getPatchPath('20130602.morediviner.sql'), ), + '20130602.namedqueries.sql' => array( + 'type' => 'sql', + 'name' => $this->getPatchPath('20130602.namedqueries.sql'), + ), ); } }