From d6341cfffec9d1f2dc03267da44fc9654b4a5598 Mon Sep 17 00:00:00 2001 From: Bob Trahan Date: Tue, 2 Dec 2014 13:10:29 -0800 Subject: [PATCH] Transactions - add pagination to application transactions Summary: Ref T4712. This adds pagination. Future diffs will need to deploy `buildTransactionTimeline` everywhere and massage this stuff as necessary if we hit any special cases. Test Plan: Set page size to "5" to make it need to paginate often. Verified proper transactions loaded in and the javascript actions worked. Reviewers: epriestley Reviewed By: epriestley Subscribers: Korvin, epriestley Maniphest Tasks: T4712 Differential Revision: https://secure.phabricator.com/D10887 --- resources/celerity/map.php | 8 + src/__phutil_library_map__.php | 2 + .../base/controller/PhabricatorController.php | 12 +- .../PhabricatorTransactionsApplication.php | 2 + ...licationTransactionShowOlderController.php | 55 +++++++ ...PhabricatorApplicationTransactionQuery.php | 8 +- .../PhabricatorApplicationTransactionView.php | 42 ++++- src/view/phui/PHUITimelineView.php | 150 ++++++++++++------ .../behavior-show-older-transactions.js | 104 ++++++++++++ 9 files changed, 328 insertions(+), 55 deletions(-) create mode 100644 src/applications/transactions/controller/PhabricatorApplicationTransactionShowOlderController.php create mode 100644 webroot/rsrc/js/application/transactions/behavior-show-older-transactions.js diff --git a/resources/celerity/map.php b/resources/celerity/map.php index c3d2d36227..93e9c99a97 100644 --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -421,6 +421,7 @@ return array( 'rsrc/js/application/repository/repository-crossreference.js' => 'f9539603', 'rsrc/js/application/search/behavior-reorder-queries.js' => 'e9581f08', 'rsrc/js/application/slowvote/behavior-slowvote-embed.js' => 'd6f54db0', + 'rsrc/js/application/transactions/behavior-show-older-transactions.js' => 'c30ccda9', 'rsrc/js/application/transactions/behavior-transaction-comment-form.js' => '9f7309fb', 'rsrc/js/application/transactions/behavior-transaction-list.js' => '13c739ea', 'rsrc/js/application/uiexample/JavelinViewExample.js' => 'd4a14807', @@ -624,6 +625,7 @@ return array( 'javelin-behavior-phabricator-reveal-content' => '60821bc7', 'javelin-behavior-phabricator-search-typeahead' => '724b1247', 'javelin-behavior-phabricator-show-all-transactions' => '7c273581', + 'javelin-behavior-phabricator-show-older-transactions' => 'c30ccda9', 'javelin-behavior-phabricator-tooltips' => '3ee3408b', 'javelin-behavior-phabricator-transaction-comment-form' => '9f7309fb', 'javelin-behavior-phabricator-transaction-list' => '13c739ea', @@ -1666,6 +1668,12 @@ return array( 'javelin-util', 'phabricator-shaped-request', ), + 'c30ccda9' => array( + 'javelin-behavior', + 'javelin-stratcom', + 'javelin-dom', + 'phabricator-busy', + ), 'c4569c05' => array( 'javelin-magical-init', 'javelin-install', diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 61fe4b2ad1..e5a277ff02 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -1250,6 +1250,7 @@ phutil_register_library_map(array( 'PhabricatorApplicationTransactionNoEffectResponse' => 'applications/transactions/response/PhabricatorApplicationTransactionNoEffectResponse.php', 'PhabricatorApplicationTransactionQuery' => 'applications/transactions/query/PhabricatorApplicationTransactionQuery.php', 'PhabricatorApplicationTransactionResponse' => 'applications/transactions/response/PhabricatorApplicationTransactionResponse.php', + 'PhabricatorApplicationTransactionShowOlderController' => 'applications/transactions/controller/PhabricatorApplicationTransactionShowOlderController.php', 'PhabricatorApplicationTransactionStructureException' => 'applications/transactions/exception/PhabricatorApplicationTransactionStructureException.php', 'PhabricatorApplicationTransactionTextDiffDetailView' => 'applications/transactions/view/PhabricatorApplicationTransactionTextDiffDetailView.php', 'PhabricatorApplicationTransactionTransactionPHIDType' => 'applications/transactions/phid/PhabricatorApplicationTransactionTransactionPHIDType.php', @@ -4336,6 +4337,7 @@ phutil_register_library_map(array( 'PhabricatorApplicationTransactionNoEffectResponse' => 'AphrontProxyResponse', 'PhabricatorApplicationTransactionQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', 'PhabricatorApplicationTransactionResponse' => 'AphrontProxyResponse', + 'PhabricatorApplicationTransactionShowOlderController' => 'PhabricatorApplicationTransactionController', 'PhabricatorApplicationTransactionStructureException' => 'Exception', 'PhabricatorApplicationTransactionTextDiffDetailView' => 'AphrontView', 'PhabricatorApplicationTransactionTransactionPHIDType' => 'PhabricatorPHIDType', diff --git a/src/applications/base/controller/PhabricatorController.php b/src/applications/base/controller/PhabricatorController.php index 989855d0cc..81348a9bc7 100644 --- a/src/applications/base/controller/PhabricatorController.php +++ b/src/applications/base/controller/PhabricatorController.php @@ -535,11 +535,18 @@ abstract class PhabricatorController extends AphrontController { $xaction = $object->getApplicationTransactionTemplate(); $view = $xaction->getApplicationTransactionViewObject(); + $pager = id(new AphrontCursorPagerView()) + ->readFromRequest($this->getRequest()) + ->setURI(new PhutilURI( + '/transactions/showolder/'.$object->getPHID().'/')); + $xactions = $query ->setViewer($viewer) ->withObjectPHIDs(array($object->getPHID())) ->needComments(true) - ->execute(); + ->setReversePaging(false) + ->executeWithCursorPager($pager); + $xactions = array_reverse($xactions); if ($engine) { foreach ($xactions as $xaction) { @@ -556,7 +563,8 @@ abstract class PhabricatorController extends AphrontController { $timeline = $view ->setUser($viewer) ->setObjectPHID($object->getPHID()) - ->setTransactions($xactions); + ->setTransactions($xactions) + ->setPager($pager); return $timeline; } diff --git a/src/applications/transactions/application/PhabricatorTransactionsApplication.php b/src/applications/transactions/application/PhabricatorTransactionsApplication.php index c1a90dccd2..1a8792b256 100644 --- a/src/applications/transactions/application/PhabricatorTransactionsApplication.php +++ b/src/applications/transactions/application/PhabricatorTransactionsApplication.php @@ -29,6 +29,8 @@ final class PhabricatorTransactionsApplication extends PhabricatorApplication { => 'PhabricatorApplicationTransactionCommentRawController', 'detail/(?[^/]+)/' => 'PhabricatorApplicationTransactionDetailController', + 'showolder/(?[^/]+)/' + => 'PhabricatorApplicationTransactionShowOlderController', '(?Pold|new)/(?[^/]+)/' => 'PhabricatorApplicationTransactionValueController', ), diff --git a/src/applications/transactions/controller/PhabricatorApplicationTransactionShowOlderController.php b/src/applications/transactions/controller/PhabricatorApplicationTransactionShowOlderController.php new file mode 100644 index 0000000000..e21ef0e075 --- /dev/null +++ b/src/applications/transactions/controller/PhabricatorApplicationTransactionShowOlderController.php @@ -0,0 +1,55 @@ +getRequest(); + $viewer = $request->getUser(); + + $object = id(new PhabricatorObjectQuery()) + ->withPHIDs(array($request->getURIData('phid'))) + ->setViewer($viewer) + ->executeOne(); + if (!$object) { + return new Aphront404Response(); + } + if (!$object instanceof PhabricatorApplicationTransactionInterface) { + return new Aphront404Response(); + } + + $template = $object->getApplicationTransactionTemplate(); + $queries = id(new PhutilSymbolLoader()) + ->setAncestorClass('PhabricatorApplicationTransactionQuery') + ->loadObjects(); + + $object_query = null; + foreach ($queries as $query) { + if ($query->getTemplateApplicationTransaction() == $template) { + $object_query = $query; + break; + } + } + + if (!$object_query) { + return new Aphront404Response(); + } + + $timeline = $this->buildTransactionTimeline( + $object, + $query); + + $phui_timeline = $timeline->buildPHUITimelineView($with_hiding = false); + $phui_timeline->setShouldAddSpacers(false); + $events = $phui_timeline->buildEvents(); + + return id(new AphrontAjaxResponse()) + ->setContent(array( + 'timeline' => hsprintf('%s', $events),)); + } + +} diff --git a/src/applications/transactions/query/PhabricatorApplicationTransactionQuery.php b/src/applications/transactions/query/PhabricatorApplicationTransactionQuery.php index 59bfb542ef..3cec8821f6 100644 --- a/src/applications/transactions/query/PhabricatorApplicationTransactionQuery.php +++ b/src/applications/transactions/query/PhabricatorApplicationTransactionQuery.php @@ -7,6 +7,7 @@ abstract class PhabricatorApplicationTransactionQuery private $objectPHIDs; private $authorPHIDs; private $transactionTypes; + private $reversePaging = true; private $needComments = true; private $needHandles = true; @@ -17,8 +18,13 @@ abstract class PhabricatorApplicationTransactionQuery return array(); } + public function setReversePaging($bool) { + $this->reversePaging = $bool; + return $this; + } + protected function getReversePaging() { - return true; + return $this->reversePaging; } public function withPHIDs(array $phids) { diff --git a/src/applications/transactions/view/PhabricatorApplicationTransactionView.php b/src/applications/transactions/view/PhabricatorApplicationTransactionView.php index 4a1f5cfac3..eae18d39e3 100644 --- a/src/applications/transactions/view/PhabricatorApplicationTransactionView.php +++ b/src/applications/transactions/view/PhabricatorApplicationTransactionView.php @@ -13,6 +13,7 @@ class PhabricatorApplicationTransactionView extends AphrontView { private $shouldTerminate = false; private $quoteTargetID; private $quoteRef; + private $pager; public function setQuoteRef($quote_ref) { $this->quoteRef = $quote_ref; @@ -71,6 +72,15 @@ class PhabricatorApplicationTransactionView extends AphrontView { return $this; } + public function setPager(AphrontCursorPagerView $pager) { + $this->pager = $pager; + return $this; + } + + public function getPager() { + return $this->pager; + } + public function buildEvents($with_hiding = false) { $user = $this->getUser(); @@ -129,6 +139,12 @@ class PhabricatorApplicationTransactionView extends AphrontView { } else { $group_event->addEventToGroup($event); } + if ($hide_by_default) { + $pager = $this->getPager(); + if ($pager) { + $pager->setNextPageID($xaction->getID()); + } + } } $events[] = $group_event; @@ -142,12 +158,7 @@ class PhabricatorApplicationTransactionView extends AphrontView { throw new Exception('Call setObjectPHID() before render()!'); } - $view = new PHUITimelineView(); - $view->setShouldTerminate($this->shouldTerminate); - $events = $this->buildEvents($with_hiding = true); - foreach ($events as $event) { - $view->addEvent($event); - } + $view = $this->buildPHUITimelineView(); if ($this->getShowEditActions()) { Javelin::initBehavior('phabricator-transaction-list'); @@ -156,6 +167,25 @@ class PhabricatorApplicationTransactionView extends AphrontView { return $view->render(); } + public function buildPHUITimelineView($with_hiding = true) { + if (!$this->getObjectPHID()) { + throw new Exception( + 'Call setObjectPHID() before buildPHUITimelineView()!'); + } + + $view = new PHUITimelineView(); + $view->setShouldTerminate($this->shouldTerminate); + $events = $this->buildEvents($with_hiding); + foreach ($events as $event) { + $view->addEvent($event); + } + if ($this->getPager()) { + $view->setPager($this->getPager()); + } + + return $view; + } + protected function getOrBuildEngine() { if (!$this->engine) { $field = PhabricatorApplicationTransactionComment::MARKUP_FIELD_COMMENT; diff --git a/src/view/phui/PHUITimelineView.php b/src/view/phui/PHUITimelineView.php index 3629b7d7fe..df8007de8c 100644 --- a/src/view/phui/PHUITimelineView.php +++ b/src/view/phui/PHUITimelineView.php @@ -5,6 +5,8 @@ final class PHUITimelineView extends AphrontView { private $events = array(); private $id; private $shouldTerminate = false; + private $shouldAddSpacers = true; + private $pager; public function setID($id) { $this->id = $id; @@ -16,12 +18,48 @@ final class PHUITimelineView extends AphrontView { return $this; } + public function setShouldAddSpacers($bool) { + $this->shouldAddSpacers = $bool; + return $this; + } + + public function setPager(AphrontCursorPagerView $pager) { + $this->pager = $pager; + return $this; + } + + public function getPager() { + return $this->pager; + } + public function addEvent(PHUITimelineEventView $event) { $this->events[] = $event; return $this; } public function render() { + if ($this->getPager()) { + if ($this->id === null) { + $this->id = celerity_generate_unique_node_id(); + } + Javelin::initBehavior( + 'phabricator-show-older-transactions', + array( + 'timelineID' => $this->id, + )); + } + $events = $this->buildEvents(); + + return phutil_tag( + 'div', + array( + 'class' => 'phui-timeline-view', + 'id' => $this->id, + ), + $events); + } + + public function buildEvents() { require_celerity_resource('phui-timeline-view-css'); $spacer = self::renderSpacer(); @@ -39,48 +77,72 @@ final class PHUITimelineView extends AphrontView { $events = array(); if ($hide) { - $hidden = phutil_implode_html($spacer, $hide); - $count = count($hide); + if ($this->getPager()) { - $show_id = celerity_generate_unique_node_id(); - $hide_id = celerity_generate_unique_node_id(); - $link_id = celerity_generate_unique_node_id(); - - Javelin::initBehavior( - 'phabricator-show-all-transactions', - array( - 'anchors' => array_filter(mpull($hide, 'getAnchor')), - 'linkID' => $link_id, - 'hideID' => $hide_id, - 'showID' => $show_id, - )); - - $events[] = phutil_tag( - 'div', - array( - 'id' => $hide_id, - 'class' => 'phui-timeline-older-transactions-are-hidden', - ), - array( - pht('%s older changes(s) are hidden.', new PhutilNumber($count)), - ' ', - javelin_tag( - 'a', - array( - 'href' => '#', + $events[] = javelin_tag( + 'div', + array( + 'sigil' => 'show-older-block', + 'class' => 'phui-timeline-older-transactions-are-hidden', + ), + array( + pht('Older changes are hidden. '), + ' ', + javelin_tag( + 'a', + array( + 'href' => (string) $this->getPager()->getNextPageURI(), 'mustcapture' => true, - 'id' => $link_id, + 'sigil' => 'show-older-link', ), - pht('Show all changes.')), - )); + pht('Show older changes.')), + )); - $events[] = phutil_tag( - 'div', - array( - 'id' => $show_id, - 'style' => 'display: none', - ), - $hidden); + } else { + + $hidden = phutil_implode_html($spacer, $hide); + $count = count($hide); + + $show_id = celerity_generate_unique_node_id(); + $hide_id = celerity_generate_unique_node_id(); + $link_id = celerity_generate_unique_node_id(); + + Javelin::initBehavior( + 'phabricator-show-all-transactions', + array( + 'anchors' => array_filter(mpull($hide, 'getAnchor')), + 'linkID' => $link_id, + 'hideID' => $hide_id, + 'showID' => $show_id, + )); + + $events[] = phutil_tag( + 'div', + array( + 'id' => $hide_id, + 'class' => 'phui-timeline-older-transactions-are-hidden', + ), + array( + pht('%s older changes(s) are hidden.', new PhutilNumber($count)), + ' ', + javelin_tag( + 'a', + array( + 'href' => '#', + 'mustcapture' => true, + 'id' => $link_id, + ), + pht('Show all changes.')), + )); + + $events[] = phutil_tag( + 'div', + array( + 'id' => $show_id, + 'style' => 'display: none', + ), + $hidden); + } } if ($hide && $show) { @@ -92,7 +154,9 @@ final class PHUITimelineView extends AphrontView { } if ($events) { - $events = array($spacer, $events, $spacer); + if ($this->shouldAddSpacers) { + $events = array($spacer, $events, $spacer); + } } else { $events = array($spacer); } @@ -101,13 +165,7 @@ final class PHUITimelineView extends AphrontView { $events[] = self::renderEnder(true); } - return phutil_tag( - 'div', - array( - 'class' => 'phui-timeline-view', - 'id' => $this->id, - ), - $events); + return $events; } public static function renderSpacer() { diff --git a/webroot/rsrc/js/application/transactions/behavior-show-older-transactions.js b/webroot/rsrc/js/application/transactions/behavior-show-older-transactions.js new file mode 100644 index 0000000000..112eac1612 --- /dev/null +++ b/webroot/rsrc/js/application/transactions/behavior-show-older-transactions.js @@ -0,0 +1,104 @@ +/** + * @provides javelin-behavior-phabricator-show-older-transactions + * @requires javelin-behavior + * javelin-stratcom + * javelin-dom + * phabricator-busy + */ + +JX.behavior('phabricator-show-older-transactions', function(config) { + + var loading = false; + + function get_hash() { + return window.location.hash.replace(/^#/, ''); + } + + function hash_is_hidden() { + var hash = get_hash(); + if (!hash) { + return false; + } + var id = 'anchor-'+hash; + try { + JX.$(id); + } catch (not_found_exception) { + return true; + } + return false; + } + + function check_hash() { + if (hash_is_hidden()) { + var showOlderBlock = null; + try { + showOlderBlock = JX.DOM.find( + JX.$(config.timelineID), + 'div', + 'show-older-block'); + } catch (not_found_exception) { + // probably a garbage hash and we loaded everything looking + // for it; just abort + if (loading) { + loading = false; + JX.Busy.done(); + } + return; + } + var showOlderLink = JX.DOM.find( + showOlderBlock, + 'a', + 'show-older-link'); + if (!loading) { + loading = true; + JX.Busy.start(); + } + fetch_older_workflow( + showOlderLink.href, + load_hidden_hash, + showOlderBlock) + .start(); + } else { + if (loading) { + loading = false; + JX.Busy.done(); + } + try { + var target = JX.$(get_hash()); + JX.DOM.scrollTo(target); + } catch (ignored) { + // We did our best. + } + } + } + + var show_older = function(swap, r) { + JX.DOM.replace(swap, JX.$H(r.timeline).getFragment()); + }; + + var load_hidden_hash = function(swap, r) { + show_older(swap, r); + check_hash(); + }; + + var fetch_older_workflow = function(href, callback, swap) { + return new JX.Workflow(href) + .setHandler(JX.bind(null, callback, swap)); + }; + + JX.Stratcom.listen( + 'click', + ['show-older-link'], + function(e) { + e.kill(); + fetch_older_workflow( + e.getNode('tag:a').href, + show_older, + e.getNode('show-older-block')) + .start(); + }); + + JX.Stratcom.listen('hashchange', null, check_hash); + check_hash(); + +});