Build a page for viewing all inline comments

Summary: Adds a very basic list of all inline comments, threaded, and their status. Kept this a little simpler than the mock, mostly because sorting here feels a little strange given threads would be all over the place. Not sure sorted is needed in practice anyways. I'd probably lean towards just adding a JS checkbox to hide certain rows if needed in the future.

Test Plan:
Test various commenting structures:

 - Leave Comment
 - Update Diff
 - Leave new comment
 - Reply to comment
 - Reply to comment as revision author
 - Mark items as done
 - Update diff again

{F4996915}

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

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

Author: Chad Little

src/__phutil_library_map__.php

@@ -547,6 +547,7 @@
     'DifferentialRevisionHeraldField' => 'applications/differential/herald/DifferentialRevisionHeraldField.php',
     'DifferentialRevisionHeraldFieldGroup' => 'applications/differential/herald/DifferentialRevisionHeraldFieldGroup.php',
     'DifferentialRevisionIDCommitMessageField' => 'applications/differential/field/DifferentialRevisionIDCommitMessageField.php',
+    'DifferentialRevisionInlinesController' => 'applications/differential/controller/DifferentialRevisionInlinesController.php',
     'DifferentialRevisionLandController' => 'applications/differential/controller/DifferentialRevisionLandController.php',
     'DifferentialRevisionListController' => 'applications/differential/controller/DifferentialRevisionListController.php',
     'DifferentialRevisionListView' => 'applications/differential/view/DifferentialRevisionListView.php',
@@ -1738,6 +1739,7 @@
     'PHUIDiffInlineCommentTableScaffold' => 'infrastructure/diff/view/PHUIDiffInlineCommentTableScaffold.php',
     'PHUIDiffInlineCommentUndoView' => 'infrastructure/diff/view/PHUIDiffInlineCommentUndoView.php',
     'PHUIDiffInlineCommentView' => 'infrastructure/diff/view/PHUIDiffInlineCommentView.php',
+    'PHUIDiffInlineThreader' => 'infrastructure/diff/view/PHUIDiffInlineThreader.php',
     'PHUIDiffOneUpInlineCommentRowScaffold' => 'infrastructure/diff/view/PHUIDiffOneUpInlineCommentRowScaffold.php',
     'PHUIDiffRevealIconView' => 'infrastructure/diff/view/PHUIDiffRevealIconView.php',
     'PHUIDiffTableOfContentsItemView' => 'infrastructure/diff/view/PHUIDiffTableOfContentsItemView.php',
@@ -5516,6 +5518,7 @@
     'DifferentialRevisionHeraldField' => 'HeraldField',
     'DifferentialRevisionHeraldFieldGroup' => 'HeraldFieldGroup',
     'DifferentialRevisionIDCommitMessageField' => 'DifferentialCommitMessageField',
+    'DifferentialRevisionInlinesController' => 'DifferentialController',
     'DifferentialRevisionLandController' => 'DifferentialController',
     'DifferentialRevisionListController' => 'DifferentialController',
     'DifferentialRevisionListView' => 'AphrontView',
@@ -6872,6 +6875,7 @@
     'PHUIDiffInlineCommentTableScaffold' => 'AphrontView',
     'PHUIDiffInlineCommentUndoView' => 'PHUIDiffInlineCommentView',
     'PHUIDiffInlineCommentView' => 'AphrontView',
+    'PHUIDiffInlineThreader' => 'Phobject',
     'PHUIDiffOneUpInlineCommentRowScaffold' => 'PHUIDiffInlineCommentRowScaffold',
     'PHUIDiffRevealIconView' => 'AphrontView',
     'PHUIDiffTableOfContentsItemView' => 'AphrontView',

src/applications/audit/storage/PhabricatorAuditInlineComment.php

@@ -331,6 +331,14 @@ public function getIsGhost() {
     return $this->isGhost;
   }
 
+  public function getDateModified() {
+    return $this->proxy->getDateModified();
+  }
+
+  public function getDateCreated() {
+    return $this->proxy->getDateCreated();
+  }
+
 
 /* -(  PhabricatorMarkupInterface Implementation  )-------------------------- */
 

src/applications/base/controller/PhabricatorController.php

@@ -483,8 +483,10 @@ public function newCurtainView($object = null) {
     // NOTE: Applications (objects of class PhabricatorApplication) can't
     // currently be set here, although they don't need any of the extensions
     // anyway. This should probably work differently than it does, though.
-    if ($object instanceof PhabricatorLiskDAO) {
-      $action_list->setObject($object);
+    if ($object) {
+      if ($object instanceof PhabricatorLiskDAO) {
+        $action_list->setObject($object);
+      }
     }
 
     $curtain = id(new PHUICurtainView())

src/applications/differential/application/PhabricatorDifferentialApplication.php

@@ -77,6 +77,8 @@ public function getRoutes() {
             => 'DifferentialDiffCreateController',
           'operation/(?P<id>[1-9]\d*)/'
             => 'DifferentialRevisionOperationController',
+          'inlines/(?P<id>[1-9]\d*)/'
+            => 'DifferentialRevisionInlinesController',
         ),
         'comment/' => array(
           'preview/(?P<id>[1-9]\d*)/' => 'DifferentialCommentPreviewController',

src/applications/differential/controller/DifferentialRevisionInlinesController.php

@@ -0,0 +1,190 @@
+<?php
+
+final class DifferentialRevisionInlinesController
+  extends DifferentialController {
+
+  public function shouldAllowPublic() {
+    return true;
+  }
+
+  public function handleRequest(AphrontRequest $request) {
+    $viewer = $this->getViewer();
+    $id = $request->getURIData('id');
+
+    $revision = id(new DifferentialRevisionQuery())
+      ->withIDs(array($id))
+      ->setViewer($viewer)
+      ->needDiffIDs(true)
+      ->executeOne();
+    if (!$revision) {
+      return new Aphront404Response();
+    }
+
+    $revision_monogram = $revision->getMonogram();
+    $revision_uri = $revision->getURI();
+    $revision_title = $revision->getTitle();
+
+    $query = id(new DifferentialInlineCommentQuery())
+      ->setViewer($viewer)
+      ->needHidden(true)
+      ->withRevisionPHIDs(array($revision->getPHID()));
+    $inlines = $query->execute();
+
+    $crumbs = $this->buildApplicationCrumbs();
+    $crumbs->addTextCrumb($revision_monogram, $revision_uri);
+    $crumbs->addTextCrumb(pht('Inline Comments'));
+    $crumbs->setBorder(true);
+
+    $content = $this->renderInlineTable($revision, $inlines);
+    $header = $this->buildHeader($revision);
+
+    $view = id(new PHUITwoColumnView())
+      ->setHeader($header)
+      ->setFooter($content);
+
+    return $this->newPage()
+      ->setTitle(
+        array(
+          "{$revision_monogram} {$revision_title}",
+          pht('Inlines'),
+        ))
+      ->setCrumbs($crumbs)
+      ->appendChild($view);
+  }
+
+  private function buildHeader(DifferentialRevision $revision) {
+    $viewer = $this->getViewer();
+
+    $button = id(new PHUIButtonView())
+      ->setTag('a')
+      ->setIcon('fa-chevron-left')
+      ->setHref($revision->getURI())
+      ->setText(pht('Back to Revision'));
+
+    return id(new PHUIHeaderView())
+      ->setHeader($revision->getTitle())
+      ->setUser($viewer)
+      ->setHeaderIcon('fa-cog')
+      ->addActionLink($button);
+  }
+
+  private function renderInlineTable(
+    DifferentialRevision $revision,
+    array $inlines) {
+
+    $viewer = $this->getViewer();
+    $inlines = id(new PHUIDiffInlineThreader())
+      ->reorderAndThreadCommments($inlines);
+
+    $handle_phids = array();
+    $changeset_ids = array();
+    foreach ($inlines as $inline) {
+      $handle_phids[] = $inline->getAuthorPHID();
+      $changeset_ids[] = $inline->getChangesetID();
+    }
+    $handles = $viewer->loadHandles($handle_phids);
+    $handles = iterator_to_array($handles);
+
+    if ($changeset_ids) {
+      $changesets = id(new DifferentialChangesetQuery())
+        ->setViewer($viewer)
+        ->withIDs($changeset_ids)
+        ->execute();
+      $changesets = mpull($changesets, null, 'getID');
+    } else {
+      $changesets = array();
+    }
+
+    $current_changeset = head($revision->getDiffIDs());
+
+    $rows = array();
+    foreach ($inlines as $inline) {
+      $status_icons = array();
+
+      $c_id = $inline->getChangesetID();
+      $d_id = $changesets[$c_id]->getDiffID();
+
+      if ($d_id == $current_changeset) {
+        $diff_id = phutil_tag('strong', array(), pht('Current'));
+      } else {
+        $diff_id = pht('Diff %d', $d_id);
+      }
+
+      $reviewer = $handles[$inline->getAuthorPHID()]->renderLink();
+      $now = PhabricatorTime::getNow();
+      $then = $inline->getDateModified();
+      $datetime = phutil_format_relative_time($now - $then);
+
+      $comment_href = $revision->getURI().'#inline-'.$inline->getID();
+      $comment = phutil_tag(
+        'a',
+        array(
+          'href' => $comment_href,
+        ),
+        $inline->getContent());
+
+      $state = $inline->getFixedState();
+      if ($state == PhabricatorInlineCommentInterface::STATE_DONE) {
+        $status_icons[] = id(new PHUIIconView())
+          ->setIcon('fa-check green')
+          ->addClass('mmr');
+      } else if ($inline->getReplyToCommentPHID() &&
+        $inline->getAuthorPHID() == $revision->getAuthorPHID()) {
+        $status_icons[] = id(new PHUIIconView())
+          ->setIcon('fa-commenting-o blue')
+          ->addClass('mmr');
+      } else {
+        $status_icons[] = id(new PHUIIconView())
+          ->setIcon('fa-circle-o grey')
+          ->addClass('mmr');
+      }
+
+
+      if ($inline->getReplyToCommentPHID()) {
+        $reply_icon = id(new PHUIIconView())
+          ->setIcon('fa-reply mmr darkgrey');
+        $comment = array($reply_icon, $comment);
+      }
+
+      $rows[] = array(
+        $diff_id,
+        $status_icons,
+        $reviewer,
+        AphrontTableView::renderSingleDisplayLine($comment),
+        $datetime,
+      );
+    }
+
+    $table = new AphrontTableView($rows);
+    $table->setHeaders(
+      array(
+        pht('Diff'),
+        pht('Status'),
+        pht('Reviewer'),
+        pht('Comment'),
+        pht('Created'),
+      ));
+    $table->setColumnClasses(
+      array(
+        '',
+        '',
+        '',
+        'wide',
+        'right',
+      ));
+    $table->setColumnVisibility(
+      array(
+        true,
+        true,
+        true,
+        true,
+        true,
+      ));
+
+    return id(new PHUIObjectBoxView())
+      ->setHeaderText(pht('Inline Comments'))
+      ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY)
+      ->setTable($table);
+  }
+
+}

src/applications/differential/controller/DifferentialRevisionViewController.php

@@ -574,6 +574,12 @@ private function buildCurtain(DifferentialRevision $revision) {
         ->setDisabled(!$can_edit)
         ->setWorkflow(!$can_edit));
 
+    $curtain->addAction(
+      id(new PhabricatorActionView())
+        ->setIcon('fa-indent')
+        ->setHref("/differential/revision/inlines/{$revision_id}/")
+        ->setName(pht('List Inline Comments')));
+
     $curtain->addAction(
       id(new PhabricatorActionView())
         ->setIcon('fa-upload')

src/applications/differential/parser/DifferentialChangesetParser.php

@@ -1045,7 +1045,8 @@ public function render(
         }
       }
 
-      $this->comments = $this->reorderAndThreadComments($this->comments);
+      $this->comments = id(new PHUIDiffInlineThreader())
+        ->reorderAndThreadCommments($this->comments);
 
       foreach ($this->comments as $comment) {
         $final = $comment->getLineNumber() +
@@ -1617,68 +1618,6 @@ private function buildLineBackmaps() {
     return array($old_back, $new_back);
   }
 
-  private function reorderAndThreadComments(array $comments) {
-    $comments = msort($comments, 'getID');
-
-    // Build an empty map of all the comments we actually have. If a comment
-    // is a reply but the parent has gone missing, we don't want it to vanish
-    // completely.
-    $comment_phids = mpull($comments, 'getPHID');
-    $replies = array_fill_keys($comment_phids, array());
-
-    // Now, remove all comments which are replies, leaving only the top-level
-    // comments.
-    foreach ($comments as $key => $comment) {
-      $reply_phid = $comment->getReplyToCommentPHID();
-      if (isset($replies[$reply_phid])) {
-        $replies[$reply_phid][] = $comment;
-        unset($comments[$key]);
-      }
-    }
-
-    // For each top level comment, add the comment, then add any replies
-    // to it. Do this recursively so threads are shown in threaded order.
-    $results = array();
-    foreach ($comments as $comment) {
-      $results[] = $comment;
-      $phid = $comment->getPHID();
-      $descendants = $this->getInlineReplies($replies, $phid, 1);
-      foreach ($descendants as $descendant) {
-        $results[] = $descendant;
-      }
-    }
-
-    // If we have anything left, they were cyclic references. Just dump
-    // them in a the end. This should be impossible, but users are very
-    // creative.
-    foreach ($replies as $phid => $comments) {
-      foreach ($comments as $comment) {
-        $results[] = $comment;
-      }
-    }
-
-    return $results;
-  }
-
-  private function getInlineReplies(array &$replies, $phid, $depth) {
-    $comments = idx($replies, $phid, array());
-    unset($replies[$phid]);
-
-    $results = array();
-    foreach ($comments as $comment) {
-      $results[] = $comment;
-      $descendants = $this->getInlineReplies(
-        $replies,
-        $comment->getPHID(),
-        $depth + 1);
-      foreach ($descendants as $descendant) {
-        $results[] = $descendant;
-      }
-    }
-
-    return $results;
-  }
-
   private function getOffset(array $map, $line) {
     if (!$map) {
       return null;

src/applications/differential/storage/DifferentialInlineComment.php

@@ -255,6 +255,13 @@ public function makeEphemeral() {
     return $this;
   }
 
+  public function getDateModified() {
+    return $this->proxy->getDateModified();
+  }
+
+  public function getDateCreated() {
+    return $this->proxy->getDateCreated();
+  }
 
 /* -(  PhabricatorMarkupInterface Implementation  )-------------------------- */
 

src/infrastructure/diff/interface/PhabricatorInlineCommentInterface.php

@@ -60,4 +60,7 @@ public function getIsGhost();
   public function supportsHiding();
   public function isHidden();
 
+  public function getDateModified();
+  public function getDateCreated();
+
 }