Page MenuHomePhabricator
Files F14035199

D12483.diff
No OneTemporary
Actions

D12483.diff
View Options

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
@@ -287,7 +287,8 @@
}
return id(new DifferentialInlineCommentQuery())
- ->withViewerAndChangesetIDs($author_phid, $changeset_ids)
+ ->setViewer($this->getViewer())
+ ->withChangesetIDs($changeset_ids)
->execute();
}
diff --git a/src/applications/differential/controller/DifferentialInlineCommentEditController.php b/src/applications/differential/controller/DifferentialInlineCommentEditController.php
--- a/src/applications/differential/controller/DifferentialInlineCommentEditController.php
+++ b/src/applications/differential/controller/DifferentialInlineCommentEditController.php
@@ -39,13 +39,17 @@
protected function loadComment($id) {
return id(new DifferentialInlineCommentQuery())
+ ->setViewer($this->getViewer())
->withIDs(array($id))
+ ->withDeletedDrafts(true)
->executeOne();
}
protected function loadCommentByPHID($phid) {
return id(new DifferentialInlineCommentQuery())
+ ->setViewer($this->getViewer())
->withPHIDs(array($phid))
+ ->withDeletedDrafts(true)
->executeOne();
}
diff --git a/src/applications/differential/controller/DifferentialInlineCommentPreviewController.php b/src/applications/differential/controller/DifferentialInlineCommentPreviewController.php
--- a/src/applications/differential/controller/DifferentialInlineCommentPreviewController.php
+++ b/src/applications/differential/controller/DifferentialInlineCommentPreviewController.php
@@ -6,8 +6,19 @@
protected function loadInlineComments() {
$viewer = $this->getViewer();
+ $revision = id(new DifferentialRevisionQuery())
+ ->setViewer($viewer)
+ ->withIDs(array($this->getRevisionID()))
+ ->executeOne();
+ if (!$revision) {
+ return array();
+ }
+
return id(new DifferentialInlineCommentQuery())
- ->withDraftComments($viewer->getPHID(), $this->getRevisionID())
+ ->setViewer($this->getViewer())
+ ->withDrafts(true)
+ ->withAuthorPHIDs(array($viewer->getPHID()))
+ ->withRevisionPHIDs(array($revision->getPHID()))
->execute();
}
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
@@ -96,7 +96,7 @@
$props = mpull($props, 'getData', 'getName');
$all_changesets = $changesets;
- $inlines = $revision->loadInlineComments($all_changesets);
+ $inlines = $revision->loadInlineComments($all_changesets, $user);
$object_phids = array_merge(
$revision->getReviewers(),
@@ -158,7 +158,10 @@
$warning = $warning->render();
$my_inlines = id(new DifferentialInlineCommentQuery())
- ->withDraftComments($user->getPHID(), $this->revisionID)
+ ->setViewer($user)
+ ->withDrafts(true)
+ ->withAuthorPHIDs(array($user->getPHID()))
+ ->withRevisionPHIDs(array($revision->getPHID()))
->execute();
$visible_changesets = array();
diff --git a/src/applications/differential/query/DifferentialInlineCommentQuery.php b/src/applications/differential/query/DifferentialInlineCommentQuery.php
--- a/src/applications/differential/query/DifferentialInlineCommentQuery.php
+++ b/src/applications/differential/query/DifferentialInlineCommentQuery.php
@@ -6,24 +6,24 @@
final class DifferentialInlineCommentQuery
extends PhabricatorOffsetPagedQuery {
- private $revisionIDs;
- private $notDraft;
+ // TODO: Remove this when this query eventually moves to PolicyAware.
+ private $viewer;
+
private $ids;
private $phids;
- private $commentIDs;
-
- private $viewerAndChangesetIDs;
- private $draftComments;
- private $draftsByAuthors;
-
- public function withRevisionIDs(array $ids) {
- $this->revisionIDs = $ids;
+ private $drafts;
+ private $authorPHIDs;
+ private $changesetIDs;
+ private $revisionPHIDs;
+ private $deletedDrafts;
+
+ public function setViewer(PhabricatorUser $viewer) {
+ $this->viewer = $viewer;
return $this;
}
- public function withNotDraft($not_draft) {
- $this->notDraft = $not_draft;
- return $this;
+ public function getViewer() {
+ return $this->viewer;
}
public function withIDs(array $ids) {
@@ -36,18 +36,28 @@
return $this;
}
- public function withViewerAndChangesetIDs($author_phid, array $ids) {
- $this->viewerAndChangesetIDs = array($author_phid, $ids);
+ public function withDrafts($drafts) {
+ $this->drafts = $drafts;
return $this;
}
- public function withDraftComments($author_phid, $revision_id) {
- $this->draftComments = array($author_phid, $revision_id);
+ public function withAuthorPHIDs(array $author_phids) {
+ $this->authorPHIDs = $author_phids;
return $this;
}
- public function withDraftsByAuthors(array $author_phids) {
- $this->draftsByAuthors = $author_phids;
+ public function withChangesetIDs(array $ids) {
+ $this->changesetIDs = $ids;
+ return $this;
+ }
+
+ public function withRevisionPHIDs(array $revision_phids) {
+ $this->revisionPHIDs = $revision_phids;
+ return $this;
+ }
+
+ public function withDeletedDrafts($deleted_drafts) {
+ $this->deletedDrafts = $deleted_drafts;
return $this;
}
@@ -73,6 +83,7 @@
}
public function executeOne() {
+ // TODO: Remove when this query moves to PolicyAware.
return head($this->execute());
}
@@ -84,33 +95,7 @@
$conn_r,
'changesetID IS NOT NULL');
- if ($this->revisionIDs) {
-
- // Look up revision PHIDs.
- $revision_phids = queryfx_all(
- $conn_r,
- 'SELECT phid FROM %T WHERE id IN (%Ld)',
- id(new DifferentialRevision())->getTableName(),
- $this->revisionIDs);
-
- if (!$revision_phids) {
- throw new PhabricatorEmptyQueryException();
- }
- $revision_phids = ipull($revision_phids, 'phid');
-
- $where[] = qsprintf(
- $conn_r,
- 'revisionPHID IN (%Ls)',
- $revision_phids);
- }
-
- if ($this->notDraft) {
- $where[] = qsprintf(
- $conn_r,
- 'transactionPHID IS NOT NULL');
- }
-
- if ($this->ids) {
+ if ($this->ids !== null) {
$where[] = qsprintf(
$conn_r,
'id IN (%Ld)',
@@ -124,44 +109,42 @@
$this->phids);
}
- if ($this->viewerAndChangesetIDs) {
- list($phid, $ids) = $this->viewerAndChangesetIDs;
+ if ($this->revisionPHIDs !== null) {
$where[] = qsprintf(
$conn_r,
- 'changesetID IN (%Ld) AND
- ((authorPHID = %s AND isDeleted = 0) OR transactionPHID IS NOT NULL)',
- $ids,
- $phid);
+ 'revisionPHID IN (%Ls)',
+ $this->revisionPHIDs);
}
- if ($this->draftComments) {
- list($phid, $rev_id) = $this->draftComments;
-
- $rev_phid = queryfx_one(
+ if ($this->changesetIDs !== null) {
+ $where[] = qsprintf(
$conn_r,
- 'SELECT phid FROM %T WHERE id = %d',
- id(new DifferentialRevision())->getTableName(),
- $rev_id);
+ 'changesetID IN (%Ld)',
+ $this->changesetIDs);
+ }
- if (!$rev_phid) {
- throw new PhabricatorEmptyQueryException();
+ if ($this->drafts === null) {
+ if ($this->deletedDrafts) {
+ $where[] = qsprintf(
+ $conn_r,
+ '(authorPHID = %s) OR (transactionPHID IS NOT NULL)',
+ $this->getViewer()->getPHID());
+ } else {
+ $where[] = qsprintf(
+ $conn_r,
+ '(authorPHID = %s AND isDeleted = 0)
+ OR (transactionPHID IS NOT NULL)',
+ $this->getViewer()->getPHID());
}
-
- $rev_phid = $rev_phid['phid'];
-
+ } else if ($this->drafts) {
$where[] = qsprintf(
$conn_r,
- 'authorPHID = %s AND revisionPHID = %s AND transactionPHID IS NULL
- AND isDeleted = 0',
- $phid,
- $rev_phid);
- }
-
- if ($this->draftsByAuthors) {
+ '(authorPHID = %s AND isDeleted = 0) AND (transactionPHID IS NULL)',
+ $this->getViewer()->getPHID());
+ } else {
$where[] = qsprintf(
$conn_r,
- 'authorPHID IN (%Ls) AND isDeleted = 0 AND transactionPHID IS NULL',
- $this->draftsByAuthors);
+ 'transactionPHID IS NOT NULL');
}
return $this->formatWhereClause($where);
diff --git a/src/applications/differential/storage/DifferentialRevision.php b/src/applications/differential/storage/DifferentialRevision.php
--- a/src/applications/differential/storage/DifferentialRevision.php
+++ b/src/applications/differential/storage/DifferentialRevision.php
@@ -281,14 +281,16 @@
}
public function loadInlineComments(
- array &$changesets) {
+ array &$changesets,
+ PhabricatorUser $viewer) {
assert_instances_of($changesets, 'DifferentialChangeset');
$inline_comments = array();
$inline_comments = id(new DifferentialInlineCommentQuery())
- ->withRevisionIDs(array($this->getID()))
- ->withNotDraft(true)
+ ->setViewer($viewer)
+ ->withDrafts(false)
+ ->withRevisionPHIDs(array($this->getPHID()))
->execute();
$load_changesets = array();
@@ -535,7 +537,7 @@
->execute();
// NOTE: this mutates $changesets to include changesets for all inline
// comments...!
- $inlines = $this->loadInlineComments($changesets);
+ $inlines = $this->loadInlineComments($changesets, $request->getViewer());
$changesets = mpull($changesets, null, 'getID');
return $timeline
@@ -569,18 +571,6 @@
self::TABLE_COMMIT,
$this->getID());
- try {
- $inlines = id(new DifferentialInlineCommentQuery())
- ->withRevisionIDs(array($this->getID()))
- ->execute();
- foreach ($inlines as $inline) {
- $inline->delete();
- }
- } catch (PhabricatorEmptyQueryException $ex) {
- // TODO: There's still some funky legacy wrapping going on here, and
- // we might catch a raw query exception.
- }
-
// we have to do paths a little differentally as they do not have
// an id or phid column for delete() to act on
$dummy_path = new DifferentialAffectedPath();

File Metadata

Mime Type
text/plain
Expires
Mon, Nov 11, 4:50 AM (1 w, 1 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6741085
Default Alt Text
D12483.diff (10 KB)

Event Timeline

Phabricator · Built by Phacility