Remove "DifferentialInlineCommentQuery"

Summary: Ref T13513. Replaces "DifferentialInlineCommentQuery" with the similar but more modern "DifferentialDiffInlineCommentQuery".

Test Plan: Viewed comments in timeline, changesets. Created, edited, and submitted comments. Hid and un-hid comments, reloading (saw state preserved).

Maniphest Tasks: T13513

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

src/__phutil_library_map__.php

@@ -561,7 +561,6 @@ phutil_register_library_map(array(
     'DifferentialInlineComment' => 'applications/differential/storage/DifferentialInlineComment.php',
     'DifferentialInlineCommentEditController' => 'applications/differential/controller/DifferentialInlineCommentEditController.php',
     'DifferentialInlineCommentMailView' => 'applications/differential/mail/DifferentialInlineCommentMailView.php',
-    'DifferentialInlineCommentQuery' => 'applications/differential/query/DifferentialInlineCommentQuery.php',
     'DifferentialJIRAIssuesCommitMessageField' => 'applications/differential/field/DifferentialJIRAIssuesCommitMessageField.php',
     'DifferentialJIRAIssuesField' => 'applications/differential/customfield/DifferentialJIRAIssuesField.php',
     'DifferentialLegacyQuery' => 'applications/differential/constants/DifferentialLegacyQuery.php',
@@ -3592,6 +3591,7 @@ phutil_register_library_map(array(
     'PhabricatorIndexableInterface' => 'applications/search/interface/PhabricatorIndexableInterface.php',
     'PhabricatorInfrastructureTestCase' => '__tests__/PhabricatorInfrastructureTestCase.php',
     'PhabricatorInlineComment' => 'infrastructure/diff/interface/PhabricatorInlineComment.php',
+    'PhabricatorInlineCommentAdjustmentEngine' => 'infrastructure/diff/engine/PhabricatorInlineCommentAdjustmentEngine.php',
     'PhabricatorInlineCommentController' => 'infrastructure/diff/PhabricatorInlineCommentController.php',
     'PhabricatorInlineCommentInterface' => 'applications/transactions/interface/PhabricatorInlineCommentInterface.php',
     'PhabricatorInlineSummaryView' => 'infrastructure/diff/view/PhabricatorInlineSummaryView.php',
@@ -6626,7 +6626,6 @@ phutil_register_library_map(array(
     'DifferentialInlineComment' => 'PhabricatorInlineComment',
     'DifferentialInlineCommentEditController' => 'PhabricatorInlineCommentController',
     'DifferentialInlineCommentMailView' => 'DifferentialMailView',
-    'DifferentialInlineCommentQuery' => 'PhabricatorOffsetPagedQuery',
     'DifferentialJIRAIssuesCommitMessageField' => 'DifferentialCommitMessageCustomField',
     'DifferentialJIRAIssuesField' => 'DifferentialStoredCustomField',
     'DifferentialLegacyQuery' => 'Phobject',
@@ -10118,6 +10117,7 @@ phutil_register_library_map(array(
       'Phobject',
       'PhabricatorMarkupInterface',
     ),
+    'PhabricatorInlineCommentAdjustmentEngine' => 'Phobject',
     'PhabricatorInlineCommentController' => 'PhabricatorController',
     'PhabricatorInlineSummaryView' => 'AphrontView',
     'PhabricatorInstructionsEditField' => 'PhabricatorEditField',

src/applications/differential/controller/DifferentialChangesetViewController.php

@@ -194,16 +194,23 @@ final class DifferentialChangesetViewController extends DifferentialController {
 
     // Load both left-side and right-side inline comments.
     if ($revision) {
-      $query = id(new DifferentialInlineCommentQuery())
+      $inlines = id(new DifferentialDiffInlineCommentQuery())
         ->setViewer($viewer)
+        ->withRevisionPHIDs(array($revision->getPHID()))
+        ->withPublishableComments(true)
+        ->withPublishedComments(true)
         ->needHidden(true)
-        ->withRevisionPHIDs(array($revision->getPHID()));
+        ->execute();
-      $inlines = $query->execute();
+
-      $inlines = $query->adjustInlinesForChangesets(
+      $inlines = mpull($inlines, 'newInlineCommentObject');
-        $inlines,
+
-        $old,
+      $inlines = id(new PhabricatorInlineCommentAdjustmentEngine())
-        $new,
+        ->setViewer($viewer)
-        $revision);
+        ->setRevision($revision)
+        ->setOldChangesets($old)
+        ->setNewChangesets($new)
+        ->setInlines($inlines)
+        ->execute();
     } else {
       $inlines = array();
     }

src/applications/differential/controller/DifferentialRevisionViewController.php

@@ -227,16 +227,20 @@ final class DifferentialRevisionViewController
       $old = array_select_keys($changesets, $old_ids);
       $new = array_select_keys($changesets, $new_ids);
 
-      $query = id(new DifferentialInlineCommentQuery())
+      $inlines = id(new DifferentialDiffInlineCommentQuery())
         ->setViewer($viewer)
-        ->needHidden(true)
+        ->withRevisionPHIDs(array($revision->getPHID()))
-        ->withRevisionPHIDs(array($revision->getPHID()));
+        ->withPublishableComments(true)
-      $inlines = $query->execute();
+        ->withPublishedComments(true)
-      $inlines = $query->adjustInlinesForChangesets(
+        ->execute();
-        $inlines,
+
-        $old,
+      $inlines = id(new PhabricatorInlineCommentAdjustmentEngine())
-        $new,
+        ->setViewer($viewer)
-        $revision);
+        ->setRevision($revision)
+        ->setOldChangesets($old)
+        ->setNewChangesets($new)
+        ->setInlines($inlines)
+        ->execute();
 
       foreach ($inlines as $inline) {
         $changeset_id = $inline->getChangesetID();

src/applications/differential/engine/DifferentialRevisionTimelineEngine.php

@@ -50,22 +50,22 @@ final class DifferentialRevisionTimelineEngine
     }
 
     foreach ($inlines as $key => $inline) {
-      $inlines[$key] = DifferentialInlineComment::newFromModernComment(
+      $inlines[$key] = $inline->newInlineCommentObject();
-        $inline);
     }
 
-    $query = id(new DifferentialInlineCommentQuery())
-      ->needHidden(true)
-      ->setViewer($viewer);
-
     // NOTE: This is a bit sketchy: this method adjusts the inlines as a
     // side effect, which means it will ultimately adjust the transaction
     // comments and affect timeline rendering.
-    $query->adjustInlinesForChangesets(
+
-      $inlines,
+    $old = array_select_keys($changesets, $old_ids);
-      array_select_keys($changesets, $old_ids),
+    $new = array_select_keys($changesets, $new_ids);
-      array_select_keys($changesets, $new_ids),
+    id(new PhabricatorInlineCommentAdjustmentEngine())
-      $revision);
+      ->setViewer($viewer)
+      ->setRevision($revision)
+      ->setOldChangesets($old)
+      ->setNewChangesets($new)
+      ->setInlines($inlines)
+      ->execute();
 
     return id(new DifferentialTransactionView())
       ->setViewData($view_data)

src/applications/differential/query/DifferentialDiffInlineCommentQuery.php

@@ -45,4 +45,26 @@ final class DifferentialDiffInlineCommentQuery
     return $where;
   }
 
+  protected function loadHiddenCommentIDs(
+    $viewer_phid,
+    array $comments) {
+
+    $table = new DifferentialHiddenComment();
+    $conn = $table->establishConnection('r');
+
+    $rows = queryfx_all(
+      $conn,
+      'SELECT commentID FROM %R
+        WHERE userPHID = %s
+        AND commentID IN (%Ld)',
+      $table,
+      $viewer_phid,
+      mpull($comments, 'getID'));
+
+    $id_map = ipull($rows, 'commentID');
+    $id_map = array_fuse($id_map);
+
+    return $id_map;
+  }
+
 }

src/applications/diffusion/query/DiffusionDiffInlineCommentQuery.php

@@ -60,4 +60,10 @@ final class DiffusionDiffInlineCommentQuery
     return $where;
   }
 
+  protected function loadHiddenCommentIDs(
+    $viewer_phid,
+    array $comments) {
+    return array();
+  }
+
 }

src/infrastructure/diff/engine/PhabricatorInlineCommentAdjustmentEngine.php

@@ -1,21 +1,13 @@
 <?php
 
-/**
+final class PhabricatorInlineCommentAdjustmentEngine
- * Temporary wrapper for transitioning Differential to ApplicationTransactions.
+  extends Phobject {
- */
-final class DifferentialInlineCommentQuery
-  extends PhabricatorOffsetPagedQuery {
 
-  // TODO: Remove this when this query eventually moves to PolicyAware.
   private $viewer;
-
+  private $inlines;
-  private $ids;
+  private $revision;
-  private $phids;
+  private $oldChangesets;
-  private $drafts;
+  private $newChangesets;
-  private $authorPHIDs;
-  private $revisionPHIDs;
-  private $deletedDrafts;
-  private $needHidden;
 
   public function setViewer(PhabricatorUser $viewer) {
     $this->viewer = $viewer;
@@ -26,154 +18,51 @@ final class DifferentialInlineCommentQuery
     return $this->viewer;
   }
 
-  public function withIDs(array $ids) {
+  public function setInlines(array $inlines) {
-    $this->ids = $ids;
+    assert_instances_of($inlines, 'DifferentialInlineComment');
+    $this->inlines = $inlines;
     return $this;
   }
 
-  public function withPHIDs(array $phids) {
+  public function getInlines() {
-    $this->phids = $phids;
+    return $this->inlines;
+  }
+
+  public function setOldChangesets(array $old_changesets) {
+    assert_instances_of($old_changesets, 'DifferentialChangeset');
+    $this->oldChangesets = $old_changesets;
     return $this;
   }
 
-  public function withDrafts($drafts) {
+  public function getOldChangesets() {
-    $this->drafts = $drafts;
+    return $this->oldChangesets;
+  }
+
+  public function setNewChangesets(array $new_changesets) {
+    assert_instances_of($new_changesets, 'DifferentialChangeset');
+    $this->newChangesets = $new_changesets;
     return $this;
   }
 
-  public function withAuthorPHIDs(array $author_phids) {
+  public function getNewChangesets() {
-    $this->authorPHIDs = $author_phids;
+    return $this->newChangesets;
+  }
+
+  public function setRevision(DifferentialRevision $revision) {
+    $this->revision = $revision;
     return $this;
   }
 
-  public function withRevisionPHIDs(array $revision_phids) {
+  public function getRevision() {
-    $this->revisionPHIDs = $revision_phids;
+    return $this->revision;
-    return $this;
-  }
-
-  public function withDeletedDrafts($deleted_drafts) {
-    $this->deletedDrafts = $deleted_drafts;
-    return $this;
-  }
-
-  public function needHidden($need) {
-    $this->needHidden = $need;
-    return $this;
   }
 
   public function execute() {
-    $table = new DifferentialTransactionComment();
-    $conn_r = $table->establishConnection('r');
-
-    $data = queryfx_all(
-      $conn_r,
-      'SELECT * FROM %T %Q %Q',
-      $table->getTableName(),
-      $this->buildWhereClause($conn_r),
-      $this->buildLimitClause($conn_r));
-
-    $comments = $table->loadAllFromArray($data);
-
-    if ($this->needHidden) {
-      $viewer_phid = $this->getViewer()->getPHID();
-      if ($viewer_phid && $comments) {
-        $hidden = queryfx_all(
-          $conn_r,
-          'SELECT commentID FROM %T WHERE userPHID = %s
-            AND commentID IN (%Ls)',
-          id(new DifferentialHiddenComment())->getTableName(),
-          $viewer_phid,
-          mpull($comments, 'getID'));
-        $hidden = array_fuse(ipull($hidden, 'commentID'));
-      } else {
-        $hidden = array();
-      }
-
-      foreach ($comments as $inline) {
-        $inline->attachIsHidden(isset($hidden[$inline->getID()]));
-      }
-    }
-
-    foreach ($comments as $key => $value) {
-      $comments[$key] = DifferentialInlineComment::newFromModernComment(
-        $value);
-    }
-
-    return $comments;
-  }
-
-  public function executeOne() {
-    // TODO: Remove when this query moves to PolicyAware.
-    return head($this->execute());
-  }
-
-  protected function buildWhereClause(AphrontDatabaseConnection $conn) {
-    $where = array();
-
-    // Only find inline comments.
-    $where[] = qsprintf(
-      $conn,
-      'changesetID IS NOT NULL');
-
-    if ($this->ids !== null) {
-      $where[] = qsprintf(
-        $conn,
-        'id IN (%Ld)',
-        $this->ids);
-    }
-
-    if ($this->phids !== null) {
-      $where[] = qsprintf(
-        $conn,
-        'phid IN (%Ls)',
-        $this->phids);
-    }
-
-    if ($this->revisionPHIDs !== null) {
-      $where[] = qsprintf(
-        $conn,
-        'revisionPHID IN (%Ls)',
-        $this->revisionPHIDs);
-    }
-
-    if ($this->drafts === null) {
-      if ($this->deletedDrafts) {
-        $where[] = qsprintf(
-          $conn,
-          '(authorPHID = %s) OR (transactionPHID IS NOT NULL)',
-          $this->getViewer()->getPHID());
-      } else {
-        $where[] = qsprintf(
-          $conn,
-          '(authorPHID = %s AND isDeleted = 0)
-            OR (transactionPHID IS NOT NULL)',
-          $this->getViewer()->getPHID());
-      }
-    } else if ($this->drafts) {
-      $where[] = qsprintf(
-        $conn,
-        '(authorPHID = %s AND isDeleted = 0) AND (transactionPHID IS NULL)',
-        $this->getViewer()->getPHID());
-    } else {
-      $where[] = qsprintf(
-        $conn,
-        'transactionPHID IS NOT NULL');
-    }
-
-    return $this->formatWhereClause($conn, $where);
-  }
-
-  public function adjustInlinesForChangesets(
-    array $inlines,
-    array $old,
-    array $new,
-    DifferentialRevision $revision) {
-
-    assert_instances_of($inlines, 'DifferentialInlineComment');
-    assert_instances_of($old, 'DifferentialChangeset');
-    assert_instances_of($new, 'DifferentialChangeset');
-
     $viewer = $this->getViewer();
+    $inlines = $this->getInlines();
+    $revision = $this->getRevision();
+    $old = $this->getOldChangesets();
+    $new = $this->getNewChangesets();
 
     $no_ghosts = $viewer->compareUserSetting(
       PhabricatorOlderInlinesSetting::SETTINGKEY,

src/infrastructure/diff/query/PhabricatorDiffInlineCommentQuery.php

@@ -7,31 +7,40 @@ abstract class PhabricatorDiffInlineCommentQuery
   private $needReplyToComments;
   private $publishedComments;
   private $publishableComments;
+  private $needHidden;
 
   abstract protected function buildInlineCommentWhereClauseParts(
     AphrontDatabaseConnection $conn);
   abstract public function withObjectPHIDs(array $phids);
+  abstract protected function loadHiddenCommentIDs(
+    $viewer_phid,
+    array $comments);
 
-  public function withFixedStates(array $states) {
+  final public function withFixedStates(array $states) {
     $this->fixedStates = $states;
     return $this;
   }
 
-  public function needReplyToComments($need_reply_to) {
+  final public function needReplyToComments($need_reply_to) {
     $this->needReplyToComments = $need_reply_to;
     return $this;
   }
 
-  public function withPublishableComments($with_publishable) {
+  final public function withPublishableComments($with_publishable) {
     $this->publishableComments = $with_publishable;
     return $this;
   }
 
-  public function withPublishedComments($with_published) {
+  final public function withPublishedComments($with_published) {
     $this->publishedComments = $with_published;
     return $this;
   }
 
+  final public function needHidden($need_hidden) {
+    $this->needHidden = $need_hidden;
+    return $this;
+  }
+
   protected function buildWhereClauseParts(AphrontDatabaseConnection $conn) {
     $where = parent::buildWhereClauseParts($conn);
     $alias = $this->getPrimaryTableAlias();
@@ -152,6 +161,27 @@ abstract class PhabricatorDiffInlineCommentQuery
       }
     }
 
+    if (!$comments) {
+      return $comments;
+    }
+
+    if ($this->needHidden) {
+      $viewer = $this->getViewer();
+      $viewer_phid = $viewer->getPHID();
+
+      if ($viewer_phid) {
+        $hidden = $this->loadHiddenCommentIDs(
+          $viewer_phid,
+          $comments);
+      } else {
+        $hidden = array();
+      }
+
+      foreach ($comments as $inline) {
+        $inline->attachIsHidden(isset($hidden[$inline->getID()]));
+      }
+    }
+
     return $comments;
   }