Extend TransactionCommentQuery for Diffusion

Summary: Ref T2009. Ref T1460. Reduces the amount of garbage involved in loading inline comments and routes more pathways through the proper Query layer.

Test Plan: Viewed, edited, previewed, submitted inline comments in Diffusion.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T2009, T1460

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

src/__phutil_library_map__.php

@@ -491,6 +491,7 @@ phutil_register_library_map(array(
     'DiffusionDefaultPushCapability' => 'applications/diffusion/capability/DiffusionDefaultPushCapability.php',
     'DiffusionDefaultViewCapability' => 'applications/diffusion/capability/DiffusionDefaultViewCapability.php',
     'DiffusionDiffController' => 'applications/diffusion/controller/DiffusionDiffController.php',
+    'DiffusionDiffInlineCommentQuery' => 'applications/diffusion/query/DiffusionDiffInlineCommentQuery.php',
     'DiffusionDiffQueryConduitAPIMethod' => 'applications/diffusion/conduit/DiffusionDiffQueryConduitAPIMethod.php',
     'DiffusionDoorkeeperCommitFeedStoryPublisher' => 'applications/diffusion/doorkeeper/DiffusionDoorkeeperCommitFeedStoryPublisher.php',
     'DiffusionEmptyResultView' => 'applications/diffusion/view/DiffusionEmptyResultView.php',
@@ -3644,6 +3645,7 @@ phutil_register_library_map(array(
     'DiffusionDefaultPushCapability' => 'PhabricatorPolicyCapability',
     'DiffusionDefaultViewCapability' => 'PhabricatorPolicyCapability',
     'DiffusionDiffController' => 'DiffusionController',
+    'DiffusionDiffInlineCommentQuery' => 'PhabricatorDiffInlineCommentQuery',
     'DiffusionDiffQueryConduitAPIMethod' => 'DiffusionQueryConduitAPIMethod',
     'DiffusionDoorkeeperCommitFeedStoryPublisher' => 'DoorkeeperFeedStoryPublisher',
     'DiffusionEmptyResultView' => 'DiffusionView',

src/applications/audit/storage/PhabricatorAuditInlineComment.php

@@ -61,16 +61,15 @@ final class PhabricatorAuditInlineComment
     PhabricatorUser $viewer,
     $commit_phid) {
 
-    $inlines = id(new PhabricatorAuditTransactionComment())->loadAllWhere(
-      'authorPHID = %s AND commitPHID = %s AND transactionPHID IS NULL
-        AND pathID IS NOT NULL
-        AND isDeleted = 0',
-      $viewer->getPHID(),
-      $commit_phid);
-
-    $inlines = PhabricatorInlineCommentController::loadAndAttachReplies(
-      $viewer,
-      $inlines);
+    $inlines = id(new DiffusionDiffInlineCommentQuery())
+      ->setViewer($viewer)
+      ->withAuthorPHIDs(array($viewer->getPHID()))
+      ->withCommitPHIDs(array($commit_phid))
+      ->withHasTransaction(false)
+      ->withHasPath(true)
+      ->withIsDeleted(false)
+      ->needReplyToComments(true)
+      ->execute();
 
     return self::buildProxies($inlines);
   }
@@ -79,10 +78,12 @@ final class PhabricatorAuditInlineComment
     PhabricatorUser $viewer,
     $commit_phid) {
 
-    $inlines = id(new PhabricatorAuditTransactionComment())->loadAllWhere(
-      'commitPHID = %s AND transactionPHID IS NOT NULL
-        AND pathID IS NOT NULL',
-      $commit_phid);
+    $inlines = id(new DiffusionDiffInlineCommentQuery())
+      ->setViewer($viewer)
+      ->withCommitPHIDs(array($commit_phid))
+      ->withHasTransaction(true)
+      ->withHasPath(true)
+      ->execute();
 
     return self::buildProxies($inlines);
   }

src/applications/diffusion/query/DiffusionDiffInlineCommentQuery.php

@@ -0,0 +1,62 @@
+<?php
+
+final class DiffusionDiffInlineCommentQuery
+  extends PhabricatorDiffInlineCommentQuery {
+
+  private $commitPHIDs;
+  private $hasPath;
+  private $pathIDs;
+
+  public function withCommitPHIDs(array $phids) {
+    $this->commitPHIDs = $phids;
+    return $this;
+  }
+
+  public function withHasPath($has_path) {
+    $this->hasPath = $has_path;
+    return $this;
+  }
+
+  public function withPathIDs(array $path_ids) {
+    $this->pathIDs = $path_ids;
+    return $this;
+  }
+
+  protected function getTemplate() {
+    return new PhabricatorAuditTransactionComment();
+  }
+
+  protected function buildWhereClauseComponents(
+    AphrontDatabaseConnection $conn_r) {
+    $where = parent::buildWhereClauseComponents($conn_r);
+
+    if ($this->commitPHIDs !== null) {
+      $where[] = qsprintf(
+        $conn_r,
+        'xcomment.commitPHID IN (%Ls)',
+        $this->commitPHIDs);
+    }
+
+    if ($this->hasPath !== null) {
+      if ($this->hasPath) {
+        $where[] = qsprintf(
+          $conn_r,
+          'xcomment.pathID IS NOT NULL');
+      } else {
+        $where[] = qsprintf(
+          $conn_r,
+          'xcomment.pathID IS NULL');
+      }
+    }
+
+    if ($this->pathIDs !== null) {
+      $where[] = qsprintf(
+        $conn_r,
+        'xcomment.pathID IN (%Ld)',
+        $this->pathIDs);
+    }
+
+    return $where;
+  }
+
+}

src/infrastructure/diff/PhabricatorInlineCommentController.php

@@ -309,45 +309,4 @@ abstract class PhabricatorInlineCommentController
       ->addRowScaffold($view);
   }
 
-  public static function loadAndAttachReplies(
-    PhabricatorUser $viewer,
-    array $comments) {
-    // TODO: This code doesn't really belong here, but we don't have a much
-    // better place to put it at the moment.
-
-    if (!$comments) {
-      return $comments;
-    }
-
-    $template = head($comments);
-
-    $reply_phids = array();
-    foreach ($comments as $comment) {
-      $reply_phid = $comment->getReplyToCommentPHID();
-      if ($reply_phid) {
-        $reply_phids[] = $reply_phid;
-      }
-    }
-
-    if ($reply_phids) {
-      $reply_comments =
-        id(new PhabricatorApplicationTransactionTemplatedCommentQuery())
-          ->setTemplate($template)
-          ->setViewer($viewer)
-          ->withPHIDs($reply_phids)
-          ->execute();
-      $reply_comments = mpull($reply_comments, null, 'getPHID');
-    } else {
-      $reply_comments = array();
-    }
-
-    foreach ($comments as $comment) {
-      $reply_phid = $comment->getReplyToCommentPHID();
-      $reply = idx($reply_comments, $reply_phid);
-      $comment->attachReplyToComment($reply);
-    }
-
-    return $comments;
-  }
-
 }