From 589ae8d26d9e47b88dbefde5e91f0733fec14ef6 Mon Sep 17 00:00:00 2001 From: epriestley Date: Fri, 2 Aug 2013 06:53:01 -0700 Subject: [PATCH] Use ApplicationSearch in Herald Summary: Ref T2769. Ref T2625. Herald is currently a giant mishmash of hard-codes and weird special cases. Move toward modernization and normality. Test Plan: {F52716} Reviewers: btrahan Reviewed By: btrahan CC: aran Maniphest Tasks: T2625, T2769 Differential Revision: https://secure.phabricator.com/D6652 --- src/__phutil_library_map__.php | 12 +- .../PhabricatorApplicationHerald.php | 4 +- .../herald/controller/HeraldController.php | 32 ++-- .../controller/HeraldHomeController.php | 156 ------------------ .../HeraldRuleEditHistoryController.php | 2 +- .../controller/HeraldRuleListController.php | 78 +++++++++ .../HeraldTestConsoleController.php | 2 +- .../HeraldTranscriptListController.php | 2 +- .../herald/query/HeraldRuleSearchEngine.php | 129 +++++++++++++++ .../herald/view/HeraldRuleListView.php | 83 ---------- 10 files changed, 231 insertions(+), 269 deletions(-) delete mode 100644 src/applications/herald/controller/HeraldHomeController.php create mode 100644 src/applications/herald/controller/HeraldRuleListController.php create mode 100644 src/applications/herald/query/HeraldRuleSearchEngine.php delete mode 100644 src/applications/herald/view/HeraldRuleListView.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 0fc4120b59..97c4d3d407 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -611,7 +611,6 @@ phutil_register_library_map(array( 'HeraldEffect' => 'applications/herald/engine/HeraldEffect.php', 'HeraldEngine' => 'applications/herald/engine/HeraldEngine.php', 'HeraldFieldConfig' => 'applications/herald/config/HeraldFieldConfig.php', - 'HeraldHomeController' => 'applications/herald/controller/HeraldHomeController.php', 'HeraldInvalidConditionException' => 'applications/herald/engine/engine/HeraldInvalidConditionException.php', 'HeraldInvalidFieldException' => 'applications/herald/engine/engine/HeraldInvalidFieldException.php', 'HeraldNewController' => 'applications/herald/controller/HeraldNewController.php', @@ -625,8 +624,9 @@ phutil_register_library_map(array( 'HeraldRuleEdit' => 'applications/herald/storage/HeraldRuleEdit.php', 'HeraldRuleEditHistoryController' => 'applications/herald/controller/HeraldRuleEditHistoryController.php', 'HeraldRuleEditHistoryView' => 'applications/herald/view/HeraldRuleEditHistoryView.php', - 'HeraldRuleListView' => 'applications/herald/view/HeraldRuleListView.php', + 'HeraldRuleListController' => 'applications/herald/controller/HeraldRuleListController.php', 'HeraldRuleQuery' => 'applications/herald/query/HeraldRuleQuery.php', + 'HeraldRuleSearchEngine' => 'applications/herald/query/HeraldRuleSearchEngine.php', 'HeraldRuleTranscript' => 'applications/herald/storage/transcript/HeraldRuleTranscript.php', 'HeraldRuleTypeConfig' => 'applications/herald/config/HeraldRuleTypeConfig.php', 'HeraldTestConsoleController' => 'applications/herald/controller/HeraldTestConsoleController.php', @@ -2618,7 +2618,6 @@ phutil_register_library_map(array( 'HeraldDifferentialRevisionAdapter' => 'HeraldObjectAdapter', 'HeraldDryRunAdapter' => 'HeraldObjectAdapter', 'HeraldEditLogQuery' => 'PhabricatorOffsetPagedQuery', - 'HeraldHomeController' => 'HeraldController', 'HeraldInvalidConditionException' => 'Exception', 'HeraldInvalidFieldException' => 'Exception', 'HeraldNewController' => 'HeraldController', @@ -2633,8 +2632,13 @@ phutil_register_library_map(array( 'HeraldRuleEdit' => 'HeraldDAO', 'HeraldRuleEditHistoryController' => 'HeraldController', 'HeraldRuleEditHistoryView' => 'AphrontView', - 'HeraldRuleListView' => 'AphrontView', + 'HeraldRuleListController' => + array( + 0 => 'HeraldController', + 1 => 'PhabricatorApplicationSearchResultsControllerInterface', + ), 'HeraldRuleQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', + 'HeraldRuleSearchEngine' => 'PhabricatorApplicationSearchEngine', 'HeraldTestConsoleController' => 'HeraldController', 'HeraldTranscript' => 'HeraldDAO', 'HeraldTranscriptController' => 'HeraldController', diff --git a/src/applications/herald/application/PhabricatorApplicationHerald.php b/src/applications/herald/application/PhabricatorApplicationHerald.php index f751971093..6aa14b8f4a 100644 --- a/src/applications/herald/application/PhabricatorApplicationHerald.php +++ b/src/applications/herald/application/PhabricatorApplicationHerald.php @@ -33,9 +33,7 @@ final class PhabricatorApplicationHerald extends PhabricatorApplication { public function getRoutes() { return array( '/herald/' => array( - '' => 'HeraldHomeController', - 'view/(?P[^/]+)/(?:(?P[^/]+)/)?' - => 'HeraldHomeController', + '(?:query/(?P[^/]+)/)?' => 'HeraldRuleListController', 'new/(?:(?P[^/]+)/(?:(?P[^/]+)/)?)?' => 'HeraldNewController', 'rule/(?:(?P[1-9]\d*)/)?' => 'HeraldRuleController', diff --git a/src/applications/herald/controller/HeraldController.php b/src/applications/herald/controller/HeraldController.php index 542622f066..a4f4eab1e0 100644 --- a/src/applications/herald/controller/HeraldController.php +++ b/src/applications/herald/controller/HeraldController.php @@ -17,7 +17,7 @@ abstract class HeraldController extends PhabricatorController { } public function buildApplicationMenu() { - return $this->renderNav()->getMenu(); + return $this->buildSideNavView(true)->getMenu(); } public function buildApplicationCrumbs() { @@ -32,22 +32,19 @@ abstract class HeraldController extends PhabricatorController { return $crumbs; } - protected function renderNav() { - $nav = id(new AphrontSideNavFilterView()) - ->setBaseURI(new PhutilURI('/herald/')) - ->addLabel(pht('My Rules')) - ->addFilter('new', pht('Create Rule')); + public function buildSideNavView($for_app = false) { + $user = $this->getRequest()->getUser(); - $rules_map = HeraldContentTypeConfig::getContentTypeMap(); - foreach ($rules_map as $key => $value) { - $nav->addFilter("view/{$key}/personal", $value); + $nav = new AphrontSideNavFilterView(); + $nav->setBaseURI(new PhutilURI($this->getApplicationURI())); + + if ($for_app) { + $nav->addFilter('create', pht('Create Rule')); } - $nav->addLabel(pht('Global Rules')); - - foreach ($rules_map as $key => $value) { - $nav->addFilter("view/{$key}/global", $value); - } + id(new HeraldRuleSearchEngine()) + ->setViewer($user) + ->addNavigationItems($nav->getMenu()); $nav ->addLabel(pht('Utilities')) @@ -55,12 +52,7 @@ abstract class HeraldController extends PhabricatorController { ->addFilter('transcript', pht('Transcripts')) ->addFilter('history', pht('Edit Log')); - if ($this->getRequest()->getUser()->getIsAdmin()) { - $nav->addLabel(pht('Admin')); - foreach ($rules_map as $key => $value) { - $nav->addFilter("view/{$key}/all", $value); - } - } + $nav->selectFilter(null); return $nav; } diff --git a/src/applications/herald/controller/HeraldHomeController.php b/src/applications/herald/controller/HeraldHomeController.php deleted file mode 100644 index 5f55c8b51d..0000000000 --- a/src/applications/herald/controller/HeraldHomeController.php +++ /dev/null @@ -1,156 +0,0 @@ -contentType = idx($data, 'content_type'); - $this->ruleType = idx($data, 'rule_type'); - } - - public function processRequest() { - $request = $this->getRequest(); - $user = $request->getUser(); - - if ($request->isFormPost()) { - $phids = $request->getArr('set_phid'); - $phid = head($phids); - - $uri = $request->getRequestURI(); - if ($phid) { - $uri = $uri->alter('phid', nonempty($phid, null)); - } - - return id(new AphrontRedirectResponse())->setURI($uri); - } - - $query = id(new HeraldRuleQuery()) - ->setViewer($user); - - $content_type_map = HeraldContentTypeConfig::getContentTypeMap(); - if (empty($content_type_map[$this->contentType])) { - $this->contentType = head_key($content_type_map); - } - $content_desc = $content_type_map[$this->contentType]; - - $query->withContentTypes(array($this->contentType)); - - $show_author = false; - $show_rule_type = false; - $has_author_filter = false; - $author_filter_phid = null; - - switch ($this->ruleType) { - case 'all': - if (!$user->getIsAdmin()) { - return new Aphront400Response(); - } - $show_rule_type = true; - $show_author = true; - $has_author_filter = true; - $author_filter_phid = $request->getStr('phid'); - if ($author_filter_phid) { - $query->withAuthorPHIDs(array($author_filter_phid)); - } - $rule_desc = pht('All'); - break; - case HeraldRuleTypeConfig::RULE_TYPE_GLOBAL: - $query->withRuleTypes(array(HeraldRuleTypeConfig::RULE_TYPE_GLOBAL)); - $rule_desc = pht('Global'); - break; - case HeraldRuleTypeConfig::RULE_TYPE_PERSONAL: - default: - $this->ruleType = HeraldRuleTypeConfig::RULE_TYPE_PERSONAL; - $query->withRuleTypes(array(HeraldRuleTypeConfig::RULE_TYPE_PERSONAL)); - $query->withAuthorPHIDs(array($user->getPHID())); - $rule_desc = pht('Personal'); - break; - } - - $pager = new AphrontPagerView(); - $pager->setURI($request->getRequestURI(), 'offset'); - $pager->setOffset($request->getStr('offset')); - - $rules = $query->executeWithOffsetPager($pager); - - $need_phids = mpull($rules, 'getAuthorPHID'); - $handles = $this->loadViewerHandles($need_phids); - - $list_view = id(new HeraldRuleListView()) - ->setRules($rules) - ->setShowAuthor($show_author) - ->setShowRuleType($show_rule_type) - ->setHandles($handles) - ->setUser($user); - - $panel = new AphrontPanelView(); - $panel->appendChild($list_view); - $panel->appendChild($pager); - $panel->setNoBackground(); - - $panel->setHeader( - pht("Herald: %s Rules for %s", $rule_desc, $content_desc)); - - $crumbs = $this - ->buildApplicationCrumbs() - ->addCrumb( - id(new PhabricatorCrumbView()) - ->setName(pht('Herald Rules')) - ->setHref($this->getApplicationURI( - 'view/'.$this->contentType.'/'.$this->ruleType))); - - $nav = $this->renderNav(); - $nav->selectFilter('view/'.$this->contentType.'/'.$this->ruleType); - - if ($has_author_filter) { - $nav->appendChild($this->renderAuthorFilter($author_filter_phid)); - } - - $nav->appendChild($panel); - $nav->setCrumbs($crumbs); - - return $this->buildApplicationPage( - $nav, - array( - 'title' => pht('Herald'), - 'dust' => true, - 'device' => true, - )); - } - - private function renderAuthorFilter($phid) { - $user = $this->getRequest()->getUser(); - if ($phid) { - $handle = PhabricatorObjectHandleData::loadOneHandle( - $phid, - $user); - $tokens = array( - $phid => $handle->getFullName(), - ); - } else { - $tokens = array(); - } - - $form = id(new AphrontFormView()) - ->setUser($user) - ->setNoShading(true) - ->appendChild( - id(new AphrontFormTokenizerControl()) - ->setName('set_phid') - ->setValue($tokens) - ->setLimit(1) - ->setLabel(pht('Filter Author')) - ->setDataSource('/typeahead/common/accounts/')) - ->appendChild( - id(new AphrontFormSubmitControl()) - ->setValue(pht('Apply Filter'))); - - $filter = new AphrontListFilterView(); - $filter->appendChild($form); - return $filter; - } - - -} diff --git a/src/applications/herald/controller/HeraldRuleEditHistoryController.php b/src/applications/herald/controller/HeraldRuleEditHistoryController.php index 50887264bf..25b0cbde9b 100644 --- a/src/applications/herald/controller/HeraldRuleEditHistoryController.php +++ b/src/applications/herald/controller/HeraldRuleEditHistoryController.php @@ -42,7 +42,7 @@ final class HeraldRuleEditHistoryController extends HeraldController { ->setName(pht('Edit History')) ->setHref($this->getApplicationURI('herald/history'))); - $nav = $this->renderNav(); + $nav = $this->buildSideNavView(); $nav->selectFilter('history'); $nav->appendChild($panel); $nav->setCrumbs($crumbs); diff --git a/src/applications/herald/controller/HeraldRuleListController.php b/src/applications/herald/controller/HeraldRuleListController.php new file mode 100644 index 0000000000..f0d8bdb4f1 --- /dev/null +++ b/src/applications/herald/controller/HeraldRuleListController.php @@ -0,0 +1,78 @@ +queryKey = idx($data, 'queryKey'); + } + + public function processRequest() { + $request = $this->getRequest(); + $controller = id(new PhabricatorApplicationSearchController($request)) + ->setQueryKey($this->queryKey) + ->setSearchEngine(new HeraldRuleSearchEngine()) + ->setNavigation($this->buildSideNavView()); + + return $this->delegateToController($controller); + } + + public function renderResultsList( + array $rules, + PhabricatorSavedQuery $query) { + assert_instances_of($rules, 'HeraldRule'); + + $viewer = $this->getRequest()->getUser(); + + $phids = mpull($rules, 'getAuthorPHID'); + $this->loadHandles($phids); + + $content_type_map = HeraldContentTypeConfig::getContentTypeMap(); + + $list = id(new PhabricatorObjectItemListView()) + ->setUser($viewer); + foreach ($rules as $rule) { + $id = $rule->getID(); + + $item = id(new PhabricatorObjectItemView()) + ->setObjectName(pht('Rule %s', $rule->getID())) + ->setHeader($rule->getName()) + ->setHref($this->getApplicationURI("rule/{$id}/")); + + if ($rule->isPersonalRule()) { + $item->addByline( + pht( + 'Authored by %s', + $this->getHandle($rule->getAuthorPHID())->renderLink())); + } else { + $item->addIcon('world', pht('Global Rule')); + } + + $item->addAction( + id(new PHUIListItemView()) + ->setHref($this->getApplicationURI("history/{$id}/")) + ->setIcon('transcript') + ->setName(pht('Edit Log'))); + + $item->addAction( + id(new PHUIListItemView()) + ->setHref('/herald/delete/'.$rule->getID().'/') + ->setIcon('delete') + ->setWorkflow(true)); + + $content_type_name = idx($content_type_map, $rule->getContentType()); + $item->addAttribute(pht('Affects: %s', $content_type_name)); + + $list->addItem($item); + } + + return $list; + } + +} diff --git a/src/applications/herald/controller/HeraldTestConsoleController.php b/src/applications/herald/controller/HeraldTestConsoleController.php index e9865a41c7..e982144add 100644 --- a/src/applications/herald/controller/HeraldTestConsoleController.php +++ b/src/applications/herald/controller/HeraldTestConsoleController.php @@ -113,7 +113,7 @@ final class HeraldTestConsoleController extends HeraldController { id(new AphrontFormSubmitControl()) ->setValue(pht('Test Rules'))); - $nav = $this->renderNav(); + $nav = $this->buildSideNavView(); $nav->selectFilter('test'); $nav->appendChild( array( diff --git a/src/applications/herald/controller/HeraldTranscriptListController.php b/src/applications/herald/controller/HeraldTranscriptListController.php index 5e38f31c45..cbf8fd5485 100644 --- a/src/applications/herald/controller/HeraldTranscriptListController.php +++ b/src/applications/herald/controller/HeraldTranscriptListController.php @@ -95,7 +95,7 @@ final class HeraldTranscriptListController extends HeraldController { $panel->appendChild($pager); $panel->setNoBackground(); - $nav = $this->renderNav(); + $nav = $this->buildSideNavView(); $nav->selectFilter('transcript'); $nav->appendChild($panel); diff --git a/src/applications/herald/query/HeraldRuleSearchEngine.php b/src/applications/herald/query/HeraldRuleSearchEngine.php new file mode 100644 index 0000000000..d516defceb --- /dev/null +++ b/src/applications/herald/query/HeraldRuleSearchEngine.php @@ -0,0 +1,129 @@ +setParameter( + 'authorPHIDs', + array_values($request->getArr('authors'))); + + $saved->setParameter('contentType', $request->getStr('contentType')); + $saved->setParameter('ruleType', $request->getStr('ruleType')); + + return $saved; + } + + public function buildQueryFromSavedQuery(PhabricatorSavedQuery $saved) { + $query = id(new HeraldRuleQuery()); + + $author_phids = $saved->getParameter('authorPHIDs'); + if ($author_phids) { + $query->withAuthorPHIDs($author_phids); + } + + $content_type = $saved->getParameter('contentType'); + $content_type = idx($this->getContentTypeValues(), $content_type); + if ($content_type) { + $query->withContentTypes(array($content_type)); + } + + $rule_type = $saved->getParameter('ruleType'); + $rule_type = idx($this->getRuleTypeValues(), $rule_type); + if ($rule_type) { + $query->withRuleTypes(array($rule_type)); + } + + return $query; + } + + public function buildSearchForm( + AphrontFormView $form, + PhabricatorSavedQuery $saved_query) { + + $phids = $saved_query->getParameter('authorPHIDs', array()); + $handles = id(new PhabricatorObjectHandleData($phids)) + ->setViewer($this->requireViewer()) + ->loadHandles(); + $author_tokens = mpull($handles, 'getFullName', 'getPHID'); + + $content_type = $saved_query->getParameter('contentType'); + $rule_type = $saved_query->getParameter('ruleType'); + + $form + ->appendChild( + id(new AphrontFormTokenizerControl()) + ->setDatasource('/typeahead/common/users/') + ->setName('authors') + ->setLabel(pht('Authors')) + ->setValue($author_tokens)) + ->appendChild( + id(new AphrontFormSelectControl()) + ->setName('contentType') + ->setLabel(pht('Content Type')) + ->setValue($content_type) + ->setOptions($this->getContentTypeOptions())) + ->appendChild( + id(new AphrontFormSelectControl()) + ->setName('ruleType') + ->setLabel(pht('Rule Type')) + ->setValue($rule_type) + ->setOptions($this->getRuleTypeOptions())); + } + + protected function getURI($path) { + return '/herald/'.$path; + } + + public function getBuiltinQueryNames() { + $names = array(); + + if ($this->requireViewer()->isLoggedIn()) { + $names['authored'] = pht('Authored'); + } + + $names['all'] = pht('All'); + + return $names; + } + + public function buildSavedQueryFromBuiltin($query_key) { + + $query = $this->newSavedQuery(); + $query->setQueryKey($query_key); + + switch ($query_key) { + case 'all': + return $query; + case 'authored': + return $query->setParameter( + 'authorPHIDs', + array($this->requireViewer()->getPHID())); + } + + return parent::buildSavedQueryFromBuiltin($query_key); + } + + private function getContentTypeOptions() { + return array( + '' => pht('(All Content Types)'), + ) + HeraldContentTypeConfig::getContentTypeMap(); + } + + private function getContentTypeValues() { + return HeraldContentTypeConfig::getContentTypeMap(); + } + + private function getRuleTypeOptions() { + return array( + '' => pht('(All Rule Types)'), + ) + HeraldRuleTypeConfig::getRuleTypeMap(); + } + + private function getRuleTypeValues() { + return HeraldRuleTypeConfig::getRuleTypeMap(); + } + +} diff --git a/src/applications/herald/view/HeraldRuleListView.php b/src/applications/herald/view/HeraldRuleListView.php deleted file mode 100644 index 2d566bf35f..0000000000 --- a/src/applications/herald/view/HeraldRuleListView.php +++ /dev/null @@ -1,83 +0,0 @@ -rules = $rules; - return $this; - } - - public function setHandles(array $handles) { - assert_instances_of($handles, 'PhabricatorObjectHandle'); - $this->handles = $handles; - return $this; - } - - public function setShowAuthor($show_author) { - $this->showAuthor = $show_author; - return $this; - } - - public function setShowRuleType($show_rule_type) { - $this->showRuleType = $show_rule_type; - return $this; - } - - public function render() { - - $type_map = HeraldRuleTypeConfig::getRuleTypeMap(); - - $list = new PhabricatorObjectItemListView(); - $list->setFlush(true); - $list->setCards(true); - foreach ($this->rules as $rule) { - - if ($rule->getRuleType() == HeraldRuleTypeConfig::RULE_TYPE_GLOBAL) { - $author = pht('Global Rule'); - } else { - $author = $this->handles[$rule->getAuthorPHID()]->renderLink(); - $author = pht('Editor: %s', $author); - } - - $edit_log = phutil_tag( - 'a', - array( - 'href' => '/herald/history/'.$rule->getID().'/', - ), - pht('View Edit Log')); - - $delete = javelin_tag( - 'a', - array( - 'href' => '/herald/delete/'.$rule->getID().'/', - 'sigil' => 'workflow', - ), - pht('Delete')); - - $item = id(new PhabricatorObjectItemView()) - ->setObjectName($type_map[$rule->getRuleType()]) - ->setHeader($rule->getName()) - ->setHref('/herald/rule/'.$rule->getID().'/') - ->addAttribute($edit_log) - ->addIcon('none', $author) - ->addAction( - id(new PHUIListItemView()) - ->setHref('/herald/delete/'.$rule->getID().'/') - ->setIcon('delete') - ->setWorkflow(true)); - - $list->addItem($item); - } - - $list->setNoDataString(pht("No matching rules.")); - - return $list; - } -}