Page MenuHomePhabricator
Files F15450270

D21233.id.diff
No OneTemporary
Actions

D21233.id.diff
View Options

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 @@
<?php
-/**
- * Temporary wrapper for transitioning Differential to ApplicationTransactions.
- */
-final class DifferentialInlineCommentQuery
- extends PhabricatorOffsetPagedQuery {
+final class PhabricatorInlineCommentAdjustmentEngine
+ extends Phobject {
- // TODO: Remove this when this query eventually moves to PolicyAware.
private $viewer;
-
- private $ids;
- private $phids;
- private $drafts;
- private $authorPHIDs;
- private $revisionPHIDs;
- private $deletedDrafts;
- private $needHidden;
+ private $inlines;
+ private $revision;
+ private $oldChangesets;
+ private $newChangesets;
public function setViewer(PhabricatorUser $viewer) {
$this->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;
}

File Metadata

Mime Type
text/plain
Expires
Sat, Mar 29, 2:48 PM (1 w, 3 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7379722
Default Alt Text
D21233.id.diff (16 KB)

Event Timeline

Phabricator · Built by Phacility