Convert Audit comment rendering to standard infrastructure

Summary: Ref T4896. Depends on D10055. This uses core rendering stuff for audit comments, and fixes all the wonkiness with inlines so we can actually land the migration.

Test Plan: Viewed, previewed and edited various types of comments in Diffusion.

Reviewers: chad, btrahan, joshuaspence

Reviewed By: joshuaspence

Subscribers: epriestley

Maniphest Tasks: T4896

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

resources/celerity/map.php

@@ -12,7 +12,7 @@ return array(
     'darkconsole.pkg.js' => 'df001cab',
     'differential.pkg.css' => '4a93db37',
     'differential.pkg.js' => '7528cfc9',
-    'diffusion.pkg.css' => '471bc9eb',
+    'diffusion.pkg.css' => '591664fa',
     'diffusion.pkg.js' => 'bfc0737b',
     'maniphest.pkg.css' => 'f5d89daf',
     'maniphest.pkg.js' => 'df4aa49f',
@@ -62,7 +62,6 @@ return array(
     'rsrc/css/application/differential/revision-history.css' => '0e8eb855',
     'rsrc/css/application/differential/revision-list.css' => 'f3c47d33',
     'rsrc/css/application/differential/table-of-contents.css' => '6bf8e1d2',
-    'rsrc/css/application/diffusion/commit-view.css' => '92d1e8f9',
     'rsrc/css/application/diffusion/diffusion-icons.css' => '9c5828da',
     'rsrc/css/application/diffusion/diffusion-source.css' => '66fdf661',
     'rsrc/css/application/feed/feed.css' => '4e544db4',
@@ -534,7 +533,6 @@ return array(
     'differential-revision-history-css' => '0e8eb855',
     'differential-revision-list-css' => 'f3c47d33',
     'differential-table-of-contents-css' => '6bf8e1d2',
-    'diffusion-commit-view-css' => '92d1e8f9',
     'diffusion-icons-css' => '9c5828da',
     'diffusion-source-css' => '66fdf661',
     'diviner-shared-css' => '38813222',
@@ -2137,7 +2135,6 @@ return array(
       'javelin-behavior-aphront-more',
     ),
     'diffusion.pkg.css' => array(
-      'diffusion-commit-view-css',
       'diffusion-icons-css',
     ),
     'diffusion.pkg.js' => array(

resources/celerity/packages.php

@@ -164,7 +164,6 @@ return array(
     'javelin-behavior-aphront-more',
   ),
   'diffusion.pkg.css' => array(
-    'diffusion-commit-view-css',
     'diffusion-icons-css',
   ),
   'diffusion.pkg.js' => array(

src/__phutil_library_map__.php

@@ -364,8 +364,6 @@ phutil_register_library_map(array(
     'DiffusionBrowseSearchController' => 'applications/diffusion/controller/DiffusionBrowseSearchController.php',
     'DiffusionBrowseTableView' => 'applications/diffusion/view/DiffusionBrowseTableView.php',
     'DiffusionChangeController' => 'applications/diffusion/controller/DiffusionChangeController.php',
-    'DiffusionCommentListView' => 'applications/diffusion/view/DiffusionCommentListView.php',
-    'DiffusionCommentView' => 'applications/diffusion/view/DiffusionCommentView.php',
     'DiffusionCommitBranchesController' => 'applications/diffusion/controller/DiffusionCommitBranchesController.php',
     'DiffusionCommitChangeTableView' => 'applications/diffusion/view/DiffusionCommitChangeTableView.php',
     'DiffusionCommitController' => 'applications/diffusion/controller/DiffusionCommitController.php',
@@ -1153,6 +1151,7 @@ phutil_register_library_map(array(
     'PhabricatorAuditTransaction' => 'applications/audit/storage/PhabricatorAuditTransaction.php',
     'PhabricatorAuditTransactionComment' => 'applications/audit/storage/PhabricatorAuditTransactionComment.php',
     'PhabricatorAuditTransactionQuery' => 'applications/audit/query/PhabricatorAuditTransactionQuery.php',
+    'PhabricatorAuditTransactionView' => 'applications/audit/view/PhabricatorAuditTransactionView.php',
     'PhabricatorAuthAccountView' => 'applications/auth/view/PhabricatorAuthAccountView.php',
     'PhabricatorAuthApplication' => 'applications/auth/application/PhabricatorAuthApplication.php',
     'PhabricatorAuthAuthFactorPHIDType' => 'applications/auth/phid/PhabricatorAuthAuthFactorPHIDType.php',
@@ -3099,8 +3098,6 @@ phutil_register_library_map(array(
     'DiffusionBrowseSearchController' => 'DiffusionBrowseController',
     'DiffusionBrowseTableView' => 'DiffusionView',
     'DiffusionChangeController' => 'DiffusionController',
-    'DiffusionCommentListView' => 'AphrontView',
-    'DiffusionCommentView' => 'AphrontView',
     'DiffusionCommitBranchesController' => 'DiffusionController',
     'DiffusionCommitChangeTableView' => 'DiffusionView',
     'DiffusionCommitController' => 'DiffusionController',
@@ -3944,6 +3941,7 @@ phutil_register_library_map(array(
     'PhabricatorAuditTransaction' => 'PhabricatorApplicationTransaction',
     'PhabricatorAuditTransactionComment' => 'PhabricatorApplicationTransactionComment',
     'PhabricatorAuditTransactionQuery' => 'PhabricatorApplicationTransactionQuery',
+    'PhabricatorAuditTransactionView' => 'PhabricatorApplicationTransactionView',
     'PhabricatorAuthAccountView' => 'AphrontView',
     'PhabricatorAuthApplication' => 'PhabricatorApplication',
     'PhabricatorAuthAuthFactorPHIDType' => 'PhabricatorPHIDType',

src/applications/audit/controller/PhabricatorAuditPreviewController.php

@@ -18,83 +18,66 @@ final class PhabricatorAuditPreviewController
       return new Aphront404Response();
     }
 
+    $xactions = array();
+
     $action = $request->getStr('action');
-
-    $phids = array(
-      $user->getPHID(),
-      $commit->getPHID(),
-    );
-
-    $comments = array();
-
     if ($action != PhabricatorAuditActionConstants::COMMENT) {
-      $action_comment = id(new PhabricatorAuditComment())
-        ->setActorPHID($user->getPHID())
-        ->setTargetPHID($commit->getPHID())
-        ->setAction($action);
+      $action_xaction = id(new PhabricatorAuditTransaction())
+        ->setAuthorPHID($user->getPHID())
+        ->setObjectPHID($commit->getPHID())
+        ->setTransactionType(PhabricatorAuditActionConstants::ACTION)
+        ->setNewValue($action);
 
       $auditors = $request->getStrList('auditors');
       if ($action == PhabricatorAuditActionConstants::ADD_AUDITORS &&
         $auditors) {
-
-        $action_comment->setMetadata(array(
-          PhabricatorAuditComment::METADATA_ADDED_AUDITORS => $auditors));
-        $phids = array_merge($phids, $auditors);
+        $action_xaction->setTransactionType($action);
+        $action_xaction->setNewValue(array_fuse($auditors));
       }
 
       $ccs = $request->getStrList('ccs');
       if ($action == PhabricatorAuditActionConstants::ADD_CCS && $ccs) {
-        $action_comment->setMetadata(array(
-          PhabricatorAuditComment::METADATA_ADDED_CCS => $ccs));
-        $phids = array_merge($phids, $ccs);
+        $action_xaction->setTransactionType($action);
+        $action_xaction->setNewValue(array_fuse($ccs));
       }
 
-      $comments[] = $action_comment;
+      $xactions[] = $action_xaction;
     }
 
     $content = $request->getStr('content');
     if (strlen($content)) {
-      $comments[] = id(new PhabricatorAuditComment())
-        ->setActorPHID($user->getPHID())
-        ->setTargetPHID($commit->getPHID())
-        ->setAction(PhabricatorAuditActionConstants::COMMENT)
-        ->setContent($content);
+      $xactions[] = id(new PhabricatorAuditTransaction())
+        ->setAuthorPHID($user->getPHID())
+        ->setObjectPHID($commit->getPHID())
+        ->setTransactionType(PhabricatorTransactions::TYPE_COMMENT)
+        ->attachComment(
+          id(new PhabricatorAuditTransactionComment())
+            ->setContent($content));
     }
 
-    $engine = new PhabricatorMarkupEngine();
-    $engine->setViewer($user);
-    foreach ($comments as $comment) {
-      $engine->addObject(
-        $comment,
-        PhabricatorAuditComment::MARKUP_FIELD_BODY);
+    $phids = array();
+    foreach ($xactions as $xaction) {
+      $phids[] = $xaction->getRequiredHandlePHIDs();
     }
-    $engine->process();
-
-    $views = array();
-    foreach ($comments as $comment) {
-      $view = id(new DiffusionCommentView())
-        ->setMarkupEngine($engine)
-        ->setUser($user)
-        ->setComment($comment)
-        ->setIsPreview(true);
-
-      $phids = array_merge($phids, $view->getRequiredHandlePHIDs());
-      $views[] = $view;
-    }
-
+    $phids = array_mergev($phids);
     $handles = $this->loadViewerHandles($phids);
-
-    foreach ($views as $view) {
-      $view->setHandles($handles);
+    foreach ($xactions as $xaction) {
+      $xaction->setHandles($handles);
     }
 
+    $view = id(new PhabricatorAuditTransactionView())
+      ->setIsPreview(true)
+      ->setUser($user)
+      ->setObjectPHID($commit->getPHID())
+      ->setTransactions($xactions);
+
     id(new PhabricatorDraft())
       ->setAuthorPHID($user->getPHID())
       ->setDraftKey('diffusion-audit-'.$this->id)
       ->setDraft($content)
       ->replaceOrDelete();
 
-    return id(new AphrontAjaxResponse())->setContent(hsprintf('%s', $views));
+    return id(new AphrontAjaxResponse())->setContent(hsprintf('%s', $view));
   }
 
 }

src/applications/audit/storage/PhabricatorAuditTransaction.php

@@ -15,4 +15,144 @@ final class PhabricatorAuditTransaction
     return new PhabricatorAuditTransactionComment();
   }
 
+  public function getRequiredHandlePHIDs() {
+    $phids = parent::getRequiredHandlePHIDs();
+
+    $type = $this->getTransactionType();
+
+    switch ($type) {
+      case PhabricatorAuditActionConstants::ADD_CCS:
+      case PhabricatorAuditActionConstants::ADD_AUDITORS:
+        $old = $this->getOldValue();
+        $new = $this->getNewValue();
+
+        if (!is_array($old)) {
+          $old = array();
+        }
+        if (!is_array($new)) {
+          $new = array();
+        }
+
+        foreach (array_keys($old + $new) as $phid) {
+          $phids[] = $phid;
+        }
+        break;
+    }
+
+    return $phids;
+  }
+
+  public function getColor() {
+
+    $type = $this->getTransactionType();
+
+    switch ($type) {
+      case PhabricatorAuditActionConstants::ACTION:
+        switch ($this->getNewValue()) {
+          case PhabricatorAuditActionConstants::CONCERN:
+            return 'red';
+          case PhabricatorAuditActionConstants::ACCEPT:
+            return 'green';
+        }
+    }
+
+    return parent::getColor();
+  }
+
+  public function getTitle() {
+    $old = $this->getOldValue();
+    $new = $this->getNewValue();
+
+    $author_handle = $this->getHandle($this->getAuthorPHID())->renderLink();
+
+    $type = $this->getTransactionType();
+
+    switch ($type) {
+      case PhabricatorAuditActionConstants::ADD_CCS:
+      case PhabricatorAuditActionConstants::ADD_AUDITORS:
+        if (!is_array($old)) {
+          $old = array();
+        }
+        if (!is_array($new)) {
+          $new = array();
+        }
+        $add = array_keys(array_diff_key($new, $old));
+        $rem = array_keys(array_diff_key($old, $new));
+        break;
+    }
+
+    switch ($type) {
+      case PhabricatorAuditActionConstants::INLINE:
+        break;
+      case PhabricatorAuditActionConstants::ADD_CCS:
+        if ($add && $rem) {
+          return pht(
+            '%s edited subscribers; added: %s, removed: %s.',
+            $author_handle,
+            $this->renderHandleList($add),
+            $this->renderHandleList($rem));
+        } else if ($add) {
+          return pht(
+            '%s added subscribers: %s.',
+            $author_handle,
+            $this->renderHandleList($add));
+        } else if ($rem) {
+          return pht(
+            '%s removed subscribers: %s.',
+            $author_handle,
+            $this->renderHandleList($rem));
+        } else {
+          return pht(
+            '%s added subscribers...',
+            $author_handle);
+        }
+
+      case PhabricatorAuditActionConstants::ADD_AUDITORS:
+        if ($add && $rem) {
+          return pht(
+            '%s edited auditors; added: %s, removed: %s.',
+            $author_handle,
+            $this->renderHandleList($add),
+            $this->renderHandleList($rem));
+        } else if ($add) {
+          return pht(
+            '%s added auditors: %s.',
+            $author_handle,
+            $this->renderHandleList($add));
+        } else if ($rem) {
+          return pht(
+            '%s removed auditors: %s.',
+            $author_handle,
+            $this->renderHandleList($rem));
+        } else {
+          return pht(
+            '%s added auditors...',
+            $author_handle);
+        }
+
+      case PhabricatorAuditActionConstants::ACTION:
+        switch ($new) {
+          case PhabricatorAuditActionConstants::ACCEPT:
+            return pht(
+              '%s accepted this commit.',
+              $author_handle);
+          case PhabricatorAuditActionConstants::CONCERN:
+            return pht(
+              '%s raised a concern with this commit.',
+              $author_handle);
+          case PhabricatorAuditActionConstants::RESIGN:
+            return pht(
+              '%s resigned from this audit.',
+              $author_handle);
+          case PhabricatorAuditActionConstants::CLOSE:
+            return pht(
+              '%s closed this audit.',
+              $author_handle);
+        }
+
+    }
+
+    return parent::getTitle();
+  }
+
 }

src/applications/audit/view/PhabricatorAuditTransactionView.php

@@ -0,0 +1,124 @@
+<?php
+
+final class PhabricatorAuditTransactionView
+  extends PhabricatorApplicationTransactionView {
+
+  private $pathMap;
+
+  public function setPathMap(array $path_map) {
+    $this->pathMap = $path_map;
+    return $this;
+  }
+
+  public function getPathMap() {
+    return $this->pathMap;
+  }
+
+  // TODO: This shares a lot of code with Differential and Pholio and should
+  // probably be merged up.
+
+  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 PhabricatorAuditActionConstants::INLINE:
+        break;
+      default:
+        return false;
+    }
+
+    switch ($v->getTransactionType()) {
+      case PhabricatorAuditActionConstants::INLINE:
+        return true;
+    }
+
+    return parent::shouldGroupTransactions($u, $v);
+  }
+
+  protected function renderTransactionContent(
+    PhabricatorApplicationTransaction $xaction) {
+
+    $out = array();
+
+    $type_inline = PhabricatorAuditActionConstants::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 PhabricatorAuditActionConstants::INLINE:
+          $inlines[] = $xaction;
+          break;
+        default:
+          throw new Exception('Unknown grouped transaction type!');
+      }
+    }
+
+    if ($inlines) {
+
+      // TODO: This should do something similar to sortAndGroupInlines() to get
+      // a stable ordering.
+
+      $inlines_by_path = array();
+      foreach ($inlines as $key => $inline) {
+        $comment = $inline->getComment();
+        if (!$comment) {
+          // TODO: Migrate these away? They probably do not exist on normal
+          // non-development installs.
+          unset($inlines[$key]);
+          continue;
+        }
+        $path_id = $comment->getPathID();
+        $inlines_by_path[$path_id][] = $inline;
+      }
+
+      $inline_view = new PhabricatorInlineSummaryView();
+      foreach ($inlines_by_path as $path_id => $group) {
+        $path = idx($this->pathMap, $path_id);
+        if ($path === null) {
+          continue;
+        }
+
+        $items = array();
+        foreach ($group as $inline) {
+          $comment = $inline->getComment();
+          $item = array(
+            'id' => $comment->getID(),
+            'line' => $comment->getLineNumber(),
+            'length' => $comment->getLineLength(),
+            'content' => parent::renderTransactionContent($inline),
+          );
+          $items[] = $item;
+        }
+        $inline_view->addCommentGroup($path, $items);
+      }
+
+      $out[] = $inline_view;
+    }
+
+    return $out;
+  }
+
+}

src/applications/diffusion/controller/DiffusionCommitController.php

@@ -100,7 +100,6 @@ final class DiffusionCommitController extends DiffusionController {
       $engine = PhabricatorMarkupEngine::newDifferentialMarkupEngine();
       $engine->setConfig('viewer', $user);
 
-      require_celerity_resource('diffusion-commit-view-css');
       require_celerity_resource('phabricator-remarkup-css');
 
       $parents = $this->callConduitWithDiffusionRequest(
@@ -642,16 +641,23 @@ final class DiffusionCommitController extends DiffusionController {
   }
 
   private function buildComments(PhabricatorRepositoryCommit $commit) {
-    $user = $this->getRequest()->getUser();
-    $comments = PhabricatorAuditComment::loadComments(
-      $user,
-      $commit->getPHID());
+    $viewer = $this->getRequest()->getUser();
 
-    $inlines = PhabricatorAuditInlineComment::loadPublishedComments(
-      $user,
-      $commit->getPHID());
+    $xactions = id(new PhabricatorAuditTransactionQuery())
+      ->setViewer($viewer)
+      ->withObjectPHIDs(array($commit->getPHID()))
+      ->needComments(true)
+      ->execute();
 
-    $path_ids = mpull($inlines, 'getPathID');
+    $path_ids = array();
+    foreach ($xactions as $xaction) {
+      if ($xaction->hasComment()) {
+        $path_id = $xaction->getComment()->getPathID();
+        if ($path_id) {
+          $path_ids[] = $path_id;
+        }
+      }
+    }
 
     $path_map = array();
     if ($path_ids) {
@@ -661,35 +667,11 @@ final class DiffusionCommitController extends DiffusionController {
       $path_map = ipull($path_map, 'path', 'id');
     }
 
-    $engine = new PhabricatorMarkupEngine();
-    $engine->setViewer($user);
-
-    foreach ($comments as $comment) {
-      $engine->addObject(
-        $comment,
-        PhabricatorAuditComment::MARKUP_FIELD_BODY);
-    }
-
-    foreach ($inlines as $inline) {
-      $engine->addObject(
-        $inline,
-        PhabricatorInlineCommentInterface::MARKUP_FIELD_BODY);
-    }
-
-    $engine->process();
-
-    $view = new DiffusionCommentListView();
-    $view->setMarkupEngine($engine);
-    $view->setUser($user);
-    $view->setComments($comments);
-    $view->setInlineComments($inlines);
-    $view->setPathMap($path_map);
-
-    $phids = $view->getRequiredHandlePHIDs();
-    $handles = $this->loadViewerHandles($phids);
-    $view->setHandles($handles);
-
-    return $view;
+    return id(new PhabricatorAuditTransactionView())
+      ->setUser($viewer)
+      ->setObjectPHID($commit->getPHID())
+      ->setPathMap($path_map)
+      ->setTransactions($xactions);
   }
 
   private function renderAddCommentPanel(

src/applications/diffusion/view/DiffusionCommentListView.php

@@ -1,96 +0,0 @@
-<?php
-
-final class DiffusionCommentListView extends AphrontView {
-
-  private $comments;
-  private $inlineComments = array();
-  private $pathMap = array();
-  private $handles = array();
-  private $markupEngine;
-
-  public function setComments(array $comments) {
-    assert_instances_of($comments, 'PhabricatorAuditComment');
-    $this->comments = $comments;
-    return $this;
-  }
-
-  public function setInlineComments(array $inline_comments) {
-    assert_instances_of($inline_comments, 'PhabricatorInlineCommentInterface');
-    $this->inlineComments = $inline_comments;
-    return $this;
-  }
-
-  public function setPathMap(array $path_map) {
-    $this->pathMap = $path_map;
-    return $this;
-  }
-
-  public function setMarkupEngine(PhabricatorMarkupEngine $markup_engine) {
-    $this->markupEngine = $markup_engine;
-    return $this;
-  }
-
-  public function getMarkupEngine() {
-    return $this->markupEngine;
-  }
-
-  public function getRequiredHandlePHIDs() {
-    $phids = array();
-    foreach ($this->comments as $comment) {
-      $phids[$comment->getActorPHID()] = true;
-      $metadata = $comment->getMetaData();
-
-      $ccs_key = PhabricatorAuditComment::METADATA_ADDED_CCS;
-      $added_ccs = idx($metadata, $ccs_key, array());
-      foreach ($added_ccs as $cc) {
-        $phids[$cc] = true;
-      }
-      $auditors_key = PhabricatorAuditComment::METADATA_ADDED_AUDITORS;
-      $added_auditors = idx($metadata, $auditors_key, array());
-      foreach ($added_auditors as $auditor) {
-        $phids[$auditor] = true;
-      }
-    }
-    foreach ($this->inlineComments as $comment) {
-      $phids[$comment->getAuthorPHID()] = true;
-    }
-    return array_keys($phids);
-  }
-
-  public function setHandles(array $handles) {
-    assert_instances_of($handles, 'PhabricatorObjectHandle');
-    $this->handles = $handles;
-    return $this;
-  }
-
-  public function render() {
-
-    $inline_comments = mgroup($this->inlineComments, 'getTransactionPHID');
-
-    $num = 1;
-
-    $comments = array();
-    foreach ($this->comments as $comment) {
-
-      $inlines = idx($inline_comments, $comment->getPHID(), array());
-
-      $view = id(new DiffusionCommentView())
-        ->setMarkupEngine($this->getMarkupEngine())
-        ->setComment($comment)
-        ->setInlineComments($inlines)
-        ->setCommentNumber($num)
-        ->setHandles($this->handles)
-        ->setPathMap($this->pathMap)
-        ->setUser($this->user);
-
-      $comments[] = $view->render();
-      ++$num;
-    }
-
-    return phutil_tag(
-      'div',
-      array('class' => 'diffusion-comment-list'),
-      $comments);
-  }
-
-}

src/applications/diffusion/view/DiffusionCommentView.php

@@ -1,209 +0,0 @@
-<?php
-
-final class DiffusionCommentView extends AphrontView {
-
-  private $comment;
-  private $commentNumber;
-  private $handles;
-  private $isPreview;
-  private $pathMap;
-
-  private $inlineComments;
-  private $markupEngine;
-
-  public function setComment(PhabricatorAuditComment $comment) {
-    $this->comment = $comment;
-    return $this;
-  }
-
-  public function setCommentNumber($comment_number) {
-    $this->commentNumber = $comment_number;
-    return $this;
-  }
-
-  public function setHandles(array $handles) {
-    assert_instances_of($handles, 'PhabricatorObjectHandle');
-    $this->handles = $handles;
-    return $this;
-  }
-
-  public function setIsPreview($is_preview) {
-    $this->isPreview = $is_preview;
-    return $this;
-  }
-
-  public function setInlineComments(array $inline_comments) {
-    assert_instances_of($inline_comments, 'PhabricatorInlineCommentInterface');
-    $this->inlineComments = $inline_comments;
-    return $this;
-  }
-
-  public function setPathMap(array $path_map) {
-    $this->pathMap = $path_map;
-    return $this;
-  }
-
-  public function setMarkupEngine(PhabricatorMarkupEngine $markup_engine) {
-    $this->markupEngine = $markup_engine;
-    return $this;
-  }
-
-  public function getMarkupEngine() {
-    return $this->markupEngine;
-  }
-
-  public function getRequiredHandlePHIDs() {
-    return array($this->comment->getActorPHID());
-  }
-
-  private function getHandle($phid) {
-    if (empty($this->handles[$phid])) {
-      throw new Exception("Unloaded handle '{$phid}'!");
-    }
-    return $this->handles[$phid];
-  }
-
-  public function render() {
-    $comment = $this->comment;
-    $author = $this->getHandle($comment->getActorPHID());
-    $author_link = $author->renderLink();
-
-    $actions = $this->renderActions();
-    $content = $this->renderContent();
-    $classes = $this->renderClasses();
-
-    $xaction_view = id(new PhabricatorTransactionView())
-      ->setUser($this->user)
-      ->setImageURI($author->getImageURI())
-      ->setActions($actions)
-      ->appendChild($content);
-
-    if ($this->isPreview) {
-      $xaction_view->setIsPreview(true);
-    } else {
-      $xaction_view
-        ->setAnchor('comment-'.$this->commentNumber, '#'.$this->commentNumber)
-        ->setEpoch($comment->getDateCreated());
-    }
-
-    foreach ($classes as $class) {
-      $xaction_view->addClass($class);
-    }
-
-    return $xaction_view->render();
-  }
-
-  private function renderActions() {
-    $comment = $this->comment;
-    $author = $this->getHandle($comment->getActorPHID());
-    $author_link = $author->renderLink();
-
-    $action = $comment->getAction();
-    $verb = PhabricatorAuditActionConstants::getActionPastTenseVerb($action);
-
-    $metadata = $comment->getMetadata();
-    $added_auditors = idx(
-      $metadata,
-      PhabricatorAuditComment::METADATA_ADDED_AUDITORS,
-      array());
-    $added_ccs = idx(
-      $metadata,
-      PhabricatorAuditComment::METADATA_ADDED_CCS,
-      array());
-
-    $actions = array();
-    if ($action == PhabricatorAuditActionConstants::ADD_CCS) {
-      $rendered_ccs = $this->renderHandleList($added_ccs);
-      $actions[] = pht('%s added CCs: %s.', $author_link, $rendered_ccs);
-    } else if ($action == PhabricatorAuditActionConstants::ADD_AUDITORS) {
-      $rendered_auditors = $this->renderHandleList($added_auditors);
-      $actions[] = pht(
-        '%s added auditors: %s.',
-        $author_link,
-        $rendered_auditors);
-    } else {
-      $actions[] = hsprintf('%s %s this commit.', $author_link, $verb);
-    }
-
-    foreach ($actions as $key => $action) {
-      $actions[$key] = phutil_tag('div', array(), $action);
-    }
-
-    return $actions;
-  }
-
-  private function renderContent() {
-    $comment = $this->comment;
-    $engine = $this->getMarkupEngine();
-
-    if (!strlen($comment->getContent()) && empty($this->inlineComments)) {
-      return null;
-    } else {
-      return phutil_tag_div('phabricator-remarkup', array(
-        $engine->getOutput(
-          $comment,
-          PhabricatorAuditComment::MARKUP_FIELD_BODY),
-        $this->renderInlines(),
-      ));
-    }
-  }
-
-  private function renderInlines() {
-    if (!$this->inlineComments) {
-      return null;
-    }
-
-    $engine = $this->getMarkupEngine();
-
-    $inlines_by_path = mgroup($this->inlineComments, 'getPathID');
-
-    $view = new PhabricatorInlineSummaryView();
-    foreach ($inlines_by_path as $path_id => $inlines) {
-      $path = idx($this->pathMap, $path_id);
-      if ($path === null) {
-        continue;
-      }
-
-      $items = array();
-      foreach ($inlines as $inline) {
-        $items[] = array(
-          'id'      => $inline->getID(),
-          'line'    => $inline->getLineNumber(),
-          'length'  => $inline->getLineLength(),
-          'content' => $engine->getOutput(
-            $inline,
-            PhabricatorInlineCommentInterface::MARKUP_FIELD_BODY),
-        );
-      }
-
-      $view->addCommentGroup($path, $items);
-    }
-
-    return $view;
-  }
-
-  private function renderHandleList(array $phids) {
-    $result = array();
-    foreach ($phids as $phid) {
-      $result[] = $this->handles[$phid]->renderLink();
-    }
-    return phutil_implode_html(', ', $result);
-  }
-
-  private function renderClasses() {
-    $comment = $this->comment;
-
-    $classes = array();
-    switch ($comment->getAction()) {
-      case PhabricatorAuditActionConstants::ACCEPT:
-        $classes[] = 'audit-accept';
-        break;
-      case PhabricatorAuditActionConstants::CONCERN:
-        $classes[] = 'audit-concern';
-        break;
-    }
-
-    return $classes;
-  }
-
-}

src/applications/transactions/storage/PhabricatorApplicationTransaction.php

@@ -264,9 +264,10 @@ abstract class PhabricatorApplicationTransaction
     if (empty($this->handles[$phid])) {
       throw new Exception(
         pht(
-          'Transaction ("%s") requires a handle ("%s") that it did not '.
-          'load.',
+          'Transaction ("%s", of type "%s") requires a handle ("%s") that it '.
+          'did not load.',
           $this->getPHID(),
+          $this->getTransactionType(),
           $phid));
     }
     return $this->handles[$phid];

src/applications/transactions/view/PhabricatorApplicationTransactionView.php

@@ -159,17 +159,7 @@ class PhabricatorApplicationTransactionView extends AphrontView {
     }
 
     if ($this->getShowEditActions()) {
-      $list_id = celerity_generate_unique_node_id();
-
-      $view->setID($list_id);
-
-      Javelin::initBehavior(
-        'phabricator-transaction-list',
-        array(
-          'listID'          => $list_id,
-          'objectPHID'      => $this->getObjectPHID(),
-          'nextAnchor'      => $this->anchorOffset + count($events),
-        ));
+      Javelin::initBehavior('phabricator-transaction-list');
     }
 
     return $view->render();

src/view/phui/PHUITimelineEventView.php

@@ -495,8 +495,20 @@ final class PHUITimelineEventView extends AphrontView {
     $xaction_phid = $this->getTransactionPHID();
 
     $items = array();
-    if ($this->getQuoteTargetID()) {
 
+    if ($this->getIsEditable()) {
+      $items[] = id(new PhabricatorActionView())
+        ->setIcon('fa-pencil')
+        ->setHref('/transactions/edit/'.$xaction_phid.'/')
+        ->setName(pht('Edit Comment'))
+        ->addSigil('transaction-edit')
+        ->setMetadata(
+          array(
+            'anchor' => $anchor,
+          ));
+    }
+
+    if ($this->getQuoteTargetID()) {
       $ref = null;
       if ($this->getQuoteRef()) {
         $ref = $this->getQuoteRef();
@@ -505,18 +517,6 @@ final class PHUITimelineEventView extends AphrontView {
         }
       }
 
-      if ($this->getIsEditable()) {
-        $items[] = id(new PhabricatorActionView())
-          ->setIcon('fa-pencil')
-          ->setHref('/transactions/edit/'.$xaction_phid.'/')
-          ->setName(pht('Edit Comment'))
-          ->addSigil('transaction-edit')
-          ->setMetadata(
-            array(
-              'anchor' => $anchor,
-            ));
-      }
-
       $items[] = id(new PhabricatorActionView())
         ->setIcon('fa-quote-left')
         ->setHref('#')

webroot/rsrc/css/application/diffusion/commit-view.css

@@ -1,15 +0,0 @@
-/**
- * @provides diffusion-commit-view-css
- */
-
-.diffusion-comment-list {
-  margin: 2em;
-}
-
-.phabricator-transaction-view .audit-accept {
-  border-color: #009933;
-}
-
-.phabricator-transaction-view .audit-concern {
-  border-color: #aa0000;
-}