Remove "willRenderTimeline()" from ApplicationTransactionInterface

Summary: Depends on D19914. Ref T11351. Some of the Phoilo rabbit holes go very deep. `PhabricatorApplicationTransactionInterface` currently requires you to implement `willRenderTimeline()`. Almost every object just implements this as `return $timeline`; only Pholio, Diffusion, and Differential specialize it. In all cases, they are specializing it mostly to render inline comments. The actual implementations are a bit of a weird mess and the way the data is threaded through the call stack is weird and not very modern. Try to clean this up: - Stop requiring `willRenderTimeline()` to be implemented. - Stop requiring `getApplicationTransactionViewObject()` to be implemented (only the three above, plus Legalpad, implement this, and Legalpad's implementation is a no-op). These two methods are inherently pretty coupled for almost any reasonable thing you might want to do with the timeline. - Simplify the handling of "renderdata" and call it "View Data". This is additional information about the current view of the transaction timeline that is required to render it correctly. This is only used in Differential, to decide if we can link an inline comment to an anchor on the same page or should link it to another page. We could perhaps do this on the client instead, but having this data doesn't seem inherently bad to me. - If objects want to customize timeline rendering, they now implement `PhabricatorTimelineInterface` and provide a `TimelineEngine` which gets a nice formal stack. This leaves a lot of empty `willRenderTimeline()` implementations hanging around. I'll remove these in the next change, it's just going to be deleting a couple dozen copies of an identical empty method implementation. Test Plan: - Viewed audits, revisions, and mocks with inline comments. - Used "Show Older" to page a revision back in history (this is relevant for "View Data"). - Grepped for symbols: willRenderTimeline, getApplicationTransactionViewObject, Legalpad classes. Reviewers: amckinley Reviewed By: amckinley Maniphest Tasks: T11351 Differential Revision: https://secure.phabricator.com/D19918
2024-11-22 06:42:42 +01:00 · 2018-12-19 11:52:53 -08:00 · 2018-12-19 11:52:53 -08:00 · 6c43d1d52c
commit 6c43d1d52c
parent 21f07bf6f7
33 changed files with 366 additions and 230 deletions
--- a/resources/celerity/map.php
+++ b/resources/celerity/map.php
@ -10,7 +10,7 @@ return array(
    'conpherence.pkg.css' => 'e68cf1fa',
    'conpherence.pkg.js' => '15191c65',
    'core.pkg.css' => '9d1148a4',
-    'core.pkg.js' => '4bde473b',
+    'core.pkg.js' => 'bd89cb1d',
    'differential.pkg.css' => '06dc617c',
    'differential.pkg.js' => 'ef0b989b',
    'diffusion.pkg.css' => 'a2d17c7d',
@ -425,7 +425,7 @@ return array(
    'rsrc/js/application/transactions/behavior-comment-actions.js' => '59e27e74',
    'rsrc/js/application/transactions/behavior-reorder-configs.js' => 'd7a74243',
    'rsrc/js/application/transactions/behavior-reorder-fields.js' => 'b59e1e96',
-    'rsrc/js/application/transactions/behavior-show-older-transactions.js' => '8f29b364',
+    'rsrc/js/application/transactions/behavior-show-older-transactions.js' => '0e1eca96',
    'rsrc/js/application/transactions/behavior-transaction-comment-form.js' => 'b23b49e6',
    'rsrc/js/application/transactions/behavior-transaction-list.js' => '1f6794f6',
    'rsrc/js/application/typeahead/behavior-typeahead-browse.js' => '635de1ec',
@ -639,7 +639,7 @@ return array(
    'javelin-behavior-phabricator-remarkup-assist' => 'acd29eee',
    'javelin-behavior-phabricator-reveal-content' => '60821bc7',
    'javelin-behavior-phabricator-search-typeahead' => 'c3e917d9',
-    'javelin-behavior-phabricator-show-older-transactions' => '8f29b364',
+    'javelin-behavior-phabricator-show-older-transactions' => '0e1eca96',
    'javelin-behavior-phabricator-tooltips' => 'c420b0b9',
    'javelin-behavior-phabricator-transaction-comment-form' => 'b23b49e6',
    'javelin-behavior-phabricator-transaction-list' => '1f6794f6',
@ -950,6 +950,12 @@ return array(
      'javelin-install',
      'phuix-button-view',
    ),
+    '0e1eca96' => array(
+      'javelin-behavior',
+      'javelin-stratcom',
+      'javelin-dom',
+      'phabricator-busy',
+    ),
    '0f764c35' => array(
      'javelin-install',
      'javelin-util',
@ -1581,12 +1587,6 @@ return array(
    '8e1baf68' => array(
      'phui-button-css',
    ),
-    '8f29b364' => array(
-      'javelin-behavior',
-      'javelin-stratcom',
-      'javelin-dom',
-      'phabricator-busy',
-    ),
    '8ff5e24c' => array(
      'javelin-behavior',
      'javelin-stratcom',
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@ -647,6 +647,7 @@ phutil_register_library_map(array(
    'DifferentialRevisionSummaryTransaction' => 'applications/differential/xaction/DifferentialRevisionSummaryTransaction.php',
    'DifferentialRevisionTestPlanHeraldField' => 'applications/differential/herald/DifferentialRevisionTestPlanHeraldField.php',
    'DifferentialRevisionTestPlanTransaction' => 'applications/differential/xaction/DifferentialRevisionTestPlanTransaction.php',
+    'DifferentialRevisionTimelineEngine' => 'applications/differential/engine/DifferentialRevisionTimelineEngine.php',
    'DifferentialRevisionTitleHeraldField' => 'applications/differential/herald/DifferentialRevisionTitleHeraldField.php',
    'DifferentialRevisionTitleTransaction' => 'applications/differential/xaction/DifferentialRevisionTitleTransaction.php',
    'DifferentialRevisionTransactionType' => 'applications/differential/xaction/DifferentialRevisionTransactionType.php',
@ -769,6 +770,7 @@ phutil_register_library_map(array(
    'DiffusionCommitSearchConduitAPIMethod' => 'applications/diffusion/conduit/DiffusionCommitSearchConduitAPIMethod.php',
    'DiffusionCommitStateTransaction' => 'applications/diffusion/xaction/DiffusionCommitStateTransaction.php',
    'DiffusionCommitTagsController' => 'applications/diffusion/controller/DiffusionCommitTagsController.php',
+    'DiffusionCommitTimelineEngine' => 'applications/diffusion/engine/DiffusionCommitTimelineEngine.php',
    'DiffusionCommitTransactionType' => 'applications/diffusion/xaction/DiffusionCommitTransactionType.php',
    'DiffusionCommitVerifyTransaction' => 'applications/diffusion/xaction/DiffusionCommitVerifyTransaction.php',
    'DiffusionCompareController' => 'applications/diffusion/controller/DiffusionCompareController.php',
@ -1630,7 +1632,6 @@ phutil_register_library_map(array(
    'LegalpadTransaction' => 'applications/legalpad/storage/LegalpadTransaction.php',
    'LegalpadTransactionComment' => 'applications/legalpad/storage/LegalpadTransactionComment.php',
    'LegalpadTransactionQuery' => 'applications/legalpad/query/LegalpadTransactionQuery.php',
-    'LegalpadTransactionView' => 'applications/legalpad/view/LegalpadTransactionView.php',
    'LiskChunkTestCase' => 'infrastructure/storage/lisk/__tests__/LiskChunkTestCase.php',
    'LiskDAO' => 'infrastructure/storage/lisk/LiskDAO.php',
    'LiskDAOTestCase' => 'infrastructure/storage/lisk/__tests__/LiskDAOTestCase.php',
@ -4433,6 +4434,7 @@ phutil_register_library_map(array(
    'PhabricatorStandardCustomFieldUsers' => 'infrastructure/customfield/standard/PhabricatorStandardCustomFieldUsers.php',
    'PhabricatorStandardPageView' => 'view/page/PhabricatorStandardPageView.php',
    'PhabricatorStandardSelectCustomFieldDatasource' => 'infrastructure/customfield/datasource/PhabricatorStandardSelectCustomFieldDatasource.php',
+    'PhabricatorStandardTimelineEngine' => 'applications/transactions/engine/PhabricatorStandardTimelineEngine.php',
    'PhabricatorStaticEditField' => 'applications/transactions/editfield/PhabricatorStaticEditField.php',
    'PhabricatorStatusController' => 'applications/system/controller/PhabricatorStatusController.php',
    'PhabricatorStatusUIExample' => 'applications/uiexample/examples/PhabricatorStatusUIExample.php',
@ -4532,6 +4534,8 @@ phutil_register_library_map(array(
    'PhabricatorTimeFormatSetting' => 'applications/settings/setting/PhabricatorTimeFormatSetting.php',
    'PhabricatorTimeGuard' => 'infrastructure/time/PhabricatorTimeGuard.php',
    'PhabricatorTimeTestCase' => 'infrastructure/time/__tests__/PhabricatorTimeTestCase.php',
+    'PhabricatorTimelineEngine' => 'applications/transactions/engine/PhabricatorTimelineEngine.php',
+    'PhabricatorTimelineInterface' => 'applications/transactions/interface/PhabricatorTimelineInterface.php',
    'PhabricatorTimezoneIgnoreOffsetSetting' => 'applications/settings/setting/PhabricatorTimezoneIgnoreOffsetSetting.php',
    'PhabricatorTimezoneSetting' => 'applications/settings/setting/PhabricatorTimezoneSetting.php',
    'PhabricatorTimezoneSetupCheck' => 'applications/config/check/PhabricatorTimezoneSetupCheck.php',
@ -4871,6 +4875,7 @@ phutil_register_library_map(array(
    'PholioMockSearchEngine' => 'applications/pholio/query/PholioMockSearchEngine.php',
    'PholioMockStatusTransaction' => 'applications/pholio/xaction/PholioMockStatusTransaction.php',
    'PholioMockThumbGridView' => 'applications/pholio/view/PholioMockThumbGridView.php',
+    'PholioMockTimelineEngine' => 'applications/pholio/engine/PholioMockTimelineEngine.php',
    'PholioMockTransactionType' => 'applications/pholio/xaction/PholioMockTransactionType.php',
    'PholioMockViewController' => 'applications/pholio/controller/PholioMockViewController.php',
    'PholioRemarkupRule' => 'applications/pholio/remarkup/PholioRemarkupRule.php',
@ -5991,6 +5996,7 @@ phutil_register_library_map(array(
      'PhabricatorSubscribableInterface',
      'PhabricatorCustomFieldInterface',
      'PhabricatorApplicationTransactionInterface',
+      'PhabricatorTimelineInterface',
      'PhabricatorMentionableInterface',
      'PhabricatorDestructibleInterface',
      'PhabricatorProjectInterface',
@ -6072,6 +6078,7 @@ phutil_register_library_map(array(
    'DifferentialRevisionSummaryTransaction' => 'DifferentialRevisionTransactionType',
    'DifferentialRevisionTestPlanHeraldField' => 'DifferentialRevisionHeraldField',
    'DifferentialRevisionTestPlanTransaction' => 'DifferentialRevisionTransactionType',
+    'DifferentialRevisionTimelineEngine' => 'PhabricatorTimelineEngine',
    'DifferentialRevisionTitleHeraldField' => 'DifferentialRevisionHeraldField',
    'DifferentialRevisionTitleTransaction' => 'DifferentialRevisionTransactionType',
    'DifferentialRevisionTransactionType' => 'PhabricatorModularTransactionType',
@ -6194,6 +6201,7 @@ phutil_register_library_map(array(
    'DiffusionCommitSearchConduitAPIMethod' => 'PhabricatorSearchEngineAPIMethod',
    'DiffusionCommitStateTransaction' => 'DiffusionCommitTransactionType',
    'DiffusionCommitTagsController' => 'DiffusionController',
+    'DiffusionCommitTimelineEngine' => 'PhabricatorTimelineEngine',
    'DiffusionCommitTransactionType' => 'PhabricatorModularTransactionType',
    'DiffusionCommitVerifyTransaction' => 'DiffusionCommitAuditTransaction',
    'DiffusionCompareController' => 'DiffusionController',
@ -7201,7 +7209,6 @@ phutil_register_library_map(array(
    'LegalpadTransaction' => 'PhabricatorModularTransaction',
    'LegalpadTransactionComment' => 'PhabricatorApplicationTransactionComment',
    'LegalpadTransactionQuery' => 'PhabricatorApplicationTransactionQuery',
-    'LegalpadTransactionView' => 'PhabricatorApplicationTransactionView',
    'LiskChunkTestCase' => 'PhabricatorTestCase',
    'LiskDAO' => array(
      'Phobject',
@ -10079,6 +10086,7 @@ phutil_register_library_map(array(
      'HarbormasterBuildkiteBuildableInterface',
      'PhabricatorCustomFieldInterface',
      'PhabricatorApplicationTransactionInterface',
+      'PhabricatorTimelineInterface',
      'PhabricatorFulltextInterface',
      'PhabricatorFerretInterface',
      'PhabricatorConduitResultInterface',
@ -10469,6 +10477,7 @@ phutil_register_library_map(array(
      'AphrontResponseProducerInterface',
    ),
    'PhabricatorStandardSelectCustomFieldDatasource' => 'PhabricatorTypeaheadDatasource',
+    'PhabricatorStandardTimelineEngine' => 'PhabricatorTimelineEngine',
    'PhabricatorStaticEditField' => 'PhabricatorEditField',
    'PhabricatorStatusController' => 'PhabricatorController',
    'PhabricatorStatusUIExample' => 'PhabricatorUIExample',
@ -10567,6 +10576,7 @@ phutil_register_library_map(array(
    'PhabricatorTimeFormatSetting' => 'PhabricatorSelectSetting',
    'PhabricatorTimeGuard' => 'Phobject',
    'PhabricatorTimeTestCase' => 'PhabricatorTestCase',
+    'PhabricatorTimelineEngine' => 'Phobject',
    'PhabricatorTimezoneIgnoreOffsetSetting' => 'PhabricatorInternalSetting',
    'PhabricatorTimezoneSetting' => 'PhabricatorOptionGroupSetting',
    'PhabricatorTimezoneSetupCheck' => 'PhabricatorSetupCheck',
@ -10967,6 +10977,7 @@ phutil_register_library_map(array(
      'PhabricatorTokenReceiverInterface',
      'PhabricatorFlaggableInterface',
      'PhabricatorApplicationTransactionInterface',
+      'PhabricatorTimelineInterface',
      'PhabricatorProjectInterface',
      'PhabricatorDestructibleInterface',
      'PhabricatorSpacesInterface',
@ -11001,6 +11012,7 @@ phutil_register_library_map(array(
    'PholioMockSearchEngine' => 'PhabricatorApplicationSearchEngine',
    'PholioMockStatusTransaction' => 'PholioMockTransactionType',
    'PholioMockThumbGridView' => 'AphrontView',
+    'PholioMockTimelineEngine' => 'PhabricatorTimelineEngine',
    'PholioMockTransactionType' => 'PholioTransactionType',
    'PholioMockViewController' => 'PholioController',
    'PholioRemarkupRule' => 'PhabricatorObjectRemarkupRule',
--- a/src/applications/audit/storage/PhabricatorAuditTransaction.php
+++ b/src/applications/audit/storage/PhabricatorAuditTransaction.php
@ -32,10 +32,6 @@ final class PhabricatorAuditTransaction
    return new PhabricatorAuditTransactionComment();
  }

-  public function getApplicationTransactionViewObject() {
-    return new PhabricatorAuditTransactionView();
-  }
-
  public function getRemarkupBlocks() {
    $blocks = parent::getRemarkupBlocks();

--- a/src/applications/badges/controller/PhabricatorBadgesCommentController.php
+++ b/src/applications/badges/controller/PhabricatorBadgesCommentController.php
@ -51,6 +51,7 @@ final class PhabricatorBadgesCommentController

    if ($request->isAjax() && $is_preview) {
      return id(new PhabricatorApplicationTransactionResponse())
+        ->setObject($badge)
        ->setViewer($viewer)
        ->setTransactions($xactions)
        ->setIsPreview($is_preview);
--- a/src/applications/base/controller/PhabricatorController.php
+++ b/src/applications/base/controller/PhabricatorController.php
@ -482,14 +482,14 @@ abstract class PhabricatorController extends AphrontController {
    PhabricatorApplicationTransactionInterface $object,
    PhabricatorApplicationTransactionQuery $query,
    PhabricatorMarkupEngine $engine = null,
-    $render_data = array()) {
+    $view_data = array()) {

-    $viewer = $this->getRequest()->getUser();
+    $request = $this->getRequest();
+    $viewer = $this->getViewer();
    $xaction = $object->getApplicationTransactionTemplate();
-    $view = $xaction->getApplicationTransactionViewObject();

    $pager = id(new AphrontCursorPagerView())
-      ->readFromRequest($this->getRequest())
+      ->readFromRequest($request)
      ->setURI(new PhutilURI(
        '/transactions/showolder/'.$object->getPHID().'/'));

@ -500,6 +500,13 @@ abstract class PhabricatorController extends AphrontController {
      ->executeWithCursorPager($pager);
    $xactions = array_reverse($xactions);

+    $timeline_engine = PhabricatorTimelineEngine::newForObject($object)
+      ->setViewer($viewer)
+      ->setTransactions($xactions)
+      ->setViewData($view_data);
+
+    $view = $timeline_engine->buildTimelineView();
+
    if ($engine) {
      foreach ($xactions as $xaction) {
        if ($xaction->getComment()) {
@ -513,14 +520,9 @@ abstract class PhabricatorController extends AphrontController {
    }

    $timeline = $view
-      ->setUser($viewer)
-      ->setObjectPHID($object->getPHID())
-      ->setTransactions($xactions)
      ->setPager($pager)
-      ->setRenderData($render_data)
      ->setQuoteTargetID($this->getRequest()->getStr('quoteTargetID'))
      ->setQuoteRef($this->getRequest()->getStr('quoteRef'));
-    $object->willRenderTimeline($timeline, $this->getRequest());

    return $timeline;
  }
--- a/src/applications/config/controller/PhabricatorConfigHistoryController.php
+++ b/src/applications/config/controller/PhabricatorConfigHistoryController.php
@ -16,18 +16,14 @@ final class PhabricatorConfigHistoryController

    $xaction = $object->getApplicationTransactionTemplate();

-    $view = $xaction->getApplicationTransactionViewObject();
-
-    $timeline = $view
-      ->setUser($viewer)
+    $timeline = id(new PhabricatorApplicationTransactionView())
+      ->setViewer($viewer)
      ->setTransactions($xactions)
      ->setRenderAsFeed(true)
      ->setObjectPHID(PhabricatorPHIDConstants::PHID_VOID);

    $timeline->setShouldTerminate(true);

-    $object->willRenderTimeline($timeline, $this->getRequest());
-
    $title = pht('Settings History');
    $header = $this->buildHeaderView($title);

--- a/src/applications/differential/engine/DifferentialRevisionTimelineEngine.php
+++ b/src/applications/differential/engine/DifferentialRevisionTimelineEngine.php
@ -0,0 +1,78 @@
+<?php
+
+final class DifferentialRevisionTimelineEngine
+  extends PhabricatorTimelineEngine {
+
+  protected function newTimelineView() {
+    $viewer = $this->getViewer();
+    $xactions = $this->getTransactions();
+    $revision = $this->getObject();
+
+    $view_data = $this->getViewData();
+    if (!$view_data) {
+      $view_data = array();
+    }
+
+    $left = idx($view_data, 'left');
+    $right = idx($view_data, 'right');
+
+    $diffs = id(new DifferentialDiffQuery())
+      ->setViewer($viewer)
+      ->withIDs(array($left, $right))
+      ->execute();
+    $diffs = mpull($diffs, null, 'getID');
+    $left_diff = $diffs[$left];
+    $right_diff = $diffs[$right];
+
+    $old_ids = idx($view_data, 'old');
+    $new_ids = idx($view_data, 'new');
+    $old_ids = array_filter(explode(',', $old_ids));
+    $new_ids = array_filter(explode(',', $new_ids));
+
+    $type_inline = DifferentialTransaction::TYPE_INLINE;
+    $changeset_ids = array_merge($old_ids, $new_ids);
+    $inlines = array();
+    foreach ($xactions as $xaction) {
+      if ($xaction->getTransactionType() == $type_inline) {
+        $inlines[] = $xaction->getComment();
+        $changeset_ids[] = $xaction->getComment()->getChangesetID();
+      }
+    }
+
+    if ($changeset_ids) {
+      $changesets = id(new DifferentialChangesetQuery())
+        ->setViewer($viewer)
+        ->withIDs($changeset_ids)
+        ->execute();
+      $changesets = mpull($changesets, null, 'getID');
+    } else {
+      $changesets = array();
+    }
+
+    foreach ($inlines as $key => $inline) {
+      $inlines[$key] = DifferentialInlineComment::newFromModernComment(
+        $inline);
+    }
+
+    $query = id(new DifferentialInlineCommentQuery())
+      ->needHidden(true)
+      ->setViewer($viewer);
+
+    // NOTE: This is a bit sketchy: this method adjusts the inlines as a
+    // side effect, which means it will ultimately adjust the transaction
+    // comments and affect timeline rendering.
+    $query->adjustInlinesForChangesets(
+      $inlines,
+      array_select_keys($changesets, $old_ids),
+      array_select_keys($changesets, $new_ids),
+      $revision);
+
+    return id(new DifferentialTransactionView())
+      ->setViewData($view_data)
+      ->setChangesets($changesets)
+      ->setRevision($revision)
+      ->setLeftDiff($left_diff)
+      ->setRightDiff($right_diff);
+  }
+
+}
--- a/src/applications/differential/storage/DifferentialRevision.php
+++ b/src/applications/differential/storage/DifferentialRevision.php
@ -11,6 +11,7 @@ final class DifferentialRevision extends DifferentialDAO
    PhabricatorSubscribableInterface,
    PhabricatorCustomFieldInterface,
    PhabricatorApplicationTransactionInterface,
+    PhabricatorTimelineInterface,
    PhabricatorMentionableInterface,
    PhabricatorDestructibleInterface,
    PhabricatorProjectInterface,
@ -998,73 +999,6 @@ final class DifferentialRevision extends DifferentialDAO
    return new DifferentialTransaction();
  }

-  public function willRenderTimeline(
-    PhabricatorApplicationTransactionView $timeline,
-    AphrontRequest $request) {
-    $viewer = $request->getViewer();
-
-    $render_data = $timeline->getRenderData();
-    $left = $request->getInt('left', idx($render_data, 'left'));
-    $right = $request->getInt('right', idx($render_data, 'right'));
-
-    $diffs = id(new DifferentialDiffQuery())
-      ->setViewer($request->getUser())
-      ->withIDs(array($left, $right))
-      ->execute();
-    $diffs = mpull($diffs, null, 'getID');
-    $left_diff = $diffs[$left];
-    $right_diff = $diffs[$right];
-
-    $old_ids = $request->getStr('old', idx($render_data, 'old'));
-    $new_ids = $request->getStr('new', idx($render_data, 'new'));
-    $old_ids = array_filter(explode(',', $old_ids));
-    $new_ids = array_filter(explode(',', $new_ids));
-
-    $type_inline = DifferentialTransaction::TYPE_INLINE;
-    $changeset_ids = array_merge($old_ids, $new_ids);
-    $inlines = array();
-    foreach ($timeline->getTransactions() as $xaction) {
-      if ($xaction->getTransactionType() == $type_inline) {
-        $inlines[] = $xaction->getComment();
-        $changeset_ids[] = $xaction->getComment()->getChangesetID();
-      }
-    }
-
-    if ($changeset_ids) {
-      $changesets = id(new DifferentialChangesetQuery())
-        ->setViewer($request->getUser())
-        ->withIDs($changeset_ids)
-        ->execute();
-      $changesets = mpull($changesets, null, 'getID');
-    } else {
-      $changesets = array();
-    }
-
-    foreach ($inlines as $key => $inline) {
-      $inlines[$key] = DifferentialInlineComment::newFromModernComment(
-        $inline);
-    }
-
-    $query = id(new DifferentialInlineCommentQuery())
-      ->needHidden(true)
-      ->setViewer($viewer);
-
-    // NOTE: This is a bit sketchy: this method adjusts the inlines as a
-    // side effect, which means it will ultimately adjust the transaction
-    // comments and affect timeline rendering.
-    $query->adjustInlinesForChangesets(
-      $inlines,
-      array_select_keys($changesets, $old_ids),
-      array_select_keys($changesets, $new_ids),
-      $this);
-
-    return $timeline
-      ->setChangesets($changesets)
-      ->setRevision($this)
-      ->setLeftDiff($left_diff)
-      ->setRightDiff($right_diff);
-  }
-

 /* -(  PhabricatorDestructibleInterface  )----------------------------------- */

@ -1206,4 +1140,13 @@ final class DifferentialRevision extends DifferentialDAO
    return new DifferentialRevisionDraftEngine();
  }

+
+/* -(  PhabricatorTimelineInterface  )--------------------------------------- */
+
+
+  public function newTimelineEngine() {
+    return new DifferentialRevisionTimelineEngine();
+  }
+
+
 }
--- a/src/applications/differential/storage/DifferentialTransaction.php
+++ b/src/applications/differential/storage/DifferentialTransaction.php
@ -65,10 +65,6 @@ final class DifferentialTransaction
    return new DifferentialTransactionComment();
  }

-  public function getApplicationTransactionViewObject() {
-    return new DifferentialTransactionView();
-  }
-
  public function shouldHide() {
    $old = $this->getOldValue();
    $new = $this->getNewValue();
--- a/src/applications/diffusion/controller/DiffusionCommitController.php
+++ b/src/applications/diffusion/controller/DiffusionCommitController.php
@ -740,8 +740,6 @@ final class DiffusionCommitController extends DiffusionController {
      $commit,
      new PhabricatorAuditTransactionQuery());

-    $commit->willRenderTimeline($timeline, $this->getRequest());
-
    $timeline->setQuoteRef($commit->getMonogram());

    return $timeline;
--- a/src/applications/diffusion/engine/DiffusionCommitTimelineEngine.php
+++ b/src/applications/diffusion/engine/DiffusionCommitTimelineEngine.php
@ -0,0 +1,30 @@
+<?php
+
+final class DiffusionCommitTimelineEngine
+  extends PhabricatorTimelineEngine {
+
+  protected function newTimelineView() {
+    $xactions = $this->getTransactions();
+
+    $path_ids = array();
+    foreach ($xactions as $xaction) {
+      if ($xaction->hasComment()) {
+        $path_id = $xaction->getComment()->getPathID();
+        if ($path_id) {
+          $path_ids[] = $path_id;
+        }
+      }
+    }
+
+    $path_map = array();
+    if ($path_ids) {
+      $path_map = id(new DiffusionPathQuery())
+        ->withPathIDs($path_ids)
+        ->execute();
+      $path_map = ipull($path_map, 'path', 'id');
+    }
+
+    return id(new PhabricatorAuditTransactionView())
+      ->setPathMap($path_map);
+  }
+}
--- a/src/applications/legalpad/storage/LegalpadTransaction.php
+++ b/src/applications/legalpad/storage/LegalpadTransaction.php
@ -14,10 +14,6 @@ final class LegalpadTransaction extends PhabricatorModularTransaction {
    return new LegalpadTransactionComment();
  }

-  public function getApplicationTransactionViewObject() {
-    return new LegalpadTransactionView();
-  }
-
  public function getBaseTransactionClass() {
    return 'LegalpadDocumentTransactionType';
  }
--- a/src/applications/legalpad/view/LegalpadTransactionView.php
+++ b/src/applications/legalpad/view/LegalpadTransactionView.php
@ -1,4 +0,0 @@
-<?php
-
-final class LegalpadTransactionView
-  extends PhabricatorApplicationTransactionView {}
--- a/src/applications/pholio/controller/PholioMockCommentController.php
+++ b/src/applications/pholio/controller/PholioMockCommentController.php
@ -68,13 +68,10 @@ final class PholioMockCommentController extends PholioController {
    }

    if ($request->isAjax() && $is_preview) {
-      $xaction_view = id(new PholioTransactionView())
-        ->setMock($mock);
-
      return id(new PhabricatorApplicationTransactionResponse())
+        ->setObject($mock)
        ->setViewer($viewer)
        ->setTransactions($xactions)
-        ->setTransactionView($xaction_view)
        ->setIsPreview($is_preview);
    } else {
      return id(new AphrontRedirectResponse())->setURI($mock_uri);
--- a/src/applications/pholio/engine/PholioMockTimelineEngine.php
+++ b/src/applications/pholio/engine/PholioMockTimelineEngine.php
@ -0,0 +1,19 @@
+<?php
+
+final class PholioMockTimelineEngine
+  extends PhabricatorTimelineEngine {
+
+  protected function newTimelineView() {
+    $viewer = $this->getViewer();
+    $object = $this->getObject();
+
+    PholioMockQuery::loadImages(
+      $viewer,
+      array($object),
+      $need_inline_comments = true);
+
+    return id(new PholioTransactionView())
+      ->setMock($object);
+  }
+
+}
--- a/src/applications/pholio/storage/PholioMock.php
+++ b/src/applications/pholio/storage/PholioMock.php
@ -7,6 +7,7 @@ final class PholioMock extends PholioDAO
    PhabricatorTokenReceiverInterface,
    PhabricatorFlaggableInterface,
    PhabricatorApplicationTransactionInterface,
+    PhabricatorTimelineInterface,
    PhabricatorProjectInterface,
    PhabricatorDestructibleInterface,
    PhabricatorSpacesInterface,
@ -228,17 +229,6 @@ final class PholioMock extends PholioDAO
    return new PholioTransaction();
  }

-  public function willRenderTimeline(
-    PhabricatorApplicationTransactionView $timeline,
-    AphrontRequest $request) {
-
-    PholioMockQuery::loadImages(
-      $request->getUser(),
-      array($this),
-      $need_inline_comments = true);
-    $timeline->setMock($this);
-    return $timeline;
-  }

 /* -(  PhabricatorTokenReceiverInterface  )---------------------------------- */

@ -288,9 +278,18 @@ final class PholioMock extends PholioDAO

 /* -(  PhabricatorFerretInterface  )----------------------------------------- */

+
  public function newFerretEngine() {
    return new PholioMockFerretEngine();
  }


+/* -(  PhabricatorTimelineInterace  )---------------------------------------- */
+
+
+  public function newTimelineEngine() {
+    return new PholioMockTimelineEngine();
+  }
+
+
 }
--- a/src/applications/pholio/storage/PholioTransaction.php
+++ b/src/applications/pholio/storage/PholioTransaction.php
@ -23,10 +23,6 @@ final class PholioTransaction extends PhabricatorModularTransaction {
    return new PholioTransactionComment();
  }

-  public function getApplicationTransactionViewObject() {
-    return new PholioTransactionView();
-  }
-
  public function getMailTags() {
    $tags = array();
    switch ($this->getTransactionType()) {
--- a/src/applications/ponder/controller/PonderAnswerCommentController.php
+++ b/src/applications/ponder/controller/PonderAnswerCommentController.php
@ -50,6 +50,7 @@ final class PonderAnswerCommentController extends PonderController {

    if ($request->isAjax() && $is_preview) {
      return id(new PhabricatorApplicationTransactionResponse())
+        ->setObject($answer)
        ->setViewer($viewer)
        ->setTransactions($xactions)
        ->setIsPreview($is_preview);
--- a/src/applications/ponder/controller/PonderQuestionCommentController.php
+++ b/src/applications/ponder/controller/PonderQuestionCommentController.php
@ -46,6 +46,7 @@ final class PonderQuestionCommentController extends PonderController {

    if ($request->isAjax() && $is_preview) {
      return id(new PhabricatorApplicationTransactionResponse())
+        ->setObject($question)
        ->setViewer($viewer)
        ->setTransactions($xactions)
        ->setIsPreview($is_preview);
--- a/src/applications/releeph/controller/request/ReleephRequestCommentController.php
+++ b/src/applications/releeph/controller/request/ReleephRequestCommentController.php
@ -51,6 +51,7 @@ final class ReleephRequestCommentController

    if ($request->isAjax() && $is_preview) {
      return id(new PhabricatorApplicationTransactionResponse())
+        ->setObject($pull)
        ->setViewer($viewer)
        ->setTransactions($xactions)
        ->setIsPreview($is_preview);
--- a/src/applications/repository/storage/PhabricatorRepositoryCommit.php
+++ b/src/applications/repository/storage/PhabricatorRepositoryCommit.php
@ -14,6 +14,7 @@ final class PhabricatorRepositoryCommit
    HarbormasterBuildkiteBuildableInterface,
    PhabricatorCustomFieldInterface,
    PhabricatorApplicationTransactionInterface,
+    PhabricatorTimelineInterface,
    PhabricatorFulltextInterface,
    PhabricatorFerretInterface,
    PhabricatorConduitResultInterface,
@ -738,33 +739,6 @@ final class PhabricatorRepositoryCommit
    return new PhabricatorAuditTransaction();
  }

-  public function willRenderTimeline(
-    PhabricatorApplicationTransactionView $timeline,
-    AphrontRequest $request) {
-
-    $xactions = $timeline->getTransactions();
-
-    $path_ids = array();
-    foreach ($xactions as $xaction) {
-      if ($xaction->hasComment()) {
-        $path_id = $xaction->getComment()->getPathID();
-        if ($path_id) {
-          $path_ids[] = $path_id;
-        }
-      }
-    }
-
-    $path_map = array();
-    if ($path_ids) {
-      $path_map = id(new DiffusionPathQuery())
-        ->withPathIDs($path_ids)
-        ->execute();
-      $path_map = ipull($path_map, 'path', 'id');
-    }
-
-    return $timeline->setPathMap($path_map);
-  }
-
 /* -(  PhabricatorFulltextInterface  )--------------------------------------- */


@ -916,4 +890,12 @@ final class PhabricatorRepositoryCommit
    return $this;
  }

+
+/* -(  PhabricatorTimelineInterface  )--------------------------------------- */
+
+
+  public function newTimelineEngine() {
+    return new DiffusionCommitTimelineEngine();
+  }
+
 }
--- a/src/applications/slowvote/controller/PhabricatorSlowvoteCommentController.php
+++ b/src/applications/slowvote/controller/PhabricatorSlowvoteCommentController.php
@ -51,6 +51,7 @@ final class PhabricatorSlowvoteCommentController

    if ($request->isAjax() && $is_preview) {
      return id(new PhabricatorApplicationTransactionResponse())
+        ->setObject($poll)
        ->setViewer($viewer)
        ->setTransactions($xactions)
        ->setIsPreview($is_preview);
--- a/src/applications/transactions/controller/PhabricatorApplicationTransactionShowOlderController.php
+++ b/src/applications/transactions/controller/PhabricatorApplicationTransactionShowOlderController.php
@ -27,7 +27,18 @@ final class PhabricatorApplicationTransactionShowOlderController
      return new Aphront404Response();
    }

-    $timeline = $this->buildTransactionTimeline($object, $query);
+    $raw_view_data = $request->getStr('viewData');
+    try {
+      $view_data = phutil_json_decode($raw_view_data);
+    } catch (Exception $ex) {
+      $view_data = array();
+    }
+
+    $timeline = $this->buildTransactionTimeline(
+      $object,
+      $query,
+      null,
+      $view_data);

    $phui_timeline = $timeline->buildPHUITimelineView($with_hiding = false);
    $phui_timeline->setShouldAddSpacers(false);
--- a/src/applications/transactions/editengine/PhabricatorEditEngine.php
+++ b/src/applications/transactions/editengine/PhabricatorEditEngine.php
@ -1955,6 +1955,7 @@ abstract class PhabricatorEditEngine
      $preview_content = $this->newCommentPreviewContent($object, $xactions);

      return id(new PhabricatorApplicationTransactionResponse())
+        ->setObject($object)
        ->setViewer($viewer)
        ->setTransactions($xactions)
        ->setIsPreview($is_preview)
--- a/src/applications/transactions/engine/PhabricatorStandardTimelineEngine.php
+++ b/src/applications/transactions/engine/PhabricatorStandardTimelineEngine.php
@ -0,0 +1,4 @@
+<?php
+
+final class PhabricatorStandardTimelineEngine
+   extends PhabricatorTimelineEngine {}
--- a/src/applications/transactions/engine/PhabricatorTimelineEngine.php
+++ b/src/applications/transactions/engine/PhabricatorTimelineEngine.php
@ -0,0 +1,95 @@
+<?php
+
+abstract class PhabricatorTimelineEngine
+  extends Phobject {
+
+  private $viewer;
+  private $object;
+  private $xactions;
+  private $viewData;
+
+  final public static function newForObject($object) {
+    if ($object instanceof PhabricatorTimelineInterface) {
+      $engine = $object->newTimelineEngine();
+    } else {
+      $engine = new PhabricatorStandardTimelineEngine();
+    }
+
+    $engine->setObject($object);
+
+    return $engine;
+  }
+
+  final public function setViewer(PhabricatorUser $viewer) {
+    $this->viewer = $viewer;
+    return $this;
+  }
+
+  final public function getViewer() {
+    return $this->viewer;
+  }
+
+  final public function setObject($object) {
+    $this->object = $object;
+    return $this;
+  }
+
+  final public function getObject() {
+    return $this->object;
+  }
+
+  final public function setTransactions(array $xactions) {
+    assert_instances_of($xactions, 'PhabricatorApplicationTransaction');
+    $this->xactions = $xactions;
+    return $this;
+  }
+
+  final public function getTransactions() {
+    return $this->xactions;
+  }
+
+  final public function setRequest(AphrontRequest $request) {
+    $this->request = $request;
+    return $this;
+  }
+
+  final public function getRequest() {
+    return $this->request;
+  }
+
+  final public function setViewData(array $view_data) {
+    $this->viewData = $view_data;
+    return $this;
+  }
+
+  final public function getViewData() {
+    return $this->viewData;
+  }
+
+  final public function buildTimelineView() {
+    $view = $this->newTimelineView();
+
+    if (!($view instanceof PhabricatorApplicationTransactionView)) {
+      throw new Exception(
+        pht(
+          'Expected "newTimelineView()" to return an object of class "%s" '.
+          '(in engine "%s").',
+          'PhabricatorApplicationTransactionView',
+          get_class($this)));
+    }
+
+    $viewer = $this->getViewer();
+    $object = $this->getObject();
+    $xactions = $this->getTransactions();
+
+    return $view
+      ->setViewer($viewer)
+      ->setObjectPHID($object->getPHID())
+      ->setTransactions($xactions);
+  }
+
+  protected function newTimelineView() {
+    return new PhabricatorApplicationTransactionView();
+  }
+
+}
--- a/src/applications/transactions/interface/PhabricatorApplicationTransactionInterface.php
+++ b/src/applications/transactions/interface/PhabricatorApplicationTransactionInterface.php
@ -35,15 +35,6 @@ interface PhabricatorApplicationTransactionInterface {
   */
  public function getApplicationTransactionTemplate();

-  /**
-   * Hook to augment the $timeline with additional data for rendering.
-   *
-   * @return PhabricatorApplicationTransactionView
-   */
-  public function willRenderTimeline(
-    PhabricatorApplicationTransactionView $timeline,
-    AphrontRequest $request);
-
 }

 // TEMPLATE IMPLEMENTATION /////////////////////////////////////////////////////
@ -64,11 +55,4 @@ interface PhabricatorApplicationTransactionInterface {
    return new <<<???>>>Transaction();
  }

-  public function willRenderTimeline(
-    PhabricatorApplicationTransactionView $timeline,
-    AphrontRequest $request) {
-
-    return $timeline;
-  }
-
 */
--- a/src/applications/transactions/interface/PhabricatorTimelineInterface.php
+++ b/src/applications/transactions/interface/PhabricatorTimelineInterface.php
@ -0,0 +1,7 @@
+<?php
+
+interface PhabricatorTimelineInterface {
+
+  public function newTimelineEngine();
+
+}
--- a/src/applications/transactions/response/PhabricatorApplicationTransactionResponse.php
+++ b/src/applications/transactions/response/PhabricatorApplicationTransactionResponse.php
@ -6,17 +6,8 @@ final class PhabricatorApplicationTransactionResponse
  private $viewer;
  private $transactions;
  private $isPreview;
-  private $transactionView;
  private $previewContent;
-
-  public function setTransactionView($transaction_view) {
-    $this->transactionView = $transaction_view;
-    return $this;
-  }
-
-  public function getTransactionView() {
-    return $this->transactionView;
-  }
+  private $object;

  protected function buildProxy() {
    return new AphrontAjaxResponse();
@ -33,6 +24,15 @@ final class PhabricatorApplicationTransactionResponse
    return $this->transactions;
  }

+  public function setObject($object) {
+    $this->object = $object;
+    return $this;
+  }
+
+  public function getObject() {
+    return $this->object;
+  }
+
  public function setViewer(PhabricatorUser $viewer) {
    $this->viewer = $viewer;
    return $this;
@ -57,19 +57,17 @@ final class PhabricatorApplicationTransactionResponse
  }

  public function reduceProxyResponse() {
-    if ($this->transactionView) {
-      $view = $this->transactionView;
-    } else if ($this->getTransactions()) {
-      $view = head($this->getTransactions())
-        ->getApplicationTransactionViewObject();
-    } else {
-      $view = new PhabricatorApplicationTransactionView();
-    }
+    $object = $this->getObject();
+    $viewer = $this->getViewer();
+    $xactions = $this->getTransactions();

-    $view
-      ->setUser($this->getViewer())
-      ->setTransactions($this->getTransactions())
-      ->setIsPreview($this->isPreview);
+    $timeline_engine = PhabricatorTimelineEngine::newForObject($object)
+      ->setViewer($viewer)
+      ->setTransactions($xactions);
+
+    $view = $timeline_engine->buildTimelineView();
+
+    $view->setIsPreview($this->isPreview);

    if ($this->isPreview) {
      $xactions = mpull($view->buildEvents(), 'render');
--- a/src/applications/transactions/storage/PhabricatorApplicationTransaction.php
+++ b/src/applications/transactions/storage/PhabricatorApplicationTransaction.php
@ -79,10 +79,6 @@ abstract class PhabricatorApplicationTransaction
    throw new PhutilMethodNotImplementedException();
  }

-  public function getApplicationTransactionViewObject() {
-    return new PhabricatorApplicationTransactionView();
-  }
-
  public function getMetadataValue($key, $default = null) {
    return idx($this->metadata, $key, $default);
  }
--- a/src/applications/transactions/view/PhabricatorApplicationTransactionView.php
+++ b/src/applications/transactions/view/PhabricatorApplicationTransactionView.php
@ -15,8 +15,8 @@ class PhabricatorApplicationTransactionView extends AphrontView {
  private $quoteRef;
  private $pager;
  private $renderAsFeed;
-  private $renderData = array();
  private $hideCommentOptions = false;
+  private $viewData = array();

  public function setRenderAsFeed($feed) {
    $this->renderAsFeed = $feed;
@ -97,21 +97,6 @@ class PhabricatorApplicationTransactionView extends AphrontView {
    return $this->pager;
  }

-  /**
-   * This is additional data that may be necessary to render the next set
-   * of transactions. Objects that implement
-   * PhabricatorApplicationTransactionInterface use this data in
-   * willRenderTimeline.
-   */
-  public function setRenderData(array $data) {
-    $this->renderData = $data;
-    return $this;
-  }
-
-  public function getRenderData() {
-    return $this->renderData;
-  }
-
  public function setHideCommentOptions($hide_comment_options) {
    $this->hideCommentOptions = $hide_comment_options;
    return $this;
@ -121,6 +106,15 @@ class PhabricatorApplicationTransactionView extends AphrontView {
    return $this->hideCommentOptions;
  }

+  public function setViewData(array $view_data) {
+    $this->viewData = $view_data;
+    return $this;
+  }
+
+  public function getViewData() {
+    return $this->viewData;
+  }
+
  public function buildEvents($with_hiding = false) {
    $user = $this->getUser();

@ -216,10 +210,11 @@ class PhabricatorApplicationTransactionView extends AphrontView {
    }

    $view = id(new PHUITimelineView())
-      ->setUser($this->getUser())
+      ->setViewer($this->getViewer())
      ->setShouldTerminate($this->shouldTerminate)
      ->setQuoteTargetID($this->getQuoteTargetID())
-      ->setQuoteRef($this->getQuoteRef());
+      ->setQuoteRef($this->getQuoteRef())
+      ->setViewData($this->getViewData());

    $events = $this->buildEvents($with_hiding);
    foreach ($events as $event) {
@ -230,10 +225,6 @@ class PhabricatorApplicationTransactionView extends AphrontView {
      $view->setPager($this->getPager());
    }

-    if ($this->getRenderData()) {
-      $view->setRenderData($this->getRenderData());
-    }
-
    return $view;
  }

@ -246,7 +237,7 @@ class PhabricatorApplicationTransactionView extends AphrontView {
      $field = PhabricatorApplicationTransactionComment::MARKUP_FIELD_COMMENT;

      $engine = id(new PhabricatorMarkupEngine())
-        ->setViewer($this->getUser());
+        ->setViewer($this->getViewer());
      foreach ($this->transactions as $xaction) {
        if (!$xaction->hasComment()) {
          continue;
@ -414,10 +405,10 @@ class PhabricatorApplicationTransactionView extends AphrontView {
  private function renderEvent(
    PhabricatorApplicationTransaction $xaction,
    array $group) {
-    $viewer = $this->getUser();
+    $viewer = $this->getViewer();

    $event = id(new PHUITimelineEventView())
-      ->setUser($viewer)
+      ->setViewer($viewer)
      ->setAuthorPHID($xaction->getAuthorPHID())
      ->setTransactionPHID($xaction->getPHID())
      ->setUserHandle($xaction->getHandle($xaction->getAuthorPHID()))
--- a/src/view/phui/PHUITimelineView.php
+++ b/src/view/phui/PHUITimelineView.php
@ -7,7 +7,7 @@ final class PHUITimelineView extends AphrontView {
  private $shouldTerminate = false;
  private $shouldAddSpacers = true;
  private $pager;
-  private $renderData = array();
+  private $viewData = array();
  private $quoteTargetID;
  private $quoteRef;

@ -40,11 +40,15 @@ final class PHUITimelineView extends AphrontView {
    return $this;
  }

-  public function setRenderData(array $data) {
-    $this->renderData = $data;
+  public function setViewData(array $data) {
+    $this->viewData = $data;
    return $this;
  }

+  public function getViewData() {
+    return $this->viewData;
+  }
+
  public function setQuoteTargetID($quote_target_id) {
    $this->quoteTargetID = $quote_target_id;
    return $this;
@ -72,7 +76,7 @@ final class PHUITimelineView extends AphrontView {
        'phabricator-show-older-transactions',
        array(
          'timelineID' => $this->id,
-          'renderData' => $this->renderData,
+          'viewData' => $this->getViewData(),
        ));
    }
    $events = $this->buildEvents();
--- a/webroot/rsrc/js/application/transactions/behavior-show-older-transactions.js
+++ b/webroot/rsrc/js/application/transactions/behavior-show-older-transactions.js
@ -83,7 +83,11 @@ JX.behavior('phabricator-show-older-transactions', function(config) {
  };

  var fetch_older_workflow = function(href, callback, swap) {
-    return new JX.Workflow(href, config.renderData)
+    var params = {
+      viewData: JX.JSON.stringify(config.viewData)
+    };
+
+    return new JX.Workflow(href, params)
      .setHandler(JX.bind(null, callback, swap));
  };