diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index d7f399560d..6f8afa6705 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -906,6 +906,7 @@ phutil_register_library_map(array( 'PhabricatorProjectCreateController' => 'applications/project/controller/PhabricatorProjectCreateController.php', 'PhabricatorProjectDAO' => 'applications/project/storage/PhabricatorProjectDAO.php', 'PhabricatorProjectEditor' => 'applications/project/editor/PhabricatorProjectEditor.php', + 'PhabricatorProjectEditorTestCase' => 'applications/project/editor/__tests__/PhabricatorProjectEditorTestCase.php', 'PhabricatorProjectListController' => 'applications/project/controller/PhabricatorProjectListController.php', 'PhabricatorProjectMembersEditController' => 'applications/project/controller/PhabricatorProjectMembersEditController.php', 'PhabricatorProjectNameCollisionException' => 'applications/project/exception/PhabricatorProjectNameCollisionException.php', @@ -1986,6 +1987,7 @@ phutil_register_library_map(array( 'PhabricatorProjectController' => 'PhabricatorController', 'PhabricatorProjectCreateController' => 'PhabricatorProjectController', 'PhabricatorProjectDAO' => 'PhabricatorLiskDAO', + 'PhabricatorProjectEditorTestCase' => 'PhabricatorTestCase', 'PhabricatorProjectListController' => 'PhabricatorProjectController', 'PhabricatorProjectMembersEditController' => 'PhabricatorProjectController', 'PhabricatorProjectNameCollisionException' => 'Exception', diff --git a/src/applications/project/controller/PhabricatorProjectUpdateController.php b/src/applications/project/controller/PhabricatorProjectUpdateController.php index 32adf77bc8..b12bcc8fa7 100644 --- a/src/applications/project/controller/PhabricatorProjectUpdateController.php +++ b/src/applications/project/controller/PhabricatorProjectUpdateController.php @@ -31,7 +31,11 @@ final class PhabricatorProjectUpdateController $request = $this->getRequest(); $user = $request->getUser(); - $project = id(new PhabricatorProject())->load($this->id); + $project = id(new PhabricatorProjectQuery()) + ->setViewer($user) + ->needMembers(true) + ->withIDs(array($this->id)) + ->executeOne(); if (!$project) { return new Aphront404Response(); } @@ -51,44 +55,14 @@ final class PhabricatorProjectUpdateController $project_uri = '/project/view/'.$project->getID().'/'; if ($process_action) { - $xactions = array(); - - switch ($this->action) { case 'join': - $member_phids = $project->loadMemberPHIDs(); - $member_map = array_fill_keys($member_phids, true); - if (empty($member_map[$user->getPHID()])) { - $member_map[$user->getPHID()] = true; - - $xaction = new PhabricatorProjectTransaction(); - $xaction->setTransactionType( - PhabricatorProjectTransactionType::TYPE_MEMBERS); - $xaction->setNewValue(array_keys($member_map)); - $xactions[] = $xaction; - } + PhabricatorProjectEditor::applyJoinProject($project, $user); break; case 'leave': - $member_phids = $project->loadMemberPHIDs(); - $member_map = array_fill_keys($member_phids, true); - if (isset($member_map[$user->getPHID()])) { - unset($member_map[$user->getPHID()]); - - $xaction = new PhabricatorProjectTransaction(); - $xaction->setTransactionType( - PhabricatorProjectTransactionType::TYPE_MEMBERS); - $xaction->setNewValue(array_keys($member_map)); - $xactions[] = $xaction; - } + PhabricatorProjectEditor::applyLeaveProject($project, $user); break; } - - if ($xactions) { - $editor = new PhabricatorProjectEditor($project); - $editor->setUser($user); - $editor->applyTransactions($xactions); - } - return id(new AphrontRedirectResponse())->setURI($project_uri); } diff --git a/src/applications/project/editor/PhabricatorProjectEditor.php b/src/applications/project/editor/PhabricatorProjectEditor.php index 86c39d0a98..4c8646d9c6 100644 --- a/src/applications/project/editor/PhabricatorProjectEditor.php +++ b/src/applications/project/editor/PhabricatorProjectEditor.php @@ -25,6 +25,51 @@ final class PhabricatorProjectEditor { private $addEdges = array(); private $remEdges = array(); + public static function applyJoinProject( + PhabricatorProject $project, + PhabricatorUser $user) { + + $members = $project->getMemberPHIDs(); + $members[] = $user->getPHID(); + + self::applyOneTransaction( + $project, + $user, + PhabricatorProjectTransactionType::TYPE_MEMBERS, + $members); + } + + public static function applyLeaveProject( + PhabricatorProject $project, + PhabricatorUser $user) { + + $members = array_fill_keys($project->getMemberPHIDs(), true); + unset($members[$user->getPHID()]); + $members = array_keys($members); + + self::applyOneTransaction( + $project, + $user, + PhabricatorProjectTransactionType::TYPE_MEMBERS, + $members); + } + + private static function applyOneTransaction( + PhabricatorProject $project, + PhabricatorUser $user, + $type, + $new_value) { + + $xaction = new PhabricatorProjectTransaction(); + $xaction->setTransactionType($type); + $xaction->setNewValue($new_value); + + $editor = new PhabricatorProjectEditor($project); + $editor->setUser($user); + $editor->applyTransactions(array($xaction)); + } + + public function __construct(PhabricatorProject $project) { $this->project = $project; } diff --git a/src/applications/project/editor/__tests__/PhabricatorProjectEditorTestCase.php b/src/applications/project/editor/__tests__/PhabricatorProjectEditorTestCase.php new file mode 100644 index 0000000000..dafedfe23b --- /dev/null +++ b/src/applications/project/editor/__tests__/PhabricatorProjectEditorTestCase.php @@ -0,0 +1,138 @@ + true, + ); + } + + public function testJoinLeaveProject() { + $user = $this->createUser(); + $user->save(); + + $proj = $this->createProjectWithNewAuthor(); + $proj->save(); + + $proj = $this->refreshProject($proj, $user, true); + $this->assertEqual( + true, + (bool)$proj, + 'Assumption that projects are default visible to any user when created.'); + + $this->assertEqual( + false, + $proj->isUserMember($user->getPHID()), + 'Arbitrary user not member of project.'); + + // Join the project. + PhabricatorProjectEditor::applyJoinProject($proj, $user); + + $proj = $this->refreshProject($proj, $user, true); + $this->assertEqual(true, (bool)$proj); + + $this->assertEqual( + true, + $proj->isUserMember($user->getPHID()), + 'Join works.'); + + + // Join the project again. + PhabricatorProjectEditor::applyJoinProject($proj, $user); + + $proj = $this->refreshProject($proj, $user, true); + $this->assertEqual(true, (bool)$proj); + + $this->assertEqual( + true, + $proj->isUserMember($user->getPHID()), + 'Joining an already-joined project is a no-op.'); + + + // Leave the project. + PhabricatorProjectEditor::applyLeaveProject($proj, $user); + + $proj = $this->refreshProject($proj, $user, true); + $this->assertEqual(true, (bool)$proj); + + $this->assertEqual( + false, + $proj->isUserMember($user->getPHID()), + 'Leave works.'); + + + // Leave the project again. + PhabricatorProjectEditor::applyLeaveProject($proj, $user); + + $proj = $this->refreshProject($proj, $user, true); + $this->assertEqual(true, (bool)$proj); + + $this->assertEqual( + false, + $proj->isUserMember($user->getPHID()), + 'Leaving an already-left project is a no-op.'); + } + + private function refreshProject( + PhabricatorProject $project, + PhabricatorUser $viewer, + $need_members = false) { + + $results = id(new PhabricatorProjectQuery()) + ->setViewer($viewer) + ->needMembers($need_members) + ->withIDs(array($project->getID())) + ->execute(); + + if ($results) { + return head($results); + } else { + return null; + } + } + + private function createProject() { + $project = new PhabricatorProject(); + $project->setName('Test Project '.mt_rand()); + + return $project; + } + + private function createProjectWithNewAuthor() { + $author = $this->createUser(); + $author->save(); + + $project = $this->createProject(); + $project->setAuthorPHID($author->getPHID()); + + return $project; + } + + private function createUser() { + $rand = mt_rand(); + + $user = new PhabricatorUser(); + $user->setUsername('unittestuser'.$rand); + $user->setRealName('Unit Test User '.$rand); + + return $user; + } + +}