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
2024-12-18 11:30:55 +01:00 · 2014-12-02 13:10:29 -08:00 · 2014-12-02 13:10:29 -08:00 · d6341cfffe
commit d6341cfffe
parent 7aedb3c04e
9 changed files with 328 additions and 55 deletions
--- 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',
--- 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',
--- 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;
  }
--- 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/(?<phid>[^/]+)/'
          => 'PhabricatorApplicationTransactionDetailController',
+        'showolder/(?<phid>[^/]+)/'
+          => 'PhabricatorApplicationTransactionShowOlderController',
        '(?P<value>old|new)/(?<phid>[^/]+)/'
          => 'PhabricatorApplicationTransactionValueController',
      ),
--- a/src/applications/transactions/controller/PhabricatorApplicationTransactionShowOlderController.php
+++ b/src/applications/transactions/controller/PhabricatorApplicationTransactionShowOlderController.php
@ -0,0 +1,55 @@
+<?php
+
+final class PhabricatorApplicationTransactionShowOlderController
+  extends PhabricatorApplicationTransactionController {
+
+  public function shouldAllowPublic() {
+    return true;
+  }
+
+  public function handleRequest(AphrontRequest $request) {
+    $request = $this->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),));
+  }
+
+}
--- 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) {
--- 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;
--- 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() {
--- a/webroot/rsrc/js/application/transactions/behavior-show-older-transactions.js
+++ 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();
+
+});