Merge Differential comment migration to ApplicationTransactions

Summary: Ref T2222. This merges the `tmp.differential` branch, including the
comment -> application transaction migration, to `master`.

Auditors: btrahan

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";

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',
@@ -464,6 +463,8 @@ 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',
+    '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',
@@ -2856,11 +2857,7 @@ phutil_register_library_map(array(
     'DifferentialChangesetTwoUpRenderer' => 'DifferentialChangesetHTMLRenderer',
     'DifferentialChangesetTwoUpTestRenderer' => 'DifferentialChangesetTestRenderer',
     'DifferentialChangesetViewController' => 'DifferentialController',
-    'DifferentialComment' =>
+    'DifferentialComment' => 'PhabricatorMarkupInterface',
-    array(
-      0 => 'DifferentialDAO',
-      1 => 'PhabricatorMarkupInterface',
-    ),
     'DifferentialCommentEditor' => 'PhabricatorEditor',
     'DifferentialCommentMail' => 'DifferentialMail',
     'DifferentialCommentPreviewController' => 'DifferentialController',
@@ -2954,7 +2951,6 @@ phutil_register_library_map(array(
       5 => 'HarbormasterBuildableInterface',
       6 => 'PhabricatorSubscribableInterface',
     ),
-    'DifferentialRevisionCommentListView' => 'AphrontView',
     'DifferentialRevisionCommentView' => 'AphrontView',
     'DifferentialRevisionDetailView' => 'AphrontView',
     'DifferentialRevisionEditController' => 'DifferentialController',
@@ -2981,6 +2977,8 @@ phutil_register_library_map(array(
     'DifferentialTitleFieldSpecification' => 'DifferentialFreeformFieldSpecification',
     'DifferentialTransaction' => 'PhabricatorApplicationTransaction',
     'DifferentialTransactionComment' => 'PhabricatorApplicationTransactionComment',
+    'DifferentialTransactionQuery' => 'PhabricatorApplicationTransactionQuery',
+    'DifferentialTransactionView' => 'PhabricatorApplicationTransactionView',
     'DifferentialUnitFieldSpecification' => 'DifferentialFieldSpecification',
     'DifferentialViewPolicyFieldSpecification' => 'DifferentialFieldSpecification',
     'DiffusionBranchTableController' => 'DiffusionController',

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())
+    $revisions = id(new DifferentialRevisionQuery())
-      ->withRevisionIDs($revision_ids)
+      ->setViewer($request->getUser())
+      ->withIDs($revision_ids)
       ->execute();
 
-    $with_inlines = $request->getValue('inlines');
+    if (!$revisions) {
-    if ($with_inlines) {
+      return $results;
-      $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');
-      }
     }
 
+    $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 = idx($revisions, $comment->getRevisionPHID());
-      $revision_id = $comment->getRevisionID();
+      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) {
+      $results[$revision->getID()][] = $result;
-        $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;
     }
 
     return $results;

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;
@@ -117,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])) {

src/applications/differential/controller/DifferentialRevisionViewController.php

@@ -258,21 +258,25 @@ final class DifferentialRevisionViewController extends DifferentialController {
     $revision_detail->setActions($actions);
     $revision_detail->setUser($user);
 
-    $comment_view = new DifferentialRevisionCommentListView();
+    $comment_view = $this->buildTransactions(
-    $comment_view->setComments($comments);
+      $revision,
-    $comment_view->setHandles($handles);
+      $diff_vs ? $diffs[$diff_vs] : $target,
-    $comment_view->setInlineComments($inlines);
+      $target,
-    $comment_view->setChangesets($all_changesets);
+      $all_changesets);
-    $comment_view->setUser($user);
+
-    $comment_view->setTargetDiff($target);
+    $wrap_id = celerity_generate_unique_node_id();
-    $comment_view->setVersusDiffID($diff_vs);
+    $comment_view = phutil_tag(
-    $comment_view->setRevision($revision);
+      '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,
         ));
     }
@@ -926,4 +930,38 @@ final class DifferentialRevisionViewController extends DifferentialController {
     return id(new AphrontRedirectResponse())->setURI($file->getBestURI());
 
   }
+
+  private function buildTransactions(
+    DifferentialRevision $revision,
+    DifferentialDiff $left_diff,
+    DifferentialDiff $right_diff,
+    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)
+      ->setRevision($revision)
+      ->setLeftDiff($left_diff)
+      ->setRightDiff($right_diff)
+      ->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;
+  }
+
 }

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
@@ -624,7 +630,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 +640,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 +650,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
+        $inline_xaction_comment = $inline->getTransactionCommentForSave();
-        // them their own transactions, but this would be fairly gross during
+        $inline_xaction_comment->setRevisionPHID($revision->getPHID());
-        // the storage transition and we'll have to do special thing with these
+
-        // during migration anyway.
+        $comments[] = id(clone $template)
-        $inline->setCommentID($last_comment->getID());
+          ->setAction(DifferentialTransaction::TYPE_INLINE)
-        $inline->save();
+          ->setProxyComment($inline_xaction_comment)
+          ->save();
       }
     }
 
@@ -702,7 +707,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())

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)

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);

src/applications/differential/query/DifferentialCommentQuery.php

@@ -6,67 +6,29 @@
 final class DifferentialCommentQuery
   extends PhabricatorOffsetPagedQuery {
 
-  private $revisionIDs;
+  private $revisionPHIDs;
 
-  public function withRevisionIDs(array $ids) {
+  public function withRevisionPHIDs(array $phids) {
-    $this->revisionIDs = $ids;
+    $this->revisionPHIDs = $phids;
     return $this;
   }
 
   public function execute() {
-    $table = new DifferentialComment();
+    // TODO: We're getting rid of this, it is the bads.
-    $conn_r = $table->establishConnection('r');
+    $viewer = PhabricatorUser::getOmnipotentUser();
 
-    $data = queryfx_all(
+    $xactions = id(new DifferentialTransactionQuery())
-      $conn_r,
+      ->setViewer($viewer)
-      'SELECT * FROM %T %Q %Q',
+      ->withObjectPHIDs($this->revisionPHIDs)
-      $table->getTableName(),
+      ->needComments(true)
-      $this->buildWhereClause($conn_r),
+      ->execute();
-      $this->buildLimitClause($conn_r));
 
-    $comments = $table->loadAllFromArray($data);
+    $results = array();
-
+    foreach ($xactions as $xaction) {
-    // We've moved the actual text storage into DifferentialTransactionComment,
+      $results[] = DifferentialComment::newFromModernTransaction($xaction);
-    // so load the relevant pieces of text we need.
-    if ($comments) {
-      $this->loadCommentText($comments);
     }
 
-    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);
-      }
-    }
-  }
-
-
 }

src/applications/differential/query/DifferentialTransactionQuery.php

@@ -0,0 +1,10 @@
+<?php
+
+final class DifferentialTransactionQuery
+  extends PhabricatorApplicationTransactionQuery {
+
+  public function getTemplateApplicationTransaction() {
+    return new DifferentialTransaction();
+  }
+
+}

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())

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();
+    switch ($this->proxy->getTransactionType()) {
-
+      case PhabricatorTransactions::TYPE_SUBSCRIBERS:
-    $metadata = $this->getMetadata();
+      case PhabricatorTransactions::TYPE_EDGE:
-    $added_reviewers = idx(
+        return array_merge(
-      $metadata,
+          array_keys($this->proxy->getOldValue()),
-      self::METADATA_ADDED_REVIEWERS);
+          array_keys($this->proxy->getNewValue()));
-    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;
-      }
     }
 
-    return $phids;
+    return array();
-  }
-
-  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);
   }
 
   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();
+    $this->proxy->openTransaction();
-      $result = parent::save();
+      $this->proxy
+        ->setViewPolicy('public')
+        ->setEditPolicy($this->getAuthorPHID())
+        ->save();
 
-      if ($this->getContent() !== null) {
+      if ($this->getContent() !== null ||
-        $content_source = PhabricatorContentSource::newForSource(
+          $this->getProxyComment()->getChangesetID()) {
-          PhabricatorContentSource::SOURCE_LEGACY,
-          array());
 
-        $xaction_phid = PhabricatorPHID::generateNewPHID(
+        $this->getProxyComment()
-          PhabricatorApplicationTransactionPHIDTypeTransaction::TYPECONST,
-          DifferentialPHIDTypeRevision::TYPECONST);
-
-        $proxy = $this->getProxyComment();
-        $proxy
           ->setAuthorPHID($this->getAuthorPHID())
           ->setViewPolicy('public')
           ->setEditPolicy($this->getAuthorPHID())
-          ->setContentSource($content_source)
           ->setCommentVersion(1)
-          ->setLegacyCommentID($this->getID())
+          ->setTransactionPHID($this->proxy->getPHID())
-          ->setTransactionPHID($xaction_phid)
+          ->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;
   }
 
 }

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)
+      ->setCommentVersion(1);
-      ->save();
+
+    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) {

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();

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';
   }
@@ -14,4 +18,101 @@ final class DifferentialTransaction extends PhabricatorApplicationTransaction {
     return new DifferentialTransactionComment();
   }
 
+  public function getTitle() {
+    $author_phid = $this->getAuthorPHID();
+    $author_handle = $this->renderHandleLink($author_phid);
+
+    $old = $this->getOldValue();
+    $new = $this->getNewValue();
+
+    switch ($this->getTransactionType()) {
+      case self::TYPE_INLINE:
+        return pht(
+          '%s added inline comments.',
+          $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();
+  }
+
 }

src/applications/differential/view/DifferentialRevisionCommentListView.php

@@ -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->getID()) {
-          // 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));
-  }
-}

src/applications/differential/view/DifferentialTransactionView.php

@@ -0,0 +1,191 @@
+<?php
+
+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');
+    $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) {
+        // Sort the group of inlines by line number.
+        $by_line = array();
+        foreach ($group as $inline) {
+          $by_line[] = array(
+            'line' => ((int)$inline->getComment()->getLineNumber() << 16) +
+                      ((int)$inline->getComment()->getLineLength()),
+            'inline' => $inline,
+          );
+        }
+        $by_line = isort($by_line, 'line');
+        $group = ipull($by_line, 'inline');
+
+        $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),
+          );
+
+          $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;
+        }
+        $inline_view->addCommentGroup(
+          $changeset->getFilename(),
+          $items);
+      }
+
+      $out[] = $inline_view;
+    }
+
+    return $out;
+  }
+
+}

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;
       }
     }
 
@@ -319,28 +343,41 @@ final class PhabricatorFeedStoryDifferential extends PhabricatorFeedStory {
     }
 
     // Roughly render inlines into the comment.
-    $comment_id = $data->getValue('temporaryCommentID');
+    $xaction_phids = $data->getValue('temporaryTransactionPHIDs');
-    if ($comment_id) {
+    if ($xaction_phids) {
-      $inlines = id(new DifferentialInlineCommentQuery())
+      $inlines = id(new DifferentialTransactionQuery())
-        ->withCommentIDs(array($comment_id))
+        ->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();
+          $linenumber = $comment->getLineNumber();
-          $inline_text = $engine->markupText($inline->getContent());
+          $inline_text = $engine->markupText($comment->getContent());
           $inline_text = rtrim($inline_text);
 
           $title .= "{$filename}:{$linenumber} {$inline_text}\n";

src/applications/pholio/storage/PholioTransaction.php

@@ -1,8 +1,5 @@
 <?php
 
-/**
- * @group pholio
- */
 final class PholioTransaction extends PhabricatorApplicationTransaction {
 
   public function getApplicationName() {

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;
     }
 

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();