Render Differential previews through ApplicationTransactions

Summary: Ref T2222. Use new code for rendering. Delete `DifferentialRevisionCommentView`, which has no remaining callsites.

Test Plan: Went through all the different actions and verified the previews rendered correctly. Reloaded page to test draft behavior.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T2222

Differential Revision: https://secure.phabricator.com/D8236

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',
-    'DifferentialRevisionCommentView' => 'applications/differential/view/DifferentialRevisionCommentView.php',
     'DifferentialRevisionControlSystem' => 'applications/differential/constants/DifferentialRevisionControlSystem.php',
     'DifferentialRevisionDetailRenderer' => 'applications/differential/controller/DifferentialRevisionDetailRenderer.php',
     'DifferentialRevisionDetailView' => 'applications/differential/view/DifferentialRevisionDetailView.php',
@@ -2957,7 +2956,6 @@ phutil_register_library_map(array(
       5 => 'HarbormasterBuildableInterface',
       6 => 'PhabricatorSubscribableInterface',
     ),
-    'DifferentialRevisionCommentView' => 'AphrontView',
     'DifferentialRevisionDetailView' => 'AphrontView',
     'DifferentialRevisionEditController' => 'DifferentialController',
     'DifferentialRevisionEditor' => 'PhabricatorEditor',

src/applications/differential/controller/DifferentialCommentPreviewController.php

@@ -21,45 +21,95 @@ final class DifferentialCommentPreviewController
       return new Aphront404Response();
     }
 
-    $author_phid = $viewer->getPHID();
+    $type_comment = PhabricatorTransactions::TYPE_COMMENT;
+    $type_action = DifferentialTransaction::TYPE_ACTION;
+    $type_edge = PhabricatorTransactions::TYPE_EDGE;
+    $type_subscribers = PhabricatorTransactions::TYPE_SUBSCRIBERS;
+
+    $xactions = array();
+
     $action = $request->getStr('action');
+    switch ($action) {
+      case DifferentialAction::ACTION_COMMENT:
+      case DifferentialAction::ACTION_ADDREVIEWERS:
+      case DifferentialAction::ACTION_ADDCCS:
+        break;
+      default:
+        $xactions[] = id(new DifferentialTransaction())
+          ->setTransactionType($type_action)
+          ->setNewValue($action);
+        break;
+    }
 
-    $comment = new DifferentialComment();
+    $edge_reviewer = PhabricatorEdgeConfig::TYPE_DREV_HAS_REVIEWER;
-    $comment->setContent($request->getStr('content'));
-    $comment->setAction($action);
-    $comment->setAuthorPHID($author_phid);
-
-    $handles = array($author_phid);
 
     $reviewers = $request->getStrList('reviewers');
     if (DifferentialAction::allowReviewers($action) && $reviewers) {
-      $comment->setMetadata(array(
+      $faux_edges = array();
-        DifferentialComment::METADATA_ADDED_REVIEWERS => $reviewers));
+      foreach ($reviewers as $phid) {
-      $handles = array_merge($handles, $reviewers);
+        $faux_edges[$phid] = array(
+          'src' => $revision->getPHID(),
+          'type' => $edge_reviewer,
+          'dst' => $phid,
+        );
+      }
+
+      $xactions[] = id(new DifferentialTransaction())
+        ->setTransactionType($type_edge)
+        ->setMetadataValue('edge:type', $edge_reviewer)
+        ->setOldValue(array())
+        ->setNewValue($faux_edges);
     }
 
     $ccs = $request->getStrList('ccs');
-    if ($action == DifferentialAction::ACTION_ADDCCS && $ccs) {
+    if ($ccs) {
-      $comment->setMetadata(array(
+      $xactions[] = id(new DifferentialTransaction())
-        DifferentialComment::METADATA_ADDED_CCS => $ccs));
+        ->setTransactionType($type_subscribers)
-      $handles = array_merge($handles, $ccs);
+        ->setOldValue(array())
+        ->setNewValue(array_fuse($ccs));
     }
 
-    $handles = $this->loadViewerHandles($handles);
+    // Add a comment transaction if there's nothing, so we'll generate a
+    // nonempty result.
+    if (strlen($request->getStr('content')) || !$xactions) {
+      $xactions[] = id(new DifferentialTransaction())
+        ->setTransactionType($type_comment)
+        ->attachComment(
+          id(new ManiphestTransactionComment())
+            ->setContent($request->getStr('content')));
+    }
+
+    foreach ($xactions as $xaction) {
+      $xaction->setAuthorPHID($viewer->getPHID());
+    }
 
     $engine = new PhabricatorMarkupEngine();
     $engine->setViewer($request->getUser());
-    $engine->addObject($comment, DifferentialComment::MARKUP_FIELD_BODY);
+    foreach ($xactions as $xaction) {
+      if ($xaction->hasComment()) {
+        $engine->addObject(
+          $xaction->getComment(),
+          PhabricatorApplicationTransactionComment::MARKUP_FIELD_COMMENT);
+      }
+    }
     $engine->process();
 
-    $view = new DifferentialRevisionCommentView();
+    $phids = mpull($xactions, 'getRequiredHandlePHIDs');
-    $view->setUser($request->getUser());
+    $phids = array_mergev($phids);
-    $view->setComment($comment);
+
-    $view->setHandles($handles);
+    $handles = id(new PhabricatorHandleQuery())
-    $view->setMarkupEngine($engine);
+      ->setViewer($viewer)
-    $view->setRevision($revision);
+      ->withPHIDs($phids)
-    $view->setPreview(true);
+      ->execute();
-    $view->setTargetDiff(null);
+
+    foreach ($xactions as $xaction) {
+      $xaction->setHandles($handles);
+    }
+
+    $view = id(new DifferentialTransactionView())
+      ->setUser($viewer)
+      ->setTransactions($xactions)
+      ->setIsPreview(true);
 
     $metadata = array(
       'reviewers' => $reviewers,
@@ -70,14 +120,14 @@ final class DifferentialCommentPreviewController
     }
 
     id(new PhabricatorDraft())
-      ->setAuthorPHID($author_phid)
+      ->setAuthorPHID($viewer->getPHID())
       ->setDraftKey('differential-comment-'.$this->id)
-      ->setDraft($comment->getContent())
+      ->setDraft($request->getStr('content'))
       ->setMetadata($metadata)
       ->replaceOrDelete();
 
     return id(new AphrontAjaxResponse())
-      ->setContent($view->render());
+      ->setContent((string)phutil_implode_html('', $view->buildEvents()));
   }
 
 }

src/applications/differential/view/DifferentialRevisionCommentView.php

@@ -1,305 +0,0 @@
-<?php
-
-final class DifferentialRevisionCommentView extends AphrontView {
-
-  private $comment;
-  private $handles;
-  private $markupEngine;
-  private $preview;
-  private $inlines;
-  private $changesets;
-  private $target;
-  private $anchorName;
-  private $versusDiffID;
-  private $revision;
-
-  public function setRevision(DifferentialRevision $revision) {
-    $this->revision = $revision;
-    return $this;
-  }
-
-  public function getRevision() {
-    return $this->revision;
-  }
-
-  public function setComment($comment) {
-    $this->comment = $comment;
-    return $this;
-  }
-
-  public function setHandles(array $handles) {
-    assert_instances_of($handles, 'PhabricatorObjectHandle');
-    $this->handles = $handles;
-    return $this;
-  }
-
-  public function setMarkupEngine(PhabricatorMarkupEngine $markup_engine) {
-    $this->markupEngine = $markup_engine;
-    return $this;
-  }
-
-  public function setPreview($preview) {
-    $this->preview = $preview;
-    return $this;
-  }
-
-  public function setInlineComments(array $inline_comments) {
-    assert_instances_of($inline_comments, 'PhabricatorInlineCommentInterface');
-    $this->inlines = $inline_comments;
-    return $this;
-  }
-
-  public function setChangesets(array $changesets) {
-    assert_instances_of($changesets, 'DifferentialChangeset');
-    // Ship these in sorted by getSortKey() and keyed by ID... or else!
-    $this->changesets = $changesets;
-    return $this;
-  }
-
-  public function setTargetDiff($target) {
-    $this->target = $target;
-    return $this;
-  }
-
-  public function setVersusDiffID($diff_vs) {
-    $this->versusDiffID = $diff_vs;
-    return $this;
-  }
-
-  public function setAnchorName($anchor_name) {
-    $this->anchorName = $anchor_name;
-    return $this;
-  }
-
-  public function render() {
-
-    if (!$this->user) {
-      throw new Exception("Call setUser() before rendering!");
-    }
-
-    $this->requireResource('phabricator-remarkup-css');
-    $this->requireResource('differential-revision-comment-css');
-
-    $comment = $this->comment;
-
-    $action = $comment->getAction();
-
-    $action_class = 'differential-comment-action-'.$action;
-
-    $info = array();
-
-    $content = $comment->getContent();
-    $hide_comments = true;
-    if (strlen(rtrim($content))) {
-      $hide_comments = false;
-
-      $content = $this->markupEngine->getOutput(
-        $comment,
-        PhabricatorInlineCommentInterface::MARKUP_FIELD_BODY);
-
-      $content = phutil_tag_div('phabricator-remarkup', $content);
-    }
-
-    $inline_render = $this->renderInlineComments();
-    if ($inline_render) {
-      $hide_comments = false;
-    }
-
-    $author = $this->handles[$comment->getAuthorPHID()];
-    $author_link = $author->renderLink();
-
-    $metadata = $comment->getMetadata();
-    $added_reviewers = idx(
-      $metadata,
-      DifferentialComment::METADATA_ADDED_REVIEWERS,
-      array());
-    $removed_reviewers = idx(
-      $metadata,
-      DifferentialComment::METADATA_REMOVED_REVIEWERS,
-      array());
-    $added_ccs = idx(
-      $metadata,
-      DifferentialComment::METADATA_ADDED_CCS,
-      array());
-
-    $actions = array();
-    switch ($comment->getAction()) {
-      case DifferentialAction::ACTION_ADDCCS:
-        $actions[] = pht(
-          "%s added CCs: %s.",
-          $author_link,
-          $this->renderHandleList($added_ccs));
-        $added_ccs = null;
-        break;
-      case DifferentialAction::ACTION_ADDREVIEWERS:
-        $actions[] = pht(
-          "%s added reviewers: %s.",
-          $author_link,
-          $this->renderHandleList($added_reviewers));
-        $added_reviewers = null;
-        break;
-      case DifferentialAction::ACTION_UPDATE:
-        $diff_id = idx($metadata, DifferentialComment::METADATA_DIFF_ID);
-        if ($diff_id) {
-          $diff_link = phutil_tag(
-            'a',
-            array(
-              'href' => '/D'.$this->getRevision()->getID().'?id='.$diff_id,
-            ),
-            'Diff #'.$diff_id);
-          $actions[] = pht(
-            "%s updated this revision to %s.",
-            $author_link,
-            $diff_link);
-        } else {
-          $actions[] = DifferentialAction::getBasicStoryText(
-            $comment->getAction(), $author_link);
-        }
-        break;
-      default:
-          $actions[] = DifferentialAction::getBasicStoryText(
-            $comment->getAction(), $author_link);
-        break;
-    }
-
-    if ($added_reviewers) {
-      $actions[] = pht(
-        "%s added reviewers: %s.",
-        $author_link,
-        $this->renderHandleList($added_reviewers));
-    }
-
-    if ($removed_reviewers) {
-      $actions[] = pht(
-        "%s removed reviewers: %s.",
-        $author_link,
-        $this->renderHandleList($removed_reviewers));
-    }
-
-    if ($added_ccs) {
-      $actions[] = pht(
-        "%s added CCs: %s.",
-        $author_link,
-        $this->renderHandleList($added_ccs));
-    }
-
-    foreach ($actions as $key => $action) {
-      $actions[$key] = phutil_tag('div', array(), $action);
-    }
-
-    $xaction_view = id(new PhabricatorTransactionView())
-      ->setUser($this->user)
-      ->setImageURI($author->getImageURI())
-      ->setContentSource($comment->getContentSource())
-      ->addClass($action_class)
-      ->setActions($actions);
-
-    if ($this->preview) {
-      $xaction_view->setIsPreview($this->preview);
-    } else {
-      $xaction_view->setEpoch($comment->getDateCreated());
-      if ($this->anchorName) {
-        $anchor_text =
-          'D'.$this->getRevision()->getID().
-          '#'.preg_replace('/^comment-/', '', $this->anchorName);
-
-        $xaction_view->setAnchor($this->anchorName, $anchor_text);
-      }
-    }
-
-    if (!$hide_comments) {
-      $xaction_view->appendChild(phutil_tag_div(
-        'differential-comment-core',
-        array($content, $inline_render)));
-    }
-
-    return $xaction_view->render();
-  }
-
-  private function renderHandleList(array $phids) {
-    $result = array();
-    foreach ($phids as $phid) {
-      $result[] = $this->handles[$phid]->renderLink();
-    }
-    return phutil_implode_html(', ', $result);
-  }
-
-  private function renderInlineComments() {
-    if (!$this->inlines) {
-      return null;
-    }
-
-    $inlines = $this->inlines;
-    $changesets = $this->changesets;
-    $inlines_by_changeset = mgroup($inlines, 'getChangesetID');
-    $inlines_by_changeset = array_select_keys(
-      $inlines_by_changeset,
-      array_keys($this->changesets));
-
-    $view = new PhabricatorInlineSummaryView();
-    foreach ($inlines_by_changeset as $changeset_id => $inlines) {
-      $changeset = $changesets[$changeset_id];
-      $items = array();
-      foreach ($inlines as $inline) {
-
-        $on_target = ($this->target) &&
-                     ($this->target->getID() == $changeset->getDiffID());
-
-        $is_visible = false;
-        if ($inline->getIsNewFile()) {
-          // This comment is on the right side of the versus diff, and visible
-          // on the left side of the page.
-          if ($this->versusDiffID) {
-            if ($changeset->getDiffID() == $this->versusDiffID) {
-              $is_visible = true;
-            }
-          }
-
-          // This comment is on the right side of the target diff, and visible
-          // on the right side of the page.
-          if ($on_target) {
-            $is_visible = true;
-          }
-        } else {
-          // Ths comment is on the left side of the target diff, and visible
-          // on the left side of the page.
-          if (!$this->versusDiffID) {
-            if ($on_target) {
-              $is_visible = true;
-            }
-          }
-
-          // 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.
-        }
-
-        $item = array(
-          'id'      => $inline->getID(),
-          'line'    => $inline->getLineNumber(),
-          'length'  => $inline->getLineLength(),
-          'content' => $this->markupEngine->getOutput(
-            $inline,
-            DifferentialInlineComment::MARKUP_FIELD_BODY),
-        );
-
-        if (!$is_visible) {
-          $diff_id = $changeset->getDiffID();
-          $item['where'] = '(On Diff #'.$diff_id.')';
-          $item['href'] =
-            'D'.$this->getRevision()->getID().
-            '?id='.$diff_id.
-            '#inline-'.$inline->getID();
-        }
-
-        $items[] = $item;
-      }
-      $view->addCommentGroup($changeset->getFilename(), $items);
-    }
-
-    return $view;
-  }
-
-}