diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -561,7 +561,6 @@ '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 @@ '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 @@ 'DifferentialInlineComment' => 'PhabricatorInlineComment', 'DifferentialInlineCommentEditController' => 'PhabricatorInlineCommentController', 'DifferentialInlineCommentMailView' => 'DifferentialMailView', - 'DifferentialInlineCommentQuery' => 'PhabricatorOffsetPagedQuery', 'DifferentialJIRAIssuesCommitMessageField' => 'DifferentialCommitMessageCustomField', 'DifferentialJIRAIssuesField' => 'DifferentialStoredCustomField', 'DifferentialLegacyQuery' => 'Phobject', @@ -10118,6 +10117,7 @@ 'Phobject', 'PhabricatorMarkupInterface', ), + 'PhabricatorInlineCommentAdjustmentEngine' => 'Phobject', 'PhabricatorInlineCommentController' => 'PhabricatorController', 'PhabricatorInlineSummaryView' => 'AphrontView', 'PhabricatorInstructionsEditField' => 'PhabricatorEditField', diff --git a/src/applications/differential/controller/DifferentialChangesetViewController.php b/src/applications/differential/controller/DifferentialChangesetViewController.php --- a/src/applications/differential/controller/DifferentialChangesetViewController.php +++ b/src/applications/differential/controller/DifferentialChangesetViewController.php @@ -194,16 +194,23 @@ // 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())); - $inlines = $query->execute(); - $inlines = $query->adjustInlinesForChangesets( - $inlines, - $old, - $new, - $revision); + ->execute(); + + $inlines = mpull($inlines, 'newInlineCommentObject'); + + $inlines = id(new PhabricatorInlineCommentAdjustmentEngine()) + ->setViewer($viewer) + ->setRevision($revision) + ->setOldChangesets($old) + ->setNewChangesets($new) + ->setInlines($inlines) + ->execute(); } else { $inlines = array(); } diff --git a/src/applications/differential/controller/DifferentialRevisionViewController.php b/src/applications/differential/controller/DifferentialRevisionViewController.php --- a/src/applications/differential/controller/DifferentialRevisionViewController.php +++ b/src/applications/differential/controller/DifferentialRevisionViewController.php @@ -227,16 +227,20 @@ $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())); - $inlines = $query->execute(); - $inlines = $query->adjustInlinesForChangesets( - $inlines, - $old, - $new, - $revision); + ->withRevisionPHIDs(array($revision->getPHID())) + ->withPublishableComments(true) + ->withPublishedComments(true) + ->execute(); + + $inlines = id(new PhabricatorInlineCommentAdjustmentEngine()) + ->setViewer($viewer) + ->setRevision($revision) + ->setOldChangesets($old) + ->setNewChangesets($new) + ->setInlines($inlines) + ->execute(); foreach ($inlines as $inline) { $changeset_id = $inline->getChangesetID(); diff --git a/src/applications/differential/engine/DifferentialRevisionTimelineEngine.php b/src/applications/differential/engine/DifferentialRevisionTimelineEngine.php --- a/src/applications/differential/engine/DifferentialRevisionTimelineEngine.php +++ b/src/applications/differential/engine/DifferentialRevisionTimelineEngine.php @@ -50,22 +50,22 @@ } foreach ($inlines as $key => $inline) { - $inlines[$key] = DifferentialInlineComment::newFromModernComment( - $inline); + $inlines[$key] = $inline->newInlineCommentObject(); } - $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, - array_select_keys($changesets, $old_ids), - array_select_keys($changesets, $new_ids), - $revision); + + $old = array_select_keys($changesets, $old_ids); + $new = array_select_keys($changesets, $new_ids); + id(new PhabricatorInlineCommentAdjustmentEngine()) + ->setViewer($viewer) + ->setRevision($revision) + ->setOldChangesets($old) + ->setNewChangesets($new) + ->setInlines($inlines) + ->execute(); return id(new DifferentialTransactionView()) ->setViewData($view_data) diff --git a/src/applications/differential/query/DifferentialDiffInlineCommentQuery.php b/src/applications/differential/query/DifferentialDiffInlineCommentQuery.php --- a/src/applications/differential/query/DifferentialDiffInlineCommentQuery.php +++ b/src/applications/differential/query/DifferentialDiffInlineCommentQuery.php @@ -45,4 +45,26 @@ 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; + } + } diff --git a/src/applications/diffusion/query/DiffusionDiffInlineCommentQuery.php b/src/applications/diffusion/query/DiffusionDiffInlineCommentQuery.php --- a/src/applications/diffusion/query/DiffusionDiffInlineCommentQuery.php +++ b/src/applications/diffusion/query/DiffusionDiffInlineCommentQuery.php @@ -60,4 +60,10 @@ return $where; } + protected function loadHiddenCommentIDs( + $viewer_phid, + array $comments) { + return array(); + } + } diff --git a/src/applications/differential/query/DifferentialInlineCommentQuery.php b/src/infrastructure/diff/engine/PhabricatorInlineCommentAdjustmentEngine.php rename from src/applications/differential/query/DifferentialInlineCommentQuery.php rename to src/infrastructure/diff/engine/PhabricatorInlineCommentAdjustmentEngine.php --- a/src/applications/differential/query/DifferentialInlineCommentQuery.php +++ b/src/infrastructure/diff/engine/PhabricatorInlineCommentAdjustmentEngine.php @@ -1,21 +1,13 @@ viewer = $viewer; @@ -26,154 +18,51 @@ return $this->viewer; } - public function withIDs(array $ids) { - $this->ids = $ids; + public function setInlines(array $inlines) { + assert_instances_of($inlines, 'DifferentialInlineComment'); + $this->inlines = $inlines; return $this; } - public function withPHIDs(array $phids) { - $this->phids = $phids; - return $this; + public function getInlines() { + return $this->inlines; } - public function withDrafts($drafts) { - $this->drafts = $drafts; + public function setOldChangesets(array $old_changesets) { + assert_instances_of($old_changesets, 'DifferentialChangeset'); + $this->oldChangesets = $old_changesets; return $this; } - public function withAuthorPHIDs(array $author_phids) { - $this->authorPHIDs = $author_phids; - return $this; + public function getOldChangesets() { + return $this->oldChangesets; } - public function withRevisionPHIDs(array $revision_phids) { - $this->revisionPHIDs = $revision_phids; + public function setNewChangesets(array $new_changesets) { + assert_instances_of($new_changesets, 'DifferentialChangeset'); + $this->newChangesets = $new_changesets; return $this; } - public function withDeletedDrafts($deleted_drafts) { - $this->deletedDrafts = $deleted_drafts; - return $this; + public function getNewChangesets() { + return $this->newChangesets; } - public function needHidden($need) { - $this->needHidden = $need; + public function setRevision(DifferentialRevision $revision) { + $this->revision = $revision; 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()); + public function getRevision() { + return $this->revision; } - 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'); - + public function execute() { $viewer = $this->getViewer(); + $inlines = $this->getInlines(); + $revision = $this->getRevision(); + $old = $this->getOldChangesets(); + $new = $this->getNewChangesets(); $no_ghosts = $viewer->compareUserSetting( PhabricatorOlderInlinesSetting::SETTINGKEY, diff --git a/src/infrastructure/diff/query/PhabricatorDiffInlineCommentQuery.php b/src/infrastructure/diff/query/PhabricatorDiffInlineCommentQuery.php --- a/src/infrastructure/diff/query/PhabricatorDiffInlineCommentQuery.php +++ b/src/infrastructure/diff/query/PhabricatorDiffInlineCommentQuery.php @@ -7,31 +7,40 @@ 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 @@ } } + 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; }