diff --git a/src/applications/differential/conduit/DifferentialGetRevisionConduitAPIMethod.php b/src/applications/differential/conduit/DifferentialGetRevisionConduitAPIMethod.php index a158d86d9f..3b4d9a6a63 100644 --- a/src/applications/differential/conduit/DifferentialGetRevisionConduitAPIMethod.php +++ b/src/applications/differential/conduit/DifferentialGetRevisionConduitAPIMethod.php @@ -42,7 +42,6 @@ final class DifferentialGetRevisionConduitAPIMethod $revision = id(new DifferentialRevisionQuery()) ->withIDs(array($revision_id)) ->setViewer($request->getUser()) - ->needRelationships(true) ->needReviewerStatus(true) ->executeOne(); @@ -50,7 +49,7 @@ final class DifferentialGetRevisionConduitAPIMethod throw new ConduitException('ERR_BAD_REVISION'); } - $reviewer_phids = array_values($revision->getReviewers()); + $reviewer_phids = $revision->getReviewerPHIDs(); $diffs = id(new DifferentialDiffQuery()) ->setViewer($request->getUser()) diff --git a/src/applications/differential/conduit/DifferentialQueryConduitAPIMethod.php b/src/applications/differential/conduit/DifferentialQueryConduitAPIMethod.php index 720361367a..c9f90d0877 100644 --- a/src/applications/differential/conduit/DifferentialQueryConduitAPIMethod.php +++ b/src/applications/differential/conduit/DifferentialQueryConduitAPIMethod.php @@ -182,7 +182,7 @@ final class DifferentialQueryConduitAPIMethod $query->withBranches($branches); } - $query->needRelationships(true); + $query->needReviewerStatus(true); $query->needCommitPHIDs(true); $query->needDiffIDs(true); $query->needActiveDiffs(true); @@ -194,6 +194,14 @@ final class DifferentialQueryConduitAPIMethod $request->getUser(), $revisions); + if ($revisions) { + $ccs = id(new PhabricatorSubscribersQuery()) + ->withObjectPHIDs(mpull($revisions, 'getPHID')) + ->execute(); + } else { + $ccs = array(); + } + $results = array(); foreach ($revisions as $revision) { $diff = $revision->getActiveDiff(); @@ -224,8 +232,8 @@ final class DifferentialQueryConduitAPIMethod 'activeDiffPHID' => $diff->getPHID(), 'diffs' => $revision->getDiffIDs(), 'commits' => $revision->getCommitPHIDs(), - 'reviewers' => array_values($revision->getReviewers()), - 'ccs' => array_values($revision->getCCPHIDs()), + 'reviewers' => $revision->getReviewerPHIDs(), + 'ccs' => idx($ccs, $phid, array()), 'hashes' => $revision->getHashes(), 'auxiliary' => idx($field_data, $phid, array()), 'repositoryPHID' => $diff->getRepositoryPHID(), diff --git a/src/applications/differential/controller/DifferentialRevisionViewController.php b/src/applications/differential/controller/DifferentialRevisionViewController.php index d31da8fefc..9b87d1163d 100644 --- a/src/applications/differential/controller/DifferentialRevisionViewController.php +++ b/src/applications/differential/controller/DifferentialRevisionViewController.php @@ -17,7 +17,6 @@ final class DifferentialRevisionViewController extends DifferentialController { $revision = id(new DifferentialRevisionQuery()) ->withIDs(array($this->revisionID)) ->setViewer($viewer) - ->needRelationships(true) ->needReviewerStatus(true) ->needReviewerAuthority(true) ->executeOne(); @@ -103,9 +102,12 @@ final class DifferentialRevisionViewController extends DifferentialController { $this->loadDiffProperties($diffs); $props = $target_manual->getDiffProperties(); + $subscriber_phids = PhabricatorSubscribersQuery::loadSubscribersForPHID( + $revision->getPHID()); + $object_phids = array_merge( - $revision->getReviewers(), - $revision->getCCPHIDs(), + $revision->getReviewerPHIDs(), + $subscriber_phids, $revision->loadCommitPHIDs(), array( $revision->getAuthorPHID(), @@ -782,7 +784,7 @@ final class DifferentialRevisionViewController extends DifferentialController { ->setLimit(10) ->needFlags(true) ->needDrafts(true) - ->needRelationships(true); + ->needReviewerStatus(true); foreach ($path_map as $path => $path_id) { $query->withPath($repository->getID(), $path_id); diff --git a/src/applications/differential/doorkeeper/DifferentialDoorkeeperRevisionFeedStoryPublisher.php b/src/applications/differential/doorkeeper/DifferentialDoorkeeperRevisionFeedStoryPublisher.php index 23ad165ca9..135e9b560d 100644 --- a/src/applications/differential/doorkeeper/DifferentialDoorkeeperRevisionFeedStoryPublisher.php +++ b/src/applications/differential/doorkeeper/DifferentialDoorkeeperRevisionFeedStoryPublisher.php @@ -26,7 +26,7 @@ final class DifferentialDoorkeeperRevisionFeedStoryPublisher return id(new DifferentialRevisionQuery()) ->setViewer($this->getViewer()) ->withIDs(array($object->getID())) - ->needRelationships(true) + ->needReviewerStatus(true) ->executeOne(); } @@ -37,7 +37,7 @@ final class DifferentialDoorkeeperRevisionFeedStoryPublisher public function getActiveUserPHIDs($object) { $status = $object->getStatus(); if ($status == ArcanistDifferentialRevisionStatus::NEEDS_REVIEW) { - return $object->getReviewers(); + return $object->getReviewerPHIDs(); } else { return array(); } @@ -48,12 +48,13 @@ final class DifferentialDoorkeeperRevisionFeedStoryPublisher if ($status == ArcanistDifferentialRevisionStatus::NEEDS_REVIEW) { return array(); } else { - return $object->getReviewers(); + return $object->getReviewerPHIDs(); } } public function getCCUserPHIDs($object) { - return $object->getCCPHIDs(); + return PhabricatorSubscribersQuery::loadSubscribersForPHID( + $object->getPHID()); } public function getObjectTitle($object) { diff --git a/src/applications/differential/herald/HeraldDifferentialRevisionAdapter.php b/src/applications/differential/herald/HeraldDifferentialRevisionAdapter.php index 53fd62fe23..19d15c5c3f 100644 --- a/src/applications/differential/herald/HeraldDifferentialRevisionAdapter.php +++ b/src/applications/differential/herald/HeraldDifferentialRevisionAdapter.php @@ -85,7 +85,6 @@ final class HeraldDifferentialRevisionAdapter $revision = id(new DifferentialRevisionQuery()) ->withIDs(array($revision->getID())) ->setViewer(PhabricatorUser::getOmnipotentUser()) - ->needRelationships(true) ->needReviewerStatus(true) ->executeOne(); diff --git a/src/applications/differential/query/DifferentialRevisionQuery.php b/src/applications/differential/query/DifferentialRevisionQuery.php index c3fb98e607..1b60df777a 100644 --- a/src/applications/differential/query/DifferentialRevisionQuery.php +++ b/src/applications/differential/query/DifferentialRevisionQuery.php @@ -43,7 +43,6 @@ final class DifferentialRevisionQuery const ORDER_MODIFIED = 'order-modified'; const ORDER_CREATED = 'order-created'; - private $needRelationships = false; private $needActiveDiffs = false; private $needDiffIDs = false; private $needCommitPHIDs = false; @@ -227,20 +226,6 @@ final class DifferentialRevisionQuery } - - /** - * Set whether or not the query will load and attach relationships. - * - * @param bool True to load and attach relationships. - * @return this - * @task config - */ - public function needRelationships($need_relationships) { - $this->needRelationships = $need_relationships; - return $this; - } - - /** * Set whether or not the query should load the active diff for each * revision. @@ -425,10 +410,6 @@ final class DifferentialRevisionQuery $table = new DifferentialRevision(); $conn_r = $table->establishConnection('r'); - if ($this->needRelationships) { - $this->loadRelationships($conn_r, $revisions); - } - if ($this->needCommitPHIDs) { $this->loadCommitPHIDs($conn_r, $revisions); } @@ -854,40 +835,6 @@ final class DifferentialRevisionQuery ); } - private function loadRelationships($conn_r, array $revisions) { - assert_instances_of($revisions, 'DifferentialRevision'); - - $type_reviewer = DifferentialRevisionHasReviewerEdgeType::EDGECONST; - $type_subscriber = PhabricatorObjectHasSubscriberEdgeType::EDGECONST; - - $edges = id(new PhabricatorEdgeQuery()) - ->withSourcePHIDs(mpull($revisions, 'getPHID')) - ->withEdgeTypes(array($type_reviewer, $type_subscriber)) - ->setOrder(PhabricatorEdgeQuery::ORDER_OLDEST_FIRST) - ->execute(); - - $type_map = array( - DifferentialRevision::RELATION_REVIEWER => $type_reviewer, - DifferentialRevision::RELATION_SUBSCRIBED => $type_subscriber, - ); - - foreach ($revisions as $revision) { - $data = array(); - foreach ($type_map as $rel_type => $edge_type) { - $revision_edges = $edges[$revision->getPHID()][$edge_type]; - foreach ($revision_edges as $dst_phid => $edge_data) { - $data[] = array( - 'relation' => $rel_type, - 'objectPHID' => $dst_phid, - 'reasonPHID' => null, - ); - } - } - - $revision->attachRelationships($data); - } - } - private function loadCommitPHIDs($conn_r, array $revisions) { assert_instances_of($revisions, 'DifferentialRevision'); $commit_phids = queryfx_all( diff --git a/src/applications/differential/query/DifferentialRevisionSearchEngine.php b/src/applications/differential/query/DifferentialRevisionSearchEngine.php index 9212860292..2c40be4813 100644 --- a/src/applications/differential/query/DifferentialRevisionSearchEngine.php +++ b/src/applications/differential/query/DifferentialRevisionSearchEngine.php @@ -19,7 +19,6 @@ final class DifferentialRevisionSearchEngine return id(new DifferentialRevisionQuery()) ->needFlags(true) ->needDrafts(true) - ->needRelationships(true) ->needReviewerStatus(true); } diff --git a/src/applications/differential/storage/DifferentialRevision.php b/src/applications/differential/storage/DifferentialRevision.php index ff826a36b7..062503451f 100644 --- a/src/applications/differential/storage/DifferentialRevision.php +++ b/src/applications/differential/storage/DifferentialRevision.php @@ -38,7 +38,6 @@ final class DifferentialRevision extends DifferentialDAO protected $editPolicy = PhabricatorPolicies::POLICY_USER; protected $properties = array(); - private $relationships = self::ATTACHABLE; private $commits = self::ATTACHABLE; private $activeDiff = self::ATTACHABLE; private $diffIDs = self::ATTACHABLE; @@ -69,7 +68,6 @@ final class DifferentialRevision extends DifferentialDAO return id(new DifferentialRevision()) ->setViewPolicy($view_policy) ->setAuthorPHID($actor->getPHID()) - ->attachRelationships(array()) ->attachRepository(null) ->attachActiveDiff(null) ->attachReviewerStatus(array()) @@ -238,64 +236,6 @@ final class DifferentialRevision extends DifferentialDAO return parent::save(); } - public function loadRelationships() { - if (!$this->getID()) { - $this->relationships = array(); - return; - } - - $data = array(); - - $subscriber_phids = PhabricatorEdgeQuery::loadDestinationPHIDs( - $this->getPHID(), - PhabricatorObjectHasSubscriberEdgeType::EDGECONST); - $subscriber_phids = array_reverse($subscriber_phids); - foreach ($subscriber_phids as $phid) { - $data[] = array( - 'relation' => self::RELATION_SUBSCRIBED, - 'objectPHID' => $phid, - 'reasonPHID' => null, - ); - } - - $reviewer_phids = PhabricatorEdgeQuery::loadDestinationPHIDs( - $this->getPHID(), - DifferentialRevisionHasReviewerEdgeType::EDGECONST); - $reviewer_phids = array_reverse($reviewer_phids); - foreach ($reviewer_phids as $phid) { - $data[] = array( - 'relation' => self::RELATION_REVIEWER, - 'objectPHID' => $phid, - 'reasonPHID' => null, - ); - } - - return $this->attachRelationships($data); - } - - public function attachRelationships(array $relationships) { - $this->relationships = igroup($relationships, 'relation'); - return $this; - } - - public function getReviewers() { - return $this->getRelatedPHIDs(self::RELATION_REVIEWER); - } - - public function getCCPHIDs() { - return $this->getRelatedPHIDs(self::RELATION_SUBSCRIBED); - } - - private function getRelatedPHIDs($relation) { - $this->assertAttached($this->relationships); - - return ipull($this->getRawRelations($relation), 'objectPHID'); - } - - public function getRawRelations($relation) { - return idx($this->relationships, $relation, array()); - } - public function getHashes() { return $this->assertAttached($this->hashes); } @@ -402,6 +342,11 @@ final class DifferentialRevision extends DifferentialDAO return $this; } + public function getReviewerPHIDs() { + $reviewers = $this->getReviewerStatus(); + return mpull($reviewers, 'getReviewerPHID'); + } + public function getReviewerPHIDsForEdit() { $reviewers = $this->getReviewerStatus(); diff --git a/src/applications/differential/view/DifferentialRevisionListView.php b/src/applications/differential/view/DifferentialRevisionListView.php index 811e74fd65..5aacafbb69 100644 --- a/src/applications/differential/view/DifferentialRevisionListView.php +++ b/src/applications/differential/view/DifferentialRevisionListView.php @@ -52,10 +52,7 @@ final class DifferentialRevisionListView extends AphrontView { $phids = array(); foreach ($this->revisions as $revision) { $phids[] = array($revision->getAuthorPHID()); - - // TODO: Switch to getReviewerStatus(), but not all callers pass us - // revisions with this data loaded. - $phids[] = $revision->getReviewers(); + $phids[] = $revision->getReviewerPHIDs(); } return array_mergev($phids); } @@ -132,8 +129,7 @@ final class DifferentialRevisionListView extends AphrontView { } $reviewers = array(); - // TODO: As above, this should be based on `getReviewerStatus()`. - foreach ($revision->getReviewers() as $reviewer) { + foreach ($revision->getReviewerPHIDs() as $reviewer) { $reviewers[] = $this->handles[$reviewer]->renderLink(); } if (!$reviewers) { diff --git a/src/applications/diffusion/controller/DiffusionBrowseController.php b/src/applications/diffusion/controller/DiffusionBrowseController.php index aa8e370e25..dc0131356d 100644 --- a/src/applications/diffusion/controller/DiffusionBrowseController.php +++ b/src/applications/diffusion/controller/DiffusionBrowseController.php @@ -1765,7 +1765,7 @@ final class DiffusionBrowseController extends DiffusionController { ->withUpdatedEpochBetween($recent, null) ->setOrder(DifferentialRevisionQuery::ORDER_MODIFIED) ->setLimit(10) - ->needRelationships(true) + ->needReviewerStatus(true) ->needFlags(true) ->needDrafts(true) ->execute(); diff --git a/src/applications/diffusion/herald/DiffusionCommitRevisionReviewersHeraldField.php b/src/applications/diffusion/herald/DiffusionCommitRevisionReviewersHeraldField.php index 684f5d75d6..50900ede78 100644 --- a/src/applications/diffusion/herald/DiffusionCommitRevisionReviewersHeraldField.php +++ b/src/applications/diffusion/herald/DiffusionCommitRevisionReviewersHeraldField.php @@ -20,7 +20,7 @@ final class DiffusionCommitRevisionReviewersHeraldField return array(); } - return $revision->getReviewers(); + return $revision->getReviewerPHIDs(); } protected function getHeraldFieldStandardType() { diff --git a/src/applications/diffusion/herald/DiffusionPreCommitContentRevisionReviewersHeraldField.php b/src/applications/diffusion/herald/DiffusionPreCommitContentRevisionReviewersHeraldField.php index bce6b5b517..936126ba89 100644 --- a/src/applications/diffusion/herald/DiffusionPreCommitContentRevisionReviewersHeraldField.php +++ b/src/applications/diffusion/herald/DiffusionPreCommitContentRevisionReviewersHeraldField.php @@ -20,8 +20,8 @@ final class DiffusionPreCommitContentRevisionReviewersHeraldField return array(); } - return $revision->getReviewers(); - } + return $revision->getReviewerPHIDs(); + } protected function getHeraldFieldStandardType() { return self::STANDARD_PHID_LIST; diff --git a/src/applications/diffusion/herald/HeraldCommitAdapter.php b/src/applications/diffusion/herald/HeraldCommitAdapter.php index 9d63b9db3d..3465924f92 100644 --- a/src/applications/diffusion/herald/HeraldCommitAdapter.php +++ b/src/applications/diffusion/herald/HeraldCommitAdapter.php @@ -190,7 +190,6 @@ final class HeraldCommitAdapter $revision = id(new DifferentialRevisionQuery()) ->withIDs(array($revision_id)) ->setViewer(PhabricatorUser::getOmnipotentUser()) - ->needRelationships(true) ->needReviewerStatus(true) ->executeOne(); if ($revision) { diff --git a/src/applications/diffusion/herald/HeraldPreCommitContentAdapter.php b/src/applications/diffusion/herald/HeraldPreCommitContentAdapter.php index 13c2695fed..5fbc970dfa 100644 --- a/src/applications/diffusion/herald/HeraldPreCommitContentAdapter.php +++ b/src/applications/diffusion/herald/HeraldPreCommitContentAdapter.php @@ -190,7 +190,7 @@ final class HeraldPreCommitContentAdapter extends HeraldPreCommitAdapter { $this->revision = id(new DifferentialRevisionQuery()) ->setViewer(PhabricatorUser::getOmnipotentUser()) ->withIDs(array($revision_id)) - ->needRelationships(true) + ->needReviewerStatus(true) ->executeOne(); } }