diff --git a/src/applications/audit/storage/PhabricatorAuditInlineComment.php b/src/applications/audit/storage/PhabricatorAuditInlineComment.php index c33a5de8e1..f7686fcf0c 100644 --- a/src/applications/audit/storage/PhabricatorAuditInlineComment.php +++ b/src/applications/audit/storage/PhabricatorAuditInlineComment.php @@ -89,29 +89,6 @@ final class PhabricatorAuditInlineComment return self::buildProxies($inlines); } - public static function loadDraftAndPublishedComments( - PhabricatorUser $viewer, - $commit_phid, - $path_id = null) { - - if ($path_id === null) { - $inlines = id(new PhabricatorAuditTransactionComment())->loadAllWhere( - 'commitPHID = %s AND (transactionPHID IS NOT NULL OR authorPHID = %s) - AND pathID IS NOT NULL', - $commit_phid, - $viewer->getPHID()); - } else { - $inlines = id(new PhabricatorAuditTransactionComment())->loadAllWhere( - 'commitPHID = %s AND pathID = %d AND - ((authorPHID = %s AND isDeleted = 0) OR transactionPHID IS NOT NULL)', - $commit_phid, - $path_id, - $viewer->getPHID()); - } - - return self::buildProxies($inlines); - } - private static function buildProxies(array $inlines) { $results = array(); foreach ($inlines as $key => $inline) { diff --git a/src/applications/diffusion/controller/DiffusionCommitController.php b/src/applications/diffusion/controller/DiffusionCommitController.php index 970999853c..c1858b3422 100644 --- a/src/applications/diffusion/controller/DiffusionCommitController.php +++ b/src/applications/diffusion/controller/DiffusionCommitController.php @@ -414,9 +414,14 @@ final class DiffusionCommitController extends DiffusionController { $visible_changesets = $changesets; } else { $visible_changesets = array(); - $inlines = PhabricatorAuditInlineComment::loadDraftAndPublishedComments( - $viewer, - $commit->getPHID()); + + $inlines = id(new DiffusionDiffInlineCommentQuery()) + ->setViewer($viewer) + ->withCommitPHIDs(array($commit->getPHID())) + ->withVisibleComments(true) + ->execute(); + $inlines = mpull($inlines, 'newInlineCommentObject'); + $path_ids = mpull($inlines, null, 'getPathID'); foreach ($changesets as $key => $changeset) { if (array_key_exists($changeset->getID(), $path_ids)) { diff --git a/src/applications/diffusion/controller/DiffusionDiffController.php b/src/applications/diffusion/controller/DiffusionDiffController.php index fc505ab245..51d815db0c 100644 --- a/src/applications/diffusion/controller/DiffusionDiffController.php +++ b/src/applications/diffusion/controller/DiffusionDiffController.php @@ -99,10 +99,13 @@ final class DiffusionDiffController extends DiffusionController { ($viewer->getPHID() == $commit->getAuthorPHID())); $parser->setObjectOwnerPHID($commit->getAuthorPHID()); - $inlines = PhabricatorAuditInlineComment::loadDraftAndPublishedComments( - $viewer, - $commit->getPHID(), - $path_id); + $inlines = id(new DiffusionDiffInlineCommentQuery()) + ->setViewer($viewer) + ->withCommitPHIDs(array($commit->getPHID())) + ->withPathIDs(array($path_id)) + ->withVisibleComments(true) + ->execute(); + $inlines = mpull($inlines, 'newInlineCommentObject'); if ($inlines) { foreach ($inlines as $inline) { diff --git a/src/infrastructure/diff/query/PhabricatorDiffInlineCommentQuery.php b/src/infrastructure/diff/query/PhabricatorDiffInlineCommentQuery.php index 48f98dc531..e978bd5614 100644 --- a/src/infrastructure/diff/query/PhabricatorDiffInlineCommentQuery.php +++ b/src/infrastructure/diff/query/PhabricatorDiffInlineCommentQuery.php @@ -5,6 +5,8 @@ abstract class PhabricatorDiffInlineCommentQuery private $fixedStates; private $needReplyToComments; + private $visibleComments; + private $publishableComments; abstract protected function buildInlineCommentWhereClauseParts( AphrontDatabaseConnection $conn); @@ -20,6 +22,16 @@ abstract class PhabricatorDiffInlineCommentQuery return $this; } + public function withVisibleComments($with_visible) { + $this->visibleComments = $with_visible; + return $this; + } + + public function withPublishableComments($with_publishable) { + $this->publishableComments = $with_publishable; + return $this; + } + protected function buildWhereClauseParts(AphrontDatabaseConnection $conn) { $where = parent::buildWhereClauseParts($conn); $alias = $this->getPrimaryTableAlias(); @@ -36,6 +48,72 @@ abstract class PhabricatorDiffInlineCommentQuery $this->fixedStates); } + $show_published = false; + $show_publishable = false; + + if ($this->visibleComments !== null) { + if (!$this->visibleComments) { + throw new Exception( + pht( + 'Querying for comments that are not visible is '. + 'not supported.')); + } + $show_published = true; + $show_publishable = true; + } + + if ($this->publishableComments !== null) { + if (!$this->publishableComments) { + throw new Exception( + pht( + 'Querying for comments that are not publishable is '. + 'not supported.')); + } + $show_publishable = true; + } + + if ($show_publishable || $show_published) { + $clauses = array(); + + if ($show_published) { + // Published comments are always visible. + $clauses[] = qsprintf( + $conn, + '%T.transactionPHID IS NOT NULL', + $alias); + } + + if ($show_publishable) { + $viewer = $this->getViewer(); + $viewer_phid = $viewer->getPHID(); + + // If the viewer has a PHID, unpublished comments they authored and + // have not deleted are visible. + if ($viewer_phid) { + $clauses[] = qsprintf( + $conn, + '%T.authorPHID = %s + AND %T.isDeleted = 0 + AND %T.transactionPHID IS NULL ', + $alias, + $viewer_phid, + $alias, + $alias); + } + } + + // We can end up with a known-empty query if we (for example) query for + // publishable comments and the viewer is logged-out. + if (!$clauses) { + throw new PhabricatorEmptyQueryException(); + } + + $where[] = qsprintf( + $conn, + '%LO', + $clauses); + } + return $where; }