diff --git a/src/applications/project/__tests__/PhabricatorProjectCoreTestCase.php b/src/applications/project/__tests__/PhabricatorProjectCoreTestCase.php index 36c78386f3..3bd38353f6 100644 --- a/src/applications/project/__tests__/PhabricatorProjectCoreTestCase.php +++ b/src/applications/project/__tests__/PhabricatorProjectCoreTestCase.php @@ -113,6 +113,38 @@ final class PhabricatorProjectCoreTestCase extends PhabricatorTestCase { $this->assertTrue($caught instanceof Exception); } + public function testAncestorMembers() { + $user1 = $this->createUser(); + $user1->save(); + + $user2 = $this->createUser(); + $user2->save(); + + $parent = $this->createProject($user1); + $child = $this->createProject($user1, $parent); + + $this->joinProject($child, $user1); + $this->joinProject($child, $user2); + + $project = id(new PhabricatorProjectQuery()) + ->setViewer($user1) + ->withPHIDs(array($child->getPHID())) + ->needAncestorMembers(true) + ->executeOne(); + + $members = array_fuse($project->getParentProject()->getMemberPHIDs()); + ksort($members); + + $expect = array_fuse( + array( + $user1->getPHID(), + $user2->getPHID(), + )); + ksort($expect); + + $this->assertEqual($expect, $members); + } + public function testAncestryQueries() { $user = $this->createUser(); $user->save(); @@ -577,24 +609,6 @@ final class PhabricatorProjectCoreTestCase extends PhabricatorTestCase { $this->assertEqual($expect, $actual); } - private function attemptProjectEdit( - PhabricatorProject $proj, - PhabricatorUser $user, - $skip_refresh = false) { - - $proj = $this->refreshProject($proj, $user, true); - - $new_name = $proj->getName().' '.mt_rand(); - - $xaction = new PhabricatorProjectTransaction(); - $xaction->setTransactionType(PhabricatorProjectTransaction::TYPE_NAME); - $xaction->setNewValue($new_name); - - $this->applyTransactions($proj, $user, array($xaction)); - - return true; - } - public function testJoinLeaveProject() { $user = $this->createUser(); $user->save(); @@ -794,6 +808,25 @@ final class PhabricatorProjectCoreTestCase extends PhabricatorTestCase { pht('Engineering + Scan')); } + private function attemptProjectEdit( + PhabricatorProject $proj, + PhabricatorUser $user, + $skip_refresh = false) { + + $proj = $this->refreshProject($proj, $user, true); + + $new_name = $proj->getName().' '.mt_rand(); + + $xaction = new PhabricatorProjectTransaction(); + $xaction->setTransactionType(PhabricatorProjectTransaction::TYPE_NAME); + $xaction->setNewValue($new_name); + + $this->applyTransactions($proj, $user, array($xaction)); + + return true; + } + + private function newTask( PhabricatorUser $viewer, array $projects, diff --git a/src/applications/project/editor/PhabricatorProjectTransactionEditor.php b/src/applications/project/editor/PhabricatorProjectTransactionEditor.php index 26cc3af86e..6db9b774e6 100644 --- a/src/applications/project/editor/PhabricatorProjectTransactionEditor.php +++ b/src/applications/project/editor/PhabricatorProjectTransactionEditor.php @@ -582,7 +582,7 @@ final class PhabricatorProjectTransactionEditor return id(new PhabricatorProjectQuery()) ->setViewer(PhabricatorUser::getOmnipotentUser()) ->withPHIDs(array($object->getPHID())) - ->needMembers(true) + ->needAncestorMembers(true) ->executeOne(); } diff --git a/src/applications/project/query/PhabricatorProjectQuery.php b/src/applications/project/query/PhabricatorProjectQuery.php index e2d678e7d7..304dd956e1 100644 --- a/src/applications/project/query/PhabricatorProjectQuery.php +++ b/src/applications/project/query/PhabricatorProjectQuery.php @@ -30,6 +30,7 @@ final class PhabricatorProjectQuery private $needSlugs; private $needMembers; + private $needAncestorMembers; private $needWatchers; private $needImages; @@ -109,6 +110,11 @@ final class PhabricatorProjectQuery return $this; } + public function needAncestorMembers($need_ancestor_members) { + $this->needAncestorMembers = $need_ancestor_members; + return $this; + } + public function needWatchers($need_watchers) { $this->needWatchers = $need_watchers; return $this; @@ -220,8 +226,16 @@ final class PhabricatorProjectQuery $types[] = $watcher_type; } + $all_graph = $this->getAllReachableAncestors($projects); + + if ($this->needAncestorMembers) { + $src_projects = $all_graph; + } else { + $src_projects = $projects; + } + $all_sources = array(); - foreach ($projects as $project) { + foreach ($src_projects as $project) { if ($project->isMilestone()) { $phid = $project->getParentProjectPHID(); } else { @@ -234,10 +248,15 @@ final class PhabricatorProjectQuery ->withSourcePHIDs($all_sources) ->withEdgeTypes($types); + $need_all_edges = + $this->needMembers || + $this->needWatchers || + $this->needAncestorMembers; + // If we only need to know if the viewer is a member, we can restrict // the query to just their PHID. $any_edges = true; - if (!$this->needMembers && !$this->needWatchers) { + if (!$need_all_edges) { if ($viewer_phid) { $edge_query->withDestinationPHIDs(array($viewer_phid)); } else { @@ -253,7 +272,7 @@ final class PhabricatorProjectQuery } $membership_projects = array(); - foreach ($projects as $project) { + foreach ($src_projects as $project) { $project_phid = $project->getPHID(); if ($project->isMilestone()) { @@ -274,7 +293,7 @@ final class PhabricatorProjectQuery $membership_projects[$project_phid] = $project; } - if ($this->needMembers) { + if ($this->needMembers || $this->needAncestorMembers) { $project->attachMemberPHIDs($member_phids); } @@ -289,12 +308,15 @@ final class PhabricatorProjectQuery } } - $all_graph = $this->getAllReachableAncestors($projects); - $member_graph = $this->getAllReachableAncestors($membership_projects); + // If we loaded ancestor members, we've already populated membership + // lists above, so we can skip this step. + if (!$this->needAncestorMembers) { + $member_graph = $this->getAllReachableAncestors($membership_projects); - foreach ($all_graph as $phid => $project) { - $is_member = isset($member_graph[$phid]); - $project->setIsUserMember($viewer_phid, $is_member); + foreach ($all_graph as $phid => $project) { + $is_member = isset($member_graph[$phid]); + $project->setIsUserMember($viewer_phid, $is_member); + } } return $projects;