Prepare TransactionCommentQuery for extension

Summary:
Ref T2009. Ref T1460. The way Diffusion and Differential load inlines is horrible garbage right now:

  - Differential does an ad-hoc query to get the PHIDs, then does a real load to policy check.
  - Diffusion completely fakes things. In practice this is not a policy violation, but it's dangerous.

Make TransactionCommentQuery extensible so we can subclass it and get the query building correctly in the right Query layer.

Specifically, the Diffusion and Differential subclasses of this Query will add appropriate `withX()` methods to let us express the query in SQL.

Test Plan: Loaded, previewed, edited, and submitted inlines in Differential and Diffusion

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T2009, T1460

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

src/__phutil_library_map__.php

@@ -1298,6 +1298,7 @@ phutil_register_library_map(array(
     'PhabricatorApplicationTransactionResponse' => 'applications/transactions/response/PhabricatorApplicationTransactionResponse.php',
     'PhabricatorApplicationTransactionShowOlderController' => 'applications/transactions/controller/PhabricatorApplicationTransactionShowOlderController.php',
     'PhabricatorApplicationTransactionStructureException' => 'applications/transactions/exception/PhabricatorApplicationTransactionStructureException.php',
+    'PhabricatorApplicationTransactionTemplatedCommentQuery' => 'applications/transactions/query/PhabricatorApplicationTransactionTemplatedCommentQuery.php',
     'PhabricatorApplicationTransactionTextDiffDetailView' => 'applications/transactions/view/PhabricatorApplicationTransactionTextDiffDetailView.php',
     'PhabricatorApplicationTransactionTransactionPHIDType' => 'applications/transactions/phid/PhabricatorApplicationTransactionTransactionPHIDType.php',
     'PhabricatorApplicationTransactionValidationError' => 'applications/transactions/error/PhabricatorApplicationTransactionValidationError.php',
@@ -4541,6 +4542,7 @@ phutil_register_library_map(array(
     'PhabricatorApplicationTransactionResponse' => 'AphrontProxyResponse',
     'PhabricatorApplicationTransactionShowOlderController' => 'PhabricatorApplicationTransactionController',
     'PhabricatorApplicationTransactionStructureException' => 'Exception',
+    'PhabricatorApplicationTransactionTemplatedCommentQuery' => 'PhabricatorApplicationTransactionCommentQuery',
     'PhabricatorApplicationTransactionTextDiffDetailView' => 'AphrontView',
     'PhabricatorApplicationTransactionTransactionPHIDType' => 'PhabricatorPHIDType',
     'PhabricatorApplicationTransactionValidationError' => 'Phobject',

src/applications/differential/query/DifferentialTransactionQuery.php

@@ -11,11 +11,7 @@ final class DifferentialTransactionQuery
     PhabricatorUser $viewer,
     DifferentialRevision $revision) {
 
-    // TODO: This probably needs to move somewhere more central as we move
-    // away from DifferentialInlineCommentQuery, but
-    // PhabricatorApplicationTransactionCommentQuery is currently `final` and
-    // I'm not yet decided on how to approach that. For now, just get the PHIDs
-    // and then execute a PHID-based query through the standard stack.
+    // TODO: Subclass ApplicationTransactionCommentQuery to do this for real.
 
     $table = new DifferentialTransactionComment();
     $conn_r = $table->establishConnection('r');
@@ -36,7 +32,7 @@ final class DifferentialTransactionQuery
       return array();
     }
 
-    $comments = id(new PhabricatorApplicationTransactionCommentQuery())
+    $comments = id(new PhabricatorApplicationTransactionTemplatedCommentQuery())
       ->setTemplate(new DifferentialTransactionComment())
       ->setViewer($viewer)
       ->withPHIDs($phids)

src/applications/transactions/controller/PhabricatorApplicationTransactionCommentHistoryController.php

@@ -36,7 +36,7 @@ final class PhabricatorApplicationTransactionCommentHistoryController
       return new Aphront400Response();
     }
 
-    $comments = id(new PhabricatorApplicationTransactionCommentQuery())
+    $comments = id(new PhabricatorApplicationTransactionTemplatedCommentQuery())
       ->setViewer($user)
       ->setTemplate($xaction->getApplicationTransactionCommentObject())
       ->withTransactionPHIDs(array($xaction->getPHID()))

src/applications/transactions/query/PhabricatorApplicationTransactionCommentQuery.php

@@ -1,16 +1,18 @@
 <?php
 
-final class PhabricatorApplicationTransactionCommentQuery
+abstract class PhabricatorApplicationTransactionCommentQuery
   extends PhabricatorCursorPagedPolicyAwareQuery {
 
-  private $template;
-
+  private $ids;
+  private $authorPHIDs;
   private $phids;
   private $transactionPHIDs;
+  private $isDeleted;
 
-  public function setTemplate(
-    PhabricatorApplicationTransactionComment $template) {
-    $this->template = $template;
+  abstract protected function getTemplate();
+
+  public function withIDs(array $ids) {
+    $this->ids = $ids;
     return $this;
   }
 
@@ -24,13 +26,23 @@ final class PhabricatorApplicationTransactionCommentQuery
     return $this;
   }
 
+  public function withAuthorPHIDs(array $phids) {
+    $this->authorPHIDs = $phids;
+    return $this;
+  }
+
+  public function withDeleted($deleted) {
+    $this->isDeleted = $deleted;
+    return $this;
+  }
+
   protected function loadPage() {
-    $table = $this->template;
+    $table = $this->getTemplate();
     $conn_r = $table->establishConnection('r');
 
     $data = queryfx_all(
       $conn_r,
-      'SELECT * FROM %T xc %Q %Q %Q',
+      'SELECT * FROM %T xcomment %Q %Q %Q',
       $table->getTableName(),
       $this->buildWhereClause($conn_r),
       $this->buildOrderClause($conn_r),
@@ -39,23 +51,44 @@ final class PhabricatorApplicationTransactionCommentQuery
     return $table->loadAllFromArray($data);
   }
 
-  private function buildWhereClause(AphrontDatabaseConnection $conn_r) {
+  protected function buildWhereClause(AphrontDatabaseConnection $conn_r) {
     $where = array();
 
-    if ($this->phids) {
+    if ($this->ids !== null) {
       $where[] = qsprintf(
         $conn_r,
-        'phid IN (%Ls)',
+        'xcomment.id IN (%Ld)',
+        $this->ids);
+    }
+
+    if ($this->phids !== null) {
+      $where[] = qsprintf(
+        $conn_r,
+        'xcomment.phid IN (%Ls)',
         $this->phids);
     }
 
-    if ($this->transactionPHIDs) {
+    if ($this->authorPHIDs !== null) {
       $where[] = qsprintf(
         $conn_r,
-        'transactionPHID IN (%Ls)',
+        'xcomment.authorPHID IN (%Ls)',
+        $this->authorPHIDs);
+    }
+
+    if ($this->transactionPHIDs !== null) {
+      $where[] = qsprintf(
+        $conn_r,
+        'xcomment.transactionPHID IN (%Ls)',
         $this->transactionPHIDs);
     }
 
+    if ($this->isDeleted !== null) {
+      $where[] = qsprintf(
+        $conn_r,
+        'xcomment.isDeleted = %d',
+        (int)$this->isDeleted);
+    }
+
     return $this->formatWhereClause($where);
   }
 

src/applications/transactions/query/PhabricatorApplicationTransactionQuery.php

@@ -80,11 +80,12 @@ abstract class PhabricatorApplicationTransactionQuery
 
       $comments = array();
       if ($comment_phids) {
-        $comments = id(new PhabricatorApplicationTransactionCommentQuery())
-          ->setTemplate($table->getApplicationTransactionCommentObject())
-          ->setViewer($this->getViewer())
-          ->withPHIDs($comment_phids)
-          ->execute();
+        $comments =
+          id(new PhabricatorApplicationTransactionTemplatedCommentQuery())
+            ->setTemplate($table->getApplicationTransactionCommentObject())
+            ->setViewer($this->getViewer())
+            ->withPHIDs($comment_phids)
+            ->execute();
         $comments = mpull($comments, null, 'getPHID');
       }
 

src/applications/transactions/query/PhabricatorApplicationTransactionTemplatedCommentQuery.php

@@ -0,0 +1,18 @@
+<?php
+
+final class PhabricatorApplicationTransactionTemplatedCommentQuery
+  extends PhabricatorApplicationTransactionCommentQuery {
+
+  private $template;
+
+  public function setTemplate(
+    PhabricatorApplicationTransactionComment $template) {
+    $this->template = $template;
+    return $this;
+  }
+
+  protected function getTemplate() {
+    return $this->template;
+  }
+
+}

src/infrastructure/diff/PhabricatorInlineCommentController.php

@@ -330,11 +330,12 @@ abstract class PhabricatorInlineCommentController
     }
 
     if ($reply_phids) {
-      $reply_comments = id(new PhabricatorApplicationTransactionCommentQuery())
-        ->setTemplate($template)
-        ->setViewer($viewer)
-        ->withPHIDs($reply_phids)
-        ->execute();
+      $reply_comments =
+        id(new PhabricatorApplicationTransactionTemplatedCommentQuery())
+          ->setTemplate($template)
+          ->setViewer($viewer)
+          ->withPHIDs($reply_phids)
+          ->execute();
       $reply_comments = mpull($reply_comments, null, 'getPHID');
     } else {
       $reply_comments = array();