From 18938b5310712b1609b8e0b1bfc84b1edab8eeaf Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Tue, 11 Feb 2014 15:36:58 -0800
Subject: [PATCH 01/11] Migrate Differential comments to
 ApplicationTransactions

Summary:
Ref T2222. This is the big one.

This migrates each `DifferentialComment` to one or more ApplicationTransactions (action, cc, reviewers, update, comment, inlines), and makes `DifferentialComment` a double-reader for ApplicationTransactions.

The migration is pretty straightforward:

  - If a comment took an action not otherwise covered, it gets an "action" transaction. This is something like "epriestley abandoned this revision.".
  - If a comment updated the diff, it gets an "updated diff" transaction. Very old transactions of this type may not have a diff ID (probably only at Facebook).
  - If a comment added or removed reviewers, it gets a "changed reviewers" transaction.
  - If a comment added CCs, it gets a "subscribers" transaction.
  - If a comment added comment text, it gets a "comment" transaction.
  - For each inline attached to a comment, we generate an "inline" transaction.

Most comments generate a small number of transactions, but a few generate a significant number.

At HEAD, the code is basically already doing this, so comments in the last day or two already obey these rules, roughly, and will all generate only one transaction (except inlines).

Because we've already preallocated PHIDs in the comment text table, we only need to write to the transaction table.

NOTE: This significantly degrades Differential, making inline comments pretty much useless (they each get their own transaction, and don't show line numbers or files). The data is all fine, but the UI is garbage now. This needs to be fixed before we can deploy this to users, but it's easily separable since it's all just display code.

Specifically, they look like this:

{F112270}

Test Plan:
I've migrated locally and put things through their paces, but it's hard to catch sketchy stuff locally because most of my test data is nonsense and bad migrations wouldn't necessarily look out of place.
IMPORTANT: I'm planning to push this to a branch and then shift production over to the branch, and run it for a day or two before bringing it to master.

I generally feel good about this change: it's not that big since we were able to separate a lot of pieces out of it, and it's pretty straightforward. That said, it's still one of the most scary/dangerous changes we've ever made.

Reviewers: btrahan

CC: chad, aran

Maniphest Tasks: T2222

Differential Revision: https://secure.phabricator.com/D8210
---
 .../autopatches/20140212.dx.1.armageddon.php  | 222 ++++++++++++
 src/__phutil_library_map__.php                |   8 +-
 ...ifferential_getrevisioncomments_Method.php |  60 ++--
 .../constants/DifferentialAction.php          |   5 +
 .../editor/DifferentialCommentEditor.php      |  19 +-
 .../query/DifferentialCommentQuery.php        |  66 +---
 .../query/DifferentialTransactionQuery.php    |  10 +
 .../search/DifferentialSearchIndexer.php      |   2 +-
 .../storage/DifferentialComment.php           | 336 ++++++++++++++----
 .../storage/DifferentialInlineComment.php     |  14 +-
 .../storage/DifferentialRevision.php          |   4 +-
 .../storage/DifferentialTransaction.php       |   4 +
 .../DifferentialRevisionCommentListView.php   |   4 +-
 .../ReleephDiffChurnFieldSpecification.php    |   2 +-
 14 files changed, 574 insertions(+), 182 deletions(-)
 create mode 100644 resources/sql/autopatches/20140212.dx.1.armageddon.php
 create mode 100644 src/applications/differential/query/DifferentialTransactionQuery.php

diff --git a/resources/sql/autopatches/20140212.dx.1.armageddon.php b/resources/sql/autopatches/20140212.dx.1.armageddon.php
new file mode 100644
index 0000000000..77d9518121
--- /dev/null
+++ b/resources/sql/autopatches/20140212.dx.1.armageddon.php
@@ -0,0 +1,222 @@
+<?php
+
+$conn_w = id(new DifferentialRevision())->establishConnection('w');
+$rows = new LiskRawMigrationIterator($conn_w, 'differential_comment');
+
+$content_source = PhabricatorContentSource::newForSource(
+  PhabricatorContentSource::SOURCE_LEGACY,
+  array())->serialize();
+
+echo "Migrating Differential comments to modern storage...\n";
+foreach ($rows as $row) {
+  $id = $row['id'];
+  echo "Migrating comment {$id}...\n";
+
+  $revision = id(new DifferentialRevision())->load($row['revisionID']);
+  if (!$revision) {
+    echo "No revision, continuing.\n";
+    continue;
+  }
+
+  $revision_phid = $revision->getPHID();
+
+  $comments = queryfx_all(
+    $conn_w,
+    'SELECT * FROM %T WHERE legacyCommentID = %d',
+    'differential_transaction_comment',
+    $id);
+
+  $main_comments = array();
+  $inline_comments = array();
+
+  foreach ($comments as $comment) {
+    if ($comment['changesetID']) {
+      $inline_comments[] = $comment;
+    } else {
+      $main_comments[] = $comment;
+    }
+  }
+
+  $metadata = json_decode($row['metadata'], true);
+  if (!is_array($metadata)) {
+    $metadata = array();
+  }
+
+  $key_cc = DifferentialComment::METADATA_ADDED_CCS;
+  $key_add_rev = DifferentialComment::METADATA_ADDED_REVIEWERS;
+  $key_rem_rev = DifferentialComment::METADATA_REMOVED_REVIEWERS;
+  $key_diff_id = DifferentialComment::METADATA_DIFF_ID;
+
+  $xactions = array();
+
+  // Build the main action transaction.
+  switch ($row['action']) {
+    case DifferentialAction::ACTION_COMMENT:
+    case DifferentialAction::ACTION_ADDREVIEWERS:
+    case DifferentialAction::ACTION_ADDCCS:
+    case DifferentialAction::ACTION_UPDATE:
+    case DifferentialTransaction::TYPE_INLINE:
+      // These actions will have their transactions created by other rules.
+      break;
+    default:
+      // Otherwise, this is a normal action (like an accept or reject).
+      $xactions[] = array(
+        'type' => DifferentialTransaction::TYPE_ACTION,
+        'old' => null,
+        'new' => $row['action'],
+      );
+      break;
+  }
+
+  // Build the diff update transaction, if one exists.
+  $diff_id = idx($metadata, $key_diff_id);
+  if (!is_scalar($diff_id)) {
+    $diff_id = null;
+  }
+
+  if ($diff_id || $row['action'] == DifferentialAction::ACTION_UPDATE) {
+    $xactions[] = array(
+      'type' => DifferentialTransaction::TYPE_UPDATE,
+      'old' => null,
+      'new' => $diff_id,
+    );
+  }
+
+  // Build the add/remove reviewers transaction, if one exists.
+  $add_rev = idx($metadata, $key_add_rev, array());
+  if (!is_array($add_rev)) {
+    $add_rev = array();
+  }
+  $rem_rev = idx($metadata, $key_rem_rev, array());
+  if (!is_array($rem_rev)) {
+    $rem_rev = array();
+  }
+
+  if ($add_rev || $rem_rev) {
+    $old = array();
+    foreach ($rem_rev as $phid) {
+      if (!is_scalar($phid)) {
+        continue;
+      }
+      $old[$phid] = array(
+        'src' => $revision_phid,
+        'type' => PhabricatorEdgeConfig::TYPE_DREV_HAS_REVIEWER,
+        'dst' => $phid,
+      );
+    }
+
+    $new = array();
+    foreach ($add_rev as $phid) {
+      if (!is_scalar($phid)) {
+        continue;
+      }
+      $new[$phid] = array(
+        'src' => $revision_phid,
+        'type' => PhabricatorEdgeConfig::TYPE_DREV_HAS_REVIEWER,
+        'dst' => $phid,
+      );
+    }
+
+    $xactions[] = array(
+      'type' => PhabricatorTransactions::TYPE_EDGE,
+      'old' => $old,
+      'new' => $new,
+      'meta' => array(
+        'edge:type' => PhabricatorEdgeConfig::TYPE_DREV_HAS_REVIEWER,
+      ),
+    );
+  }
+
+  // Build the CC transaction, if one exists.
+  $add_cc = idx($metadata, $key_cc, array());
+  if (!is_array($add_cc)) {
+    $add_cc = array();
+  }
+
+  if ($add_cc) {
+    $xactions[] = array(
+      'type' => PhabricatorTransactions::TYPE_SUBSCRIBERS,
+      'old' => array(),
+      'new' => array_fuse($add_cc),
+    );
+  }
+
+
+  // Build the main comment transaction.
+  foreach ($main_comments as $main) {
+    $xactions[] = array(
+      'type' => PhabricatorTransactions::TYPE_COMMENT,
+      'old' => null,
+      'new' => null,
+      'phid' => $main['transactionPHID'],
+      'comment' => $main,
+    );
+  }
+
+  // Build inline comment transactions.
+  foreach ($inline_comments as $inline) {
+    $xactions[] = array(
+      'type' => DifferentialTransaction::TYPE_INLINE,
+      'old' => null,
+      'new' => null,
+      'phid' => $inline['transactionPHID'],
+      'comment' => $inline,
+    );
+  }
+
+  foreach ($xactions as $xaction) {
+    // Generate a new PHID, if we don't already have one from the comment
+    // table. We pregenerated into the comment table to make this a little
+    // easier, so we only need to write to one table.
+    $xaction_phid = idx($xaction, 'phid');
+    if (!$xaction_phid) {
+      $xaction_phid = PhabricatorPHID::generateNewPHID(
+        PhabricatorApplicationTransactionPHIDTypeTransaction::TYPECONST,
+        DifferentialPHIDTypeRevision::TYPECONST);
+    }
+    unset($xaction['phid']);
+
+    $comment_phid = null;
+    $comment_version = 0;
+    if (idx($xaction, 'comment')) {
+      $comment_phid = $xaction['comment']['phid'];
+      $comment_version = 1;
+    }
+
+    $old = idx($xaction, 'old');
+    $new = idx($xaction, 'new');
+    $meta = idx($xaction, 'meta', array());
+
+    queryfx(
+      $conn_w,
+      'INSERT INTO %T (phid, authorPHID, objectPHID, viewPolicy, editPolicy,
+          commentPHID, commentVersion, transactionType, oldValue, newValue,
+          contentSource, metadata, dateCreated, dateModified)
+        VALUES (%s, %s, %s, %s, %s, %ns, %d, %s, %ns, %ns, %s, %s, %d, %d)',
+      'differential_transaction',
+
+      // PHID, authorPHID, objectPHID
+      $xaction_phid,
+      (string)$row['authorPHID'],
+      $revision_phid,
+
+      // viewPolicy, editPolicy, commentPHID, commentVersion
+      'public',
+      (string)$row['authorPHID'],
+      $comment_phid,
+      $comment_version,
+
+      // transactionType, oldValue, newValue, contentSource, metadata
+      $xaction['type'],
+      json_encode($old),
+      json_encode($new),
+      $content_source,
+      json_encode($meta),
+
+      // dates
+      $row['dateCreated'],
+      $row['dateModified']);
+  }
+
+}
+echo "Done.\n";
diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
index 0ac7ce966c..e5fd4b4590 100644
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -464,6 +464,7 @@ phutil_register_library_map(array(
     'DifferentialTitleFieldSpecification' => 'applications/differential/field/specification/DifferentialTitleFieldSpecification.php',
     'DifferentialTransaction' => 'applications/differential/storage/DifferentialTransaction.php',
     'DifferentialTransactionComment' => 'applications/differential/storage/DifferentialTransactionComment.php',
+    'DifferentialTransactionQuery' => 'applications/differential/query/DifferentialTransactionQuery.php',
     'DifferentialUnitFieldSpecification' => 'applications/differential/field/specification/DifferentialUnitFieldSpecification.php',
     'DifferentialUnitStatus' => 'applications/differential/constants/DifferentialUnitStatus.php',
     'DifferentialUnitTestResult' => 'applications/differential/constants/DifferentialUnitTestResult.php',
@@ -2856,11 +2857,7 @@ phutil_register_library_map(array(
     'DifferentialChangesetTwoUpRenderer' => 'DifferentialChangesetHTMLRenderer',
     'DifferentialChangesetTwoUpTestRenderer' => 'DifferentialChangesetTestRenderer',
     'DifferentialChangesetViewController' => 'DifferentialController',
-    'DifferentialComment' =>
-    array(
-      0 => 'DifferentialDAO',
-      1 => 'PhabricatorMarkupInterface',
-    ),
+    'DifferentialComment' => 'PhabricatorMarkupInterface',
     'DifferentialCommentEditor' => 'PhabricatorEditor',
     'DifferentialCommentMail' => 'DifferentialMail',
     'DifferentialCommentPreviewController' => 'DifferentialController',
@@ -2981,6 +2978,7 @@ phutil_register_library_map(array(
     'DifferentialTitleFieldSpecification' => 'DifferentialFreeformFieldSpecification',
     'DifferentialTransaction' => 'PhabricatorApplicationTransaction',
     'DifferentialTransactionComment' => 'PhabricatorApplicationTransactionComment',
+    'DifferentialTransactionQuery' => 'PhabricatorApplicationTransactionQuery',
     'DifferentialUnitFieldSpecification' => 'DifferentialFieldSpecification',
     'DifferentialViewPolicyFieldSpecification' => 'DifferentialFieldSpecification',
     'DiffusionBranchTableController' => 'DiffusionController',
diff --git a/src/applications/differential/conduit/ConduitAPI_differential_getrevisioncomments_Method.php b/src/applications/differential/conduit/ConduitAPI_differential_getrevisioncomments_Method.php
index ee1531add0..140429d4ec 100644
--- a/src/applications/differential/conduit/ConduitAPI_differential_getrevisioncomments_Method.php
+++ b/src/applications/differential/conduit/ConduitAPI_differential_getrevisioncomments_Method.php
@@ -1,11 +1,16 @@
 <?php
 
-/**
- * @group conduit
- */
 final class ConduitAPI_differential_getrevisioncomments_Method
   extends ConduitAPI_differential_Method {
 
+  public function getMethodStatus() {
+    return self::METHOD_STATUS_DEPRECATED;
+  }
+
+  public function getMethodStatusDescription() {
+    return pht('Obsolete and doomed, see T2222.');
+  }
+
   public function getMethodDescription() {
     return "Retrieve Differential Revision Comments.";
   }
@@ -13,7 +18,7 @@ final class ConduitAPI_differential_getrevisioncomments_Method
   public function defineParamTypes() {
     return array(
       'ids' => 'required list<int>',
-      'inlines' => 'optional bool',
+      'inlines' => 'optional bool (deprecated)',
     );
   }
 
@@ -34,47 +39,36 @@ final class ConduitAPI_differential_getrevisioncomments_Method
       return $results;
     }
 
-    $comments = id(new DifferentialCommentQuery())
-      ->withRevisionIDs($revision_ids)
+    $revisions = id(new DifferentialRevisionQuery())
+      ->setViewer($request->getUser())
+      ->withIDs($revision_ids)
       ->execute();
 
-    $with_inlines = $request->getValue('inlines');
-    if ($with_inlines) {
-      $inlines = id(new DifferentialInlineCommentQuery())
-        ->withRevisionIDs($revision_ids)
-        ->execute();
-      $changesets = array();
-      if ($inlines) {
-        $changesets = id(new DifferentialChangeset())->loadAllWhere(
-          'id IN (%Ld)',
-          array_unique(mpull($inlines, 'getChangesetID')));
-        $inlines = mgroup($inlines, 'getCommentID');
-      }
+    if (!$revisions) {
+      return $results;
     }
 
+    $comments = id(new DifferentialCommentQuery())
+      ->withRevisionPHIDs(mpull($revisions, 'getPHID'))
+      ->execute();
+
+    $revisions = mpull($revisions, null, 'getPHID');
+
     foreach ($comments as $comment) {
-      // TODO: Sort this out in the ID -> PHID change.
-      $revision_id = $comment->getRevisionID();
+      $revision = idx($revisions, $comment->getRevisionPHID());
+      if (!$revision) {
+        continue;
+      }
+
       $result = array(
-        'revisionID'  => $revision_id,
+        'revisionID'  => $revision->getID(),
         'action'      => $comment->getAction(),
         'authorPHID'  => $comment->getAuthorPHID(),
         'dateCreated' => $comment->getDateCreated(),
         'content'     => $comment->getContent(),
       );
 
-      if ($with_inlines) {
-        $result['inlines'] = array();
-        foreach (idx($inlines, $comment->getID(), array()) as $inline) {
-          $changeset = idx($changesets, $inline->getChangesetID());
-          $result['inlines'][] = $this->buildInlineInfoDictionary(
-            $inline,
-            $changeset);
-        }
-        // TODO: Put synthetic inlines without an attached comment somewhere.
-      }
-
-      $results[$revision_id][] = $result;
+      $results[$revision->getID()][] = $result;
     }
 
     return $results;
diff --git a/src/applications/differential/constants/DifferentialAction.php b/src/applications/differential/constants/DifferentialAction.php
index f79c619793..9b37c3995b 100644
--- a/src/applications/differential/constants/DifferentialAction.php
+++ b/src/applications/differential/constants/DifferentialAction.php
@@ -90,6 +90,11 @@ final class DifferentialAction {
           $title = pht('%s reopened this revision.',
             $author_name);
         break;
+        case DifferentialTransaction::TYPE_INLINE:
+          $title = pht(
+            '%s added an inline comment.',
+            $author_name);
+          break;
         default:
           $title = pht('Ghosts happened to this revision.');
         break;
diff --git a/src/applications/differential/editor/DifferentialCommentEditor.php b/src/applications/differential/editor/DifferentialCommentEditor.php
index 8dac959187..204a7a65b8 100644
--- a/src/applications/differential/editor/DifferentialCommentEditor.php
+++ b/src/applications/differential/editor/DifferentialCommentEditor.php
@@ -624,7 +624,7 @@ final class DifferentialCommentEditor extends PhabricatorEditor {
 
     // If this edit has comments or inline comments, save a transaction for
     // the comment content.
-    if (strlen($this->message) || $inline_comments) {
+    if (strlen($this->message)) {
       $comments[] = id(clone $template)
         ->setAction(DifferentialAction::ACTION_COMMENT)
         ->setContent((string)$this->message);
@@ -634,8 +634,6 @@ final class DifferentialCommentEditor extends PhabricatorEditor {
       $comment->save();
     }
 
-    $last_comment = last($comments);
-
     $changesets = array();
     if ($inline_comments) {
       $load_ids = mpull($inline_comments, 'getChangesetID');
@@ -646,12 +644,13 @@ final class DifferentialCommentEditor extends PhabricatorEditor {
           $load_ids);
       }
       foreach ($inline_comments as $inline) {
-        // For now, attach inlines to the last comment. We'll eventually give
-        // them their own transactions, but this would be fairly gross during
-        // the storage transition and we'll have to do special thing with these
-        // during migration anyway.
-        $inline->setCommentID($last_comment->getID());
-        $inline->save();
+        $inline_xaction_comment = $inline->getTransactionCommentForSave();
+        $inline_xaction_comment->setRevisionPHID($revision->getPHID());
+
+        $comments[] = id(clone $template)
+          ->setAction(DifferentialTransaction::TYPE_INLINE)
+          ->setProxyComment($inline_xaction_comment)
+          ->save();
       }
     }
 
@@ -702,7 +701,7 @@ final class DifferentialCommentEditor extends PhabricatorEditor {
 
       // NOTE: Don't use this, it will be removed after ApplicationTransactions.
       // For now, it powers inline comment rendering over the Asana brdige.
-      'temporaryCommentID'   => $last_comment->getID(),
+      'temporaryTransactionPHIDs' => mpull($comments, 'getPHID'),
     );
 
     id(new PhabricatorFeedStoryPublisher())
diff --git a/src/applications/differential/query/DifferentialCommentQuery.php b/src/applications/differential/query/DifferentialCommentQuery.php
index 3c61cc2345..3926a5ca14 100644
--- a/src/applications/differential/query/DifferentialCommentQuery.php
+++ b/src/applications/differential/query/DifferentialCommentQuery.php
@@ -6,67 +6,29 @@
 final class DifferentialCommentQuery
   extends PhabricatorOffsetPagedQuery {
 
-  private $revisionIDs;
+  private $revisionPHIDs;
 
-  public function withRevisionIDs(array $ids) {
-    $this->revisionIDs = $ids;
+  public function withRevisionPHIDs(array $phids) {
+    $this->revisionPHIDs = $phids;
     return $this;
   }
 
   public function execute() {
-    $table = new DifferentialComment();
-    $conn_r = $table->establishConnection('r');
+    // TODO: We're getting rid of this, it is the bads.
+    $viewer = PhabricatorUser::getOmnipotentUser();
 
-    $data = queryfx_all(
-      $conn_r,
-      'SELECT * FROM %T %Q %Q',
-      $table->getTableName(),
-      $this->buildWhereClause($conn_r),
-      $this->buildLimitClause($conn_r));
+    $xactions = id(new DifferentialTransactionQuery())
+      ->setViewer($viewer)
+      ->withObjectPHIDs($this->revisionPHIDs)
+      ->needComments(true)
+      ->execute();
 
-    $comments = $table->loadAllFromArray($data);
-
-    // We've moved the actual text storage into DifferentialTransactionComment,
-    // so load the relevant pieces of text we need.
-    if ($comments) {
-      $this->loadCommentText($comments);
+    $results = array();
+    foreach ($xactions as $xaction) {
+      $results[] = DifferentialComment::newFromModernTransaction($xaction);
     }
 
-    return $comments;
+    return $results;
   }
 
-  private function buildWhereClause(AphrontDatabaseConnection $conn_r) {
-    $where = array();
-
-    if ($this->revisionIDs) {
-      $where[] = qsprintf(
-        $conn_r,
-        'revisionID IN (%Ld)',
-        $this->revisionIDs);
-    }
-
-    return $this->formatWhereClause($where);
-  }
-
-  private function loadCommentText(array $comments) {
-    $table = new DifferentialTransactionComment();
-    $conn_r = $table->establishConnection('r');
-
-    $data = queryfx_all(
-      $conn_r,
-      'SELECT * FROM %T WHERE legacyCommentID IN (%Ld) AND changesetID IS NULL',
-      $table->getTableName(),
-      mpull($comments, 'getID'));
-    $texts = $table->loadAllFromArray($data);
-    $texts = mpull($texts, null, 'getLegacyCommentID');
-
-    foreach ($comments as $comment) {
-      $text = idx($texts, $comment->getID());
-      if ($text) {
-        $comment->setProxyComment($text);
-      }
-    }
-  }
-
-
 }
diff --git a/src/applications/differential/query/DifferentialTransactionQuery.php b/src/applications/differential/query/DifferentialTransactionQuery.php
new file mode 100644
index 0000000000..d413782301
--- /dev/null
+++ b/src/applications/differential/query/DifferentialTransactionQuery.php
@@ -0,0 +1,10 @@
+<?php
+
+final class DifferentialTransactionQuery
+  extends PhabricatorApplicationTransactionQuery {
+
+  public function getTemplateApplicationTransaction() {
+    return new DifferentialTransaction();
+  }
+
+}
diff --git a/src/applications/differential/search/DifferentialSearchIndexer.php b/src/applications/differential/search/DifferentialSearchIndexer.php
index b07c6b6263..2dee73de54 100644
--- a/src/applications/differential/search/DifferentialSearchIndexer.php
+++ b/src/applications/differential/search/DifferentialSearchIndexer.php
@@ -53,7 +53,7 @@ final class DifferentialSearchIndexer
       time());
 
     $comments = id(new DifferentialCommentQuery())
-      ->withRevisionIDs(array($rev->getID()))
+      ->withRevisionPHIDs(array($rev->getPHID()))
       ->execute();
 
     $inlines = id(new DifferentialInlineCommentQuery())
diff --git a/src/applications/differential/storage/DifferentialComment.php b/src/applications/differential/storage/DifferentialComment.php
index e5ee912957..25a2c9662c 100644
--- a/src/applications/differential/storage/DifferentialComment.php
+++ b/src/applications/differential/storage/DifferentialComment.php
@@ -1,6 +1,9 @@
 <?php
 
-final class DifferentialComment extends DifferentialDAO
+/**
+ * Temporary proxy shell around ApplicationTransactions.
+ */
+final class DifferentialComment
   implements PhabricatorMarkupInterface {
 
   const METADATA_ADDED_REVIEWERS   = 'added-reviewers';
@@ -10,36 +13,244 @@ final class DifferentialComment extends DifferentialDAO
 
   const MARKUP_FIELD_BODY          = 'markup:body';
 
-  protected $authorPHID;
-  protected $revisionID;
-  protected $action;
-  protected $content = '';
-  protected $cache;
-  protected $metadata = array();
-  protected $contentSource;
-
   private $arbitraryDiffForFacebook;
   private $proxyComment;
+  private $proxy;
+
+  public function __construct() {
+    $this->proxy = new DifferentialTransaction();
+  }
 
   public function __clone() {
+    $this->proxy = clone $this->proxy;
     if ($this->proxyComment) {
       $this->proxyComment = clone $this->proxyComment;
     }
   }
 
+  public static function newFromModernTransaction(
+    DifferentialTransaction $xaction) {
+
+    $obj = new DifferentialComment();
+    $obj->proxy = $xaction;
+
+    if ($xaction->hasComment()) {
+      $obj->proxyComment = $xaction->getComment();
+    }
+
+    return $obj;
+  }
+
+  public function getPHID() {
+    return $this->proxy->getPHID();
+  }
+
   public function getContent() {
     return $this->getProxyComment()->getContent();
   }
 
   public function setContent($content) {
-    // NOTE: We no longer read this field, but there's no cost to continuing
-    // to write it in case something goes horribly wrong, since it makes it
-    // far easier to back out of this.
-    $this->content = $content;
     $this->getProxyComment()->setContent($content);
     return $this;
   }
 
+  public function getAuthorPHID() {
+    return $this->proxy->getAuthorPHID();
+  }
+
+  public function setAuthorPHID($author_phid) {
+    $this->proxy->setAuthorPHID($author_phid);
+    return $this;
+  }
+
+  public function setContentSource($content_source) {
+    $this->proxy->setContentSource($content_source);
+    $this->proxyComment->setContentSource($content_source);
+    return $this;
+  }
+
+  public function setAction($action) {
+    $meta = array();
+    switch ($action) {
+      case DifferentialAction::ACTION_COMMENT:
+        $type = PhabricatorTransactions::TYPE_COMMENT;
+        $old = null;
+        $new = null;
+        break;
+      case DifferentialAction::ACTION_ADDREVIEWERS:
+        $type = PhabricatorTransactions::TYPE_EDGE;
+        $old = array();
+        $new = array();
+        $meta = array(
+          'edge:type' => PhabricatorEdgeConfig::TYPE_DREV_HAS_REVIEWER,
+        );
+        break;
+      case DifferentialAction::ACTION_ADDCCS:
+        $type = PhabricatorTransactions::TYPE_SUBSCRIBERS;
+        $old = array();
+        $new = array();
+        break;
+      case DifferentialAction::ACTION_UPDATE:
+        $type = DifferentialTransaction::TYPE_UPDATE;
+        $old = null;
+        $new = null;
+        break;
+      case DifferentialTransaction::TYPE_INLINE:
+        $type = $action;
+        $old = null;
+        $new = null;
+        break;
+      default:
+        $type = DifferentialTransaction::TYPE_ACTION;
+        $old = null;
+        $new = $action;
+        break;
+    }
+
+    $xaction = $this->proxy;
+
+    $xaction
+      ->setTransactionType($type)
+      ->setOldValue($old)
+      ->setNewValue($new);
+
+    if ($meta) {
+      foreach ($meta as $key => $value) {
+        $xaction->setMetadataValue($key, $value);
+      }
+    }
+
+    return $this;
+  }
+
+  public function getAction() {
+    switch ($this->proxy->getTransactionType()) {
+      case PhabricatorTransactions::TYPE_SUBSCRIBERS:
+        return DifferentialAction::ACTION_ADDCCS;
+      case DifferentialTransaction::TYPE_UPDATE:
+        return DifferentialAction::ACTION_UPDATE;
+      case PhabricatorTransactions::TYPE_EDGE:
+        return DifferentialAction::ACTION_ADDREVIEWERS;
+      case PhabricatorTransactions::TYPE_COMMENT:
+        return DifferentialAction::ACTION_COMMENT;
+      case DifferentialTransaction::TYPE_INLINE:
+        return DifferentialTransaction::TYPE_INLINE;
+      default:
+        return $this->proxy->getNewValue();
+    }
+  }
+
+  public function setMetadata(array $metadata) {
+    if (!$this->proxy->getTransactionType()) {
+      throw new Exception(pht('Call setAction() before setMetadata()!'));
+    }
+
+    $key_cc = self::METADATA_ADDED_CCS;
+    $key_add_rev = self::METADATA_ADDED_REVIEWERS;
+    $key_rem_rev = self::METADATA_REMOVED_REVIEWERS;
+    $key_diff_id = self::METADATA_DIFF_ID;
+
+    switch ($this->proxy->getTransactionType()) {
+      case DifferentialTransaction::TYPE_UPDATE:
+        $id = idx($metadata, $key_diff_id);
+        $this->proxy->setNewValue($id);
+        break;
+      case PhabricatorTransactions::TYPE_EDGE:
+        $rem = idx($metadata, $key_rem_rev, array());
+        $old = array();
+        foreach ($rem as $phid) {
+          $old[$phid] = array(
+            'src' => $this->proxy->getObjectPHID(),
+            'type' => PhabricatorEdgeConfig::TYPE_DREV_HAS_REVIEWER,
+            'dst' => $phid,
+          );
+        }
+        $this->proxy->setOldValue($old);
+
+        $add = idx($metadata, $key_add_rev, array());
+        $new = array();
+        foreach ($add as $phid) {
+          $new[$phid] = array(
+            'src' => $this->proxy->getObjectPHID(),
+            'type' => PhabricatorEdgeConfig::TYPE_DREV_HAS_REVIEWER,
+            'dst' => $phid,
+          );
+        }
+        $this->proxy->setNewValue($new);
+        break;
+      case PhabricatorTransactions::TYPE_SUBSCRIBERS:
+        $phids = idx($metadata, $key_cc, array());
+        $new = array();
+        foreach ($phids as $phid) {
+          $new[$phid] = $phid;
+        }
+        $this->proxy->setNewValue($new);
+        break;
+      default:
+        break;
+    }
+
+    return $this;
+  }
+
+  public function getMetadata() {
+    if (!$this->proxy->getTransactionType()) {
+      throw new Exception(pht('Call setAction() before getMetadata()!'));
+    }
+
+    $key_cc = self::METADATA_ADDED_CCS;
+    $key_add_rev = self::METADATA_ADDED_REVIEWERS;
+    $key_rem_rev = self::METADATA_REMOVED_REVIEWERS;
+    $key_diff_id = self::METADATA_DIFF_ID;
+
+    $type = $this->proxy->getTransactionType();
+
+    switch ($type) {
+      case PhabricatorTransactions::TYPE_SUBSCRIBERS:
+        $value = $this->proxy->getNewValue();
+        if (!$value) {
+          $value = array();
+        }
+        return array(
+          $key_cc => $value,
+        );
+      case DifferentialTransaction::TYPE_UPDATE:
+        return array(
+          $key_diff_id => $this->proxy->getNewValue(),
+        );
+      case PhabricatorTransactions::TYPE_EDGE:
+      case PhabricatorTransactions::TYPE_SUBSCRIBERS:
+        $old = $this->proxy->getOldValue();
+        if (!$old) {
+          $old = array();
+        }
+        $new = $this->proxy->getNewValue();
+        if (!$new) {
+          $new = array();
+        }
+
+        $rem = array_diff_key($old, $new);
+        $add = array_diff_key($new, $old);
+
+        if ($type == PhabricatorTransactions::TYPE_EDGE) {
+          return array(
+            $key_add_rev => array_keys($add),
+            $key_rem_rev => array_keys($rem),
+          );
+        } else {
+          return array(
+            $key_cc => array_keys($add),
+          );
+        }
+      default:
+        return array();
+    }
+  }
+
+  public function getContentSource() {
+    return $this->proxy->getContentSource();
+  }
+
   private function getProxyComment() {
     if (!$this->proxyComment) {
       $this->proxyComment = new DifferentialTransactionComment();
@@ -48,16 +259,14 @@ final class DifferentialComment extends DifferentialDAO
   }
 
   public function setProxyComment(DifferentialTransactionComment $proxy) {
-    if ($this->proxyComment) {
-      throw new Exception(pht('You can not overwrite a proxy comment.'));
-    }
     $this->proxyComment = $proxy;
     return $this;
   }
 
   public function setRevision(DifferentialRevision $revision) {
     $this->getProxyComment()->setRevisionPHID($revision->getPHID());
-    return $this->setRevisionID($revision->getID());
+    $this->proxy->setObjectPHID($revision->getPHID());
+    return $this;
   }
 
   public function giveFacebookSomeArbitraryDiff(DifferentialDiff $diff) {
@@ -66,48 +275,19 @@ final class DifferentialComment extends DifferentialDAO
   }
 
   public function getRequiredHandlePHIDs() {
-    $phids = array();
-
-    $metadata = $this->getMetadata();
-    $added_reviewers = idx(
-      $metadata,
-      self::METADATA_ADDED_REVIEWERS);
-    if ($added_reviewers) {
-      foreach ($added_reviewers as $phid) {
-        $phids[] = $phid;
-      }
-    }
-    $added_ccs = idx(
-      $metadata,
-      self::METADATA_ADDED_CCS);
-    if ($added_ccs) {
-      foreach ($added_ccs as $phid) {
-        $phids[] = $phid;
-      }
+    switch ($this->proxy->getTransactionType()) {
+      case PhabricatorTransactions::TYPE_SUBSCRIBERS:
+      case PhabricatorTransactions::TYPE_EDGE:
+        return array_merge(
+          array_keys($this->proxy->getOldValue()),
+          array_keys($this->proxy->getNewValue()));
     }
 
-    return $phids;
-  }
-
-  public function getConfiguration() {
-    return array(
-      self::CONFIG_SERIALIZATION => array(
-        'metadata' => self::SERIALIZATION_JSON,
-      ),
-    ) + parent::getConfiguration();
-  }
-
-  public function setContentSource(PhabricatorContentSource $content_source) {
-    $this->contentSource = $content_source->serialize();
-    return $this;
-  }
-
-  public function getContentSource() {
-    return PhabricatorContentSource::newFromSerialized($this->contentSource);
+    return array();
   }
 
   public function getMarkupFieldKey($field) {
-    return 'DC:'.$this->getID();
+    return 'DC:'.$this->getPHID();
   }
 
   public function newMarkupEngine($field) {
@@ -126,37 +306,49 @@ final class DifferentialComment extends DifferentialDAO
   }
 
   public function shouldUseMarkupCache($field) {
-    return (bool)$this->getID();
+    return (bool)$this->getPHID();
+  }
+
+  public function getDateCreated() {
+    return $this->proxy->getDateCreated();
+  }
+
+  public function getRevisionPHID() {
+    return $this->proxy->getObjectPHID();
   }
 
   public function save() {
-    $this->openTransaction();
-      $result = parent::save();
+    $this->proxy->openTransaction();
+      $this->proxy
+        ->setViewPolicy('public')
+        ->setEditPolicy($this->getAuthorPHID())
+        ->save();
 
-      if ($this->getContent() !== null) {
-        $content_source = PhabricatorContentSource::newForSource(
-          PhabricatorContentSource::SOURCE_LEGACY,
-          array());
+      if ($this->getContent() !== null ||
+          $this->getProxyComment()->getChangesetID()) {
 
-        $xaction_phid = PhabricatorPHID::generateNewPHID(
-          PhabricatorApplicationTransactionPHIDTypeTransaction::TYPECONST,
-          DifferentialPHIDTypeRevision::TYPECONST);
-
-        $proxy = $this->getProxyComment();
-        $proxy
+        $this->getProxyComment()
           ->setAuthorPHID($this->getAuthorPHID())
           ->setViewPolicy('public')
           ->setEditPolicy($this->getAuthorPHID())
-          ->setContentSource($content_source)
           ->setCommentVersion(1)
-          ->setLegacyCommentID($this->getID())
-          ->setTransactionPHID($xaction_phid)
+          ->setTransactionPHID($this->proxy->getPHID())
+          ->save();
+
+        $this->proxy
+          ->setCommentVersion(1)
+          ->setCommentPHID($this->getProxyComment()->getPHID())
           ->save();
       }
 
-    $this->saveTransaction();
+    $this->proxy->saveTransaction();
 
-    return $result;
+    return $this;
+  }
+
+  public function delete() {
+    $this->proxy->delete();
+    return $this;
   }
 
 }
diff --git a/src/applications/differential/storage/DifferentialInlineComment.php b/src/applications/differential/storage/DifferentialInlineComment.php
index af30563776..4e7dcf47a7 100644
--- a/src/applications/differential/storage/DifferentialInlineComment.php
+++ b/src/applications/differential/storage/DifferentialInlineComment.php
@@ -14,7 +14,7 @@ final class DifferentialInlineComment
     $this->proxy = clone $this->proxy;
   }
 
-  public function save() {
+  public function getTransactionCommentForSave() {
     $content_source = PhabricatorContentSource::newForSource(
       PhabricatorContentSource::SOURCE_LEGACY,
       array());
@@ -23,8 +23,14 @@ final class DifferentialInlineComment
       ->setViewPolicy('public')
       ->setEditPolicy($this->getAuthorPHID())
       ->setContentSource($content_source)
-      ->setCommentVersion(1)
-      ->save();
+      ->setCommentVersion(1);
+
+    return $this->proxy;
+  }
+
+
+  public function save() {
+    $this->getTransactionCommentForSave()->save();
 
     return $this;
   }
@@ -74,7 +80,7 @@ final class DifferentialInlineComment
   }
 
   public function isDraft() {
-    return !$this->getCommentID();
+    return !$this->proxy->getTransactionPHID();
   }
 
   public function setChangesetID($id) {
diff --git a/src/applications/differential/storage/DifferentialRevision.php b/src/applications/differential/storage/DifferentialRevision.php
index 1d31361fa5..664d417bc4 100644
--- a/src/applications/differential/storage/DifferentialRevision.php
+++ b/src/applications/differential/storage/DifferentialRevision.php
@@ -158,7 +158,7 @@ final class DifferentialRevision extends DifferentialDAO
       return array();
     }
     return id(new DifferentialCommentQuery())
-      ->withRevisionIDs(array($this->getID()))
+      ->withRevisionPHIDs(array($this->getPHID()))
       ->execute();
   }
 
@@ -194,7 +194,7 @@ final class DifferentialRevision extends DifferentialDAO
         $this->getID());
 
       $comments = id(new DifferentialCommentQuery())
-        ->withRevisionIDs(array($this->getID()))
+        ->withRevisionPHIDs(array($this->getPHID()))
         ->execute();
       foreach ($comments as $comment) {
         $comment->delete();
diff --git a/src/applications/differential/storage/DifferentialTransaction.php b/src/applications/differential/storage/DifferentialTransaction.php
index 745805e4a6..68fb963ee4 100644
--- a/src/applications/differential/storage/DifferentialTransaction.php
+++ b/src/applications/differential/storage/DifferentialTransaction.php
@@ -2,6 +2,10 @@
 
 final class DifferentialTransaction extends PhabricatorApplicationTransaction {
 
+  const TYPE_INLINE = 'differential:inline';
+  const TYPE_UPDATE = 'differential:update';
+  const TYPE_ACTION = 'differential:action';
+
   public function getApplicationName() {
     return 'differential';
   }
diff --git a/src/applications/differential/view/DifferentialRevisionCommentListView.php b/src/applications/differential/view/DifferentialRevisionCommentListView.php
index a6f72854ce..29473bf9e3 100644
--- a/src/applications/differential/view/DifferentialRevisionCommentListView.php
+++ b/src/applications/differential/view/DifferentialRevisionCommentListView.php
@@ -93,7 +93,7 @@ final class DifferentialRevisionCommentListView extends AphrontView {
       $view->setUser($this->user);
       $view->setHandles($this->handles);
       $view->setMarkupEngine($engine);
-      $view->setInlineComments(idx($inlines, $comment->getID(), array()));
+//      $view->setInlineComments(idx($inlines, $comment->getID(), array()));
       $view->setChangesets($this->changesets);
       $view->setTargetDiff($this->target);
       $view->setRevision($this->getRevision());
@@ -132,7 +132,7 @@ final class DifferentialRevisionCommentListView extends AphrontView {
     $hidden = array();
     if ($last_comment !== null) {
       foreach ($objs as $position => $comment) {
-        if (!$comment->getID()) {
+        if (!$comment->getPHID()) {
           // These are synthetic comments with summary/test plan information.
           $header[] = $html[$position];
           unset($html[$position]);
diff --git a/src/applications/releeph/field/specification/ReleephDiffChurnFieldSpecification.php b/src/applications/releeph/field/specification/ReleephDiffChurnFieldSpecification.php
index 6f5a85c2d0..f77f13252d 100644
--- a/src/applications/releeph/field/specification/ReleephDiffChurnFieldSpecification.php
+++ b/src/applications/releeph/field/specification/ReleephDiffChurnFieldSpecification.php
@@ -24,7 +24,7 @@ final class ReleephDiffChurnFieldSpecification
 
     $diff_rev = $this->getReleephRequest()->loadDifferentialRevision();
     $comments = id(new DifferentialCommentQuery())
-      ->withRevisionIDs(array($diff_rev->getID()))
+      ->withRevisionPHIDs(array($diff_rev->getPHID()))
       ->execute();
 
     $counts = array();

From 0dc73d5d9357640f2fc7ddbacf23dec90dc4ec5f Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Thu, 13 Feb 2014 06:19:55 -0800
Subject: [PATCH 02/11] Always provide a valid content source for
 DifferentialCommentEditor

Summary: Ref T2222. On the `tmp.differential` branch, we're currently having
issues parsing commits which reference Differential revisions, because the
"user closed this revision (closed by commit xyz)" message is fataling:

  [2014-02-13 14:12:36] EXCEPTION: (PhutilProxyException) Error while
  executing task ID 345358 from queue. {>} (AphrontQueryException)
  #1048: Column 'contentSource' cannot be null

Specifically, the MessageParser pathway for CommentEditor doesn't set a content
source. Make sure CommentEditor always sets a content source.

(This is also causing a buildup of diffs on D8212 and D8211.)

Auditors: btrahan
---
 .../differential/editor/DifferentialCommentEditor.php     | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/src/applications/differential/editor/DifferentialCommentEditor.php b/src/applications/differential/editor/DifferentialCommentEditor.php
index 204a7a65b8..e494d36030 100644
--- a/src/applications/differential/editor/DifferentialCommentEditor.php
+++ b/src/applications/differential/editor/DifferentialCommentEditor.php
@@ -561,9 +561,15 @@ final class DifferentialCommentEditor extends PhabricatorEditor {
       ->setRevision($revision);
 
     if ($this->contentSource) {
-      $template->setContentSource($this->contentSource);
+      $content_source = $this->contentSource;
+    } else {
+      $content_source = PhabricatorContentSource::newForSource(
+        PhabricatorContentSource::SOURCE_LEGACY,
+        array());
     }
 
+    $template->setContentSource($content_source);
+
     $comments = array();
 
     // If this edit performs a meaningful action, save a transaction for the

From 62cb58408346c53d47cdb798b62a48a890bdd484 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Wed, 12 Feb 2014 14:05:40 -0800
Subject: [PATCH 03/11] Use timeline view in Differential and make inlines
 somewhat usable again

Summary:
Ref T2222. This gets rid of Differential's custom view and uses a standard view instead.

This also mostly fixes the rendering logic for inlines.

This is headed to the `tmp.differential` branch.

Test Plan: {F112696}

Reviewers: btrahan

Reviewed By: btrahan

CC: chad, aran

Maniphest Tasks: T1790, T2222

Differential Revision: https://secure.phabricator.com/D8215
---
 src/__phutil_library_map__.php                |   4 +-
 .../DifferentialRevisionViewController.php    |  36 ++-
 .../storage/DifferentialTransaction.php       |  16 ++
 .../DifferentialRevisionCommentListView.php   | 209 ------------------
 .../view/DifferentialTransactionView.php      | 127 +++++++++++
 .../pholio/storage/PholioTransaction.php      |   3 -
 .../pholio/view/PholioTransactionView.php     |   2 +-
 7 files changed, 173 insertions(+), 224 deletions(-)
 delete mode 100644 src/applications/differential/view/DifferentialRevisionCommentListView.php
 create mode 100644 src/applications/differential/view/DifferentialTransactionView.php

diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
index e5fd4b4590..29eef4a517 100644
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -437,7 +437,6 @@ phutil_register_library_map(array(
     'DifferentialReviewersFieldSpecification' => 'applications/differential/field/specification/DifferentialReviewersFieldSpecification.php',
     'DifferentialReviewersView' => 'applications/differential/view/DifferentialReviewersView.php',
     'DifferentialRevision' => 'applications/differential/storage/DifferentialRevision.php',
-    'DifferentialRevisionCommentListView' => 'applications/differential/view/DifferentialRevisionCommentListView.php',
     'DifferentialRevisionCommentView' => 'applications/differential/view/DifferentialRevisionCommentView.php',
     'DifferentialRevisionControlSystem' => 'applications/differential/constants/DifferentialRevisionControlSystem.php',
     'DifferentialRevisionDetailRenderer' => 'applications/differential/controller/DifferentialRevisionDetailRenderer.php',
@@ -465,6 +464,7 @@ phutil_register_library_map(array(
     'DifferentialTransaction' => 'applications/differential/storage/DifferentialTransaction.php',
     'DifferentialTransactionComment' => 'applications/differential/storage/DifferentialTransactionComment.php',
     'DifferentialTransactionQuery' => 'applications/differential/query/DifferentialTransactionQuery.php',
+    'DifferentialTransactionView' => 'applications/differential/view/DifferentialTransactionView.php',
     'DifferentialUnitFieldSpecification' => 'applications/differential/field/specification/DifferentialUnitFieldSpecification.php',
     'DifferentialUnitStatus' => 'applications/differential/constants/DifferentialUnitStatus.php',
     'DifferentialUnitTestResult' => 'applications/differential/constants/DifferentialUnitTestResult.php',
@@ -2951,7 +2951,6 @@ phutil_register_library_map(array(
       5 => 'HarbormasterBuildableInterface',
       6 => 'PhabricatorSubscribableInterface',
     ),
-    'DifferentialRevisionCommentListView' => 'AphrontView',
     'DifferentialRevisionCommentView' => 'AphrontView',
     'DifferentialRevisionDetailView' => 'AphrontView',
     'DifferentialRevisionEditController' => 'DifferentialController',
@@ -2979,6 +2978,7 @@ phutil_register_library_map(array(
     'DifferentialTransaction' => 'PhabricatorApplicationTransaction',
     'DifferentialTransactionComment' => 'PhabricatorApplicationTransactionComment',
     'DifferentialTransactionQuery' => 'PhabricatorApplicationTransactionQuery',
+    'DifferentialTransactionView' => 'PhabricatorApplicationTransactionView',
     'DifferentialUnitFieldSpecification' => 'DifferentialFieldSpecification',
     'DifferentialViewPolicyFieldSpecification' => 'DifferentialFieldSpecification',
     'DiffusionBranchTableController' => 'DiffusionController',
diff --git a/src/applications/differential/controller/DifferentialRevisionViewController.php b/src/applications/differential/controller/DifferentialRevisionViewController.php
index ef913b7863..536e213bde 100644
--- a/src/applications/differential/controller/DifferentialRevisionViewController.php
+++ b/src/applications/differential/controller/DifferentialRevisionViewController.php
@@ -258,15 +258,9 @@ final class DifferentialRevisionViewController extends DifferentialController {
     $revision_detail->setActions($actions);
     $revision_detail->setUser($user);
 
-    $comment_view = new DifferentialRevisionCommentListView();
-    $comment_view->setComments($comments);
-    $comment_view->setHandles($handles);
-    $comment_view->setInlineComments($inlines);
-    $comment_view->setChangesets($all_changesets);
-    $comment_view->setUser($user);
-    $comment_view->setTargetDiff($target);
-    $comment_view->setVersusDiffID($diff_vs);
-    $comment_view->setRevision($revision);
+    $comment_view = $this->buildTransactions(
+      $revision,
+      $changesets);
 
     if ($arc_project) {
       Javelin::initBehavior(
@@ -926,4 +920,28 @@ final class DifferentialRevisionViewController extends DifferentialController {
     return id(new AphrontRedirectResponse())->setURI($file->getBestURI());
 
   }
+
+  private function buildTransactions(
+    DifferentialRevision $revision,
+    array $changesets) {
+    $viewer = $this->getRequest()->getUser();
+
+    $xactions = id(new DifferentialTransactionQuery())
+      ->setViewer($viewer)
+      ->withObjectPHIDs(array($revision->getPHID()))
+      ->needComments(true)
+      ->execute();
+
+    $engine = id(new PhabricatorMarkupEngine())
+      ->setViewer($viewer);
+
+    $timeline = id(new DifferentialTransactionView())
+      ->setUser($viewer)
+      ->setObjectPHID($revision->getPHID())
+      ->setChangesets($changesets)
+      ->setTransactions($xactions);
+
+    return $timeline;
+  }
+
 }
diff --git a/src/applications/differential/storage/DifferentialTransaction.php b/src/applications/differential/storage/DifferentialTransaction.php
index 68fb963ee4..f410b868b4 100644
--- a/src/applications/differential/storage/DifferentialTransaction.php
+++ b/src/applications/differential/storage/DifferentialTransaction.php
@@ -18,4 +18,20 @@ final class DifferentialTransaction extends PhabricatorApplicationTransaction {
     return new DifferentialTransactionComment();
   }
 
+  public function getTitle() {
+    $author_phid = $this->getAuthorPHID();
+
+    $old = $this->getOldValue();
+    $new = $this->getNewValue();
+
+    switch ($this->getTransactionType()) {
+      case self::TYPE_INLINE:
+        return pht(
+          '%s added inline comments.',
+          $this->renderHandleLink($author_phid));
+    }
+
+    return parent::getTitle();
+  }
+
 }
diff --git a/src/applications/differential/view/DifferentialRevisionCommentListView.php b/src/applications/differential/view/DifferentialRevisionCommentListView.php
deleted file mode 100644
index 29473bf9e3..0000000000
--- a/src/applications/differential/view/DifferentialRevisionCommentListView.php
+++ /dev/null
@@ -1,209 +0,0 @@
-<?php
-
-final class DifferentialRevisionCommentListView extends AphrontView {
-
-  private $comments;
-  private $handles;
-  private $inlines;
-  private $changesets;
-  private $target;
-  private $versusDiffID;
-  private $id;
-  private $revision;
-
-  public function setRevision(DifferentialRevision $revision) {
-    $this->revision = $revision;
-    return $this;
-  }
-
-  public function getRevision() {
-    return $this->revision;
-  }
-
-  public function setComments(array $comments) {
-    assert_instances_of($comments, 'DifferentialComment');
-    $this->comments = $comments;
-    return $this;
-  }
-
-  public function setInlineComments(array $inline_comments) {
-    assert_instances_of($inline_comments, 'PhabricatorInlineCommentInterface');
-    $this->inlines = $inline_comments;
-    return $this;
-  }
-
-  public function setHandles(array $handles) {
-    assert_instances_of($handles, 'PhabricatorObjectHandle');
-    $this->handles = $handles;
-    return $this;
-  }
-
-  public function setChangesets(array $changesets) {
-    assert_instances_of($changesets, 'DifferentialChangeset');
-    $this->changesets = $changesets;
-    return $this;
-  }
-
-  public function setTargetDiff(DifferentialDiff $target) {
-    $this->target = $target;
-    return $this;
-  }
-
-  public function setVersusDiffID($diff_vs) {
-    $this->versusDiffID = $diff_vs;
-    return $this;
-  }
-
-  public function getID() {
-    if (!$this->id) {
-      $this->id = celerity_generate_unique_node_id();
-    }
-    return $this->id;
-  }
-
-  public function render() {
-
-    $this->requireResource('differential-revision-comment-list-css');
-
-    $engine = new PhabricatorMarkupEngine();
-    $engine->setViewer($this->user);
-    foreach ($this->comments as $comment) {
-      $comment->giveFacebookSomeArbitraryDiff($this->target);
-
-      $engine->addObject(
-        $comment,
-        DifferentialComment::MARKUP_FIELD_BODY);
-    }
-
-    foreach ($this->inlines as $inline) {
-      $engine->addObject(
-        $inline,
-        PhabricatorInlineCommentInterface::MARKUP_FIELD_BODY);
-    }
-
-    $engine->process();
-
-    $inlines = mgroup($this->inlines, 'getCommentID');
-
-    $num = 1;
-    $html = array();
-    foreach ($this->comments as $comment) {
-      $view = new DifferentialRevisionCommentView();
-      $view->setComment($comment);
-      $view->setUser($this->user);
-      $view->setHandles($this->handles);
-      $view->setMarkupEngine($engine);
-//      $view->setInlineComments(idx($inlines, $comment->getID(), array()));
-      $view->setChangesets($this->changesets);
-      $view->setTargetDiff($this->target);
-      $view->setRevision($this->getRevision());
-      $view->setVersusDiffID($this->versusDiffID);
-      if ($comment->getAction() == DifferentialAction::ACTION_SUMMARIZE) {
-        $view->setAnchorName('summary');
-      } else if ($comment->getAction() == DifferentialAction::ACTION_TESTPLAN) {
-        $view->setAnchorName('test-plan');
-      } else {
-        $view->setAnchorName('comment-'.$num);
-        $num++;
-      }
-
-      $html[] = $view->render();
-    }
-
-    $objs = array_reverse(array_values($this->comments));
-    $html = array_reverse(array_values($html));
-    $user = $this->user;
-
-    $last_comment = null;
-    // Find the most recent comment by the viewer.
-    foreach ($objs as $position => $comment) {
-      if ($user && ($comment->getAuthorPHID() == $user->getPHID())) {
-        if ($last_comment === null) {
-          $last_comment = $position;
-        } else if ($last_comment == $position - 1) {
-          // If the viewer made several comments in a row, show them all. This
-          // is a spaz rule for epriestley.
-          $last_comment = $position;
-        }
-      }
-    }
-
-    $header = array();
-    $hidden = array();
-    if ($last_comment !== null) {
-      foreach ($objs as $position => $comment) {
-        if (!$comment->getPHID()) {
-          // These are synthetic comments with summary/test plan information.
-          $header[] = $html[$position];
-          unset($html[$position]);
-          continue;
-        }
-        if ($position <= $last_comment) {
-          // Always show comments after the viewer's last comment.
-          continue;
-        }
-        if ($position < 3) {
-          // Always show the 3 most recent comments.
-          continue;
-        }
-        $hidden[] = $position;
-      }
-    }
-
-    if (count($hidden) <= 3) {
-      // Don't hide if there's not much to hide.
-      $hidden = array();
-    }
-
-    $header = array_reverse($header);
-
-
-    $hidden = array_select_keys($html, $hidden);
-    $visible = array_diff_key($html, $hidden);
-
-    $hidden = array_reverse($hidden);
-    $visible = array_reverse($visible);
-
-    if ($hidden) {
-      $this->initBehavior(
-        'differential-show-all-comments',
-        array(
-          'markup' => implode("\n", $hidden),
-        ));
-      $hidden = javelin_tag(
-        'div',
-        array(
-          'sigil' =>  "differential-all-comments-container",
-        ),
-        phutil_tag(
-          'div',
-          array(
-            'class' => 'differential-older-comments-are-hidden',
-          ),
-          array(
-            pht(
-              '%s older comments are hidden.',
-              new PhutilNumber(count($hidden))),
-            ' ',
-            javelin_tag(
-              'a',
-              array(
-                'href' => '#',
-                'mustcapture' => true,
-                'sigil' => 'differential-show-all-comments',
-              ),
-              pht('Show all comments.')),
-          )));
-    } else {
-      $hidden = null;
-    }
-
-    return javelin_tag(
-      'div',
-      array(
-        'class' => 'differential-comment-list',
-        'id' => $this->getID(),
-      ),
-      array_merge($header, array($hidden), $visible));
-  }
-}
diff --git a/src/applications/differential/view/DifferentialTransactionView.php b/src/applications/differential/view/DifferentialTransactionView.php
new file mode 100644
index 0000000000..9bec0e5fa5
--- /dev/null
+++ b/src/applications/differential/view/DifferentialTransactionView.php
@@ -0,0 +1,127 @@
+<?php
+
+final class DifferentialTransactionView
+  extends PhabricatorApplicationTransactionView {
+
+  private $changesets;
+
+  public function setChangesets(array $changesets) {
+    assert_instances_of($changesets, 'DifferentialChangeset');
+    $this->changesets = $changesets;
+    return $this;
+  }
+
+  public function getChangesets() {
+    return $this->changesets;
+  }
+
+  // TODO: There's a whole lot of code duplication between this and
+  // PholioTransactionView to handle inlines. Merge this into the core? Some of
+  // it can probably be shared, while other parts are trickier.
+
+  protected function shouldGroupTransactions(
+    PhabricatorApplicationTransaction $u,
+    PhabricatorApplicationTransaction $v) {
+
+    if ($u->getAuthorPHID() != $v->getAuthorPHID()) {
+      // Don't group transactions by different authors.
+      return false;
+    }
+
+    if (($v->getDateCreated() - $u->getDateCreated()) > 60) {
+      // Don't group if transactions that happened more than 60s apart.
+      return false;
+    }
+
+    switch ($u->getTransactionType()) {
+      case PhabricatorTransactions::TYPE_COMMENT:
+      case DifferentialTransaction::TYPE_INLINE:
+        break;
+      default:
+        return false;
+    }
+
+    switch ($v->getTransactionType()) {
+      case DifferentialTransaction::TYPE_INLINE:
+        return true;
+    }
+
+    return parent::shouldGroupTransactions($u, $v);
+  }
+
+  protected function renderTransactionContent(
+    PhabricatorApplicationTransaction $xaction) {
+
+    $out = array();
+
+    $type_inline = DifferentialTransaction::TYPE_INLINE;
+
+    $group = $xaction->getTransactionGroup();
+    if ($xaction->getTransactionType() == $type_inline) {
+      array_unshift($group, $xaction);
+    } else {
+      $out[] = parent::renderTransactionContent($xaction);
+    }
+
+    if (!$group) {
+      return $out;
+    }
+
+    $inlines = array();
+    foreach ($group as $xaction) {
+      switch ($xaction->getTransactionType()) {
+        case DifferentialTransaction::TYPE_INLINE:
+          $inlines[] = $xaction;
+          break;
+        default:
+          throw new Exception("Unknown grouped transaction type!");
+      }
+    }
+
+    if ($inlines) {
+      $inline_view = new PhabricatorInlineSummaryView();
+
+      $changesets = $this->getChangesets();
+      $changesets = mpull($changesets, null, 'getID');
+
+      // Group the changesets by file and reorder them by display order.
+      $inline_groups = array();
+      foreach ($inlines as $inline) {
+        $inline_groups[$inline->getComment()->getChangesetID()][] = $inline;
+      }
+
+      $changsets = msort($changesets, 'getFilename');
+      $inline_groups = array_select_keys(
+        $inline_groups,
+        array_keys($changesets));
+
+      foreach ($inline_groups as $changeset_id => $group) {
+        $group = msort($group, 'getLineNumber');
+
+        $changeset = $changesets[$changeset_id];
+        $items = array();
+        foreach ($group as $inline) {
+          $comment = $inline->getComment();
+          $item = array(
+            'id' => $comment->getID(),
+            'line' => $comment->getLineNumber(),
+            'length' => $comment->getLineLength(),
+            'content' => parent::renderTransactionContent($inline),
+          );
+
+          // TODO: Fix the where/href stuff for nonlocal inlines.
+
+          $items[] = $item;
+        }
+        $inline_view->addCommentGroup(
+          $changeset->getFilename(),
+          $items);
+      }
+
+      $out[] = $inline_view;
+    }
+
+    return $out;
+  }
+
+}
diff --git a/src/applications/pholio/storage/PholioTransaction.php b/src/applications/pholio/storage/PholioTransaction.php
index af9c0502a8..c20865b914 100644
--- a/src/applications/pholio/storage/PholioTransaction.php
+++ b/src/applications/pholio/storage/PholioTransaction.php
@@ -1,8 +1,5 @@
 <?php
 
-/**
- * @group pholio
- */
 final class PholioTransaction extends PhabricatorApplicationTransaction {
 
   public function getApplicationName() {
diff --git a/src/applications/pholio/view/PholioTransactionView.php b/src/applications/pholio/view/PholioTransactionView.php
index 33ca7444f3..5ba716f638 100644
--- a/src/applications/pholio/view/PholioTransactionView.php
+++ b/src/applications/pholio/view/PholioTransactionView.php
@@ -16,7 +16,7 @@ final class PholioTransactionView
     }
 
     if (($v->getDateCreated() - $u->getDateCreated()) > 60) {
-      // Don't group if transactions happend more than 60s apart.
+      // Don't group if transactions happened more than 60s apart.
       return false;
     }
 

From 29918e1bd47f98413adc3066a9cb4100ded85fce Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Thu, 13 Feb 2014 12:12:17 -0800
Subject: [PATCH 04/11] Fix double-rendering of inline comments in mail

Summary: Ref T2222. This is a `tmp.differential`-only issue. Inline comment transactions now have content, so we treat them like body text. We also render them separately as inline text. This produces mail where inlines are rendered twice.

Test Plan: Sent myself mail, saw only one copy of inlines.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T2222

Differential Revision: https://secure.phabricator.com/D8216
---
 .../differential/constants/DifferentialAction.php             | 1 +
 .../differential/mail/DifferentialCommentMail.php             | 4 ++++
 2 files changed, 5 insertions(+)

diff --git a/src/applications/differential/constants/DifferentialAction.php b/src/applications/differential/constants/DifferentialAction.php
index 9b37c3995b..655e720838 100644
--- a/src/applications/differential/constants/DifferentialAction.php
+++ b/src/applications/differential/constants/DifferentialAction.php
@@ -122,6 +122,7 @@ final class DifferentialAction {
       self::ACTION_ADDCCS         => 'added CCs to',
       self::ACTION_CLAIM          => 'commandeered',
       self::ACTION_REOPEN         => 'reopened',
+      DifferentialTransaction::TYPE_INLINE => 'commented on',
     );
 
     if (!empty($verbs[$action])) {
diff --git a/src/applications/differential/mail/DifferentialCommentMail.php b/src/applications/differential/mail/DifferentialCommentMail.php
index 2c7c3c07f8..e4c462ba4b 100644
--- a/src/applications/differential/mail/DifferentialCommentMail.php
+++ b/src/applications/differential/mail/DifferentialCommentMail.php
@@ -154,6 +154,10 @@ final class DifferentialCommentMail extends DifferentialMail {
     $body[] = null;
 
     foreach ($this->getComments() as $comment) {
+      if ($comment->getAction() == DifferentialTransaction::TYPE_INLINE) {
+        // These have comment content now, but are rendered below.
+        continue;
+      }
       $content = $comment->getContent();
       if (strlen($content)) {
         $body[] = $this->formatText($content);

From 348b07a4becf487b0bd654fa7a6993912ca9fce0 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Thu, 13 Feb 2014 12:26:44 -0800
Subject: [PATCH 05/11] Make Asana bridge work with new Differential
 transactions

Summary: Ref T2222. Unbreak the Asana/JIRA bridge.

Test Plan: {F112712}

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T2222

Differential Revision: https://secure.phabricator.com/D8217
---
 .../PhabricatorFeedStoryDifferential.php      | 29 ++++++++++++++-----
 1 file changed, 21 insertions(+), 8 deletions(-)

diff --git a/src/applications/feed/story/PhabricatorFeedStoryDifferential.php b/src/applications/feed/story/PhabricatorFeedStoryDifferential.php
index 6a5c70b525..f6dc133804 100644
--- a/src/applications/feed/story/PhabricatorFeedStoryDifferential.php
+++ b/src/applications/feed/story/PhabricatorFeedStoryDifferential.php
@@ -319,28 +319,41 @@ final class PhabricatorFeedStoryDifferential extends PhabricatorFeedStory {
     }
 
     // Roughly render inlines into the comment.
-    $comment_id = $data->getValue('temporaryCommentID');
-    if ($comment_id) {
-      $inlines = id(new DifferentialInlineCommentQuery())
-        ->withCommentIDs(array($comment_id))
+    $xaction_phids = $data->getValue('temporaryTransactionPHIDs');
+    if ($xaction_phids) {
+      $inlines = id(new DifferentialTransactionQuery())
+        ->setViewer(PhabricatorUser::getOmnipotentUser())
+        ->withPHIDs($xaction_phids)
+        ->needComments(true)
+        ->withTransactionTypes(
+          array(
+            DifferentialTransaction::TYPE_INLINE,
+          ))
         ->execute();
       if ($inlines) {
         $title .= "\n\n";
         $title .= pht('Inline Comments');
         $title .= "\n";
-        $changeset_ids = mpull($inlines, 'getChangesetID');
+
+        $changeset_ids = array();
+        foreach ($inlines as $inline) {
+          $changeset_ids[] = $inline->getComment()->getChangesetID();
+        }
+
         $changesets = id(new DifferentialChangeset())->loadAllWhere(
           'id IN (%Ld)',
           $changeset_ids);
+
         foreach ($inlines as $inline) {
-          $changeset = idx($changesets, $inline->getChangesetID());
+          $comment = $inline->getComment();
+          $changeset = idx($changesets, $comment->getChangesetID());
           if (!$changeset) {
             continue;
           }
 
           $filename = $changeset->getDisplayFilename();
-          $linenumber = $inline->getLineNumber();
-          $inline_text = $engine->markupText($inline->getContent());
+          $linenumber = $comment->getLineNumber();
+          $inline_text = $engine->markupText($comment->getContent());
           $inline_text = rtrim($inline_text);
 
           $title .= "{$filename}:{$linenumber} {$inline_text}\n";

From 015931576330ddc48af77256268b7ffcb134427e Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Thu, 13 Feb 2014 13:12:14 -0800
Subject: [PATCH 06/11] Add some missing text for Differential inline
 transactions

Summary: Ref T2222. Add inline and default text.

Test Plan: Feed works again.

Reviewers: chad, btrahan

Reviewed By: chad

CC: aran

Maniphest Tasks: T2222

Differential Revision: https://secure.phabricator.com/D8218
---
 .../PhabricatorFeedStoryDifferential.php      | 24 +++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/src/applications/feed/story/PhabricatorFeedStoryDifferential.php b/src/applications/feed/story/PhabricatorFeedStoryDifferential.php
index f6dc133804..20b6013608 100644
--- a/src/applications/feed/story/PhabricatorFeedStoryDifferential.php
+++ b/src/applications/feed/story/PhabricatorFeedStoryDifferential.php
@@ -117,6 +117,14 @@ final class PhabricatorFeedStoryDifferential extends PhabricatorFeedStory {
         $one_line = pht('%s reopened revision %s',
         $actor_link, $revision_link);
       break;
+      case DifferentialTransaction::TYPE_INLINE:
+        $one_line = pht('%s added inline comments to %s',
+        $actor_link, $revision_link);
+      break;
+      default:
+        $one_line = pht('%s edited %s',
+        $actor_link, $revision_link);
+      break;
     }
 
     return $one_line;
@@ -200,6 +208,14 @@ final class PhabricatorFeedStoryDifferential extends PhabricatorFeedStory {
         $one_line = pht('%s reopened revision %s %s',
           $author_name, $revision_title, $revision_uri);
       break;
+      case DifferentialTransaction::TYPE_INLINE:
+        $one_line = pht('%s added inline comments to %s %s',
+          $author_name, $revision_title, $revision_uri);
+        break;
+      default:
+        $one_line = pht('%s edited %s %s',
+          $author_name, $revision_title, $revision_uri);
+        break;
     }
 
     return $one_line;
@@ -308,6 +324,14 @@ final class PhabricatorFeedStoryDifferential extends PhabricatorFeedStory {
           $title = pht('%s reopened revision %s',
           $author_name, $revision_name);
         break;
+        case DifferentialTransaction::TYPE_INLINE:
+          $title = pht('%s added inline comments to %s',
+          $author_name, $revision_name);
+        break;
+        default:
+          $title = pht('%s edited revision %s',
+          $author_name, $revision_name);
+        break;
       }
     }
 

From ee38e334963bedddb54adc6cf8a197f6d43e4403 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Thu, 13 Feb 2014 15:06:38 -0800
Subject: [PATCH 07/11] Fix two issues with Differential Transactions

Summary: With symbols, the transactions fataled on `getID()`. Also my line
number sorting was a bit goofy.

Auditors: btrahan
---
 .../DifferentialRevisionViewController.php           | 10 +++++++++-
 .../view/DifferentialTransactionView.php             | 12 +++++++++++-
 2 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/src/applications/differential/controller/DifferentialRevisionViewController.php b/src/applications/differential/controller/DifferentialRevisionViewController.php
index 536e213bde..9c9a6c1a77 100644
--- a/src/applications/differential/controller/DifferentialRevisionViewController.php
+++ b/src/applications/differential/controller/DifferentialRevisionViewController.php
@@ -262,11 +262,19 @@ final class DifferentialRevisionViewController extends DifferentialController {
       $revision,
       $changesets);
 
+    $wrap_id = celerity_generate_unique_node_id();
+    $comment_view = phutil_tag(
+      'div',
+      array(
+        'id' => $wrap_id,
+      ),
+      $comment_view);
+
     if ($arc_project) {
       Javelin::initBehavior(
         'repository-crossreference',
         array(
-          'section' => $comment_view->getID(),
+          'section' => $wrap_id,
           'projects' => $project_phids,
         ));
     }
diff --git a/src/applications/differential/view/DifferentialTransactionView.php b/src/applications/differential/view/DifferentialTransactionView.php
index 9bec0e5fa5..23ef06f637 100644
--- a/src/applications/differential/view/DifferentialTransactionView.php
+++ b/src/applications/differential/view/DifferentialTransactionView.php
@@ -96,7 +96,17 @@ final class DifferentialTransactionView
         array_keys($changesets));
 
       foreach ($inline_groups as $changeset_id => $group) {
-        $group = msort($group, 'getLineNumber');
+        // Sort the group of inlines by line number.
+        $by_line = array();
+        foreach ($group as $inline) {
+          $by_line[] = array(
+            'line' => $inline->getComment()->getLineNumber().','.
+                      $inline->getComment()->getLineLength(),
+            'inline' => $inline,
+          );
+        }
+        $by_line = isort($by_line, 'line');
+        $group = ipull($by_line, 'inline');
 
         $changeset = $changesets[$changeset_id];
         $items = array();

From c7ca3cd4abd8cf662810be5f8a107130a0eaf5cf Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Thu, 13 Feb 2014 15:12:42 -0800
Subject: [PATCH 08/11] Pass $all_changesets, not $changesets, to inline
 rendering logic

Summary: `$all_changesets` has all the changesets.

Auditors: btrahan
---
 .../controller/DifferentialRevisionViewController.php           | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/applications/differential/controller/DifferentialRevisionViewController.php b/src/applications/differential/controller/DifferentialRevisionViewController.php
index 9c9a6c1a77..f3581aca96 100644
--- a/src/applications/differential/controller/DifferentialRevisionViewController.php
+++ b/src/applications/differential/controller/DifferentialRevisionViewController.php
@@ -260,7 +260,7 @@ final class DifferentialRevisionViewController extends DifferentialController {
 
     $comment_view = $this->buildTransactions(
       $revision,
-      $changesets);
+      $all_changesets);
 
     $wrap_id = celerity_generate_unique_node_id();
     $comment_view = phutil_tag(

From cb20205aee1031ae9127065e809636f3be5752c5 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Thu, 13 Feb 2014 15:20:22 -0800
Subject: [PATCH 09/11] Don't show "edit" links in Differential for now

Summary: Ref T2222. These don't work yet. We just have to copy a couple fields, but let's sort that out later since this is purely a new feature.

Test Plan: Looked at a revision, no edit links.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T2222

Differential Revision: https://secure.phabricator.com/D8222
---
 .../controller/DifferentialRevisionViewController.php        | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/src/applications/differential/controller/DifferentialRevisionViewController.php b/src/applications/differential/controller/DifferentialRevisionViewController.php
index f3581aca96..cffb9b00b2 100644
--- a/src/applications/differential/controller/DifferentialRevisionViewController.php
+++ b/src/applications/differential/controller/DifferentialRevisionViewController.php
@@ -949,6 +949,11 @@ final class DifferentialRevisionViewController extends DifferentialController {
       ->setChangesets($changesets)
       ->setTransactions($xactions);
 
+    // TODO: Make this work and restore edit links. We need to copy
+    // `revisionPHID` to the new version of the comment. This should be simple,
+    // but can happen post-merge. See T2222.
+    $timeline->setShowEditActions(false);
+
     return $timeline;
   }
 

From 9afe52de51d75398a87436dc245f453491fbde33 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Thu, 13 Feb 2014 15:35:44 -0800
Subject: [PATCH 10/11] Provide transaction strings, icons and colors for
 Differential

Summary: Ref T2222. Once these are live, yell if any of them seem off. I tried to mostly stay consistent-ish with what we had before.

Test Plan: Looked at a bunch of revisions and saw more detailed, colorful transactions.

Reviewers: btrahan, chad

Reviewed By: chad

CC: aran

Maniphest Tasks: T2222

Differential Revision: https://secure.phabricator.com/D8223
---
 .../storage/DifferentialTransaction.php       | 83 ++++++++++++++++++-
 1 file changed, 82 insertions(+), 1 deletion(-)

diff --git a/src/applications/differential/storage/DifferentialTransaction.php b/src/applications/differential/storage/DifferentialTransaction.php
index f410b868b4..d8cbbb6e36 100644
--- a/src/applications/differential/storage/DifferentialTransaction.php
+++ b/src/applications/differential/storage/DifferentialTransaction.php
@@ -20,6 +20,7 @@ final class DifferentialTransaction extends PhabricatorApplicationTransaction {
 
   public function getTitle() {
     $author_phid = $this->getAuthorPHID();
+    $author_handle = $this->renderHandleLink($author_phid);
 
     $old = $this->getOldValue();
     $new = $this->getNewValue();
@@ -28,10 +29,90 @@ final class DifferentialTransaction extends PhabricatorApplicationTransaction {
       case self::TYPE_INLINE:
         return pht(
           '%s added inline comments.',
-          $this->renderHandleLink($author_phid));
+          $author_handle);
+      case self::TYPE_UPDATE:
+        if ($new) {
+          // TODO: Migrate to PHIDs and use handles here?
+          // TODO: Link this?
+          return pht(
+            '%s updated this revision to Diff #%d.',
+            $author_handle,
+            $new);
+        } else {
+          return pht(
+            '%s updated this revision.',
+            $author_handle);
+        }
+      case self::TYPE_ACTION:
+        return DifferentialAction::getBasicStoryText($new, $author_handle);
     }
 
     return parent::getTitle();
   }
 
+  public function getIcon() {
+    switch ($this->getTransactionType()) {
+      case self::TYPE_INLINE:
+        return 'comment';
+      case self::TYPE_UPDATE:
+        return 'refresh';
+      case self::TYPE_ACTION:
+        switch ($this->getNewValue()) {
+          case DifferentialAction::ACTION_CLOSE:
+            return 'ok';
+          case DifferentialAction::ACTION_ACCEPT:
+            return 'enable';
+          case DifferentialAction::ACTION_REJECT:
+          case DifferentialAction::ACTION_ABANDON:
+            return 'delete';
+          case DifferentialAction::ACTION_RETHINK:
+            return 'disable';
+          case DifferentialAction::ACTION_REQUEST:
+            return 'refresh';
+          case DifferentialAction::ACTION_RECLAIM:
+          case DifferentialAction::ACTION_REOPEN:
+            return 'new';
+          case DifferentialAction::ACTION_RESIGN:
+            return 'undo';
+          case DifferentialAction::ACTION_CLAIM:
+            return 'user';
+        }
+    }
+
+    return parent::getIcon();
+  }
+
+  public function getColor() {
+    switch ($this->getTransactionType()) {
+      case self::TYPE_UPDATE:
+        return PhabricatorTransactions::COLOR_SKY;
+      case self::TYPE_ACTION:
+        switch ($this->getNewValue()) {
+          case DifferentialAction::ACTION_CLOSE:
+            return PhabricatorTransactions::COLOR_BLUE;
+          case DifferentialAction::ACTION_ACCEPT:
+            return PhabricatorTransactions::COLOR_GREEN;
+          case DifferentialAction::ACTION_REJECT:
+            return PhabricatorTransactions::COLOR_RED;
+          case DifferentialAction::ACTION_ABANDON:
+            return PhabricatorTransactions::COLOR_BLACK;
+          case DifferentialAction::ACTION_RETHINK:
+            return PhabricatorTransactions::COLOR_RED;
+          case DifferentialAction::ACTION_REQUEST:
+            return PhabricatorTransactions::COLOR_SKY;
+          case DifferentialAction::ACTION_RECLAIM:
+            return PhabricatorTransactions::COLOR_SKY;
+          case DifferentialAction::ACTION_REOPEN:
+            return PhabricatorTransactions::COLOR_SKY;
+          case DifferentialAction::ACTION_RESIGN:
+            return PhabricatorTransactions::COLOR_ORANGE;
+          case DifferentialAction::ACTION_CLAIM:
+            return PhabricatorTransactions::COLOR_YELLOW;
+        }
+    }
+
+
+    return parent::getColor();
+  }
+
 }

From 7374a0a4d4f57d180569f8952a16166e3c10aa5a Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Thu, 13 Feb 2014 16:03:23 -0800
Subject: [PATCH 11/11] Fix inline comment links for non-visible comments

Summary: Ref T2222. Restore this funky is-visible / inline-is-elsewhere logic.

Test Plan: Updated a revision, saw all the inlines render properly when looking at various diffs and versus-diffs. Clicked inline links.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T2222

Differential Revision: https://secure.phabricator.com/D8224
---
 .../DifferentialRevisionViewController.php    |  7 +++
 .../editor/DifferentialRevisionEditor.php     | 10 +++-
 .../view/DifferentialTransactionView.php      | 60 ++++++++++++++++++-
 3 files changed, 73 insertions(+), 4 deletions(-)

diff --git a/src/applications/differential/controller/DifferentialRevisionViewController.php b/src/applications/differential/controller/DifferentialRevisionViewController.php
index cffb9b00b2..494669e261 100644
--- a/src/applications/differential/controller/DifferentialRevisionViewController.php
+++ b/src/applications/differential/controller/DifferentialRevisionViewController.php
@@ -260,6 +260,8 @@ final class DifferentialRevisionViewController extends DifferentialController {
 
     $comment_view = $this->buildTransactions(
       $revision,
+      $diff_vs ? $diffs[$diff_vs] : $target,
+      $target,
       $all_changesets);
 
     $wrap_id = celerity_generate_unique_node_id();
@@ -931,6 +933,8 @@ final class DifferentialRevisionViewController extends DifferentialController {
 
   private function buildTransactions(
     DifferentialRevision $revision,
+    DifferentialDiff $left_diff,
+    DifferentialDiff $right_diff,
     array $changesets) {
     $viewer = $this->getRequest()->getUser();
 
@@ -947,6 +951,9 @@ final class DifferentialRevisionViewController extends DifferentialController {
       ->setUser($viewer)
       ->setObjectPHID($revision->getPHID())
       ->setChangesets($changesets)
+      ->setRevision($revision)
+      ->setLeftDiff($left_diff)
+      ->setRightDiff($right_diff)
       ->setTransactions($xactions);
 
     // TODO: Make this work and restore edit links. We need to copy
diff --git a/src/applications/differential/editor/DifferentialRevisionEditor.php b/src/applications/differential/editor/DifferentialRevisionEditor.php
index 732d12d1dd..246c80f9d2 100644
--- a/src/applications/differential/editor/DifferentialRevisionEditor.php
+++ b/src/applications/differential/editor/DifferentialRevisionEditor.php
@@ -701,10 +701,18 @@ final class DifferentialRevisionEditor extends PhabricatorEditor {
     $template = id(new DifferentialComment())
       ->setAuthorPHID($this->getActorPHID())
       ->setRevision($this->revision);
+
     if ($this->contentSource) {
-      $template->setContentSource($this->contentSource);
+      $content_source = $this->contentSource;
+    } else {
+      $content_source = PhabricatorContentSource::newForSource(
+        PhabricatorContentSource::SOURCE_LEGACY,
+        array());
     }
 
+    $template->setContentSource($content_source);
+
+
     // Write the "update active diff" transaction.
     id(clone $template)
       ->setAction(DifferentialAction::ACTION_UPDATE)
diff --git a/src/applications/differential/view/DifferentialTransactionView.php b/src/applications/differential/view/DifferentialTransactionView.php
index 23ef06f637..8d7c3aa045 100644
--- a/src/applications/differential/view/DifferentialTransactionView.php
+++ b/src/applications/differential/view/DifferentialTransactionView.php
@@ -4,6 +4,36 @@ final class DifferentialTransactionView
   extends PhabricatorApplicationTransactionView {
 
   private $changesets;
+  private $revision;
+  private $rightDiff;
+  private $leftDiff;
+
+  public function setLeftDiff(DifferentialDiff $left_diff) {
+    $this->leftDiff = $left_diff;
+    return $this;
+  }
+
+  public function getLeftDiff() {
+    return $this->leftDiff;
+  }
+
+  public function setRightDiff(DifferentialDiff $right_diff) {
+    $this->rightDiff = $right_diff;
+    return $this;
+  }
+
+  public function getRightDiff() {
+    return $this->rightDiff;
+  }
+
+  public function setRevision(DifferentialRevision $revision) {
+    $this->revision = $revision;
+    return $this;
+  }
+
+  public function getRevision() {
+    return $this->revision;
+  }
 
   public function setChangesets(array $changesets) {
     assert_instances_of($changesets, 'DifferentialChangeset');
@@ -100,8 +130,8 @@ final class DifferentialTransactionView
         $by_line = array();
         foreach ($group as $inline) {
           $by_line[] = array(
-            'line' => $inline->getComment()->getLineNumber().','.
-                      $inline->getComment()->getLineLength(),
+            'line' => ((int)$inline->getComment()->getLineNumber() << 16) +
+                      ((int)$inline->getComment()->getLineLength()),
             'inline' => $inline,
           );
         }
@@ -119,7 +149,31 @@ final class DifferentialTransactionView
             'content' => parent::renderTransactionContent($inline),
           );
 
-          // TODO: Fix the where/href stuff for nonlocal inlines.
+          $changeset_diff_id = $changeset->getDiffID();
+          if ($comment->getIsNewFile()) {
+            $visible_diff_id = $this->getRightDiff()->getID();
+          } else {
+            $visible_diff_id = $this->getLeftDiff()->getID();
+          }
+
+          // TODO: We still get one edge case wrong here, when we have a
+          // versus diff and the file didn't exist in the old version. The
+          // comment is visible because we show the left side of the target
+          // diff when there's no corresponding file in the versus diff, but
+          // we incorrectly link it off-page.
+
+          $is_visible = ($changeset_diff_id == $visible_diff_id);
+          if (!$is_visible) {
+            $item['where'] = pht('(On Diff #%d)', $changeset_diff_id);
+
+            $revision_id = $this->getRevision()->getID();
+            $comment_id = $comment->getID();
+
+            $item['href'] =
+              "/D".$revision_id.
+              "?id=".$changeset_diff_id.
+              "#inline-".$comment_id;
+          }
 
           $items[] = $item;
         }