From 7427a6e648cf913dcf7cc82ef4151ad07c4d42e6 Mon Sep 17 00:00:00 2001 From: epriestley Date: Mon, 9 Mar 2015 12:40:29 -0700 Subject: [PATCH] Extend TransactionCommentQuery for Differential Summary: Ref T2009. Ref T1460. Replace hard-coded garbage with a real Query-layer query. Test Plan: Submitted inline comments in Differential. Reviewers: btrahan Reviewed By: btrahan Subscribers: epriestley Maniphest Tasks: T2009, T1460 Differential Revision: https://secure.phabricator.com/D12027 --- src/__phutil_library_map__.php | 4 ++ .../DifferentialDiffInlineCommentQuery.php | 31 +++++++++++ .../query/DifferentialTransactionQuery.php | 36 +++---------- ...atorApplicationTransactionCommentQuery.php | 30 +++++++++-- .../PhabricatorDiffInlineCommentQuery.php | 53 +++++++++++++++++++ 5 files changed, 121 insertions(+), 33 deletions(-) create mode 100644 src/applications/differential/query/DifferentialDiffInlineCommentQuery.php create mode 100644 src/infrastructure/diff/query/PhabricatorDiffInlineCommentQuery.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 05b8ad18e9..717692109e 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -339,6 +339,7 @@ phutil_register_library_map(array( 'DifferentialDiff' => 'applications/differential/storage/DifferentialDiff.php', 'DifferentialDiffCreateController' => 'applications/differential/controller/DifferentialDiffCreateController.php', 'DifferentialDiffEditor' => 'applications/differential/editor/DifferentialDiffEditor.php', + 'DifferentialDiffInlineCommentQuery' => 'applications/differential/query/DifferentialDiffInlineCommentQuery.php', 'DifferentialDiffPHIDType' => 'applications/differential/phid/DifferentialDiffPHIDType.php', 'DifferentialDiffProperty' => 'applications/differential/storage/DifferentialDiffProperty.php', 'DifferentialDiffQuery' => 'applications/differential/query/DifferentialDiffQuery.php', @@ -1692,6 +1693,7 @@ phutil_register_library_map(array( 'PhabricatorDestructionEngine' => 'applications/system/engine/PhabricatorDestructionEngine.php', 'PhabricatorDeveloperConfigOptions' => 'applications/config/option/PhabricatorDeveloperConfigOptions.php', 'PhabricatorDeveloperPreferencesSettingsPanel' => 'applications/settings/panel/PhabricatorDeveloperPreferencesSettingsPanel.php', + 'PhabricatorDiffInlineCommentQuery' => 'infrastructure/diff/query/PhabricatorDiffInlineCommentQuery.php', 'PhabricatorDiffPreferencesSettingsPanel' => 'applications/settings/panel/PhabricatorDiffPreferencesSettingsPanel.php', 'PhabricatorDifferenceEngine' => 'infrastructure/diff/PhabricatorDifferenceEngine.php', 'PhabricatorDifferentialApplication' => 'applications/differential/application/PhabricatorDifferentialApplication.php', @@ -3487,6 +3489,7 @@ phutil_register_library_map(array( ), 'DifferentialDiffCreateController' => 'DifferentialController', 'DifferentialDiffEditor' => 'PhabricatorApplicationTransactionEditor', + 'DifferentialDiffInlineCommentQuery' => 'PhabricatorDiffInlineCommentQuery', 'DifferentialDiffPHIDType' => 'PhabricatorPHIDType', 'DifferentialDiffProperty' => 'DifferentialDAO', 'DifferentialDiffQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', @@ -4974,6 +4977,7 @@ phutil_register_library_map(array( 'PhabricatorDestructionEngine' => 'Phobject', 'PhabricatorDeveloperConfigOptions' => 'PhabricatorApplicationConfigOptions', 'PhabricatorDeveloperPreferencesSettingsPanel' => 'PhabricatorSettingsPanel', + 'PhabricatorDiffInlineCommentQuery' => 'PhabricatorApplicationTransactionCommentQuery', 'PhabricatorDiffPreferencesSettingsPanel' => 'PhabricatorSettingsPanel', 'PhabricatorDifferentialApplication' => 'PhabricatorApplication', 'PhabricatorDifferentialConfigOptions' => 'PhabricatorApplicationConfigOptions', diff --git a/src/applications/differential/query/DifferentialDiffInlineCommentQuery.php b/src/applications/differential/query/DifferentialDiffInlineCommentQuery.php new file mode 100644 index 0000000000..e29d63320e --- /dev/null +++ b/src/applications/differential/query/DifferentialDiffInlineCommentQuery.php @@ -0,0 +1,31 @@ +revisionPHIDs = $phids; + return $this; + } + + protected function getTemplate() { + return new DifferentialTransactionComment(); + } + + protected function buildWhereClauseComponents( + AphrontDatabaseConnection $conn_r) { + $where = parent::buildWhereClauseComponents($conn_r); + + if ($this->revisionPHIDs !== null) { + $where[] = qsprintf( + $conn_r, + 'revisionPHID IN (%Ls)', + $this->revisionPHIDs); + } + + return $where; + } + +} diff --git a/src/applications/differential/query/DifferentialTransactionQuery.php b/src/applications/differential/query/DifferentialTransactionQuery.php index 72e247aa36..8b60bbc03f 100644 --- a/src/applications/differential/query/DifferentialTransactionQuery.php +++ b/src/applications/differential/query/DifferentialTransactionQuery.php @@ -11,38 +11,14 @@ final class DifferentialTransactionQuery PhabricatorUser $viewer, DifferentialRevision $revision) { - // TODO: Subclass ApplicationTransactionCommentQuery to do this for real. - - $table = new DifferentialTransactionComment(); - $conn_r = $table->establishConnection('r'); - - $phids = queryfx_all( - $conn_r, - 'SELECT phid FROM %T - WHERE revisionPHID = %s - AND authorPHID = %s - AND transactionPHID IS NULL - AND isDeleted = 0', - $table->getTableName(), - $revision->getPHID(), - $viewer->getPHID()); - - $phids = ipull($phids, 'phid'); - if (!$phids) { - return array(); - } - - $comments = id(new PhabricatorApplicationTransactionTemplatedCommentQuery()) - ->setTemplate(new DifferentialTransactionComment()) + return id(new DifferentialDiffInlineCommentQuery()) ->setViewer($viewer) - ->withPHIDs($phids) + ->withRevisionPHIDs(array($revision->getPHID())) + ->withAuthorPHIDs(array($viewer->getPHID())) + ->withHasTransaction(false) + ->withIsDeleted(false) + ->needReplyToComments(true) ->execute(); - - $comments = PhabricatorInlineCommentController::loadAndAttachReplies( - $viewer, - $comments); - - return $comments; } } diff --git a/src/applications/transactions/query/PhabricatorApplicationTransactionCommentQuery.php b/src/applications/transactions/query/PhabricatorApplicationTransactionCommentQuery.php index df188829f0..cd61c0fd30 100644 --- a/src/applications/transactions/query/PhabricatorApplicationTransactionCommentQuery.php +++ b/src/applications/transactions/query/PhabricatorApplicationTransactionCommentQuery.php @@ -8,6 +8,7 @@ abstract class PhabricatorApplicationTransactionCommentQuery private $phids; private $transactionPHIDs; private $isDeleted; + private $hasTransaction; abstract protected function getTemplate(); @@ -31,11 +32,16 @@ abstract class PhabricatorApplicationTransactionCommentQuery return $this; } - public function withDeleted($deleted) { + public function withIsDeleted($deleted) { $this->isDeleted = $deleted; return $this; } + public function withHasTransaction($has_transaction) { + $this->hasTransaction = $has_transaction; + return $this; + } + protected function loadPage() { $table = $this->getTemplate(); $conn_r = $table->establishConnection('r'); @@ -51,7 +57,13 @@ abstract class PhabricatorApplicationTransactionCommentQuery return $table->loadAllFromArray($data); } - protected function buildWhereClause(AphrontDatabaseConnection $conn_r) { + private function buildWhereClause(AphrontDatabaseConnection $conn_r) { + return $this->formatWhereClause($this->buildWhereClauseComponents($conn_r)); + } + + protected function buildWhereClauseComponents( + AphrontDatabaseConnection $conn_r) { + $where = array(); if ($this->ids !== null) { @@ -89,7 +101,19 @@ abstract class PhabricatorApplicationTransactionCommentQuery (int)$this->isDeleted); } - return $this->formatWhereClause($where); + if ($this->hasTransaction !== null) { + if ($this->hasTransaction) { + $where[] = qsprintf( + $conn_r, + 'xcomment.transactionPHID IS NOT NULL'); + } else { + $where[] = qsprintf( + $conn_r, + 'xcomment.transactionPHID IS NULL'); + } + } + + return $where; } public function getQueryApplicationClass() { diff --git a/src/infrastructure/diff/query/PhabricatorDiffInlineCommentQuery.php b/src/infrastructure/diff/query/PhabricatorDiffInlineCommentQuery.php new file mode 100644 index 0000000000..9af8cd9d44 --- /dev/null +++ b/src/infrastructure/diff/query/PhabricatorDiffInlineCommentQuery.php @@ -0,0 +1,53 @@ +needReplyToComments = $need_reply_to; + return $this; + } + + protected function willFilterPage(array $comments) { + if ($this->needReplyToComments) { + $reply_phids = array(); + foreach ($comments as $comment) { + $reply_phid = $comment->getReplyToCommentPHID(); + if ($reply_phid) { + $reply_phids[] = $reply_phid; + } + } + + if ($reply_phids) { + $reply_comments = newv(get_class($this), array()) + ->setViewer($this->getViewer()) + ->setParentQuery($this) + ->withPHIDs($reply_phids) + ->execute(); + $reply_comments = mpull($reply_comments, null, 'getPHID'); + } else { + $reply_comments = array(); + } + + foreach ($comments as $key => $comment) { + $reply_phid = $comment->getReplyToCommentPHID(); + if (!$reply_phid) { + $comment->attachReplyToComment(null); + continue; + } + $reply = idx($reply_comments, $reply_phid); + if (!$reply) { + $this->didRejectResult($comment); + unset($comments[$key]); + continue; + } + $comment->attachReplyToComment($reply); + } + } + + return $comments; + } + +}