From dadd9a9dd98d9f651f291ff8487486f24c54f292 Mon Sep 17 00:00:00 2001 From: epriestley Date: Thu, 8 May 2014 08:24:47 -0700 Subject: [PATCH] Let feed panels render something meaningful-ish Summary: Ref T4986. We need to introduce alternate views to make this more pleasant, but let rendering move to engines so it can be shared between panels and controllers. I also moved some of the pagination logic in to avoid duplicating that. So far, only Feed works. I'm going to do these gradually since we have ~40-50 of them. Test Plan: - Used global search to check for collateral damage. - Used not-global search too. - Used normal feed. {F151541} Reviewers: btrahan Reviewed By: btrahan Subscribers: chad, epriestley Maniphest Tasks: T4986 Differential Revision: https://secure.phabricator.com/D9008 --- src/__phutil_library_map__.php | 6 +- .../PhabricatorDashboardPanelTypeQuery.php | 13 +---- .../PhabricatorFeedListController.php | 15 +---- .../query/PhabricatorFeedSearchEngine.php | 12 ++++ ...PhabricatorApplicationSearchController.php | 52 +++++------------ .../PhabricatorSearchController.php | 1 - .../PhabricatorApplicationSearchEngine.php | 57 ++++++++++++++++++- ...abricatorSearchApplicationSearchEngine.php | 4 ++ 8 files changed, 90 insertions(+), 70 deletions(-) diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index efdfefc676..f6d979de86 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -4377,11 +4377,7 @@ phutil_register_library_map(array( 'PhabricatorFeedController' => 'PhabricatorController', 'PhabricatorFeedDAO' => 'PhabricatorLiskDAO', 'PhabricatorFeedDetailController' => 'PhabricatorFeedController', - 'PhabricatorFeedListController' => - array( - 0 => 'PhabricatorFeedController', - 1 => 'PhabricatorApplicationSearchResultsControllerInterface', - ), + 'PhabricatorFeedListController' => 'PhabricatorFeedController', 'PhabricatorFeedManagementRepublishWorkflow' => 'PhabricatorFeedManagementWorkflow', 'PhabricatorFeedManagementWorkflow' => 'PhabricatorManagementWorkflow', 'PhabricatorFeedPublicStreamController' => 'PhabricatorFeedController', diff --git a/src/applications/dashboard/paneltype/PhabricatorDashboardPanelTypeQuery.php b/src/applications/dashboard/paneltype/PhabricatorDashboardPanelTypeQuery.php index 8cbddd436e..ab00d6beef 100644 --- a/src/applications/dashboard/paneltype/PhabricatorDashboardPanelTypeQuery.php +++ b/src/applications/dashboard/paneltype/PhabricatorDashboardPanelTypeQuery.php @@ -66,17 +66,10 @@ final class PhabricatorDashboardPanelTypeQuery } $query = $engine->buildQueryFromSavedQuery($saved); + $pager = $engine->newPagerForSavedQuery($saved); + $results = $engine->executeQuery($query, $pager); - $results = $query - ->setViewer($viewer) - ->execute(); - - $out = array(); - foreach ($results as $result) { - $out[] = phutil_tag('div', array(), $result->getPHID()); - } - - return $out; + return $engine->renderResults($results, $saved); } } diff --git a/src/applications/feed/controller/PhabricatorFeedListController.php b/src/applications/feed/controller/PhabricatorFeedListController.php index 42dd0fb5f9..d6a81cd23a 100644 --- a/src/applications/feed/controller/PhabricatorFeedListController.php +++ b/src/applications/feed/controller/PhabricatorFeedListController.php @@ -1,7 +1,6 @@ delegateToController($controller); } - public function renderResultsList( - array $feed, - PhabricatorSavedQuery $query) { - - $builder = new PhabricatorFeedBuilder($feed); - $builder->setShowHovercards(true); - $builder->setUser($this->getRequest()->getUser()); - $view = $builder->buildView(); - - return phutil_tag_div('phabricator-feed-frame', $view); - } - } diff --git a/src/applications/feed/query/PhabricatorFeedSearchEngine.php b/src/applications/feed/query/PhabricatorFeedSearchEngine.php index c4048d7f53..7aca133cc3 100644 --- a/src/applications/feed/query/PhabricatorFeedSearchEngine.php +++ b/src/applications/feed/query/PhabricatorFeedSearchEngine.php @@ -124,4 +124,16 @@ final class PhabricatorFeedSearchEngine return parent::buildSavedQueryFromBuiltin($query_key); } + public function renderResults( + array $objects, + PhabricatorSavedQuery $query) { + + $builder = new PhabricatorFeedBuilder($objects); + $builder->setShowHovercards(true); + $builder->setUser($this->requireViewer()); + $view = $builder->buildView(); + + return phutil_tag_div('phabricator-feed-frame', $view); + } + } diff --git a/src/applications/search/controller/PhabricatorApplicationSearchController.php b/src/applications/search/controller/PhabricatorApplicationSearchController.php index f326c71ddd..5a9563c417 100644 --- a/src/applications/search/controller/PhabricatorApplicationSearchController.php +++ b/src/applications/search/controller/PhabricatorApplicationSearchController.php @@ -7,16 +7,6 @@ final class PhabricatorApplicationSearchController private $navigation; private $queryKey; private $preface; - private $useOffsetPaging; - - public function setUseOffsetPaging($use_offset_paging) { - $this->useOffsetPaging = $use_offset_paging; - return $this; - } - - public function getUseOffsetPaging() { - return $this->useOffsetPaging; - } public function setPreface($preface) { $this->preface = $preface; @@ -78,11 +68,6 @@ final class PhabricatorApplicationSearchController $engine->setViewer($this->getRequest()->getUser()); $parent = $this->getDelegatingController(); - $interface = 'PhabricatorApplicationSearchResultsControllerInterface'; - if (!$parent instanceof $interface) { - throw new Exception( - "Delegating controller must implement '{$interface}'."); - } } public function processRequest() { @@ -223,34 +208,23 @@ final class PhabricatorApplicationSearchController $query = $engine->buildQueryFromSavedQuery($saved_query); - $use_offset_paging = $this->getUseOffsetPaging(); - if ($use_offset_paging) { - $pager = new AphrontPagerView(); - } else { - $pager = new AphrontCursorPagerView(); - } + $pager = $engine->newPagerForSavedQuery($saved_query); $pager->readFromRequest($request); - $page_size = $engine->getPageSize($saved_query); - if (is_finite($page_size)) { - $pager->setPageSize($page_size); + + $objects = $engine->executeQuery($query, $pager); + + // TODO: To support Dashboard panels, rendering is moving into + // SearchEngines. Move it all the way in and then get rid of this. + + $interface = 'PhabricatorApplicationSearchResultsControllerInterface'; + if ($parent instanceof $interface) { + $list = $parent->renderResultsList($objects, $saved_query); } else { - // Consider an INF pagesize to mean a large finite pagesize. - - // TODO: It would be nice to handle this more gracefully, but math - // with INF seems to vary across PHP versions, systems, and runtimes. - $pager->setPageSize(0xFFFF); + $list = $engine->renderResults( + $objects, + $saved_query); } - $query->setViewer($request->getUser()); - - if ($use_offset_paging) { - $objects = $query->executeWithOffsetPager($pager); - } else { - $objects = $query->executeWithCursorPager($pager); - } - - $list = $parent->renderResultsList($objects, $saved_query); - $nav->appendChild($list); // TODO: This is a bit hacky. diff --git a/src/applications/search/controller/PhabricatorSearchController.php b/src/applications/search/controller/PhabricatorSearchController.php index cb57c48a2f..62401bbf49 100644 --- a/src/applications/search/controller/PhabricatorSearchController.php +++ b/src/applications/search/controller/PhabricatorSearchController.php @@ -76,7 +76,6 @@ final class PhabricatorSearchController $controller = id(new PhabricatorApplicationSearchController($request)) ->setQueryKey($this->queryKey) ->setSearchEngine($engine) - ->setUseOffsetPaging(true) ->setNavigation($this->buildSideNavView()); return $this->delegateToController($controller); diff --git a/src/applications/search/engine/PhabricatorApplicationSearchEngine.php b/src/applications/search/engine/PhabricatorApplicationSearchEngine.php index 9f0ecdcdb0..a999635fde 100644 --- a/src/applications/search/engine/PhabricatorApplicationSearchEngine.php +++ b/src/applications/search/engine/PhabricatorApplicationSearchEngine.php @@ -9,6 +9,8 @@ * @task uri Query URIs * @task dates Date Filters * @task read Reading Utilities + * @task exec Paging and Executing Queries + * @task render Rendering Results * * @group search */ @@ -502,7 +504,7 @@ abstract class PhabricatorApplicationSearchEngine { } -/* -( Pagination )--------------------------------------------------------- */ +/* -( Paging and Executing Queries )--------------------------------------- */ public function getPageSize(PhabricatorSavedQuery $saved) { @@ -510,6 +512,59 @@ abstract class PhabricatorApplicationSearchEngine { } + public function shouldUseOffsetPaging() { + return false; + } + + + public function newPagerForSavedQuery(PhabricatorSavedQuery $saved) { + if ($this->shouldUseOffsetPaging()) { + $pager = new AphrontPagerView(); + } else { + $pager = new AphrontCursorPagerView(); + } + + $page_size = $this->getPageSize($saved); + if (is_finite($page_size)) { + $pager->setPageSize($page_size); + } else { + // Consider an INF pagesize to mean a large finite pagesize. + + // TODO: It would be nice to handle this more gracefully, but math + // with INF seems to vary across PHP versions, systems, and runtimes. + $pager->setPageSize(0xFFFF); + } + + return $pager; + } + + + public function executeQuery( + PhabricatorPolicyAwareQuery $query, + AphrontView $pager) { + + $query->setViewer($this->requireViewer()); + + if ($this->shouldUseOffsetPaging()) { + $objects = $query->executeWithOffsetPager($pager); + } else { + $objects = $query->executeWithCursorPager($pager); + } + + return $objects; + } + + +/* -( Rendering )---------------------------------------------------------- */ + + + public function renderResults( + array $objects, + PhabricatorSavedQuery $query) { + throw new Exception(pht('Not supported here yet!')); + } + + /* -( Application Search )------------------------------------------------- */ diff --git a/src/applications/search/query/PhabricatorSearchApplicationSearchEngine.php b/src/applications/search/query/PhabricatorSearchApplicationSearchEngine.php index 53461d019a..8e73feeb1b 100644 --- a/src/applications/search/query/PhabricatorSearchApplicationSearchEngine.php +++ b/src/applications/search/query/PhabricatorSearchApplicationSearchEngine.php @@ -229,5 +229,9 @@ final class PhabricatorSearchApplicationSearchEngine return $results; } + public function shouldUseOffsetPaging() { + return true; + } + }